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

refactor: define a single method to remove entities from authz #425

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

refactor: define a single method to remove entities from authz #425

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
81c955d
refactor: define a single method to remove entities from authz
leafty Sep 25, 2024
a3f4ee7
chore: fix polylith error in tests
olevski Sep 25, 2024
eb49b8b
Merge remote-tracking branch 'origin/chore-fix-polylith-fakeredis' in…
leafty Sep 25, 2024
03e70ab
update error message
leafty Sep 25, 2024
871b24a
Merge branch 'main' into leafty/refactor-authz-remove-entity
leafty Sep 25, 2024
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
112 changes: 63 additions & 49 deletions components/renku_data_services/authz/authz.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -473,7 +473,7 @@ async def _get_authz_change(
authz_change = db_repo.authz._add_project(result)
case AuthzOperation.delete, ResourceType.project if isinstance(result, Project):
user = _extract_user_from_args(*func_args, **func_kwargs)
authz_change = await db_repo.authz._remove_project(user, result)
authz_change = await db_repo.authz._remove_entity(user, result)
case AuthzOperation.delete, ResourceType.project if result is None:
# NOTE: This means that the project does not exist in the first place so nothing was deleted
pass
Expand All @@ -489,7 +489,7 @@ async def _get_authz_change(
authz_change = db_repo.authz._add_group(result)
case AuthzOperation.delete, ResourceType.group if isinstance(result, Group):
user = _extract_user_from_args(*func_args, **func_kwargs)
authz_change = await db_repo.authz._remove_group(user, result)
authz_change = await db_repo.authz._remove_entity(user, result)
case AuthzOperation.delete, ResourceType.group if result is None:
# NOTE: This means that the group does not exist in the first place so nothing was deleted
pass
Expand Down Expand Up @@ -569,6 +569,65 @@ async def decorated_function(

return decorator

async def _remove_entity(
self, user: base_models.APIUser, resource: UserInfo | Group | Namespace | Project
) -> _AuthzChange:
resource_type: ResourceType
match resource:
case _ if isinstance(resource, UserInfo):
resource_type = ResourceType.user
case _ if isinstance(resource, Group):
resource_type = ResourceType.group
case _ if isinstance(resource, Namespace) and resource.kind == NamespaceKind.user:
resource_type = ResourceType.user_namespace
case _ if isinstance(resource, Namespace):
raise errors.ProgrammingError(
message=f"Cannot handle deletetion of namespace {resource.slug} of kind {resource.kind.value}."
)
case _ if isinstance(resource, Project):
resource_type = ResourceType.project
case _:
raise errors.ProgrammingError(message="Cannot handle deletion of unknown resource.")
resource_id = str(resource.id)

@_is_allowed_on_resource(Scope.DELETE, resource_type)
async def _remove_entity_wrapped(
authz: Authz,
user: base_models.APIUser,
resource: UserInfo | Group | Namespace | Project,
*,
zed_token: ZedToken | None = None,
) -> _AuthzChange:
consistency = Consistency(at_least_as_fresh=zed_token) if zed_token else Consistency(fully_consistent=True)
rels: list[Relationship] = []
# Get relations where the entity is the resource
rel_filter = RelationshipFilter(resource_type=resource_type.value, optional_resource_id=resource_id)
responses: AsyncIterable[ReadRelationshipsResponse] = authz.client.ReadRelationships(
ReadRelationshipsRequest(consistency=consistency, relationship_filter=rel_filter)
)
async for response in responses:
rels.append(response.relationship)
# Get relations where the entity is the subject
rel_filter = RelationshipFilter(
optional_subject_filter=SubjectFilter(subject_type=resource_type, optional_subject_id=resource_id)
)
responses: AsyncIterable[ReadRelationshipsResponse] = authz.client.ReadRelationships(
ReadRelationshipsRequest(consistency=consistency, relationship_filter=rel_filter)
)
async for response in responses:
rels.append(response.relationship)
apply = WriteRelationshipsRequest(
updates=[
RelationshipUpdate(operation=RelationshipUpdate.OPERATION_DELETE, relationship=i) for i in rels
]
)
undo = WriteRelationshipsRequest(
updates=[RelationshipUpdate(operation=RelationshipUpdate.OPERATION_TOUCH, relationship=i) for i in rels]
)
return _AuthzChange(apply=apply, undo=undo)

return await _remove_entity_wrapped(self, user, resource)

def _add_project(self, project: Project) -> _AuthzChange:
"""Create the new project and associated resources and relations in the DB."""
creator = SubjectReference(object=_AuthzConverter.user(project.created_by))
Expand Down Expand Up @@ -618,27 +677,6 @@ def _add_project(self, project: Project) -> _AuthzChange:
)
return _AuthzChange(apply=apply, undo=undo)

@_is_allowed_on_resource(Scope.DELETE, ResourceType.project)
async def _remove_project(
self, user: base_models.APIUser, project: Project, *, zed_token: ZedToken | None = None
) -> _AuthzChange:
"""Remove the relationships associated with the project."""
consistency = Consistency(at_least_as_fresh=zed_token) if zed_token else Consistency(fully_consistent=True)
rel_filter = RelationshipFilter(resource_type=ResourceType.project.value, optional_resource_id=str(project.id))
responses: AsyncIterable[ReadRelationshipsResponse] = self.client.ReadRelationships(
ReadRelationshipsRequest(consistency=consistency, relationship_filter=rel_filter)
)
rels: list[Relationship] = []
async for response in responses:
rels.append(response.relationship)
apply = WriteRelationshipsRequest(
updates=[RelationshipUpdate(operation=RelationshipUpdate.OPERATION_DELETE, relationship=i) for i in rels]
)
undo = WriteRelationshipsRequest(
updates=[RelationshipUpdate(operation=RelationshipUpdate.OPERATION_TOUCH, relationship=i) for i in rels]
)
return _AuthzChange(apply=apply, undo=undo)

# NOTE changing visibility is the same access level as removal
@_is_allowed_on_resource(Scope.DELETE, ResourceType.project)
async def _update_project_visibility(
Expand Down Expand Up @@ -1025,7 +1063,7 @@ def _add_admin(self, user_id: str) -> _AuthzChange:
return _AuthzChange(apply=apply, undo=undo)

async def _remove_admin(self, user_id: str) -> _AuthzChange:
"""Add a deployment-wide administrator in the authorization database."""
"""Remove a deployment-wide administrator from the authorization database."""
existing_admin_ids = await self._get_admin_user_ids()
rel = Relationship(
resource=_AuthzConverter.platform(),
Expand Down Expand Up @@ -1086,31 +1124,6 @@ def _add_group(self, group: Group) -> _AuthzChange:
)
return _AuthzChange(apply=apply, undo=undo)

@_is_allowed_on_resource(Scope.DELETE, ResourceType.group)
async def _remove_group(
self, user: base_models.APIUser, group: Group, *, zed_token: ZedToken | None = None
) -> _AuthzChange:
"""Remove the group from the authorization database."""
if not group.id:
raise errors.ProgrammingError(
message="Cannot remove a group in the authorization database if the group has no ID"
)
consistency = Consistency(at_least_as_fresh=zed_token) if zed_token else Consistency(fully_consistent=True)
rel_filter = RelationshipFilter(resource_type=ResourceType.group.value, optional_resource_id=str(group.id))
responses = self.client.ReadRelationships(
ReadRelationshipsRequest(consistency=consistency, relationship_filter=rel_filter)
)
rels: list[Relationship] = []
async for response in responses:
rels.append(response.relationship)
apply = WriteRelationshipsRequest(
updates=[RelationshipUpdate(operation=RelationshipUpdate.OPERATION_DELETE, relationship=i) for i in rels]
)
undo = WriteRelationshipsRequest(
updates=[RelationshipUpdate(operation=RelationshipUpdate.OPERATION_TOUCH, relationship=i) for i in rels]
)
return _AuthzChange(apply=apply, undo=undo)

@_is_allowed(Scope.CHANGE_MEMBERSHIP)
async def upsert_group_members(
self,
Expand Down Expand Up @@ -1341,6 +1354,7 @@ def _add_user_namespace(self, namespace: Namespace) -> _AuthzChange:
)
return _AuthzChange(apply=apply, undo=undo)

# TODO: remove this method and replace it with _remove_entity()
async def _remove_user_namespace(self, user_id: str, zed_token: ZedToken | None = None) -> _AuthzChange:
"""Remove the user namespace from the authorization database."""
consistency = Consistency(at_least_as_fresh=zed_token) if zed_token else Consistency(fully_consistent=True)
Expand Down
4 changes: 2 additions & 2 deletions test/components/renku_data_services/authz/test_authorization.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ async def test_adding_deleting_project(app_config: Config, bootstrap_admins, pub
assert not await authz.has_permission(anon_user, ResourceType.project, project_id, Scope.DELETE)
assert not await authz.has_permission(regular_user2, ResourceType.project, project_id, Scope.WRITE)
assert not await authz.has_permission(regular_user2, ResourceType.project, project_id, Scope.DELETE)
authz_changes = await authz._remove_project(project_owner, project)
authz_changes = await authz._remove_entity(project_owner, project)
await authz.client.WriteRelationships(authz_changes.apply)
assert not await authz.has_permission(admin_user, ResourceType.project, project_id, Scope.READ)
assert not await authz.has_permission(admin_user, ResourceType.project, project_id, Scope.WRITE)
Expand Down Expand Up @@ -291,7 +291,7 @@ async def test_listing_projects_with_access(app_config: Config, bootstrap_admins
== 0
)
# Test project deletion
changes = await authz._remove_project(project_owner, private_project1)
changes = await authz._remove_entity(project_owner, private_project1)
await authz.client.WriteRelationships(changes.apply)
assert private_project_id1_str not in set(
await authz.resources_with_permission(admin_user, project_owner.id, ResourceType.project, Scope.READ)
Expand Down
Loading