diff --git a/api/changelog.d/saml-config-requires-manage-account.security.md b/api/changelog.d/saml-config-requires-manage-account.security.md new file mode 100644 index 0000000000..23fc926c7d --- /dev/null +++ b/api/changelog.d/saml-config-requires-manage-account.security.md @@ -0,0 +1 @@ +SAML SSO configuration endpoints require the `manage_account` permission instead of `manage_integrations`, since the configured IdP controls who signs in and which role they receive diff --git a/api/src/backend/api/tests/test_rbac.py b/api/src/backend/api/tests/test_rbac.py index 772742eeda..49922d0ae7 100644 --- a/api/src/backend/api/tests/test_rbac.py +++ b/api/src/backend/api/tests/test_rbac.py @@ -12,6 +12,7 @@ from api.models import ( ProviderSecret, Role, RoleProviderGroupRelationship, + SAMLConfiguration, Scan, User, UserRoleRelationship, @@ -1896,6 +1897,90 @@ class TestUserRoleLinkPermissions: assert response.status_code == status.HTTP_403_FORBIDDEN +SAML_METADATA_XML = """ + + + + + + TEST + + + + urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress + + + + +""" + + +@pytest.mark.django_db +class TestSAMLConfigurationPermissions: + def _create(self, client, email_domain): + return client.post( + reverse("saml-config-list"), + data={"email_domain": email_domain, "metadata_xml": SAML_METADATA_XML}, + format="json", + ) + + def test_manage_integrations_only_cannot_create( + self, authenticated_client_rbac_manage_integrations_only + ): + response = self._create( + authenticated_client_rbac_manage_integrations_only, + "integrations-only-create.com", + ) + + assert response.status_code == status.HTTP_403_FORBIDDEN + assert not SAMLConfiguration.objects.filter( + email_domain="integrations-only-create.com" + ).exists() + + def test_manage_integrations_only_cannot_read_or_replace( + self, authenticated_client_rbac_manage_integrations_only + ): + client = authenticated_client_rbac_manage_integrations_only + tenant = Membership.objects.filter(user=client.user).first().tenant + config = SAMLConfiguration.objects.create( + tenant_id=str(tenant.id), + email_domain="integrations-only-replace.com", + metadata_xml=SAML_METADATA_XML, + ) + + list_response = client.get(reverse("saml-config-list")) + patch_response = client.patch( + reverse("saml-config-detail", kwargs={"pk": config.id}), + data={"metadata_xml": SAML_METADATA_XML}, + format="json", + ) + delete_response = client.delete( + reverse("saml-config-detail", kwargs={"pk": config.id}) + ) + + assert list_response.status_code == status.HTTP_403_FORBIDDEN + assert patch_response.status_code == status.HTTP_403_FORBIDDEN + assert delete_response.status_code == status.HTTP_403_FORBIDDEN + assert SAMLConfiguration.objects.filter(id=config.id).exists() + + def test_manage_account_only_can_create_and_delete( + self, authenticated_client_rbac_manage_account + ): + client = authenticated_client_rbac_manage_account + + create_response = self._create(client, "account-only.com") + + assert create_response.status_code == status.HTTP_201_CREATED + config = SAMLConfiguration.objects.get(email_domain="account-only.com") + + delete_response = client.delete( + reverse("saml-config-detail", kwargs={"pk": config.id}) + ) + + assert delete_response.status_code == status.HTTP_204_NO_CONTENT + assert not SAMLConfiguration.objects.filter(id=config.id).exists() + + @pytest.mark.django_db class TestCrossTenantRoleLeak: """Regression tests for get_role() cross-tenant privilege leak. diff --git a/api/src/backend/api/v1/views.py b/api/src/backend/api/v1/views.py index a94200ba11..3880042b3f 100644 --- a/api/src/backend/api/v1/views.py +++ b/api/src/backend/api/v1/views.py @@ -808,7 +808,9 @@ class SAMLConfigurationViewSet(BaseRLSViewSet): """ serializer_class = SAMLConfigurationSerializer - required_permissions = [Permissions.MANAGE_INTEGRATIONS] + # The IdP behind this configuration decides who signs in and which role + # they get, so writing it is account administration, not an integration. + required_permissions = [Permissions.MANAGE_ACCOUNT] queryset = SAMLConfiguration.objects.all() def get_queryset(self): diff --git a/api/src/backend/conftest.py b/api/src/backend/conftest.py index f2a3de5bd3..d4247cee7a 100644 --- a/api/src/backend/conftest.py +++ b/api/src/backend/conftest.py @@ -443,6 +443,47 @@ def authenticated_client_rbac_manage_users_only( ) +@pytest.fixture(scope="function") +def create_test_user_rbac_manage_integrations_only(django_db_setup, django_db_blocker): + """User with only manage_integrations permission (no manage_account).""" + with django_db_blocker.unblock(): + user = User.objects.create_user( + name="testing_manage_integrations_only", + email="rbac_manage_integrations_only@rbac.com", + password=TEST_PASSWORD, + ) + tenant = Tenant.objects.create(name="Tenant Test Manage Integrations Only") + Membership.objects.create( + user=user, + tenant=tenant, + role=Membership.RoleChoices.OWNER, + ) + role = Role.objects.create( + name="manage_integrations_only", + tenant_id=tenant.id, + manage_users=False, + manage_account=False, + manage_billing=False, + manage_providers=False, + manage_integrations=True, + manage_scans=False, + unlimited_visibility=False, + ) + UserRoleRelationship.objects.create(user=user, role=role, tenant_id=tenant.id) + return user + + +@pytest.fixture +def authenticated_client_rbac_manage_integrations_only( + create_test_user_rbac_manage_integrations_only, client +): + return authenticate_client_for_tenant( + client, + create_test_user_rbac_manage_integrations_only, + first_membership_tenant(create_test_user_rbac_manage_integrations_only), + ) + + @pytest.fixture def authenticated_client_rbac(create_test_user_rbac, tenants_fixture, client): return authenticate_client_for_tenant( diff --git a/docs/user-guide/tutorials/prowler-app-rbac.mdx b/docs/user-guide/tutorials/prowler-app-rbac.mdx index 65b5a54fbc..e54ab5bab3 100644 --- a/docs/user-guide/tutorials/prowler-app-rbac.mdx +++ b/docs/user-guide/tutorials/prowler-app-rbac.mdx @@ -249,7 +249,7 @@ Assign administrative permissions by selecting from the following options: | Permission | Scope | Description | |------------|-------|-------------| | Invite and Manage Users | All | Invite new users and manage existing ones. | -| Manage Account | All | Adjust account settings, delete users and read/manage users permissions. | +| Manage Account | All | Adjust account settings, delete users, read/manage users permissions and configure SAML SSO. | | Manage Scans | All | Run and review scans, and manage [Scan Configuration](/user-guide/tutorials/prowler-app-scan-configuration) settings. | | Manage Providers | All | Add or modify connected providers, and attach or detach providers from a [Scan Configuration](/user-guide/tutorials/prowler-app-scan-configuration) (in addition to Manage Scans). | | Manage Integrations | All | Add or modify the Prowler Integrations. | diff --git a/docs/user-guide/tutorials/prowler-app-sso.mdx b/docs/user-guide/tutorials/prowler-app-sso.mdx index 388b8d4e1e..9ae221c7c1 100644 --- a/docs/user-guide/tutorials/prowler-app-sso.mdx +++ b/docs/user-guide/tutorials/prowler-app-sso.mdx @@ -41,7 +41,7 @@ If the SAML configuration is removed, users who previously authenticated via SAM ### Prerequisites -- Administrator access to the Prowler organization is required. +- A role with the **Manage Account** permission in the Prowler organization is required. - Administrative access to the SAML 2.0 compliant Identity Provider (e.g., Okta, Azure AD, Google Workspace) is necessary. ### Configuration Steps diff --git a/ui/app/(prowler)/attack-paths/(workflow)/query-builder/attack-paths-page.integration.test.tsx b/ui/app/(prowler)/attack-paths/(workflow)/query-builder/attack-paths-page.integration.test.tsx index 6d26564274..515292627c 100644 --- a/ui/app/(prowler)/attack-paths/(workflow)/query-builder/attack-paths-page.integration.test.tsx +++ b/ui/app/(prowler)/attack-paths/(workflow)/query-builder/attack-paths-page.integration.test.tsx @@ -632,8 +632,9 @@ describe("auto-fitting the viewport", () => { await graph.executeQuery(); await graph.waitForGraphStable(3); await graph.expandAllFindings(); - - const beforeFilter = graph.viewportTransform; + // Expanding starts its own fit; settle it so the click targets a still node + // and the change below can only come from the filtered-view fit. + const beforeFilter = await graph.waitForViewportSettled(); expect(beforeFilter).toBeTruthy(); await graph.clickFirstFindingNode(); diff --git a/ui/app/(prowler)/profile/page.tsx b/ui/app/(prowler)/profile/page.tsx index 4aedf1038b..f1d8d79dd0 100644 --- a/ui/app/(prowler)/profile/page.tsx +++ b/ui/app/(prowler)/profile/page.tsx @@ -89,13 +89,7 @@ const SSRDataUser = async ({ role.attributes.manage_account === true && userRoleIds.includes(role.id), ); - const hasManageIntegrations = roleDetails.some( - (role) => - role.attributes.manage_integrations === true && - userRoleIds.includes(role.id), - ); - - const samlConfig = hasManageIntegrations ? await getSamlConfig() : undefined; + const samlConfig = hasManageAccount ? await getSamlConfig() : undefined; return ( )} - {hasManageIntegrations && ( + {hasManageAccount && ( )} { expect( screen.getByRole("heading", { name: "Findings" }), ).toBeInTheDocument(); - expect(screen.getAllByText("See how it works")).not.toHaveLength(0); + expect(await screen.findAllByText("See how it works")).not.toHaveLength(0); expect( screen.getByRole("button", { name: /start product tour: explore your findings/i, diff --git a/ui/vitest.config.ts b/ui/vitest.config.ts index d4485dc474..86d168a90e 100644 --- a/ui/vitest.config.ts +++ b/ui/vitest.config.ts @@ -96,7 +96,9 @@ export default defineConfig(() => { // vertically and ends up overlapping the graph, so Playwright // can't click nodes. Use a standard desktop viewport. viewport: { width: 1280, height: 800 }, - provider: playwright(), + // Playwright waits forever for an actionable element by default; + // fail inside the test budget so the log names the blocker. + provider: playwright({ actionTimeout: 10_000 }), headless: true, instances: [{ browser: "chromium" }], },