Skip to content

Commit

Permalink
fix: properly cascade group permissions to all projects in the group (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
olevski authored Jun 26, 2024
1 parent 66eba81 commit dd68ce2
Show file tree
Hide file tree
Showing 16 changed files with 388 additions and 99 deletions.
11 changes: 10 additions & 1 deletion bases/renku_data_services/background_jobs/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
from renku_data_services.message_queue.db import EventRepository
from renku_data_services.message_queue.redis_queue import RedisQueue
from renku_data_services.namespace.db import GroupRepository
from renku_data_services.project.db import ProjectRepository
from renku_data_services.users.db import UsersSync
from renku_data_services.users.kc_api import IKeycloakAPI, KeycloakAPI

Expand All @@ -27,6 +28,7 @@ class SyncConfig:
authz_config: AuthzConfig
group_repo: GroupRepository
event_repo: EventRepository
project_repo: ProjectRepository
session_maker: Callable[..., AsyncSession]

@classmethod
Expand Down Expand Up @@ -58,6 +60,13 @@ def from_env(cls, prefix: str = "") -> "SyncConfig":
group_authz=Authz(authz_config),
message_queue=message_queue,
)
project_repo = ProjectRepository(
session_maker=session_maker,
message_queue=message_queue,
event_repo=event_repo,
group_repo=group_repo,
authz=Authz(authz_config),
)
syncer = UsersSync(
session_maker,
message_queue=message_queue,
Expand All @@ -70,4 +79,4 @@ def from_env(cls, prefix: str = "") -> "SyncConfig":
client_secret = os.environ[f"{prefix}KEYCLOAK_CLIENT_SECRET"]
realm = os.environ.get(f"{prefix}KEYCLOAK_REALM", "Renku")
kc_api = KeycloakAPI(keycloak_url=keycloak_url, client_id=client_id, client_secret=client_secret, realm=realm)
return cls(syncer, kc_api, authz_config, group_repo, event_repo, session_maker)
return cls(syncer, kc_api, authz_config, group_repo, event_repo, project_repo, session_maker)
62 changes: 61 additions & 1 deletion bases/renku_data_services/background_jobs/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,21 @@

import logging

from authzed.api.v1.permission_service_pb2 import ReadRelationshipsRequest, RelationshipFilter
from authzed.api.v1.core_pb2 import ObjectReference, Relationship, RelationshipUpdate, SubjectReference
from authzed.api.v1.permission_service_pb2 import (
Consistency,
ReadRelationshipsRequest,
RelationshipFilter,
SubjectFilter,
WriteRelationshipsRequest,
)

from renku_data_services.authz.authz import Authz, ResourceType, _Relation
from renku_data_services.background_jobs.config import SyncConfig
from renku_data_services.base_models.core import InternalServiceAdmin, ServiceAdminId
from renku_data_services.message_queue.avro_models.io.renku.events import v2
from renku_data_services.message_queue.converters import EventConverter
from renku_data_services.namespace.models import NamespaceKind


async def sync_user_namespaces(config: SyncConfig) -> None:
Expand Down Expand Up @@ -67,3 +76,54 @@ async def bootstrap_user_namespaces(config: SyncConfig) -> None:
)
return
await sync_user_namespaces(config)


async def fix_mismatched_project_namespace_ids(config: SyncConfig) -> None:
"""Fixes a problem where the project namespace relationship for projects has the wrong group ID."""
api_user = InternalServiceAdmin(id=ServiceAdminId.migrations)
authz = Authz(config.authz_config)
res = authz.client.ReadRelationships(
ReadRelationshipsRequest(
consistency=Consistency(fully_consistent=True),
relationship_filter=RelationshipFilter(
resource_type=ResourceType.project,
optional_relation=_Relation.project_namespace.value,
optional_subject_filter=SubjectFilter(subject_type=ResourceType.group.value),
),
)
)
async for rel in res:
logging.info(f"Checking project namespace - group relation {rel} for correct group ID")
project_id = rel.relationship.resource.object_id
project = await config.project_repo.get_project(api_user, project_id)
if project.namespace.kind != NamespaceKind.group:
continue
correct_group_id = project.namespace.underlying_resource_id
authzed_group_id = rel.relationship.subject.object.object_id
if authzed_group_id != correct_group_id:
logging.info(
f"The project namespace ID in Authzed {authzed_group_id} "
f"does not match the expected group ID {correct_group_id}, correcting it..."
)
authz.client.WriteRelationships(
WriteRelationshipsRequest(
updates=[
RelationshipUpdate(
operation=RelationshipUpdate.OPERATION_TOUCH,
relationship=Relationship(
resource=rel.relationship.resource,
relation=rel.relationship.relation,
subject=SubjectReference(
object=ObjectReference(
object_type=ResourceType.group.value, object_id=correct_group_id
)
),
),
),
RelationshipUpdate(
operation=RelationshipUpdate.OPERATION_DELETE,
relationship=rel.relationship,
),
]
)
)
3 changes: 2 additions & 1 deletion bases/renku_data_services/background_jobs/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from renku_data_services.authz.admin_sync import sync_admins_from_keycloak
from renku_data_services.authz.authz import Authz
from renku_data_services.background_jobs.config import SyncConfig
from renku_data_services.background_jobs.core import bootstrap_user_namespaces
from renku_data_services.background_jobs.core import bootstrap_user_namespaces, fix_mismatched_project_namespace_ids
from renku_data_services.migrations.core import run_migrations_for_app

logging.basicConfig(level=logging.INFO)
Expand All @@ -20,6 +20,7 @@ async def short_period_sync() -> None:
await bootstrap_user_namespaces(config)
await config.syncer.events_sync(config.kc_api)
await sync_admins_from_keycloak(config.kc_api, Authz(config.authz_config))
await fix_mismatched_project_namespace_ids(config)


async def long_period_sync() -> None:
Expand Down
13 changes: 9 additions & 4 deletions components/renku_data_services/authz/authz.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
from renku_data_services import base_models
from renku_data_services.authz.config import AuthzConfig
from renku_data_services.authz.models import Change, Member, MembershipChange, Role, Scope, Visibility
from renku_data_services.base_models.core import InternalServiceAdmin
from renku_data_services.errors import errors
from renku_data_services.namespace.models import Group, GroupUpdate, Namespace, NamespaceKind, NamespaceUpdate
from renku_data_services.project.models import Project, ProjectUpdate
Expand Down Expand Up @@ -232,7 +233,8 @@ async def decorated_function(
message=f"The user with ID {user.id} cannot perform operation {operation} "
f"on resource {resource_type} with ID {resource.id} or the resource does not exist."
)
return await f(self, user, *args, **kwargs, zed_token=zed_token)
kwargs["zed_token"] = zed_token
return await f(self, user, *args, **kwargs)

return decorated_function

Expand Down Expand Up @@ -267,7 +269,8 @@ async def decorated_function(
message=f"The user with ID {user.id} cannot perform operation {operation} on {resource_type.value} "
f"with ID {resource_id} or the resource does not exist."
)
return await f(self, user, resource_type, resource_id, *args, **kwargs, zed_token=zed_token)
kwargs["zed_token"] = zed_token
return await f(self, user, resource_type, resource_id, *args, **kwargs)

return decorated_function

Expand All @@ -291,12 +294,14 @@ def client(self) -> AsyncClient:

async def _has_permission(
self, user: base_models.APIUser, resource_type: ResourceType, resource_id: str | None, scope: Scope
) -> tuple[bool, ZedToken]:
) -> tuple[bool, ZedToken | None]:
"""Checks whether the provided user has a specific permission on the specific resource."""
if not resource_id:
raise errors.ProgrammingError(
message=f"Cannot check permissions on a resource of type {resource_type} with missing resource ID."
)
if isinstance(user, InternalServiceAdmin):
return True, None
res = _AuthzConverter.to_object(resource_type, resource_id)
sub = SubjectReference(
object=_AuthzConverter.to_object(ResourceType.user, user.id)
Expand Down Expand Up @@ -565,7 +570,7 @@ def _add_project(self, project: Project) -> _AuthzChange:
project_namespace = SubjectReference(
object=_AuthzConverter.user_namespace(project.namespace.id)
if project.namespace.kind == NamespaceKind.user
else _AuthzConverter.group(project.namespace.id)
else _AuthzConverter.group(project.namespace.underlying_resource_id)
)
project_in_platform = Relationship(
resource=project_res,
Expand Down
4 changes: 2 additions & 2 deletions components/renku_data_services/git/gitlab.py
Original file line number Diff line number Diff line change
Expand Up @@ -119,11 +119,11 @@ def _process_projects(
class DummyGitlabAPI:
"""Dummy gitlab API.
The user with name John Doe has admin access to project 123456 and member access to 999999.
The user with name Admin Doe has admin access to project 123456 and member access to 999999.
"""

_store = {
"John Doe": {
"Admin Doe": {
GitlabAccessLevel.MEMBER: ["999999", "123456"],
GitlabAccessLevel.ADMIN: ["123456"],
},
Expand Down
2 changes: 1 addition & 1 deletion components/renku_data_services/migrations/alembic.ini
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ keys = console
keys = generic

[logger_root]
level = WARN
level = INFO
handlers = console
qualname =

Expand Down
4 changes: 2 additions & 2 deletions components/renku_data_services/namespace/blueprints.py
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ async def _get_namespaces(
name=ns.name,
slug=ns.latest_slug if ns.latest_slug else ns.slug,
created_by=ns.created_by,
creation_date=ns.creation_date,
creation_date=None, # NOTE: we do not save creation date in the DB
namespace_kind=apispec.NamespaceKind(ns.kind.value),
).model_dump(exclude_none=True, mode="json")
for ns in nss
Expand All @@ -186,7 +186,7 @@ async def _get_namespace(_: Request, user: base_models.APIUser, slug: str) -> JS
name=ns.name,
slug=ns.latest_slug if ns.latest_slug else ns.slug,
created_by=ns.created_by,
creation_date=ns.creation_date,
creation_date=None, # NOTE: we do not save creation date in the DB
namespace_kind=apispec.NamespaceKind(ns.kind.value),
).model_dump(exclude_none=True, mode="json")
)
Expand Down
47 changes: 10 additions & 37 deletions components/renku_data_services/namespace/db.py
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,7 @@ async def insert_group(
message="The slug for the group should be unique but it already exists in the database",
detail="Please modify the slug field and then retry",
)
raise err
# NOTE: This is needed to populate the relationship fields in the group after inserting the ID above
await session.refresh(group)
return group.dump()
Expand Down Expand Up @@ -344,35 +345,15 @@ async def get_namespace_by_slug(self, user: base_models.APIUser, slug: str) -> m
raise errors.MissingResourceError(
message=f"The group with slug {slug} does not exist or you do not have permissions to view it"
)
return models.Namespace(
id=ns.id,
name=ns.group.name,
created_by=ns.group.created_by,
creation_date=ns.group.creation_date,
kind=models.NamespaceKind.group,
slug=old_ns.slug if old_ns else ns.slug,
latest_slug=ns.slug,
)
return ns.dump()
if not ns.user or not ns.user_id:
raise errors.ProgrammingError(message="Found a namespace that has no group or user associated with it.")
is_allowed = await self.authz.has_permission(user, ResourceType.user_namespace, ns.id, Scope.READ)
if not is_allowed:
raise errors.MissingResourceError(
message=f"The namespace with slug {slug} does not exist or you do not have permissions to view it"
)
name: str | None
if ns.user.first_name and ns.user.last_name:
name = f"{ns.user.first_name} {ns.user.last_name}"
else:
name = ns.user.first_name or ns.user.last_name
return models.Namespace(
id=ns.id,
name=name,
created_by=ns.user.keycloak_id,
kind=models.NamespaceKind.user,
slug=old_ns.slug if old_ns else ns.slug,
latest_slug=ns.slug,
)
return ns.dump()

async def _get_user_namespace(self, user_id: str) -> models.Namespace | None:
"""Get the namespace for a slug."""
Expand All @@ -382,19 +363,7 @@ async def _get_user_namespace(self, user_id: str) -> models.Namespace | None:
return None
if not ns.user or not ns.user_id:
raise errors.ProgrammingError(message="Found a namespace that has no user associated with it.")
name: str | None
if ns.user.first_name and ns.user.last_name:
name = f"{ns.user.first_name} {ns.user.last_name}"
else:
name = ns.user.first_name or ns.user.last_name
return models.Namespace(
id=ns.id,
name=name,
created_by=ns.user.keycloak_id,
kind=models.NamespaceKind.user,
slug=ns.slug,
latest_slug=ns.slug,
)
return ns.dump()

async def _insert_user_namespace(
self, session: AsyncSession, user: schemas.UserORM, retry_enumerate: int = 0, retry_random: bool = False
Expand All @@ -411,12 +380,14 @@ async def _insert_user_namespace(
try:
async with session.begin_nested():
session.add(ns)
await session.flush()
except IntegrityError:
if retry_enumerate == 0:
raise errors.ValidationError(message=f"The user namespace slug {slug.value} already exists")
continue
else:
return models.Namespace(ns.id, slug=ns.slug, created_by=ns.user_id, kind=models.NamespaceKind.user)
await session.refresh(ns)
return ns.dump()
if not retry_random:
raise errors.ValidationError(
message=f"Cannot create generate a unique namespace slug for the user with ID {user.keycloak_id}"
Expand All @@ -427,4 +398,6 @@ async def _insert_user_namespace(
slug = base_models.Slug.from_name(original_slug.value.lower() + suffix)
ns = schemas.NamespaceORM(slug.value, user_id=user.keycloak_id)
session.add(ns)
return models.Namespace(ns.id, slug=ns.slug, created_by=ns.user_id, kind=models.NamespaceKind.user)
await session.flush()
await session.refresh(ns)
return ns.dump()
6 changes: 3 additions & 3 deletions components/renku_data_services/namespace/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,10 @@ class Namespace:
id: str
slug: str
kind: NamespaceKind
name: str | None = None
creation_date: datetime | None = None
created_by: str | None = None
created_by: str
underlying_resource_id: str # The user or group ID depending on the Namespace kind
latest_slug: str | None = None
name: str | None = None


@dataclass
Expand Down
Loading

0 comments on commit dd68ce2

Please sign in to comment.