mirror of
https://github.com/prowler-cloud/prowler.git
synced 2026-03-22 03:08:23 +00:00
fix(saml): prevent SAML role mapping from removing last manage-account user (#10007)
This commit is contained in:
@@ -11,6 +11,14 @@ All notable changes to the **Prowler API** are documented in this file.
|
|||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
|
## [1.19.2] (Prowler v5.18.2)
|
||||||
|
|
||||||
|
### 🐞 Fixed
|
||||||
|
|
||||||
|
- SAML role mapping now prevents removing the last MANAGE_ACCOUNT user [(#10007)](https://github.com/prowler-cloud/prowler/pull/10007)
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
## [1.19.0] (Prowler v5.18.0)
|
## [1.19.0] (Prowler v5.18.0)
|
||||||
|
|
||||||
### 🚀 Added
|
### 🚀 Added
|
||||||
|
|||||||
@@ -10844,25 +10844,20 @@ class TestTenantFinishACSView:
|
|||||||
assert "sso_saml_failed=true" in response.url
|
assert "sso_saml_failed=true" in response.url
|
||||||
|
|
||||||
def test_dispatch_skips_role_mapping_when_single_manage_account_user(
|
def test_dispatch_skips_role_mapping_when_single_manage_account_user(
|
||||||
self, create_test_user, tenants_fixture, saml_setup, settings, monkeypatch
|
self,
|
||||||
|
create_test_user,
|
||||||
|
tenants_fixture,
|
||||||
|
admin_role_fixture,
|
||||||
|
saml_setup,
|
||||||
|
settings,
|
||||||
|
monkeypatch,
|
||||||
):
|
):
|
||||||
"""Test that role mapping is skipped when tenant has only one user with MANAGE_ACCOUNT role"""
|
"""Test that role mapping is skipped when tenant has only one user with MANAGE_ACCOUNT role"""
|
||||||
monkeypatch.setenv("SAML_SSO_CALLBACK_URL", "http://localhost/sso-complete")
|
monkeypatch.setenv("SAML_SSO_CALLBACK_URL", "http://localhost/sso-complete")
|
||||||
user = create_test_user
|
user = create_test_user
|
||||||
tenant = tenants_fixture[0]
|
tenant = tenants_fixture[0]
|
||||||
|
|
||||||
# Create a single role with manage_account=True for the user
|
admin_role = admin_role_fixture
|
||||||
admin_role = Role.objects.using(MainRouter.admin_db).create(
|
|
||||||
name="admin",
|
|
||||||
tenant=tenant,
|
|
||||||
manage_account=True,
|
|
||||||
manage_users=True,
|
|
||||||
manage_billing=True,
|
|
||||||
manage_providers=True,
|
|
||||||
manage_integrations=True,
|
|
||||||
manage_scans=True,
|
|
||||||
unlimited_visibility=True,
|
|
||||||
)
|
|
||||||
UserRoleRelationship.objects.using(MainRouter.admin_db).create(
|
UserRoleRelationship.objects.using(MainRouter.admin_db).create(
|
||||||
user=user, role=admin_role, tenant_id=tenant.id
|
user=user, role=admin_role, tenant_id=tenant.id
|
||||||
)
|
)
|
||||||
@@ -10933,35 +10928,26 @@ class TestTenantFinishACSView:
|
|||||||
.exists()
|
.exists()
|
||||||
)
|
)
|
||||||
|
|
||||||
def test_dispatch_applies_role_mapping_when_multiple_manage_account_users(
|
def test_dispatch_skips_role_mapping_when_last_manage_account_user_maps_to_existing_role(
|
||||||
self, create_test_user, tenants_fixture, saml_setup, settings, monkeypatch
|
self,
|
||||||
|
create_test_user,
|
||||||
|
tenants_fixture,
|
||||||
|
admin_role_fixture,
|
||||||
|
roles_fixture,
|
||||||
|
saml_setup,
|
||||||
|
settings,
|
||||||
|
monkeypatch,
|
||||||
):
|
):
|
||||||
"""Test that role mapping is applied when tenant has multiple users with MANAGE_ACCOUNT role"""
|
"""Test that role mapping is skipped when it would remove the last MANAGE_ACCOUNT user"""
|
||||||
monkeypatch.setenv("SAML_SSO_CALLBACK_URL", "http://localhost/sso-complete")
|
monkeypatch.setenv("SAML_SSO_CALLBACK_URL", "http://localhost/sso-complete")
|
||||||
user = create_test_user
|
user = create_test_user
|
||||||
tenant = tenants_fixture[0]
|
tenant = tenants_fixture[0]
|
||||||
|
|
||||||
# Create a second user with manage_account=True
|
admin_role = admin_role_fixture
|
||||||
second_admin = User.objects.using(MainRouter.admin_db).create(
|
viewer_role = roles_fixture[3]
|
||||||
email="admin2@prowler.com", name="Second Admin"
|
|
||||||
)
|
|
||||||
admin_role = Role.objects.using(MainRouter.admin_db).create(
|
|
||||||
name="admin",
|
|
||||||
tenant=tenant,
|
|
||||||
manage_account=True,
|
|
||||||
manage_users=True,
|
|
||||||
manage_billing=True,
|
|
||||||
manage_providers=True,
|
|
||||||
manage_integrations=True,
|
|
||||||
manage_scans=True,
|
|
||||||
unlimited_visibility=True,
|
|
||||||
)
|
|
||||||
UserRoleRelationship.objects.using(MainRouter.admin_db).create(
|
UserRoleRelationship.objects.using(MainRouter.admin_db).create(
|
||||||
user=user, role=admin_role, tenant_id=tenant.id
|
user=user, role=admin_role, tenant_id=tenant.id
|
||||||
)
|
)
|
||||||
UserRoleRelationship.objects.using(MainRouter.admin_db).create(
|
|
||||||
user=second_admin, role=admin_role, tenant_id=tenant.id
|
|
||||||
)
|
|
||||||
|
|
||||||
social_account = SocialAccount(
|
social_account = SocialAccount(
|
||||||
user=user,
|
user=user,
|
||||||
@@ -10970,7 +10956,7 @@ class TestTenantFinishACSView:
|
|||||||
"firstName": ["John"],
|
"firstName": ["John"],
|
||||||
"lastName": ["Doe"],
|
"lastName": ["Doe"],
|
||||||
"organization": ["testing_company"],
|
"organization": ["testing_company"],
|
||||||
"userType": ["viewer"], # This SHOULD be applied
|
"userType": [viewer_role.name],
|
||||||
},
|
},
|
||||||
)
|
)
|
||||||
|
|
||||||
@@ -11008,10 +10994,91 @@ class TestTenantFinishACSView:
|
|||||||
|
|
||||||
assert response.status_code == 302
|
assert response.status_code == 302
|
||||||
|
|
||||||
# Verify the viewer role was created and assigned (role mapping was applied)
|
assert (
|
||||||
viewer_role = Role.objects.using(MainRouter.admin_db).get(
|
UserRoleRelationship.objects.using(MainRouter.admin_db)
|
||||||
name="viewer", tenant=tenant
|
.filter(user=user, role=admin_role, tenant_id=tenant.id)
|
||||||
|
.exists()
|
||||||
)
|
)
|
||||||
|
assert not (
|
||||||
|
UserRoleRelationship.objects.using(MainRouter.admin_db)
|
||||||
|
.filter(user=user, role=viewer_role, tenant_id=tenant.id)
|
||||||
|
.exists()
|
||||||
|
)
|
||||||
|
|
||||||
|
def test_dispatch_applies_role_mapping_when_multiple_manage_account_users(
|
||||||
|
self,
|
||||||
|
create_test_user,
|
||||||
|
tenants_fixture,
|
||||||
|
admin_role_fixture,
|
||||||
|
roles_fixture,
|
||||||
|
saml_setup,
|
||||||
|
settings,
|
||||||
|
monkeypatch,
|
||||||
|
):
|
||||||
|
"""Test that role mapping is applied when tenant has multiple users with MANAGE_ACCOUNT role"""
|
||||||
|
monkeypatch.setenv("SAML_SSO_CALLBACK_URL", "http://localhost/sso-complete")
|
||||||
|
user = create_test_user
|
||||||
|
tenant = tenants_fixture[0]
|
||||||
|
|
||||||
|
# Create a second user with manage_account=True
|
||||||
|
second_admin = User.objects.using(MainRouter.admin_db).create(
|
||||||
|
email="admin2@prowler.com", name="Second Admin"
|
||||||
|
)
|
||||||
|
admin_role = admin_role_fixture
|
||||||
|
viewer_role = roles_fixture[3]
|
||||||
|
UserRoleRelationship.objects.using(MainRouter.admin_db).create(
|
||||||
|
user=user, role=admin_role, tenant_id=tenant.id
|
||||||
|
)
|
||||||
|
UserRoleRelationship.objects.using(MainRouter.admin_db).create(
|
||||||
|
user=second_admin, role=admin_role, tenant_id=tenant.id
|
||||||
|
)
|
||||||
|
|
||||||
|
social_account = SocialAccount(
|
||||||
|
user=user,
|
||||||
|
provider="saml",
|
||||||
|
extra_data={
|
||||||
|
"firstName": ["John"],
|
||||||
|
"lastName": ["Doe"],
|
||||||
|
"organization": ["testing_company"],
|
||||||
|
"userType": [viewer_role.name], # This SHOULD be applied
|
||||||
|
},
|
||||||
|
)
|
||||||
|
|
||||||
|
request = RequestFactory().get(
|
||||||
|
reverse("saml_finish_acs", kwargs={"organization_slug": "testtenant"})
|
||||||
|
)
|
||||||
|
request.user = user
|
||||||
|
request.session = {}
|
||||||
|
|
||||||
|
with (
|
||||||
|
patch(
|
||||||
|
"allauth.socialaccount.providers.saml.views.get_app_or_404"
|
||||||
|
) as mock_get_app_or_404,
|
||||||
|
patch(
|
||||||
|
"allauth.socialaccount.models.SocialApp.objects.get"
|
||||||
|
) as mock_socialapp_get,
|
||||||
|
patch(
|
||||||
|
"allauth.socialaccount.models.SocialAccount.objects.get"
|
||||||
|
) as mock_sa_get,
|
||||||
|
patch("api.models.SAMLDomainIndex.objects.get") as mock_saml_domain_get,
|
||||||
|
patch("api.models.SAMLConfiguration.objects.get") as mock_saml_config_get,
|
||||||
|
patch("api.models.User.objects.get") as mock_user_get,
|
||||||
|
):
|
||||||
|
mock_get_app_or_404.return_value = MagicMock(
|
||||||
|
provider="saml", client_id="testtenant", name="Test App", settings={}
|
||||||
|
)
|
||||||
|
mock_sa_get.return_value = social_account
|
||||||
|
mock_socialapp_get.return_value = MagicMock(provider_id="saml")
|
||||||
|
mock_saml_domain_get.return_value = SimpleNamespace(tenant_id=tenant.id)
|
||||||
|
mock_saml_config_get.return_value = MagicMock()
|
||||||
|
mock_user_get.return_value = user
|
||||||
|
|
||||||
|
view = TenantFinishACSView.as_view()
|
||||||
|
response = view(request, organization_slug="testtenant")
|
||||||
|
|
||||||
|
assert response.status_code == 302
|
||||||
|
|
||||||
|
# Verify the viewer role was assigned (role mapping was applied)
|
||||||
assert (
|
assert (
|
||||||
UserRoleRelationship.objects.using(MainRouter.admin_db)
|
UserRoleRelationship.objects.using(MainRouter.admin_db)
|
||||||
.filter(user=user, role=viewer_role, tenant_id=tenant.id)
|
.filter(user=user, role=viewer_role, tenant_id=tenant.id)
|
||||||
@@ -11025,6 +11092,86 @@ class TestTenantFinishACSView:
|
|||||||
.exists()
|
.exists()
|
||||||
)
|
)
|
||||||
|
|
||||||
|
def test_dispatch_applies_role_mapping_for_non_admin_user_with_single_admin(
|
||||||
|
self,
|
||||||
|
create_test_user,
|
||||||
|
tenants_fixture,
|
||||||
|
admin_role_fixture,
|
||||||
|
roles_fixture,
|
||||||
|
saml_setup,
|
||||||
|
settings,
|
||||||
|
monkeypatch,
|
||||||
|
):
|
||||||
|
"""Test that role mapping is applied for a non-admin user when a single admin exists"""
|
||||||
|
monkeypatch.setenv("SAML_SSO_CALLBACK_URL", "http://localhost/sso-complete")
|
||||||
|
admin_user = create_test_user
|
||||||
|
tenant = tenants_fixture[0]
|
||||||
|
non_admin_user = User.objects.using(MainRouter.admin_db).create(
|
||||||
|
email="viewer@prowler.com", name="Viewer"
|
||||||
|
)
|
||||||
|
|
||||||
|
admin_role = admin_role_fixture
|
||||||
|
viewer_role = roles_fixture[3]
|
||||||
|
UserRoleRelationship.objects.using(MainRouter.admin_db).create(
|
||||||
|
user=admin_user, role=admin_role, tenant_id=tenant.id
|
||||||
|
)
|
||||||
|
|
||||||
|
social_account = SocialAccount(
|
||||||
|
user=non_admin_user,
|
||||||
|
provider="saml",
|
||||||
|
extra_data={
|
||||||
|
"firstName": ["Jane"],
|
||||||
|
"lastName": ["Doe"],
|
||||||
|
"organization": ["testing_company"],
|
||||||
|
"userType": [viewer_role.name],
|
||||||
|
},
|
||||||
|
)
|
||||||
|
|
||||||
|
request = RequestFactory().get(
|
||||||
|
reverse("saml_finish_acs", kwargs={"organization_slug": "testtenant"})
|
||||||
|
)
|
||||||
|
request.user = non_admin_user
|
||||||
|
request.session = {}
|
||||||
|
|
||||||
|
with (
|
||||||
|
patch(
|
||||||
|
"allauth.socialaccount.providers.saml.views.get_app_or_404"
|
||||||
|
) as mock_get_app_or_404,
|
||||||
|
patch(
|
||||||
|
"allauth.socialaccount.models.SocialApp.objects.get"
|
||||||
|
) as mock_socialapp_get,
|
||||||
|
patch(
|
||||||
|
"allauth.socialaccount.models.SocialAccount.objects.get"
|
||||||
|
) as mock_sa_get,
|
||||||
|
patch("api.models.SAMLDomainIndex.objects.get") as mock_saml_domain_get,
|
||||||
|
patch("api.models.SAMLConfiguration.objects.get") as mock_saml_config_get,
|
||||||
|
patch("api.models.User.objects.get") as mock_user_get,
|
||||||
|
):
|
||||||
|
mock_get_app_or_404.return_value = MagicMock(
|
||||||
|
provider="saml", client_id="testtenant", name="Test App", settings={}
|
||||||
|
)
|
||||||
|
mock_sa_get.return_value = social_account
|
||||||
|
mock_socialapp_get.return_value = MagicMock(provider_id="saml")
|
||||||
|
mock_saml_domain_get.return_value = SimpleNamespace(tenant_id=tenant.id)
|
||||||
|
mock_saml_config_get.return_value = MagicMock()
|
||||||
|
mock_user_get.return_value = non_admin_user
|
||||||
|
|
||||||
|
view = TenantFinishACSView.as_view()
|
||||||
|
response = view(request, organization_slug="testtenant")
|
||||||
|
|
||||||
|
assert response.status_code == 302
|
||||||
|
|
||||||
|
assert (
|
||||||
|
UserRoleRelationship.objects.using(MainRouter.admin_db)
|
||||||
|
.filter(user=non_admin_user, role=viewer_role, tenant_id=tenant.id)
|
||||||
|
.exists()
|
||||||
|
)
|
||||||
|
assert (
|
||||||
|
UserRoleRelationship.objects.using(MainRouter.admin_db)
|
||||||
|
.filter(user=admin_user, role=admin_role, tenant_id=tenant.id)
|
||||||
|
.exists()
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.django_db
|
@pytest.mark.django_db
|
||||||
class TestLighthouseConfigViewSet:
|
class TestLighthouseConfigViewSet:
|
||||||
|
|||||||
@@ -763,27 +763,40 @@ class TenantFinishACSView(FinishACSView):
|
|||||||
.tenant
|
.tenant
|
||||||
)
|
)
|
||||||
|
|
||||||
# Check if tenant has only one user with MANAGE_ACCOUNT role
|
role_name = (
|
||||||
users_with_manage_account = (
|
extra.get("userType", ["no_permissions"])[0].strip()
|
||||||
|
if extra.get("userType")
|
||||||
|
else "no_permissions"
|
||||||
|
)
|
||||||
|
role = (
|
||||||
|
Role.objects.using(MainRouter.admin_db)
|
||||||
|
.filter(name=role_name, tenant=tenant)
|
||||||
|
.first()
|
||||||
|
)
|
||||||
|
|
||||||
|
# Only skip mapping if it would remove the last MANAGE_ACCOUNT user
|
||||||
|
remaining_manage_account_users = (
|
||||||
UserRoleRelationship.objects.using(MainRouter.admin_db)
|
UserRoleRelationship.objects.using(MainRouter.admin_db)
|
||||||
.filter(role__manage_account=True, tenant_id=tenant.id)
|
.filter(role__manage_account=True, tenant_id=tenant.id)
|
||||||
|
.exclude(user_id=user_id)
|
||||||
.values("user")
|
.values("user")
|
||||||
.distinct()
|
.distinct()
|
||||||
.count()
|
.count()
|
||||||
)
|
)
|
||||||
|
user_has_manage_account = (
|
||||||
|
UserRoleRelationship.objects.using(MainRouter.admin_db)
|
||||||
|
.filter(role__manage_account=True, tenant_id=tenant.id, user_id=user_id)
|
||||||
|
.exists()
|
||||||
|
)
|
||||||
|
role_manage_account = role.manage_account if role else False
|
||||||
|
would_remove_last_manage_account = (
|
||||||
|
user_has_manage_account
|
||||||
|
and remaining_manage_account_users == 0
|
||||||
|
and not role_manage_account
|
||||||
|
)
|
||||||
|
|
||||||
# Only apply role mapping from userType if tenant does NOT have exactly one user with MANAGE_ACCOUNT
|
if not would_remove_last_manage_account:
|
||||||
if users_with_manage_account != 1:
|
if role is None:
|
||||||
role_name = (
|
|
||||||
extra.get("userType", ["no_permissions"])[0].strip()
|
|
||||||
if extra.get("userType")
|
|
||||||
else "no_permissions"
|
|
||||||
)
|
|
||||||
try:
|
|
||||||
role = Role.objects.using(MainRouter.admin_db).get(
|
|
||||||
name=role_name, tenant=tenant
|
|
||||||
)
|
|
||||||
except Role.DoesNotExist:
|
|
||||||
role = Role.objects.using(MainRouter.admin_db).create(
|
role = Role.objects.using(MainRouter.admin_db).create(
|
||||||
name=role_name,
|
name=role_name,
|
||||||
tenant=tenant,
|
tenant=tenant,
|
||||||
|
|||||||
Reference in New Issue
Block a user