Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove system flag to use public_tenant for access consistency #1451

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Remove system flag to use public_tenant for access consistency #1451

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2395d1c
Remove system flag to use public_tenant for access consistency
EvanCasey13 Jan 21, 2025
d0245f0
Consistent use of public_tenant_only for filtering
EvanCasey13 Jan 22, 2025
66fb202
Merge branch 'master' into system_flag_remove_resources_access
EvanCasey13 Jan 30, 2025
a95640b
test fix for cross account
EvanCasey13 Jan 30, 2025
c82c87d
Merge branch 'system_flag_remove_resources_access' of github.com:Evan…
EvanCasey13 Jan 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions rbac/api/cross_access/serializer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,9 +111,8 @@ def get_roles(self, obj):
def validate_roles(self, roles):
"""Format role list as expected for cross-account-request."""
public_tenant = Tenant.objects.get(tenant_name="public")

role_display_names = [role["display_name"] for role in roles]
roles_queryset = Role.objects.filter(tenant=public_tenant, display_name__in=role_display_names)
roles_queryset = Role.objects.filter(display_name__in=role_display_names, tenant=public_tenant)
role_dict = {role.display_name: role for role in roles_queryset}

system_role_uuids = []
Expand Down
6 changes: 2 additions & 4 deletions rbac/management/group/definer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,6 @@ def clone_default_group_in_public_schema(group, tenant) -> Optional[Group]:
else:
group_uuid = uuid4()

public_tenant = Tenant.objects.get(tenant_name="public")
tenant_default_policy = group.policies.get(system=True)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the policies, the situation is different, we used system True to indicate the policies can not be updated, but the tenant is in each tenant. Maybe we can just ignore the system=True for policies. You might need to do some research if we have any system not true policies

group.name = "Custom default access"
group.system = False
Expand All @@ -118,7 +117,7 @@ def clone_default_group_in_public_schema(group, tenant) -> Optional[Group]:
if Group.objects.filter(name=group.name, platform_default=group.platform_default, tenant=tenant):
# TODO: returning none can break other code
return None
public_default_roles = Role.objects.filter(platform_default=True, tenant=public_tenant)
public_default_roles = Role.objects.filter(platform_default=True).public_tenant_only()

group.save()
tenant_default_policy.group = group
Expand Down Expand Up @@ -148,8 +147,7 @@ def add_roles(group, roles_or_role_ids, tenant, user=None):
if system_policy_created:
logger.info(f"Created new system policy for tenant {tenant.org_id}.")

system_roles = roles.filter(tenant=Tenant.objects.get(tenant_name="public"))

system_roles = roles.public_tenant_only()
# Custom roles are locked to prevent resources from being added/removed concurrently,
# in the case that the Roles had _no_ resources specified to begin with.
# This should not be necessary for system roles.
Expand Down
7 changes: 2 additions & 5 deletions rbac/management/role/serializer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
from management.utils import filter_queryset_by_tenant, get_principal, validate_and_get_key
from rest_framework import serializers

from api.models import Tenant
from .model import Access, BindingMapping, Permission, ResourceDefinition, Role
from ..querysets import ORG_ID_SCOPE, PRINCIPAL_SCOPE, SCOPE_KEY, VALID_SCOPES

Expand Down Expand Up @@ -359,11 +358,9 @@ def obtain_groups_in(obj, request):
else:
assigned_groups = filter_queryset_by_tenant(Group.objects.filter(policies__in=policy_ids), request.tenant)

public_tenant = Tenant.objects.get(tenant_name="public")

platform_default_groups = Group.platform_default_set().filter(tenant=request.tenant).filter(
policies__in=policy_ids
) or Group.platform_default_set().filter(tenant=public_tenant).filter(policies__in=policy_ids)
) or Group.platform_default_set().public_tenant_only().filter(policies__in=policy_ids)

if username_param and scope_param != PRINCIPAL_SCOPE:
is_org_admin = request.user_from_query.admin
Expand All @@ -375,7 +372,7 @@ def obtain_groups_in(obj, request):
if is_org_admin:
admin_default_groups = Group.admin_default_set().filter(tenant=request.tenant).filter(
policies__in=policy_ids
) or Group.admin_default_set().filter(tenant=public_tenant).filter(policies__in=policy_ids)
) or Group.admin_default_set().public_tenant_only().filter(policies__in=policy_ids)

qs = qs | admin_default_groups

Expand Down