Restrict setting platfrom and platfrom_resource_identifier (#516)

* set platform to aiod

* set platform_resource_identifier to aiod_entry_identifier

* donot expose platform using POST endpoint

* ruff formatting

* create connector user in keycloak

* remove setting platform and platform_resource_identifier in test env

* platform name set using platform_names.py

* update test_authorisation

* not set platfrom and platform_resource_identifier in test

* update tests post and get_count

* do not set platform and platfrom_resource_identifier in test

* debug test_router_contact

* diable upload test

* disable uploader for this PR

* revert concept.py to original form

* correct error code

* add additional test

* simplfy

* remove deprecated v1, fix bugs, add env variable to docker-compose.yaml

* fix test post merge conflict resolution

* incorrect merge conflict resolution

* contant method for status code, minor fix

* update test

* platform_resource_identifer = identifier, connector platform name is verified

* small fixes

* set platform to aiod

* set platform_resource_identifier to aiod_entry_identifier

* donot expose platform using POST endpoint

* ruff formatting

* create connector user in keycloak

* remove setting platform and platform_resource_identifier in test env

* platform name set using platform_names.py

* update test_authorisation

* not set platfrom and platform_resource_identifier in test

* update tests post and get_count

* do not set platform and platfrom_resource_identifier in test

* debug test_router_contact

* diable upload test

* disable uploader for this PR

* revert concept.py to original form

* correct error code

* add additional test

* simplfy

* remove deprecated v1, fix bugs, add env variable to docker-compose.yaml

* fix test post merge conflict resolution

* incorrect merge conflict resolution

* contant method for status code, minor fix

* update test

* platform_resource_identifer = identifier, connector platform name is verified

* small fixes

* remove old unused code

* improve test_router_get_count

* remove commented code, minor fixes

* resolve pr comments

* remove dependency injection

* user permission check to set platform is moved to register_resource_func()

Author: Taniya-Das

src/authentication.py

@@ -34,7 +34,6 @@
 
 oidc = OpenIdConnect(openIdConnectUrl=KEYCLOAK_CONFIG.get("openid_connect_url"), auto_error=False)
 
-
 REVIEWER_ROLE = os.getenv("REVIEWER_ROLE_NAME")
 client_secret = os.getenv("KEYCLOAK_CLIENT_SECRET")
 
@@ -66,10 +65,23 @@ def has_role(self, role: str) -> bool:
     def has_any_role(self, *roles: str) -> bool:
         return bool(set(roles) & self.roles)
 
+    def is_connector_for_platform(self, platform_name: str) -> bool:
+        """
+        Check if the user is a connector for a specific platform.
+        """
+        return f"platform_{platform_name}" in self.roles
+
     @property
     def is_reviewer(self):
         return REVIEWER_ROLE in self.roles
 
+    @property
+    def is_connector(self) -> bool:
+        """
+        Check if the user is a connector.
+        """
+        return any(role.startswith("platform_") for role in self.roles)
+
 
 async def _get_user(token) -> KeycloakUser:
     """

src/database/model/concept/concept.py

@@ -31,6 +31,7 @@ class AIoDConceptBase(SQLModel):
         schema_extra={"example": PlatformName.example},
         foreign_key="platform.name",
     )
+
     platform_resource_identifier: str | None = Field(
         max_length=NORMAL,
         description="A unique identifier issued by the external platform that's specified in "
@@ -78,6 +79,7 @@ class AIoDConcept(AIoDConceptBase):
         foreign_key=AIoDEntryORM.__tablename__ + ".identifier",
         unique=True,
     )
+
     aiod_entry: AIoDEntryORM = Relationship()
 
     _id_generator: Callable[[], str] | None = None

src/main.py

@@ -84,7 +84,7 @@ def counts() -> dict:
         + parent_routers.router_list
         + enum_routers.router_list
         + search_routers.router_list
-        + uploader_routers.router_list
+        # + uploader_routers.router_list # TODO: uploaders disabled until issue #538 is resolved.
         + [review_router, user_router]
     ):
         app.include_router(router.create(url_prefix))

src/routers/resource_router.py

@@ -468,10 +468,36 @@ def register_resource(
             resource_create: clz_create,  # type: ignore
             user: KeycloakUser = Depends(get_user_or_raise),
         ):
+            platform = getattr(resource_create, "platform", None)
+            platform_resource_identifier = getattr(
+                resource_create, "platform_resource_identifier", None
+            )
+            if user.is_connector:
+                # Check if connector belongs to the specific platform it is registering the resource for.
+                if platform is None or not user.is_connector_for_platform(platform):
+                    raise HTTPException(
+                        status_code=HTTPStatus.FORBIDDEN,
+                        detail=f"No permission to upload assets for {platform} platform.",
+                    )
+                if platform_resource_identifier is None:
+                    raise HTTPException(
+                        status_code=HTTPStatus.FORBIDDEN,
+                        detail=f"Platform resource identifier may not be none.",
+                    )
+
+            # 2. Normal user: must NOT provide platform/platform_resource_identifier
+            else:
+                if platform is not None or platform_resource_identifier is not None:
+                    raise HTTPException(
+                        status_code=HTTPStatus.FORBIDDEN,
+                        detail="No permission to set platform or platform resource identifier.",
+                    )
+
             try:
                 with DbSession() as session:
                     try:
                         resource = self.create_resource(session, resource_create)
+
                         register_user(user, session)
                         set_permission(
                             user, resource.aiod_entry, session, type_=PermissionType.ADMIN
@@ -485,13 +511,24 @@ def register_resource(
 
         return register_resource
 
-    def create_resource(self, session: Session, resource_create_instance: SQLModel):
+    def create_resource(
+        self,
+        session: Session,
+        resource_create_instance: SQLModel,
+    ):
         """Store a resource in the database"""
         resource = self.resource_class.from_orm(resource_create_instance)
         deserialize_resource_relationships(
             session, self.resource_class, resource, resource_create_instance
         )
         session.add(resource)
+        session.flush()
+
+        if resource.platform is None and resource.platform_resource_identifier is None:
+            # Set these fields as required for normal users
+            resource.platform = PlatformName.aiod
+            resource.platform_resource_identifier = resource.identifier
+
         session.commit()
         return resource
 

src/tests/authorization/test_authorization.py

@@ -232,9 +232,7 @@ def test_an_published_asset_is_not_pending_for_review(client, publication):
 def test_retrieving_single_submission_works(user: KeycloakUser, mode: ListMode, asset: int, reason: str, client: TestClient, publication_factory):
     publication = publication_factory()
     oldest = publication_factory()
-    oldest.platform_resource_identifier = "OLDEST"
     newest = publication_factory()
-    newest.platform_resource_identifier = "NEWEST"
 
     register_asset(publication, owner=ALICE, status=EntryStatus.PUBLISHED)
     register_asset(oldest, owner=ALICE, status=EntryStatus.SUBMITTED)

src/tests/resources/schemes/aiod/aiod_concept.json

@@ -1,6 +1,4 @@
 {
-    "platform": "example",
-    "platform_resource_identifier": "1",
     "aiod_entry": {
     }
 }

src/tests/routers/generic/test_router_get_count.py

@@ -60,26 +60,37 @@ def test_get_count_detailed_happy_path(client_test_resource: TestClient):
     assert response_json == {"aiod": 1, "example": 2, "openml": 1}
     assert "deprecated" not in response.headers
 
-
+from database.model.concept.aiod_entry import EntryStatus, AIoDEntryORM
+# default platfrom is "aiod"
 def test_get_count_total(
     client: TestClient,
     person: Person,
     publication: Publication,
     contact: Contact,
 ):
+
     register_asset(person)
     register_asset(publication)
-    register_asset(Publication(name="2"))
-    register_asset(Publication(name="3"))
     register_asset(contact)
 
+    resources = [
+        Publication(name="2", platform="example", platform_resource_identifier=2),
+        Publication(name="3", platform="example", platform_resource_identifier=3)
+    ]
+    for res in resources:
+        res.aiod_entry = AIoDEntryORM()
+        res.aiod_entry.status = EntryStatus.PUBLISHED
+
+    with DbSession() as session:
+        session.add_all(resources)
+        session.commit()
+
     response = client.get("/counts")
     assert response.status_code == 200, response.json()
     response_json = response.json()
-
     assert response_json == {
-        "contacts": {"example": 1},
-        "persons": {"example": 1},
-        "publications": {"aiod": 2, "example": 1},
+        "contacts": {"aiod": 1},
+        "persons": {"aiod": 1},
+        "publications": {"aiod": 1, "example": 2},
     }
     assert "deprecated" not in response.headers

src/tests/routers/generic/test_router_post.py

@@ -2,7 +2,9 @@
 import pytest
 from starlette.testclient import TestClient
 
-from tests.testutils.users import logged_in_user
+from tests.testutils.users import logged_in_user, kc_connector_with_roles
+from database.model.platform.platform_names import PlatformName
+from http import HTTPStatus
 
 
 @pytest.mark.parametrize(
@@ -13,20 +15,22 @@ def test_unicode(client_test_resource: TestClient, title: str, auto_publish: Non
     with logged_in_user():
         response = client_test_resource.post(
             "/test_resources/v0",
-            json={"title": title, "platform": "example", "platform_resource_identifier": "1"},
+            json={"title": title},
             headers={"Authorization": "Fake token"},
         )
     assert response.status_code == 200, response.json()
-    identifier = response.json()['identifier']
+    assert "identifier" in response.json()
+    identifier = response.json()["identifier"]
 
     response = client_test_resource.get(f"/test_resources/v0/{identifier}")
     assert response.status_code == 200, response.json()
     response_json = response.json()
     assert response_json["title"] == title
+    assert response_json["platform"] == PlatformName.aiod
 
 
 def test_missing_value(client_test_resource: TestClient):
-    body = {"platform": "example", "platform_resource_identifier": "1"}
+    body: dict[str, str] = {}
     with logged_in_user():
         response = client_test_resource.post(
             "/test_resources/v0", json=body, headers={"Authorization": "Fake token"}
@@ -38,7 +42,7 @@ def test_missing_value(client_test_resource: TestClient):
 
 
 def test_null_value(client_test_resource: TestClient):
-    body = {"title": None, "platform": "example", "platform_resource_identifier": "1"}
+    body = {"title": None}
     with logged_in_user():
         response = client_test_resource.post(
             "/test_resources/v0", json=body, headers={"Authorization": "Fake token"}
@@ -52,57 +56,78 @@ def test_null_value(client_test_resource: TestClient):
         }
     ]
 
-
+# Prevents a connector from posting the same item twice.
 def test_posting_same_item_twice(client_test_resource: TestClient):
     headers = {"Authorization": "Fake token"}
     body = {"title": "title1", "platform": "example", "platform_resource_identifier": "1"}
-    with logged_in_user():
+    connector_user = kc_connector_with_roles()
+
+    with logged_in_user(connector_user):
         response = client_test_resource.post("/test_resources/v0", json=body, headers=headers)
+
     assert response.status_code == 200, response.json()
-    identifier = response.json()['identifier']
+    identifier = response.json()["identifier"]
     body = {"title": "title2", "platform": "example", "platform_resource_identifier": "1"}
-    with logged_in_user():
+    with logged_in_user(connector_user):
         response = client_test_resource.post("/test_resources/v0", json=body, headers=headers)
     assert response.status_code == 409, response.json()
     assert (
-        response.json()["detail"] == "There already exists a test_resource with the same "
-        f"platform and platform_resource_identifier, with identifier={identifier}."
+        response.json()["detail"]
+        == f"There already exists a test_resource with the same platform and platform_resource_identifier, with identifier={identifier}."
     )
 
 
 def test_posting_same_item_twice_but_deleted(
     client_test_resource: TestClient
 ):
     headers = {"Authorization": "Fake token"}
-    body = {"title": "title1", "platform": "example", "platform_resource_identifier": "1"}
+    body = {"title": "title1"}
     with logged_in_user():
         response = client_test_resource.post("/test_resources/v0", json=body, headers=headers)
+    identifier = response.json()["identifier"]
     assert response.status_code == 200, response.json()
     identifier = response.json()['identifier']
 
     with logged_in_user():
         response = client_test_resource.delete(f"/test_resources/v0/{identifier}", headers=headers)
     assert response.status_code == 200, response.json()
 
-    body = {"title": "title2", "platform": "example", "platform_resource_identifier": "1"}
+    body = {"title": "title2"}
     with logged_in_user():
         response = client_test_resource.post("/test_resources/v0", json=body, headers=headers)
     assert response.status_code == 200, response.json()
 
 
-def test_no_platform_no_platform_resource_identifier(
+def test_platform_and_platform_identifier_defaults_are_set_if_not_provided(
     client_test_resource: TestClient
 ):
+    """
+    The platform and platform_resource_identifier are set by the server.
+    """
     headers = {"Authorization": "Fake token"}
     body = {"title": "title1", "platform": None, "platform_resource_identifier": None}
     with logged_in_user():
         response = client_test_resource.post("/test_resources/v0", json=body, headers=headers)
     assert response.status_code == 200, response.json()
+
     body = {"title": "title2", "platform": None, "platform_resource_identifier": None}
     with logged_in_user():
         response = client_test_resource.post("/test_resources/v0", json=body, headers=headers)
     assert response.status_code == 200, response.json()
 
+def test_post_platform_and_platform_resource_identifier_rejected(
+    client_test_resource: TestClient
+):
+    headers = {"Authorization": "Fake token"}
+    body = {"title": "title1", "platform": "aiod", "platform_resource_identifier": 2}
+    with logged_in_user():
+        response = client_test_resource.post("/test_resources/v0", json=body, headers=headers)
+
+    assert response.status_code == HTTPStatus.FORBIDDEN
+    assert response.json()["detail"] == (
+        "No permission to set platform or platform resource identifier.")
+
+
 
 def test_no_platform_with_platform_resource_identifier(
     client_test_resource: TestClient
@@ -111,11 +136,10 @@ def test_no_platform_with_platform_resource_identifier(
     body = {"title": "title1", "platform": None, "platform_resource_identifier": "1"}
     with logged_in_user():
         response = client_test_resource.post("/test_resources/v0", json=body, headers=headers)
-    assert response.status_code == 400, response.json()
+    assert response.status_code == HTTPStatus.FORBIDDEN, response.json()
     assert (
         response.json()["detail"]
-        == "If platform is NULL, platform_resource_identifier should also be "
-        "NULL, and vice versa."
+        == "No permission to set platform or platform resource identifier."
     )
 
 
@@ -126,21 +150,38 @@ def test_platform_with_no_platform_resource_identifier(
     body = {"title": "title1", "platform": "example", "platform_resource_identifier": None}
     with logged_in_user():
         response = client_test_resource.post("/test_resources/v0", json=body, headers=headers)
-    assert response.status_code == 400, response.json()
+    assert response.status_code == HTTPStatus.FORBIDDEN, response.json()
     assert (
         response.json()["detail"]
-        == "If platform is NULL, platform_resource_identifier should also be "
-        "NULL, and vice versa."
+        == "No permission to set platform or platform resource identifier."
     )
 
+def test_connector_can_post_to_valid_platform(
+    client_test_resource: TestClient,
+):
+    headers = {"Authorization": "Fake token"}
+    connector_user = kc_connector_with_roles()
+    body = {
+        "title": "ConnectorResource",
+        "platform": "example",
+        "platform_resource_identifier": "conn-123"
+    }
+    with logged_in_user(connector_user):
+        response = client_test_resource.post("/test_resources/v0", json=body, headers=headers)
+    assert response.status_code == 200
+
 
-def test_non_existent_platform(client_test_resource: TestClient):
+def test_connector_cannot_post_to_other_platform(
+    client_test_resource: TestClient,
+):
     headers = {"Authorization": "Fake token"}
-    body = {"title": "title1", "platform": "this_does_not_exist", "platform_resource_identifier": 1}
-    with logged_in_user():
+    connector_user = kc_connector_with_roles()
+    body = {
+        "title": "ConnectorResource",
+        "platform": "aiod",
+        "platform_resource_identifier": "conn-123"
+    }
+    with logged_in_user(connector_user):
         response = client_test_resource.post("/test_resources/v0", json=body, headers=headers)
-    assert response.status_code == 412
-    assert (
-        response.json()["detail"] == "Platform this_does_not_exist does not exist. You can "
-        "register it using the POST platforms endpoint."
-    )
+    assert response.status_code == HTTPStatus.FORBIDDEN
+    assert response.json()["detail"] == "No permission to upload assets for aiod platform."