diff --git a/api/CHANGELOG.md b/api/CHANGELOG.md index 4b404586a1..50b2a8370b 100644 --- a/api/CHANGELOG.md +++ b/api/CHANGELOG.md @@ -2,6 +2,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) ### 🚀 Added diff --git a/api/src/backend/api/tests/test_views.py b/api/src/backend/api/tests/test_views.py index 4bb7509bc7..da73befd3e 100644 --- a/api/src/backend/api/tests/test_views.py +++ b/api/src/backend/api/tests/test_views.py @@ -10841,25 +10841,20 @@ class TestTenantFinishACSView: assert "sso_saml_failed=true" in response.url 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""" monkeypatch.setenv("SAML_SSO_CALLBACK_URL", "http://localhost/sso-complete") user = create_test_user tenant = tenants_fixture[0] - # Create a single role with manage_account=True for the user - 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, - ) + admin_role = admin_role_fixture UserRoleRelationship.objects.using(MainRouter.admin_db).create( user=user, role=admin_role, tenant_id=tenant.id ) @@ -10930,35 +10925,26 @@ class TestTenantFinishACSView: .exists() ) - def test_dispatch_applies_role_mapping_when_multiple_manage_account_users( - self, create_test_user, tenants_fixture, saml_setup, settings, monkeypatch + def test_dispatch_skips_role_mapping_when_last_manage_account_user_maps_to_existing_role( + 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") 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 = 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, - ) + 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, @@ -10967,7 +10953,7 @@ class TestTenantFinishACSView: "firstName": ["John"], "lastName": ["Doe"], "organization": ["testing_company"], - "userType": ["viewer"], # This SHOULD be applied + "userType": [viewer_role.name], }, ) @@ -11005,10 +10991,91 @@ class TestTenantFinishACSView: assert response.status_code == 302 - # Verify the viewer role was created and assigned (role mapping was applied) - viewer_role = Role.objects.using(MainRouter.admin_db).get( - name="viewer", tenant=tenant + assert ( + UserRoleRelationship.objects.using(MainRouter.admin_db) + .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 ( UserRoleRelationship.objects.using(MainRouter.admin_db) .filter(user=user, role=viewer_role, tenant_id=tenant.id) @@ -11022,6 +11089,86 @@ class TestTenantFinishACSView: .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 class TestLighthouseConfigViewSet: diff --git a/api/src/backend/api/v1/views.py b/api/src/backend/api/v1/views.py index 108d587cf3..542ff8bfe7 100644 --- a/api/src/backend/api/v1/views.py +++ b/api/src/backend/api/v1/views.py @@ -763,27 +763,40 @@ class TenantFinishACSView(FinishACSView): .tenant ) - # Check if tenant has only one user with MANAGE_ACCOUNT role - users_with_manage_account = ( + role_name = ( + 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) .filter(role__manage_account=True, tenant_id=tenant.id) + .exclude(user_id=user_id) .values("user") .distinct() .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 users_with_manage_account != 1: - 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: + if not would_remove_last_manage_account: + if role is None: role = Role.objects.using(MainRouter.admin_db).create( name=role_name, tenant=tenant,