Allow users to configure permissions for their assets (#638)

* add endpoint for managing asset permissions

* Add admin role for modifying resources

* Update assign permissions to allow the use of subject identifier

* Allow deletion of permission by subject identifier

* Support removal of permissions by administrators

* Use the service account to query for the user

* Add admin user to aoid realm and view-users role to aiod-api client

* Add test for adding user by nickname

* Force exact name match

* add doc of new users and roles

* minor corrections/clarifications

Author: PGijsbers

.env

@@ -18,6 +18,7 @@ REDIRECT_URIS=http://${HOSTNAME}/docs/oauth2-redirect
 POST_LOGOUT_REDIRECT_URIS=http://${HOSTNAME}/aiod-auth/realms/aiod/protocol/openid-connect/logout
 AIOD_KEYCLOAK_PORT=8080
 REVIEWER_ROLE_NAME=review_aiod_resources
+ADMIN_ROLE_NAME=admin_aiod_resources
 
 EGICHECKINALIAS=
 

authentication/import/aiod-realm.json

@@ -53,6 +53,14 @@
       "clientRole" : false,
       "containerId" : "3df7e07d-ebbd-41c4-bc0c-1ba0e1a40ac5",
       "attributes" : { }
+    }, {
+      "id" : "22918626-68f6-4bc4-802a-96f8262a2d44",
+      "name" : "admin_aiod_resources",
+      "description" : "Allow the modification or deletion of any asset on the platform, as well as assigning or removing any permissions.",
+      "composite" : false,
+      "clientRole" : false,
+      "containerId" : "3df7e07d-ebbd-41c4-bc0c-1ba0e1a40ac5",
+      "attributes" : { }
     }, {
       "id" : "25a45d6d-fe0d-4855-945f-f5a27ec86ad0",
       "name" : "uma_authorization",
@@ -1424,7 +1432,7 @@
       "subType" : "authenticated",
       "subComponents" : { },
       "config" : {
-        "allowed-protocol-mapper-types" : [ "saml-user-attribute-mapper", "oidc-full-name-mapper", "oidc-usermodel-property-mapper", "saml-role-list-mapper", "oidc-address-mapper", "oidc-sha256-pairwise-sub-mapper", "saml-user-property-mapper", "oidc-usermodel-attribute-mapper" ]
+        "allowed-protocol-mapper-types" : [ "oidc-sha256-pairwise-sub-mapper", "oidc-usermodel-property-mapper", "oidc-address-mapper", "saml-user-property-mapper", "saml-role-list-mapper", "saml-user-attribute-mapper", "oidc-full-name-mapper", "oidc-usermodel-attribute-mapper" ]
       }
     }, {
       "id" : "10f8b9b2-1038-4c98-b7a5-a9ac88fed69e",
@@ -1466,7 +1474,7 @@
       "subType" : "anonymous",
       "subComponents" : { },
       "config" : {
-        "allowed-protocol-mapper-types" : [ "saml-user-attribute-mapper", "oidc-usermodel-attribute-mapper", "oidc-address-mapper", "oidc-sha256-pairwise-sub-mapper", "oidc-full-name-mapper", "saml-user-property-mapper", "saml-role-list-mapper", "oidc-usermodel-property-mapper" ]
+        "allowed-protocol-mapper-types" : [ "oidc-usermodel-attribute-mapper", "oidc-sha256-pairwise-sub-mapper", "oidc-usermodel-property-mapper", "saml-user-attribute-mapper", "oidc-address-mapper", "oidc-full-name-mapper", "saml-user-property-mapper", "saml-role-list-mapper" ]
       }
     }, {
       "id" : "1d21f027-e0ae-4b80-b95e-f21d9426f115",

authentication/import/aiod-users-0.json

@@ -1,6 +1,26 @@
 {
   "realm" : "aiod",
   "users" : [ {
+    "id" : "ca9c5ba7-0c53-437e-98d0-8bba5eaac5f0",
+    "username" : "admin",
+    "emailVerified" : false,
+    "createdTimestamp" : 1761654821859,
+    "enabled" : true,
+    "totp" : false,
+    "credentials" : [ {
+      "id" : "20e60836-478e-4e77-bfcf-7730324cf1cd",
+      "type" : "password",
+      "userLabel" : "My password",
+      "createdDate" : 1761654837022,
+      "secretData" : "{\"value\":\"YDfyAyfKhCIdefMBzk/aieDFuuaLrhzYtAPPeWlipnLR062Pw+gGO0XfpcZbypiXBsuKI4O5QXmJy6mnwHZWgw==\",\"salt\":\"gQLs0a7jinjZC9V9n7SFdg==\",\"additionalParameters\":{}}",
+      "credentialData" : "{\"hashIterations\":210000,\"algorithm\":\"pbkdf2-sha512\",\"additionalParameters\":{}}"
+    } ],
+    "disableableCredentialTypes" : [ ],
+    "requiredActions" : [ ],
+    "realmRoles" : [ "admin_aiod_resources", "default-roles-aiod" ],
+    "notBefore" : 0,
+    "groups" : [ ]
+  }, {
     "id" : "5e827ac1-259d-406a-b3de-cf4f79f88364",
     "username" : "reviewer",
     "emailVerified" : false,
@@ -33,6 +53,7 @@
     "requiredActions" : [ ],
     "realmRoles" : [ "default-roles-aiod" ],
     "clientRoles" : {
+      "realm-management" : [ "view-users" ],
       "aiod-api" : [ "uma_protection" ]
     },
     "notBefore" : 0,

docs/developer/authentication.md

@@ -4,9 +4,11 @@ For authentication, we use a [keycloak](https://www.keycloak.org) service.
 For development, make sure to use the `USE_LOCAL_DEV=true` environment variable so that the local
 keycloak server is configured with default users:
 
-| User | Password | Role(s)                                               | Comment |
-|------|----------|-------------------------------------------------------|---------|
-| user | password | default-roles-aiod, offline_access, uma_authorization |         |
+| User     | Password | Role(s)                                               | Comment |
+|----------|----------|-------------------------------------------------------|---------|
+| user     | password | default-roles-aiod, offline_access, uma_authorization |         |
+| reviewer | password | default-roles-aiod, review_aiod_resources             |         |
+| admin    | password | default-roles-aiod, admin_aiod_resources              |         |
 
 For a description of the roles, see ["AIoD Keycloak Roles"](../hosting/authentication.md#roles).
 With the local development configuration, you will only be able to authenticate with keycloak users (OAuth2, password) not by other means.

docs/hosting/authentication.md

@@ -43,6 +43,7 @@ The normal flow for granting individual users permissions for individual assets
 These are the roles the metadata catalogue uses (`*` in a role indicates its defined for each asset type individually):
 
  * `review_aiod_resources`: identifies the user as having permission to view asset submissions and review them.
+ * `admin_aiod_resources`: treats the user as an owner of every asset on the platform.
  * `read_*`: allows the user read access to all assets on the platform, regardless of the asset-specific permissions.
  * `update_*`: allows the user update permission for all assets on the platform, regardless of the asset-specific permissions.
  * `delete_*`: allows the user delete permission for all assets on the platform, regardless of the asset-specific permissions.

src/authentication.py

@@ -19,13 +19,14 @@
 """
 
 import dataclasses
+import functools
 import logging
 import os
 
 from dotenv import load_dotenv
 from fastapi import HTTPException, Security, status
 from fastapi.security import OpenIdConnect
-from keycloak import KeycloakOpenID
+from keycloak import KeycloakOpenID, KeycloakAdmin
 
 from config import KEYCLOAK_CONFIG
 
@@ -35,6 +36,7 @@
 oidc = OpenIdConnect(openIdConnectUrl=KEYCLOAK_CONFIG.get("openid_connect_url"), auto_error=False)
 
 REVIEWER_ROLE = os.getenv("REVIEWER_ROLE_NAME")
+ADMIN_ROLE = os.getenv("ADMIN_ROLE_NAME")
 client_secret = os.getenv("KEYCLOAK_CLIENT_SECRET")
 
 keycloak_openid = KeycloakOpenID(
@@ -46,6 +48,16 @@
 )
 
 
+@functools.cache
+def keycloak_api() -> KeycloakAdmin:
+    return KeycloakAdmin(
+        server_url=KEYCLOAK_CONFIG.get("server_url"),
+        realm_name=KEYCLOAK_CONFIG.get("realm"),
+        client_id=KEYCLOAK_CONFIG.get("client_id"),
+        client_secret_key=os.getenv("KEYCLOAK_CLIENT_SECRET"),
+    )
+
+
 def assert_required_settings_configured() -> None:
     # These variables are required for the API to function, so we provide context beyond KeyError.
     # Should be managed together with other settings in the future (#67)
@@ -75,6 +87,10 @@ def is_connector_for_platform(self, platform_name: str) -> bool:
     def is_reviewer(self):
         return REVIEWER_ROLE in self.roles
 
+    @property
+    def is_admin(self):
+        return ADMIN_ROLE in self.roles
+
     @property
     def is_connector(self) -> bool:
         """
@@ -160,6 +176,24 @@ async def get_user_or_raise(token=Security(oidc)) -> KeycloakUser:
         ) from err
 
 
+def get_user_by_username(username: str) -> KeycloakUser | None:
+    """Gets the keycloak user by its username. `user.roles` will always be empty."""
+    users = keycloak_api().get_users(query={"username": username, "exact": True})
+    if not users:
+        return None
+
+    if len(users) > 1:
+        raise NotImplementedError(
+            f"Multiple users with username {username} found, expected behavior undefined."
+        )
+    user = users[0]
+    return KeycloakUser(
+        name=user.get("username"),
+        roles=set(),  # Not included with the call
+        _subject_identifier=user.get("id"),
+    )
+
+
 class InvalidUserError(Exception):
     """Raise an error on invalid userinfo or inactive user."""
 

src/database/authorization.py

@@ -55,7 +55,7 @@ class Permission(SQLModel, table=True):  # type: ignore [call-arg]
 def _user_has_permission(
     user: KeycloakUser, aiod_entry: AIoDEntryORM, *, at_least: PermissionType
 ) -> bool:
-    return any(
+    return user.is_admin or any(
         permission.user_identifier == user._subject_identifier and permission.type_ >= at_least
         for permission in aiod_entry.permissions
     )

src/routers/asset_router.py

@@ -1,19 +1,81 @@
+import re
 from http import HTTPStatus
-from fastapi import APIRouter, Depends, HTTPException
+from fastapi import APIRouter, Depends, HTTPException, Body
 from sqlmodel import Session
 
-from authentication import KeycloakUser, get_user_or_none
+from authentication import KeycloakUser, get_user_or_none, get_user_or_raise, get_user_by_username
+from database.authorization import user_can_administer, set_permission, Permission, register_user
 from database.session import get_session
 from routers.helper_functions import get_asset_type_by_abbreviation
 from routers.resource_routers import versioned_routers
 from database.model.concept.aiod_entry import EntryStatus
-from database.authorization import user_can_read
+from database.authorization import user_can_read, PermissionType
 from versioning import Version
 
 
 def create(url_prefix: str = "", version: Version = Version.LATEST) -> APIRouter:
     router = APIRouter()
 
+    @router.post(
+        "/assets/permissions",
+        tags=["Assets"],
+        description="Manage permissions that a user has for an asset.",
+    )
+    def add_or_update_permission(
+        asset_identifier: str = Body(
+            description="The identifier of the asset for which to update the permission."
+        ),
+        user: str = Body(
+            description="The username or subject identifier of the user.",
+            examples=["jsmith01", "4a80f256-3928-4cfa-ba66-5e22bb36fc01"],
+        ),
+        permission_type: PermissionType | None = Body(
+            description="The permission to add for the user. "
+            "If not set, their permissions will be removed.",
+            default=None,
+        ),
+        session: Session = Depends(get_session),
+        current_user: KeycloakUser = Depends(get_user_or_raise),
+    ):
+        _, resource = get_asset_by_identifier(asset_identifier, session)
+        if not user_can_administer(current_user, resource.aiod_entry):
+            raise HTTPException(
+                status_code=HTTPStatus.FORBIDDEN,
+                detail=f"You are not allowed to update permissions for asset {asset_identifier}.",
+            )
+        sub_pattern = r"\S{8}(-\S{4}){3}-\S{12}"
+        if re.match(sub_pattern, user):
+            other = KeycloakUser(name="unknown", roles=set(), _subject_identifier=user)
+        else:
+            other = get_user_by_username(user)  # type: ignore[assignment]
+        if not other:
+            raise HTTPException(
+                status_code=HTTPStatus.NOT_FOUND,
+                detail=f"User with name {user!r} not found.",
+            )
+
+        register_user(other, session)  # Should be replaced by KC pushing to REST API
+        if other._subject_identifier == current_user._subject_identifier:
+            # This request is more likely to be an accident than on purpose.
+            # Additionally, we do not want to allow people to accidentally remove all
+            # administrators from an asset which this restriction ensures.
+            raise HTTPException(
+                status_code=HTTPStatus.UNPROCESSABLE_ENTITY,
+                detail="You cannot change permissions that pertain to yourself.",
+            )
+        if permission_type:
+            set_permission(other, resource.aiod_entry, session, type_=permission_type)
+            session.commit()
+        else:
+            key = {
+                "user_identifier": other._subject_identifier,
+                "aiod_entry_identifier": resource.aiod_entry.identifier,
+            }
+            permission = session.get(Permission, key)
+            if permission:
+                session.delete(permission)
+                session.commit()
+
     @router.get(
         f"/assets/{{identifier}}",
         tags=["Assets"],
@@ -27,17 +89,7 @@ def asset(
         """
         Get the resource identified by AIoD identifier, return in aiod schema.
         """
-        asset_type_map = get_asset_type_by_abbreviation()
-        prefix = identifier.split("_")[0]
-        model_class = asset_type_map.get(prefix)
-
-        if not model_class:
-            raise HTTPException(
-                status_code=HTTPStatus.NOT_FOUND,
-                detail=f"Unknown asset type with identifier '{identifier}'",
-            )
-
-        resource = session.get(model_class, identifier)
+        model_class, resource = get_asset_by_identifier(identifier, session)
 
         if not resource or resource.date_deleted is not None:
             raise HTTPException(
@@ -66,4 +118,16 @@ def asset(
             detail=f"No router found to deserialize asset of type '{model_class.__name__}'",
         )
 
+    def get_asset_by_identifier(identifier, session):
+        asset_type_map = get_asset_type_by_abbreviation()
+        prefix = identifier.split("_")[0]
+        model_class = asset_type_map.get(prefix)
+        if not model_class:
+            raise HTTPException(
+                status_code=HTTPStatus.NOT_FOUND,
+                detail=f"Unknown asset type with identifier '{identifier}'",
+            )
+        resource = session.get(model_class, identifier)
+        return model_class, resource
+
     return router

src/routers/review_router.py

@@ -192,7 +192,7 @@ def _review_resource(
     user: KeycloakUser = Depends(get_user_or_raise),
     session: Session = Depends(get_session),
 ):
-    if not user.is_reviewer:
+    if not (user.is_reviewer or user.is_admin):
         raise HTTPException(
             status_code=status.HTTP_403_FORBIDDEN,
             detail="You must have reviewing privileges to use this endpoint.",

src/tests/authorization/test_authorization.py

@@ -6,20 +6,69 @@
 import pytest
 from starlette.testclient import TestClient
 
-from authentication import KeycloakUser
+from authentication import KeycloakUser, ADMIN_ROLE
 from database.authorization import (
-    PermissionType, user_can_read, user_can_write, user_can_administer, set_permission,
+    PermissionType, user_can_read, user_can_write, user_can_administer, set_permission, Permission,
 )
 from database.model.concept.aiod_entry import EntryStatus
 from database.review import Decision, ReviewCreate
 from database.session import DbSession
 from database.model.knowledge_asset.publication import Publication
 from routers.review_router import ListMode
 from tests.testutils.users import ALICE, BOB, REVIEWER, _register_user_in_db, \
-    logged_in_user, register_asset
+    logged_in_user, register_asset, kc_user_with_roles
 from versioning import Version
 
 
+def test_admin_can_delete_asset(client, publication):
+    identifier = register_asset(publication, owner=ALICE, status=EntryStatus.PUBLISHED)
+    with logged_in_user(kc_user_with_roles(ADMIN_ROLE)):
+        response = client.delete(
+            f"/publications/{identifier}",  headers={"Authorization": "Fake token"}
+        )
+        assert response.status_code == HTTPStatus.OK, response.json()
+
+def test_admin_can_remove_permission(client, publication):
+    identifier = register_asset(publication, owner=ALICE, status=EntryStatus.PUBLISHED)
+    with logged_in_user(kc_user_with_roles(ADMIN_ROLE)):
+        response = client.post(
+            f"/assets/permissions",
+            json={
+                "asset_identifier": identifier,
+                "user": ALICE._subject_identifier,
+                "permission_type": None,
+            },
+            headers={"Authorization": "Fake token"}
+        )
+        assert response.status_code == HTTPStatus.OK, response.json()
+
+    with DbSession() as session:
+        permission = session.get(Permission, {"user_identifier": ALICE._subject_identifier, "aiod_entry_identifier": 1})
+        assert permission is None
+
+
+def test_admin_can_change_permission(client, publication):
+    identifier = register_asset(publication, owner=ALICE, status=EntryStatus.PUBLISHED)
+    _register_user_in_db(BOB)
+
+    with logged_in_user(kc_user_with_roles(ADMIN_ROLE)):
+        response = client.post(
+            f"/assets/permissions",
+            json={
+                "asset_identifier": identifier,
+                "user": BOB._subject_identifier,
+                "permission_type": PermissionType.ADMIN.value,
+            },
+            headers={"Authorization": "Fake token"}
+        )
+        assert response.status_code == HTTPStatus.OK, response.json()
+
+    with DbSession() as session:
+        permission = session.get(Permission, {"user_identifier": BOB._subject_identifier, "aiod_entry_identifier": 1})
+        assert permission is not None
+        assert permission.type_ == PermissionType.ADMIN
+
+
 def test_user_must_be_logged_in_to_publish(client, publication):
     response = client.post("/publications", content=publication.json(), headers=None)
     assert response.status_code == HTTPStatus.UNAUTHORIZED