mirror of
https://github.com/prowler-cloud/prowler.git
synced 2026-10-31 07:44:21 +00:00
feat(ui): Add user expulsion from tenants with JWT authentication fix (#10787)
Co-authored-by: Pablo F.G <pablo.fernandez@prowler.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: Adrián Peña <adrianjpr@gmail.com>
This commit is contained in:
15 files changed
+832
-55
No files matched your search
@@ -330,6 +330,7 @@ class MembershipFilter(FilterSet):
|
||||
model = Membership
|
||||
fields = {
|
||||
"tenant": ["exact"],
|
||||
"user": ["exact"],
|
||||
"role": ["exact"],
|
||||
"date_joined": ["date", "gte", "lte"],
|
||||
}
|
||||
|
||||
@@ -32,6 +32,11 @@ from django_celery_results.models import TaskResult
|
||||
from rest_framework import status
|
||||
from rest_framework.exceptions import PermissionDenied
|
||||
from rest_framework.response import Response
|
||||
from rest_framework_simplejwt.token_blacklist.models import (
|
||||
BlacklistedToken,
|
||||
OutstandingToken,
|
||||
)
|
||||
from rest_framework_simplejwt.tokens import RefreshToken
|
||||
|
||||
from api.attack_paths import (
|
||||
AttackPathsQueryDefinition,
|
||||
@@ -47,6 +52,7 @@ from api.models import (
|
||||
Finding,
|
||||
Integration,
|
||||
Invitation,
|
||||
InvitationRoleRelationship,
|
||||
LighthouseProviderConfiguration,
|
||||
LighthouseProviderModels,
|
||||
LighthouseTenantConfiguration,
|
||||
@@ -746,6 +752,39 @@ class TestTenantViewSet:
|
||||
# Test user + 2 extra users for tenant 2
|
||||
assert len(response.json()["data"]) == 3
|
||||
|
||||
def test_tenants_list_memberships_filter_by_user(
|
||||
self, authenticated_client, tenants_fixture, extra_users
|
||||
):
|
||||
_, tenant2, _ = tenants_fixture
|
||||
_, user3_membership = extra_users
|
||||
user3, membership3 = user3_membership
|
||||
|
||||
response = authenticated_client.get(
|
||||
reverse("tenant-membership-list", kwargs={"tenant_pk": tenant2.id}),
|
||||
{"filter[user]": str(user3.id)},
|
||||
)
|
||||
assert response.status_code == status.HTTP_200_OK
|
||||
data = response.json()["data"]
|
||||
assert len(data) == 1
|
||||
assert data[0]["id"] == str(membership3.id)
|
||||
|
||||
def test_tenants_list_memberships_filter_by_user_no_match(
|
||||
self, authenticated_client, tenants_fixture, extra_users
|
||||
):
|
||||
_, tenant2, _ = tenants_fixture
|
||||
unrelated_user = User.objects.create_user(
|
||||
name="unrelated",
|
||||
password=TEST_PASSWORD,
|
||||
email="unrelated@gmail.com",
|
||||
)
|
||||
|
||||
response = authenticated_client.get(
|
||||
reverse("tenant-membership-list", kwargs={"tenant_pk": tenant2.id}),
|
||||
{"filter[user]": str(unrelated_user.id)},
|
||||
)
|
||||
assert response.status_code == status.HTTP_200_OK
|
||||
assert response.json()["data"] == []
|
||||
|
||||
def test_tenants_list_memberships_as_member(
|
||||
self, authenticated_client, tenants_fixture, extra_users
|
||||
):
|
||||
@@ -803,6 +842,7 @@ class TestTenantViewSet:
|
||||
):
|
||||
_, tenant2, _ = tenants_fixture
|
||||
user_membership = Membership.objects.get(tenant=tenant2, user__email=TEST_USER)
|
||||
user_id = user_membership.user_id
|
||||
response = authenticated_client.delete(
|
||||
reverse(
|
||||
"tenant-membership-detail",
|
||||
@@ -811,6 +851,127 @@ class TestTenantViewSet:
|
||||
)
|
||||
assert response.status_code == status.HTTP_403_FORBIDDEN
|
||||
assert Membership.objects.filter(id=user_membership.id).exists()
|
||||
assert User.objects.filter(id=user_id).exists()
|
||||
|
||||
def test_expel_user_deletes_account_if_last_membership(
|
||||
self, authenticated_client, tenants_fixture, extra_users
|
||||
):
|
||||
# TEST_USER is OWNER of tenant2; user3 is MEMBER only in tenant2
|
||||
_, tenant2, _ = tenants_fixture
|
||||
_, user3_membership = extra_users
|
||||
user3, membership3 = user3_membership
|
||||
|
||||
assert Membership.objects.filter(user=user3).count() == 1
|
||||
|
||||
response = authenticated_client.delete(
|
||||
reverse(
|
||||
"tenant-membership-detail",
|
||||
kwargs={"tenant_pk": tenant2.id, "pk": membership3.id},
|
||||
)
|
||||
)
|
||||
assert response.status_code == status.HTTP_204_NO_CONTENT
|
||||
assert not Membership.objects.filter(id=membership3.id).exists()
|
||||
assert not User.objects.filter(id=user3.id).exists()
|
||||
|
||||
def test_expel_user_blacklists_refresh_tokens(
|
||||
self, authenticated_client, tenants_fixture, extra_users
|
||||
):
|
||||
_, tenant2, _ = tenants_fixture
|
||||
_, user3_membership = extra_users
|
||||
user3, membership3 = user3_membership
|
||||
|
||||
# Issue two refresh tokens to simulate active sessions
|
||||
RefreshToken.for_user(user3)
|
||||
RefreshToken.for_user(user3)
|
||||
outstanding_ids = list(
|
||||
OutstandingToken.objects.filter(user=user3).values_list("id", flat=True)
|
||||
)
|
||||
assert len(outstanding_ids) == 2
|
||||
assert not BlacklistedToken.objects.filter(
|
||||
token_id__in=outstanding_ids
|
||||
).exists()
|
||||
|
||||
response = authenticated_client.delete(
|
||||
reverse(
|
||||
"tenant-membership-detail",
|
||||
kwargs={"tenant_pk": tenant2.id, "pk": membership3.id},
|
||||
)
|
||||
)
|
||||
assert response.status_code == status.HTTP_204_NO_CONTENT
|
||||
assert (
|
||||
BlacklistedToken.objects.filter(token_id__in=outstanding_ids).count() == 2
|
||||
)
|
||||
|
||||
def test_expel_user_blacklists_refresh_tokens_is_idempotent(
|
||||
self, authenticated_client, tenants_fixture, extra_users
|
||||
):
|
||||
# Regression test for the bulk blacklisting path: if one of the
|
||||
# user's refresh tokens is already blacklisted when the expel
|
||||
# endpoint runs, the remaining tokens must still be blacklisted
|
||||
# and the already-blacklisted one must not be duplicated.
|
||||
tenant1, tenant2, _ = tenants_fixture
|
||||
_, user3_membership = extra_users
|
||||
user3, membership3 = user3_membership
|
||||
|
||||
# Keep the user alive after the expel so the assertions below can
|
||||
# still query OutstandingToken by user_id.
|
||||
Membership.objects.create(
|
||||
user=user3,
|
||||
tenant=tenant1,
|
||||
role=Membership.RoleChoices.MEMBER,
|
||||
)
|
||||
|
||||
RefreshToken.for_user(user3)
|
||||
RefreshToken.for_user(user3)
|
||||
outstanding_ids = list(
|
||||
OutstandingToken.objects.filter(user=user3).values_list("id", flat=True)
|
||||
)
|
||||
assert len(outstanding_ids) == 2
|
||||
|
||||
# Pre-blacklist one of the two tokens to simulate a prior revocation.
|
||||
BlacklistedToken.objects.create(token_id=outstanding_ids[0])
|
||||
assert (
|
||||
BlacklistedToken.objects.filter(token_id__in=outstanding_ids).count() == 1
|
||||
)
|
||||
|
||||
response = authenticated_client.delete(
|
||||
reverse(
|
||||
"tenant-membership-detail",
|
||||
kwargs={"tenant_pk": tenant2.id, "pk": membership3.id},
|
||||
)
|
||||
)
|
||||
assert response.status_code == status.HTTP_204_NO_CONTENT
|
||||
|
||||
blacklisted = BlacklistedToken.objects.filter(token_id__in=outstanding_ids)
|
||||
assert blacklisted.count() == 2
|
||||
assert set(blacklisted.values_list("token_id", flat=True)) == set(
|
||||
outstanding_ids
|
||||
)
|
||||
|
||||
def test_expel_user_keeps_account_if_has_other_memberships(
|
||||
self, authenticated_client, tenants_fixture, extra_users
|
||||
):
|
||||
tenant1, tenant2, _ = tenants_fixture
|
||||
_, user3_membership = extra_users
|
||||
user3, membership3 = user3_membership
|
||||
|
||||
# Give user3 an additional membership in tenant1 so they are not orphaned
|
||||
other_membership = Membership.objects.create(
|
||||
user=user3,
|
||||
tenant=tenant1,
|
||||
role=Membership.RoleChoices.MEMBER,
|
||||
)
|
||||
|
||||
response = authenticated_client.delete(
|
||||
reverse(
|
||||
"tenant-membership-detail",
|
||||
kwargs={"tenant_pk": tenant2.id, "pk": membership3.id},
|
||||
)
|
||||
)
|
||||
assert response.status_code == status.HTTP_204_NO_CONTENT
|
||||
assert not Membership.objects.filter(id=membership3.id).exists()
|
||||
assert User.objects.filter(id=user3.id).exists()
|
||||
assert Membership.objects.filter(id=other_membership.id).exists()
|
||||
|
||||
def test_tenants_delete_another_membership_as_owner(
|
||||
self, authenticated_client, tenants_fixture, extra_users
|
||||
@@ -882,6 +1043,128 @@ class TestTenantViewSet:
|
||||
assert response.status_code == status.HTTP_404_NOT_FOUND
|
||||
assert Membership.objects.filter(id=other_membership.id).exists()
|
||||
|
||||
def test_delete_membership_cleans_up_orphaned_role_grants(
|
||||
self, authenticated_client, tenants_fixture
|
||||
):
|
||||
"""Test that deleting a membership removes UserRoleRelationship records
|
||||
for that tenant while preserving grants in other tenants."""
|
||||
tenant1, tenant2, _ = tenants_fixture
|
||||
|
||||
# Create a user with memberships in both tenants
|
||||
user = User.objects.create_user(
|
||||
name="Multi-tenant User",
|
||||
password=TEST_PASSWORD,
|
||||
email="multitenant@test.com",
|
||||
)
|
||||
|
||||
# Create memberships in both tenants
|
||||
Membership.objects.create(
|
||||
user=user, tenant=tenant1, role=Membership.RoleChoices.MEMBER
|
||||
)
|
||||
membership2 = Membership.objects.create(
|
||||
user=user, tenant=tenant2, role=Membership.RoleChoices.MEMBER
|
||||
)
|
||||
|
||||
# Create roles in both tenants
|
||||
role1 = Role.objects.create(
|
||||
name="Test Role 1", tenant=tenant1, manage_providers=True
|
||||
)
|
||||
role2 = Role.objects.create(
|
||||
name="Test Role 2", tenant=tenant2, manage_scans=True
|
||||
)
|
||||
|
||||
# Create user role relationships for both tenants
|
||||
UserRoleRelationship.objects.create(user=user, role=role1, tenant=tenant1)
|
||||
UserRoleRelationship.objects.create(user=user, role=role2, tenant=tenant2)
|
||||
|
||||
# Verify initial state
|
||||
assert UserRoleRelationship.objects.filter(user=user, tenant=tenant1).exists()
|
||||
assert UserRoleRelationship.objects.filter(user=user, tenant=tenant2).exists()
|
||||
assert Role.objects.filter(id=role1.id).exists()
|
||||
assert Role.objects.filter(id=role2.id).exists()
|
||||
|
||||
# Delete membership from tenant2 (authenticated user is owner of tenant2)
|
||||
response = authenticated_client.delete(
|
||||
reverse(
|
||||
"tenant-membership-detail",
|
||||
kwargs={"tenant_pk": tenant2.id, "pk": membership2.id},
|
||||
)
|
||||
)
|
||||
|
||||
assert response.status_code == status.HTTP_204_NO_CONTENT
|
||||
|
||||
# Verify the membership was deleted
|
||||
assert not Membership.objects.filter(id=membership2.id).exists()
|
||||
|
||||
# Verify UserRoleRelationship for tenant2 was deleted
|
||||
assert not UserRoleRelationship.objects.filter(
|
||||
user=user, tenant=tenant2
|
||||
).exists()
|
||||
|
||||
# Verify UserRoleRelationship for tenant1 is preserved
|
||||
assert UserRoleRelationship.objects.filter(user=user, tenant=tenant1).exists()
|
||||
|
||||
# Verify orphaned role2 was deleted (no more user or invitation relationships)
|
||||
assert not Role.objects.filter(id=role2.id).exists()
|
||||
|
||||
# Verify role1 is preserved (still has user relationship)
|
||||
assert Role.objects.filter(id=role1.id).exists()
|
||||
|
||||
# Verify the user still exists (has other memberships)
|
||||
assert User.objects.filter(id=user.id).exists()
|
||||
|
||||
def test_delete_membership_preserves_role_with_invitation_relationship(
|
||||
self, authenticated_client, tenants_fixture
|
||||
):
|
||||
"""Test that roles are not deleted if they have invitation relationships."""
|
||||
_, tenant2, _ = tenants_fixture
|
||||
|
||||
# Create a user with membership
|
||||
user = User.objects.create_user(
|
||||
name="Test User", password=TEST_PASSWORD, email="testuser@test.com"
|
||||
)
|
||||
membership = Membership.objects.create(
|
||||
user=user, tenant=tenant2, role=Membership.RoleChoices.MEMBER
|
||||
)
|
||||
|
||||
# Create a role and user relationship
|
||||
role = Role.objects.create(
|
||||
name="Shared Role", tenant=tenant2, manage_providers=True
|
||||
)
|
||||
UserRoleRelationship.objects.create(user=user, role=role, tenant=tenant2)
|
||||
|
||||
# Create an invitation with the same role
|
||||
invitation = Invitation.objects.create(email="pending@test.com", tenant=tenant2)
|
||||
InvitationRoleRelationship.objects.create(
|
||||
invitation=invitation, role=role, tenant=tenant2
|
||||
)
|
||||
|
||||
# Verify initial state
|
||||
assert UserRoleRelationship.objects.filter(user=user, role=role).exists()
|
||||
assert InvitationRoleRelationship.objects.filter(
|
||||
invitation=invitation, role=role
|
||||
).exists()
|
||||
assert Role.objects.filter(id=role.id).exists()
|
||||
|
||||
# Delete the membership
|
||||
response = authenticated_client.delete(
|
||||
reverse(
|
||||
"tenant-membership-detail",
|
||||
kwargs={"tenant_pk": tenant2.id, "pk": membership.id},
|
||||
)
|
||||
)
|
||||
|
||||
assert response.status_code == status.HTTP_204_NO_CONTENT
|
||||
|
||||
# Verify UserRoleRelationship was deleted
|
||||
assert not UserRoleRelationship.objects.filter(user=user, role=role).exists()
|
||||
|
||||
# Verify role is preserved because invitation relationship exists
|
||||
assert Role.objects.filter(id=role.id).exists()
|
||||
assert InvitationRoleRelationship.objects.filter(
|
||||
invitation=invitation, role=role
|
||||
).exists()
|
||||
|
||||
def test_tenants_list_no_permissions(
|
||||
self, authenticated_client_no_permissions_rbac, tenants_fixture
|
||||
):
|
||||
|
||||
@@ -83,6 +83,10 @@ from rest_framework.permissions import SAFE_METHODS
|
||||
from rest_framework_json_api import filters as jsonapi_filters
|
||||
from rest_framework_json_api.views import RelationshipView, Response
|
||||
from rest_framework_simplejwt.exceptions import InvalidToken, TokenError
|
||||
from rest_framework_simplejwt.token_blacklist.models import (
|
||||
BlacklistedToken,
|
||||
OutstandingToken,
|
||||
)
|
||||
from tasks.beat import schedule_provider_scan
|
||||
from tasks.jobs.attack_paths import db_utils as attack_paths_db_utils
|
||||
from tasks.jobs.export import get_s3_client
|
||||
@@ -169,6 +173,7 @@ from api.models import (
|
||||
FindingGroupDailySummary,
|
||||
Integration,
|
||||
Invitation,
|
||||
InvitationRoleRelationship,
|
||||
LighthouseConfiguration,
|
||||
LighthouseProviderConfiguration,
|
||||
LighthouseProviderModels,
|
||||
@@ -1330,9 +1335,11 @@ class MembershipViewSet(BaseTenantViewset):
|
||||
),
|
||||
destroy=extend_schema(
|
||||
summary="Delete tenant memberships",
|
||||
description="Delete the membership details of users in a tenant. You need to be one of the owners to delete a "
|
||||
"membership that is not yours. If you are the last owner of a tenant, you cannot delete your own "
|
||||
"membership.",
|
||||
description="Delete a user's membership from a tenant. This action: (1) removes the membership, "
|
||||
"(2) revokes all refresh tokens for the expelled user, (3) removes their role grants for this tenant, "
|
||||
"(4) cleans up orphaned roles, and (5) deletes the user account if this was their last membership. "
|
||||
"You must be a tenant owner to delete another user's membership. The last owner of a tenant cannot "
|
||||
"delete their own membership.",
|
||||
tags=["Tenant"],
|
||||
),
|
||||
)
|
||||
@@ -1341,6 +1348,7 @@ class TenantMembersViewSet(BaseTenantViewset):
|
||||
http_method_names = ["get", "delete"]
|
||||
serializer_class = MembershipSerializer
|
||||
queryset = Membership.objects.none()
|
||||
filterset_class = MembershipFilter
|
||||
# Authorization is handled by get_requesting_membership (owner/member checks),
|
||||
# not by RBAC, since the target tenant differs from the JWT tenant.
|
||||
required_permissions = []
|
||||
@@ -1398,7 +1406,84 @@ class TenantMembersViewSet(BaseTenantViewset):
|
||||
"You do not have permission to delete this membership."
|
||||
)
|
||||
|
||||
membership_to_delete.delete()
|
||||
user_to_check_id = membership_to_delete.user_id
|
||||
tenant_id = membership_to_delete.tenant_id
|
||||
# All writes run on the admin connection so that the uncommitted
|
||||
# membership delete is visible to the subsequent "other memberships"
|
||||
# check. Splitting the delete and the check across the default
|
||||
# (prowler_user, RLS) and admin connections caused the admin side to
|
||||
# miss the just-deleted row and leave the User row orphaned.
|
||||
with transaction.atomic(using=MainRouter.admin_db):
|
||||
Membership.objects.using(MainRouter.admin_db).filter(
|
||||
id=membership_to_delete.id
|
||||
).delete()
|
||||
|
||||
# Remove role grants for this user in this tenant to prevent
|
||||
# orphaned permissions that could allow access after expulsion
|
||||
deleted_role_relationships = UserRoleRelationship.objects.using(
|
||||
MainRouter.admin_db
|
||||
).filter(user_id=user_to_check_id, tenant_id=tenant_id)
|
||||
|
||||
# Collect role IDs that might become orphaned after deletion
|
||||
role_ids_to_check = list(
|
||||
deleted_role_relationships.values_list("role_id", flat=True)
|
||||
)
|
||||
|
||||
# Delete the user role relationships for this tenant
|
||||
deleted_role_relationships.delete()
|
||||
|
||||
# Clean up orphaned roles that have no remaining user or invitation relationships
|
||||
if role_ids_to_check:
|
||||
for role_id in role_ids_to_check:
|
||||
has_user_relationships = (
|
||||
UserRoleRelationship.objects.using(MainRouter.admin_db)
|
||||
.filter(role_id=role_id)
|
||||
.exists()
|
||||
)
|
||||
|
||||
has_invitation_relationships = (
|
||||
InvitationRoleRelationship.objects.using(MainRouter.admin_db)
|
||||
.filter(role_id=role_id)
|
||||
.exists()
|
||||
)
|
||||
|
||||
if not has_user_relationships and not has_invitation_relationships:
|
||||
Role.objects.using(MainRouter.admin_db).filter(
|
||||
id=role_id
|
||||
).delete()
|
||||
|
||||
# Revoke any refresh tokens the expelled user still holds so they
|
||||
# cannot mint fresh access tokens. This must happen before the
|
||||
# User row is deleted, because OutstandingToken.user is
|
||||
# on_delete=SET_NULL in djangorestframework-simplejwt 5.5.1
|
||||
# (see rest_framework_simplejwt/token_blacklist/models.py): once
|
||||
# the user row is gone, user_id becomes NULL and we can no longer
|
||||
# look up that user's outstanding tokens. Access tokens already
|
||||
# issued remain valid until SIMPLE_JWT["ACCESS_TOKEN_LIFETIME"]
|
||||
# expires.
|
||||
outstanding_token_ids = list(
|
||||
OutstandingToken.objects.using(MainRouter.admin_db)
|
||||
.filter(user_id=user_to_check_id)
|
||||
.values_list("id", flat=True)
|
||||
)
|
||||
if outstanding_token_ids:
|
||||
BlacklistedToken.objects.using(MainRouter.admin_db).bulk_create(
|
||||
[
|
||||
BlacklistedToken(token_id=token_id)
|
||||
for token_id in outstanding_token_ids
|
||||
],
|
||||
ignore_conflicts=True,
|
||||
)
|
||||
|
||||
has_other_memberships = (
|
||||
Membership.objects.using(MainRouter.admin_db)
|
||||
.filter(user_id=user_to_check_id)
|
||||
.exists()
|
||||
)
|
||||
if not has_other_memberships:
|
||||
User.objects.using(MainRouter.admin_db).filter(
|
||||
id=user_to_check_id
|
||||
).delete()
|
||||
return Response(status=status.HTTP_204_NO_CONTENT)
|
||||
|
||||
|
||||
|
||||
Reference in new issue
Block a user