Feat/read permissions (#509)

Author: PGijsbers

src/database/authorization.py

@@ -3,11 +3,10 @@
 
 import sqlalchemy
 from sqlalchemy import Column
-from sqlmodel import SQLModel, Field, Relationship, select, Session, and_
+from sqlmodel import SQLModel, Field, Relationship, select, Session
 
 from authentication import KeycloakUser
 from database.model.concept.aiod_entry import AIoDEntryORM, EntryStatus
-from database.model.concept.concept import AIoDConcept
 
 
 class User(SQLModel, table=True):  # type: ignore [call-arg]

src/database/model/resource_bundle/resource_bundle.py

@@ -1,4 +1,4 @@
-from typing import List, Optional
+from typing import List
 from sqlmodel import Relationship
 
 from database.model.ai_resource.resource import AIResource, AIResourceBase

src/routers/resource_ai_asset_router.py

@@ -1,8 +1,9 @@
 from typing import Annotated
 
-from fastapi import APIRouter, HTTPException, status, Path
+from fastapi import APIRouter, HTTPException, status, Path, Depends
 from fastapi.responses import RedirectResponse
 
+from authentication import get_user_or_none, KeycloakUser
 from database.model.ai_asset.ai_asset import AIAsset
 from .resource_router import ResourceRouter
 
@@ -56,9 +57,13 @@ def get_resource_content(
                 int,
                 Path(description=f"The index of the distribution within the {self.resource_name}"),
             ],
+            user: KeycloakUser | None = Depends(get_user_or_none),
         ):
             metadata: AIAsset = self.get_resource(
-                identifier=identifier, schema="aiod", platform=None
+                identifier=identifier,
+                schema="aiod",
+                platform=None,
+                user=user,
             )  # type: ignore
 
             distributions = metadata.distribution
@@ -88,8 +93,9 @@ def get_resource_content_default(
             identifier: Annotated[
                 str, Path(description=f"The identifier of the {self.resource_name}")
             ],
+            user: KeycloakUser | None = Depends(get_user_or_none),
         ):
-            return get_resource_content(identifier=identifier, distribution_idx=0)
+            return get_resource_content(identifier=identifier, distribution_idx=0, user=user)
 
         if default:
             return get_resource_content_default

src/routers/resource_router.py

@@ -2,6 +2,7 @@
 import datetime
 import traceback
 from functools import partial
+from http import HTTPStatus
 from typing import Annotated, Any, Literal, Sequence, Type, TypeVar, Union
 from wsgiref.handlers import format_date_time
 
@@ -13,14 +14,14 @@
 from starlette.responses import JSONResponse
 
 from authentication import KeycloakUser, get_user_or_none, get_user_or_raise
-from config import KEYCLOAK_CONFIG
 from converters.schema_converters.schema_converter import SchemaConverter
 from database.authorization import (
     user_can_administer,
     set_permission,
     register_user,
     PermissionType,
     user_can_write,
+    user_can_read,
 )
 from database.model.ai_resource.resource import AIResource
 from database.model.concept.aiod_entry import AIoDEntryORM, EntryStatus
@@ -32,7 +33,7 @@
     resource_read,
 )
 from database.model.serializers import deserialize_resource_relationships
-from database.review import Decision, Review, Submission, ReviewCreate, SubmissionCreate
+from database.review import Submission, SubmissionCreate
 from database.session import DbSession
 from dependencies.filtering import ResourceFilters, ResourceFiltersParams
 from dependencies.pagination import Pagination, PaginationParams
@@ -211,7 +212,7 @@ def get_resources(
         user: KeycloakUser | None = None,
         platform: str | None = None,
     ):
-        """Fetch all resources of this platform in given schema, using pagination"""
+        """Fetch all published resources of this platform in given schema, using pagination"""
         _raise_error_on_invalid_schema(self._possible_schemas, schema)
         with DbSession(autoflush=False) as session:
             try:
@@ -244,6 +245,17 @@ def get_resource(
                 resource: Any = self._retrieve_resource_and_post_process(
                     session, identifier, user, platform=platform
                 )
+                if resource.aiod_entry.status != EntryStatus.PUBLISHED:
+                    if user is None:
+                        raise HTTPException(
+                            status_code=HTTPStatus.UNAUTHORIZED,
+                            detail="This asset is not published. It requires authentication to access.",
+                        )
+                    if not user_can_read(user, resource.aiod_entry):
+                        raise HTTPException(
+                            status_code=HTTPStatus.FORBIDDEN,
+                            detail="You are not allowed to view this resource.",
+                        )
                 if schema != "aiod":
                     return self.schema_converters[schema].convert(session, resource)
                 return self._wrap_with_headers(self.resource_class_read.from_orm(resource))
@@ -276,7 +288,7 @@ def get_resources(
 
     def get_resource_count_func(self):
         """
-        Gets the total number of resources from the database.
+        Gets the total number of published resources from the database.
         This function returns a function (instead of being that function directly) because the
         docstring and the variables are dynamic, and used in Swagger.
         """
@@ -291,7 +303,11 @@ def get_resource_count(
                     if not detailed:
                         return (
                             session.query(self.resource_class)
-                            .where(is_(self.resource_class.date_deleted, None))
+                            .join(self.resource_class.aiod_entry, isouter=True)
+                            .where(
+                                is_(self.resource_class.date_deleted, None),
+                                AIoDEntryORM.status == EntryStatus.PUBLISHED,
+                            )
                             .count()
                         )
                     else:
@@ -300,7 +316,11 @@ def get_resource_count(
                                 self.resource_class.platform,
                                 func.count(self.resource_class.identifier),
                             )
-                            .where(is_(self.resource_class.date_deleted, None))
+                            .join(self.resource_class.aiod_entry, isouter=True)
+                            .where(
+                                is_(self.resource_class.date_deleted, None),
+                                AIoDEntryORM.status == EntryStatus.PUBLISHED,
+                            )
                             .group_by(self.resource_class.platform)
                             .all()
                         )
@@ -612,8 +632,8 @@ def _retrieve_resources(
         platform: str | None = None,
     ) -> Sequence[type[RESOURCE_MODEL]]:
         """
-        Retrieve a sequence of resources from the database based on the provided identifier,
-        platform and resource filters (if applicable).
+        Retrieve a sequence of published resources from the database based on the
+        provided identifier, platform and resource filters (if applicable).
         """
         where_clause = and_(
             is_(self.resource_class.date_deleted, None),
@@ -624,6 +644,7 @@ def _retrieve_resources(
             AIoDEntryORM.date_modified < resource_filters.date_modified_before
             if resource_filters.date_modified_before is not None
             else True,
+            AIoDEntryORM.status == EntryStatus.PUBLISHED,
         )
         query = (
             select(self.resource_class)

src/tests/authorization/test_authorization.py

@@ -36,7 +36,10 @@ def test_new_asset_is_draft(client, publication, mocked_privileged_token: Mock):
         )
         assert response.status_code == HTTPStatus.OK, response.json()
 
-        server_data = client.get(f"/publications/v1/{response.json()['identifier']}").json()
+        server_data = client.get(
+            f"/publications/v1/{response.json()['identifier']}",
+            headers={"Authorization": "Fake token"},
+        ).json()
         assert server_data["aiod_entry"]["status"] == EntryStatus.DRAFT
 
 
@@ -291,7 +294,10 @@ def test_user_can_edit_asset_in_draft(publication, client):
             headers={"Authorization": "Fake token"},
         )
         assert response.status_code == HTTPStatus.OK, response.json()
-        updated_publication = client.get(f"/publications/v1/{identifier}").json()
+        updated_publication = client.get(
+            f"/publications/v1/{identifier}",
+            headers={"Authorization": "Fake token"},
+        ).json()
         assert updated_publication["name"] == new_name
 
 
@@ -351,7 +357,12 @@ def test_reviewer_can_reject_submission(publication, client):
         )
         assert response.status_code == HTTPStatus.OK, response.json()
 
-    response = client.get(f"/publications/v1/{identifier}")
+    # Because the rejected asset is back in draft status, it requires authentication to access.
+    with logged_in_user(ALICE):
+        response = client.get(
+            f"/publications/v1/{identifier}",
+            headers={"Authorization": "Fake token"},
+        )
     assert response.status_code == HTTPStatus.OK, response.json()
     assert response.json()["aiod_entry"]["status"] == EntryStatus.DRAFT
 

src/tests/authorization/test_status_update.py

@@ -1,11 +1,11 @@
 import copy
 from http import HTTPStatus
-from unittest.mock import Mock
 
 import pytest
 from starlette.testclient import TestClient
 
 from database.model.concept.aiod_entry import EntryStatus
+from tests.testutils.users import logged_in_user, ALICE
 
 
 @pytest.fixture()
@@ -19,31 +19,34 @@ def publication_body(body_asset: dict) -> dict:
     return body
 
 
-def test_entry_status_can_update_on_put(
+def test_entry_status_does_not_update_on_put(
     client: TestClient,
-    mocked_privileged_token: Mock,
     publication_body: dict,
 ):
-    response = client.post(
-        "/publications/v1", json=publication_body, headers={"Authorization": "Fake token"}
-    )
+    with logged_in_user(ALICE):
+        response = client.post(
+            "/publications/v1", json=publication_body, headers={"Authorization": "Fake token"}
+        )
     assert response.status_code == 200, response.json()
 
     # Default is DRAFT
-    response = client.get("/publications/v1/1")
+    with logged_in_user(ALICE):
+        response = client.get("/publications/v1/1", headers={"Authorization": "Fake token"})
     assert response.status_code == 200, response.json()
     assert response.json()["aiod_entry"]["status"] == EntryStatus.DRAFT
     identifier = response.json()["identifier"]
 
     publication_body["aiod_entry"]["status"] = EntryStatus.PUBLISHED
-    response = client.put(
-        f"/publications/v1/{identifier}",
-        json=publication_body,
-        headers={"Authorization": "Fake token"},
-    )
+    with logged_in_user(ALICE):
+        response = client.put(
+            f"/publications/v1/{identifier}",
+            json=publication_body,
+            headers={"Authorization": "Fake token"},
+        )
     assert response.status_code == HTTPStatus.UNPROCESSABLE_ENTITY, response.json()
 
     # Status is not updated to published
-    response = client.get(f"/publications/v1/{identifier}")
+    with logged_in_user(ALICE):
+        response = client.get("/publications/v1/1", headers={"Authorization": "Fake token"})
     assert response.status_code == 200, response.json()
     assert response.json()["aiod_entry"]["status"] == EntryStatus.DRAFT

src/tests/database/deletion/test_hard_delete.py

@@ -5,7 +5,7 @@
 from database.deletion import hard_delete
 from database.model.concept.aiod_entry import AIoDEntryORM, EntryStatus
 from database.session import DbSession
-from tests.testutils.test_resource import factory, TestResource
+from tests.testutils.test_resource import factory_test_resource, TestResource
 
 
 def test_hard_delete():
@@ -14,34 +14,18 @@ def test_hard_delete():
     with DbSession() as session:
         session.add_all(
             [
-                factory(
-                    title="test_resource_to_keep",
-                    platform="example",
-                    platform_resource_identifier=1,
-                    status=EntryStatus.DRAFT,
-                    date_deleted=None,
-                ),
-                factory(
-                    title="test_resource_to_keep_2",
-                    platform="example",
-                    platform_resource_identifier=2,
-                    status=EntryStatus.DRAFT,
-                    date_deleted=now,
-                ),
-                factory(
-                    title="my_test_resource",
-                    platform="example",
-                    platform_resource_identifier=3,
-                    status=EntryStatus.DRAFT,
-                    date_deleted=deletion_time,
-                ),
-                factory(
-                    title="second_test_resource",
-                    platform="example",
-                    platform_resource_identifier=4,
-                    status=EntryStatus.DRAFT,
-                    date_deleted=deletion_time,
-                ),
+                factory_test_resource(title="test_resource_to_keep", status=EntryStatus.DRAFT,
+                                      platform="example", platform_resource_identifier=1,
+                                      date_deleted=None),
+                factory_test_resource(title="test_resource_to_keep_2", status=EntryStatus.DRAFT,
+                                      platform="example", platform_resource_identifier=2,
+                                      date_deleted=now),
+                factory_test_resource(title="my_test_resource", status=EntryStatus.DRAFT,
+                                      platform="example", platform_resource_identifier=3,
+                                      date_deleted=deletion_time),
+                factory_test_resource(title="second_test_resource", status=EntryStatus.DRAFT,
+                                      platform="example", platform_resource_identifier=4,
+                                      date_deleted=deletion_time),
             ]
         )
         session.commit()