From 1ba22f6f45ff6c320ee5138ac0d8748ebde7cc98 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Jes=C3=BAs=20Pe=C3=B1a=20Rodr=C3=ADguez?= Date: Thu, 9 Oct 2025 14:30:26 +0200 Subject: [PATCH] feat(api): update role mapping logic in TenantFinishACSView to handle single/manage account users (#8882) --- api/CHANGELOG.md | 1 + api/src/backend/api/tests/test_views.py | 182 ++++++++++++++++++++++++ api/src/backend/api/v1/views.py | 66 +++++---- 3 files changed, 222 insertions(+), 27 deletions(-) diff --git a/api/CHANGELOG.md b/api/CHANGELOG.md index d6b692c110..1a67cd741a 100644 --- a/api/CHANGELOG.md +++ b/api/CHANGELOG.md @@ -8,6 +8,7 @@ All notable changes to the **Prowler API** are documented in this file. - Default JWT keys are generated and stored if they are missing from configuration [(#8655)](https://github.com/prowler-cloud/prowler/pull/8655) - `compliance_name` for each compliance [(#7920)](https://github.com/prowler-cloud/prowler/pull/7920) - API Key support [(#8805)](https://github.com/prowler-cloud/prowler/pull/8805) +- SAML role mapping protection for single-admin tenants to prevent accidental lockout [(#8882)](https://github.com/prowler-cloud/prowler/pull/8882) - Support for `passed_findings` and `total_findings` fields in compliance requirement overview for accurate Prowler ThreatScore calculation [(#8582)](https://github.com/prowler-cloud/prowler/pull/8582) ### Changed diff --git a/api/src/backend/api/tests/test_views.py b/api/src/backend/api/tests/test_views.py index cfad06c777..d94caa41f7 100644 --- a/api/src/backend/api/tests/test_views.py +++ b/api/src/backend/api/tests/test_views.py @@ -6892,6 +6892,188 @@ class TestTenantFinishACSView: assert response.status_code == 302 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 + ): + """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, + ) + UserRoleRelationship.objects.using(MainRouter.admin_db).create( + user=user, role=admin_role, tenant_id=tenant.id + ) + + social_account = SocialAccount( + user=user, + provider="saml", + extra_data={ + "firstName": ["John"], + "lastName": ["Doe"], + "organization": ["testing_company"], + "userType": ["no_permissions"], # This should be ignored + }, + ) + + 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 admin role is still assigned (not changed to no_permissions) + assert ( + UserRoleRelationship.objects.using(MainRouter.admin_db) + .filter(user=user, role=admin_role, tenant_id=tenant.id) + .exists() + ) + + # Verify no_permissions role was NOT created in the database + assert ( + not Role.objects.using(MainRouter.admin_db) + .filter(name="no_permissions", tenant=tenant) + .exists() + ) + + # Verify no_permissions role was NOT assigned to the user + assert not ( + UserRoleRelationship.objects.using(MainRouter.admin_db) + .filter(user=user, role__name="no_permissions", tenant_id=tenant.id) + .exists() + ) + + def test_dispatch_applies_role_mapping_when_multiple_manage_account_users( + self, create_test_user, tenants_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 = 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( + 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"], # 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 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=viewer_role, tenant_id=tenant.id) + .exists() + ) + + # Verify the admin role was removed (replaced by viewer) + assert not ( + UserRoleRelationship.objects.using(MainRouter.admin_db) + .filter(user=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 777afac907..8be9f3aed3 100644 --- a/api/src/backend/api/v1/views.py +++ b/api/src/backend/api/v1/views.py @@ -666,36 +666,48 @@ class TenantFinishACSView(FinishACSView): .get(email_domain=email_domain) .tenant ) - role_name = ( - extra.get("userType", ["no_permissions"])[0].strip() - if extra.get("userType") - else "no_permissions" + + # Check if tenant has only one user with MANAGE_ACCOUNT role + users_with_manage_account = ( + UserRoleRelationship.objects.using(MainRouter.admin_db) + .filter(role__manage_account=True, tenant_id=tenant.id) + .values("user") + .distinct() + .count() ) - try: - role = Role.objects.using(MainRouter.admin_db).get( - name=role_name, tenant=tenant + + # 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" ) - except Role.DoesNotExist: - role = Role.objects.using(MainRouter.admin_db).create( - name=role_name, - tenant=tenant, - manage_users=False, - manage_account=False, - manage_billing=False, - manage_providers=False, - manage_integrations=False, - manage_scans=False, - unlimited_visibility=False, + 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( + name=role_name, + tenant=tenant, + manage_users=False, + manage_account=False, + manage_billing=False, + manage_providers=False, + manage_integrations=False, + manage_scans=False, + unlimited_visibility=False, + ) + UserRoleRelationship.objects.using(MainRouter.admin_db).filter( + user=user, + tenant_id=tenant.id, + ).delete() + UserRoleRelationship.objects.using(MainRouter.admin_db).create( + user=user, + role=role, + tenant_id=tenant.id, ) - UserRoleRelationship.objects.using(MainRouter.admin_db).filter( - user=user, - tenant_id=tenant.id, - ).delete() - UserRoleRelationship.objects.using(MainRouter.admin_db).create( - user=user, - role=role, - tenant_id=tenant.id, - ) membership, _ = Membership.objects.using(MainRouter.admin_db).get_or_create( user=user, tenant=tenant,