From d082722ace36de9fe68eaea3f1810e5b77565e2f Mon Sep 17 00:00:00 2001 From: Chris Arridge Date: Mon, 27 Apr 2026 12:37:52 +0100 Subject: [PATCH 01/16] test: FGA provider db contains client id and multiple provider_admin This commit enhances the FGA provider database test to account for adding the client id to the ToolDbModel and a tool restriction to the FineGrainedAuthorisationRoleAuthorisation. The main update in this test is to validate storing tools and getting multiple provider_admin roles per reporting organisation per user. It also tidies up the provider test to reduce the verbosity. --- tests/helpers/utilities.py | 38 ++++ tests/integration/test_fga_provider_pgdb.py | 231 +++++++++++++------- 2 files changed, 185 insertions(+), 84 deletions(-) diff --git a/tests/helpers/utilities.py b/tests/helpers/utilities.py index 99481a7..7b103e6 100644 --- a/tests/helpers/utilities.py +++ b/tests/helpers/utilities.py @@ -1,7 +1,11 @@ import datetime +import secrets +import string import uuid from typing import Any +from register_your_data_api.auth.fga.models import FineGrainedAuthorisationRoleAssociation + def find_record_in_response(resp_as_object: dict[str, Any], record_id: str) -> dict[str, Any] | None: """Finds a record with the given ID in the response object.""" @@ -38,3 +42,37 @@ def get_current_timestamp_as_str(format_with_z: bool = False) -> str: return s.replace("+00:00", "Z") return s + + +def gen_random_client_id(size: int = 16) -> str: + """Generates a random client ID""" + return "".join([secrets.choice(string.ascii_letters + string.digits) for _ in range(size)]) + + +def association_lists_equal_ignore_id( + assocs1: list[FineGrainedAuthorisationRoleAssociation], assocs2: list[FineGrainedAuthorisationRoleAssociation] +) -> bool: + """Test two lists of FineGrainedAuthorisationRoleAssociations are equal (excluding ids) + + Parameters + ---------- + assocs1 : list[FineGrainedAuthorisationRoleAssociation] + First list of associations. + assocs2 : list[FineGrainedAuthorisationRoleAssociation] + Second list of associations. + + Returns + ------- + bool + """ + + _a1 = [x.model_dump(exclude={"id"}) for x in assocs1] + _a2 = [x.model_dump(exclude={"id"}) for x in assocs2] + + def _sortfun(x: dict[str, uuid.UUID | str | None]) -> str: + return str(x["user"]) + str(x["reporting_org"]) + str(x["role"]) + str(x["restricted_to_tool"]) + + _a1.sort(key=_sortfun) + _a2.sort(key=_sortfun) + + return _a1 == _a2 diff --git a/tests/integration/test_fga_provider_pgdb.py b/tests/integration/test_fga_provider_pgdb.py index 22aa9bc..ee6fbf1 100644 --- a/tests/integration/test_fga_provider_pgdb.py +++ b/tests/integration/test_fga_provider_pgdb.py @@ -1,4 +1,4 @@ -from uuid import uuid4 +from uuid import UUID, uuid4 import pytest import sqlmodel @@ -14,9 +14,12 @@ ) from register_your_data_api.auth.fga.models import ( FineGrainedAuthorisationRole, + FineGrainedAuthorisationRoleAssociation, + FineGrainedAuthorisationTool, ) from ..helpers.setup_and_teardown import setup_db +from ..helpers.utilities import association_lists_equal_ignore_id, gen_random_client_id def test_assignment_of_permissions() -> None: @@ -149,7 +152,7 @@ def test_provider_users_cannot_have_role_for_org() -> None: ) ) - session.add(ToolDbModel(id=tool, name="Tool 1", provider="Tool Maker")) + session.add(ToolDbModel(id=tool, name="Tool 1", provider="Tool Maker", client_id="JnZ63UFhp03LY5N6")) session.add(ToolAuthorisationDbModel(tool=tool, reporting_org=org, id=uuid4())) session.add(ToolAdminUserDbModel(tool=tool, user=user_provider, id=uuid4())) session.add(ToolAdminUserDbModel(tool=tool, user=user_both, id=uuid4())) @@ -164,44 +167,135 @@ def test_provider_users_cannot_have_role_for_org() -> None: with pytest.raises(FineGrainedAuthorisationIntegrityError) as excinfo: fga.get_user_associations_for_org(org) - assert "Reporting org has user(s) that have multiple conflicting roles" in str(excinfo.value) + assert "Reporting org has user(s) with both provider admin and reporting org roles" in str(excinfo.value) - association = fga.get_user_role_for_org(user_provider, org) - assert association is not None - assert association.role == FineGrainedAuthorisationRole.PROVIDER_ADMIN + associations = fga.get_user_roles_for_org(user_provider, org) + assert associations + assert associations[0].role == FineGrainedAuthorisationRole.PROVIDER_ADMIN - association = fga.get_user_role_for_org(user_org, org) - assert association is not None - assert association.role == FineGrainedAuthorisationRole.ADMIN + associations = fga.get_user_roles_for_org(user_org, org) + assert associations + assert associations[0].role == FineGrainedAuthorisationRole.ADMIN with pytest.raises(FineGrainedAuthorisationIntegrityError) as excinfo: - fga.get_user_role_for_org(user_both, org) + fga.get_user_roles_for_org(user_both, org) assert "User has both reporting org role and a provider admin role" in str(excinfo.value) def test_provider_roles_are_correctly_applied() -> None: - """Test FGA provider authorisations using SQLite in-memory DB""" setup_db("sqlite:///test.db") fga = FineGrainedAuthorisationProviderDb("sqlite:///test.db") fga.setup() - user_org1, user_tool1, user_tool2, user_both_tools, user_no_perms = uuid4(), uuid4(), uuid4(), uuid4(), uuid4() - org1, org2 = uuid4(), uuid4() - tool1, tool2 = uuid4(), uuid4() + u_o1, u_t1, u_t2, u_bothtools, u_noperms = uuid4(), uuid4(), uuid4(), uuid4(), uuid4() + o1, o2 = uuid4(), uuid4() + t1, t2 = uuid4(), uuid4() + t1_clientid, t2_clientid = gen_random_client_id(), gen_random_client_id() with sqlmodel.Session(fga._engine) as session: session.add( FineGrainedAuthorisationDbModel( - user=user_org1, reporting_org=org1, role=FineGrainedAuthorisationRole.ADMIN, id=uuid4() + user=u_o1, reporting_org=o1, role=FineGrainedAuthorisationRole.ADMIN, id=uuid4() ) ) - session.add(ToolDbModel(id=tool1, name="Tool 1", provider="Tool Maker")) - session.add(ToolDbModel(id=tool2, name="Tool 2", provider="Tool Maker")) - session.add(ToolAuthorisationDbModel(tool=tool1, reporting_org=org1, id=uuid4())) - session.add(ToolAuthorisationDbModel(tool=tool2, reporting_org=org2, id=uuid4())) + session.add(ToolDbModel(id=t1, name="Tool 1", provider="Tool Maker", client_id=t1_clientid)) + session.add(ToolDbModel(id=t2, name="Tool 2", provider="Tool Maker", client_id=t2_clientid)) + session.add(ToolAuthorisationDbModel(tool=t1, reporting_org=o1, id=uuid4())) + session.add(ToolAuthorisationDbModel(tool=t2, reporting_org=o1, id=uuid4())) + session.add(ToolAuthorisationDbModel(tool=t2, reporting_org=o2, id=uuid4())) + session.add(ToolAdminUserDbModel(tool=t1, user=u_t1, id=uuid4())) + session.add(ToolAdminUserDbModel(tool=t2, user=u_t2, id=uuid4())) + session.add(ToolAdminUserDbModel(tool=t1, user=u_bothtools, id=uuid4())) + session.add(ToolAdminUserDbModel(tool=t2, user=u_bothtools, id=uuid4())) + + session.commit() + + # fga.get_user_fine_grained_permissions(u_bothtools) + + # Check associations by user. + ASSOCIATIONS_BY_USER_CHECKS = { + u_o1: [ + (o1, FineGrainedAuthorisationRole.ADMIN, None), + ], + u_t1: [ + (o1, FineGrainedAuthorisationRole.PROVIDER_ADMIN, t1), + ], + u_t2: [ + (o1, FineGrainedAuthorisationRole.PROVIDER_ADMIN, t2), + (o2, FineGrainedAuthorisationRole.PROVIDER_ADMIN, t2), + ], + u_bothtools: [ + (o1, FineGrainedAuthorisationRole.PROVIDER_ADMIN, t1), + (o1, FineGrainedAuthorisationRole.PROVIDER_ADMIN, t2), + (o2, FineGrainedAuthorisationRole.PROVIDER_ADMIN, t2), + ], + u_noperms: [], + } + for u, data in ASSOCIATIONS_BY_USER_CHECKS.items(): + expected_associations = [ + FineGrainedAuthorisationRoleAssociation(user=u, reporting_org=o, role=r, restricted_to_tool=t, id=uuid4()) + for o, r, t in data # type: ignore[attr-defined] + ] + assert association_lists_equal_ignore_id(fga.get_user_fine_grained_permissions(u), expected_associations) + + # Check associations by org. + ASSOCIATIONS_BY_ORG_CHECKS = { + o1: [ + (u_o1, FineGrainedAuthorisationRole.ADMIN, None), + (u_t1, FineGrainedAuthorisationRole.PROVIDER_ADMIN, t1), + (u_t2, FineGrainedAuthorisationRole.PROVIDER_ADMIN, t2), + (u_bothtools, FineGrainedAuthorisationRole.PROVIDER_ADMIN, t1), + (u_bothtools, FineGrainedAuthorisationRole.PROVIDER_ADMIN, t2), + ], + o2: [ + (u_t2, FineGrainedAuthorisationRole.PROVIDER_ADMIN, t2), + (u_bothtools, FineGrainedAuthorisationRole.PROVIDER_ADMIN, t2), + ], + } + for o, data in ASSOCIATIONS_BY_ORG_CHECKS.items(): + expected_associations = [ + FineGrainedAuthorisationRoleAssociation(user=u, reporting_org=o, role=r, restricted_to_tool=t, id=uuid4()) + for u, r, t in data # type: ignore[attr-defined] + ] + assert association_lists_equal_ignore_id(fga.get_user_associations_for_org(o), expected_associations) + + # Check associations by user and org + ASSOCIATIONS_BY_USER_AND_ORG_CHECKS = { + (u_o1, o1): [(FineGrainedAuthorisationRole.ADMIN, None)], + (u_t1, o1): [(FineGrainedAuthorisationRole.PROVIDER_ADMIN, t1)], + (u_t2, o1): [(FineGrainedAuthorisationRole.PROVIDER_ADMIN, t2)], + (u_t2, o2): [(FineGrainedAuthorisationRole.PROVIDER_ADMIN, t2)], + (u_bothtools, o1): [ + (FineGrainedAuthorisationRole.PROVIDER_ADMIN, t1), + (FineGrainedAuthorisationRole.PROVIDER_ADMIN, t2), + ], + (u_bothtools, o2): [(FineGrainedAuthorisationRole.PROVIDER_ADMIN, t2)], + (u_noperms, o1): [], + (u_noperms, o2): [], + } + for (u, o), data in ASSOCIATIONS_BY_USER_AND_ORG_CHECKS.items(): + expected_associations = [ + FineGrainedAuthorisationRoleAssociation(user=u, reporting_org=o, role=r, restricted_to_tool=t, id=uuid4()) + for r, t in data # type: ignore[attr-defined] + ] + assert association_lists_equal_ignore_id(fga.get_user_roles_for_org(u, o), expected_associations) + + +def test_tool_lists_correctly_fetched() -> None: + setup_db("sqlite:///test.db") + + fga = FineGrainedAuthorisationProviderDb("sqlite:///test.db") + fga.setup() + + tool1, tool2 = UUID("0415675e-8767-42e2-9f51-b44211b09aa8"), UUID("d3ef34c9-3dd2-445e-aeff-644853753c43") + user_tool1, user_tool2, user_both_tools = uuid4(), uuid4(), uuid4() + + with sqlmodel.Session(fga._engine) as session: + session.add(ToolDbModel(id=tool1, name="Tool 1", provider="Tool Maker", client_id="JnZ63UFhp03LY5N6")) + session.add(ToolDbModel(id=tool2, name="Tool 2", provider="Tool Maker", client_id="v4amDn43kirvBmB9")) session.add(ToolAdminUserDbModel(tool=tool1, user=user_tool1, id=uuid4())) session.add(ToolAdminUserDbModel(tool=tool2, user=user_tool2, id=uuid4())) session.add(ToolAdminUserDbModel(tool=tool1, user=user_both_tools, id=uuid4())) @@ -209,68 +303,37 @@ def test_provider_roles_are_correctly_applied() -> None: session.commit() - # Check user permissions. - associations = fga.get_user_fine_grained_permissions(user_org1) - assert len(associations) == 1 - assert associations[0].reporting_org == org1 - assert associations[0].role == FineGrainedAuthorisationRole.ADMIN - - associations = fga.get_user_fine_grained_permissions(user_tool1) - assert len(associations) == 1 - assert associations[0].reporting_org == org1 - assert associations[0].role == FineGrainedAuthorisationRole.PROVIDER_ADMIN - - associations = fga.get_user_fine_grained_permissions(user_tool2) - assert len(associations) == 1 - assert associations[0].reporting_org == org2 - assert associations[0].role == FineGrainedAuthorisationRole.PROVIDER_ADMIN - - associations = fga.get_user_fine_grained_permissions(user_both_tools) - assert len(associations) == 2 - assert all([association.role == FineGrainedAuthorisationRole.PROVIDER_ADMIN for association in associations]) - assert org1 in [association.reporting_org for association in associations] - assert org2 in [association.reporting_org for association in associations] - - associations = fga.get_user_fine_grained_permissions(user_no_perms) - assert len(associations) == 0 - - # Check associations each user has for each org - USER_ORG_ASSOCIATION_CHECKS = [ - (user_org1, org1, FineGrainedAuthorisationRole.ADMIN), - (user_tool1, org1, FineGrainedAuthorisationRole.PROVIDER_ADMIN), - (user_tool2, org1, None), - (user_both_tools, org1, FineGrainedAuthorisationRole.PROVIDER_ADMIN), - (user_no_perms, org1, None), - (user_org1, org2, None), - (user_tool1, org2, None), - (user_tool2, org2, FineGrainedAuthorisationRole.PROVIDER_ADMIN), - (user_both_tools, org2, FineGrainedAuthorisationRole.PROVIDER_ADMIN), - (user_no_perms, org2, None), - ] - - for check in USER_ORG_ASSOCIATION_CHECKS: - association = fga.get_user_role_for_org(check[0], check[1]) - if check[2] is None: - assert association is None - else: - assert association is not None - assert association.user == check[0] - assert association.reporting_org == check[1] - assert association.role == check[2] - - # Check user associations for org. - associations_by_user = {association.user: association for association in fga.get_user_associations_for_org(org1)} - assert len(associations_by_user) == 3 - assert user_org1 in associations_by_user - assert associations_by_user[user_org1].role == FineGrainedAuthorisationRole.ADMIN - assert user_tool1 in associations_by_user - assert associations_by_user[user_tool1].role == FineGrainedAuthorisationRole.PROVIDER_ADMIN - assert user_both_tools in associations_by_user - assert associations_by_user[user_both_tools].role == FineGrainedAuthorisationRole.PROVIDER_ADMIN - - associations_by_user = {association.user: association for association in fga.get_user_associations_for_org(org2)} - assert len(associations_by_user) == 2 - assert user_tool2 in associations_by_user - assert associations_by_user[user_tool2].role == FineGrainedAuthorisationRole.PROVIDER_ADMIN - assert user_both_tools in associations_by_user - assert associations_by_user[user_both_tools].role == FineGrainedAuthorisationRole.PROVIDER_ADMIN + # Check all tool list. + tools = fga.get_all_tools() + tools.sort(key=lambda x: x.id) + + assert len(tools) == 2 + assert tools[0] == FineGrainedAuthorisationTool( + id=tool1, name="Tool 1", provider="Tool Maker", client_id="JnZ63UFhp03LY5N6" + ) + assert tools[1] == FineGrainedAuthorisationTool( + id=tool2, name="Tool 2", provider="Tool Maker", client_id="v4amDn43kirvBmB9" + ) + + # Check tool list for each user. + tools = fga.get_tools_for_user(user_tool1) + assert len(tools) == 1 + assert tools[0] == FineGrainedAuthorisationTool( + id=tool1, name="Tool 1", provider="Tool Maker", client_id="JnZ63UFhp03LY5N6" + ) + + tools = fga.get_tools_for_user(user_tool2) + assert len(tools) == 1 + assert tools[0] == FineGrainedAuthorisationTool( + id=tool2, name="Tool 2", provider="Tool Maker", client_id="v4amDn43kirvBmB9" + ) + + tools = fga.get_tools_for_user(user_both_tools) + tools.sort(key=lambda x: x.id) + assert len(tools) == 2 + assert tools[0] == FineGrainedAuthorisationTool( + id=tool1, name="Tool 1", provider="Tool Maker", client_id="JnZ63UFhp03LY5N6" + ) + assert tools[1] == FineGrainedAuthorisationTool( + id=tool2, name="Tool 2", provider="Tool Maker", client_id="v4amDn43kirvBmB9" + ) From 44dd4d7b305cadaf14fc3e47ac617ae5ac78e515 Mon Sep 17 00:00:00 2001 From: Chris Arridge Date: Mon, 27 Apr 2026 13:10:22 +0100 Subject: [PATCH 02/16] feat: FGA provider stores client id and handles multiple provider admins This commit adds two features to the FGA functionality as a prelude to restricting the clients that provider_admin can be used with. Firstly, it adds tool client IDs to the FGA ToolDBModel table. It adds a field to the FineGrainedAuthorisationRoleAssociation model so that we can track which tool (client) an association is valid for. For a regular reporting org role this is null, for a provider_admin it will be linked to a tool. To enable this functionality, a small refactoring was added to allow get_user_role_for_org to return more than one role per user and per org (e.g., provider admin via multiple tools). --- ...c6d1170383_add_client_id_to_tooldbmodel.py | 66 ++++++++++ .../auth/fga/fga_provider.py | 14 ++- .../auth/fga/fga_provider_db.py | 113 ++++++++++++++---- src/register_your_data_api/auth/fga/models.py | 8 ++ src/register_your_data_api/routers/users.py | 18 +-- 5 files changed, 182 insertions(+), 37 deletions(-) create mode 100644 alembic/versions/b2c6d1170383_add_client_id_to_tooldbmodel.py diff --git a/alembic/versions/b2c6d1170383_add_client_id_to_tooldbmodel.py b/alembic/versions/b2c6d1170383_add_client_id_to_tooldbmodel.py new file mode 100644 index 0000000..ed76bb1 --- /dev/null +++ b/alembic/versions/b2c6d1170383_add_client_id_to_tooldbmodel.py @@ -0,0 +1,66 @@ +"""add client id to tooldbmodel + +Revision ID: b2c6d1170383 +Revises: 22951c5f0f0c +Create Date: 2026-04-27 13:06:36.123347 + +""" + +from typing import Sequence, Union + +import sqlalchemy as sa +import sqlmodel +from alembic import op +from sqlalchemy.dialects import postgresql + +# revision identifiers, used by Alembic. +revision: str = "b2c6d1170383" +down_revision: Union[str, Sequence[str], None] = "22951c5f0f0c" +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + """Upgrade schema.""" + # ### commands auto generated by Alembic - please adjust! ### + op.alter_column( + "finegrainedauthorisationdbmodel", + "role", + existing_type=postgresql.ENUM( + "CONTRIBUTOR", "EDITOR", "PROVIDER_ADMIN", "ADMIN", "CONTRIBUTOR_PENDING", name="role" + ), + type_=sa.Enum( + "CONTRIBUTOR", + "EDITOR", + "PROVIDER_ADMIN", + "ADMIN", + "SUPER_ADMIN", + "CONTRIBUTOR_PENDING", + name="finegrainedauthorisationrole", + ), + existing_nullable=False, + ) + op.add_column("tooldbmodel", sa.Column("client_id", sqlmodel.sql.sqltypes.AutoString(), nullable=False)) + # ### end Alembic commands ### + + +def downgrade() -> None: + """Downgrade schema.""" + # ### commands auto generated by Alembic - please adjust! ### + op.drop_column("tooldbmodel", "client_id") + op.alter_column( + "finegrainedauthorisationdbmodel", + "role", + existing_type=sa.Enum( + "CONTRIBUTOR", + "EDITOR", + "PROVIDER_ADMIN", + "ADMIN", + "SUPER_ADMIN", + "CONTRIBUTOR_PENDING", + name="finegrainedauthorisationrole", + ), + type_=postgresql.ENUM("CONTRIBUTOR", "EDITOR", "PROVIDER_ADMIN", "ADMIN", "CONTRIBUTOR_PENDING", name="role"), + existing_nullable=False, + ) + # ### end Alembic commands ### diff --git a/src/register_your_data_api/auth/fga/fga_provider.py b/src/register_your_data_api/auth/fga/fga_provider.py index 1705418..c82a3c3 100644 --- a/src/register_your_data_api/auth/fga/fga_provider.py +++ b/src/register_your_data_api/auth/fga/fga_provider.py @@ -1,7 +1,7 @@ from abc import ABC, abstractmethod from uuid import UUID -from .models import FineGrainedAuthorisationRoleAssociation +from .models import FineGrainedAuthorisationRoleAssociation, FineGrainedAuthorisationTool class FineGrainedAuthorisationIntegrityError(Exception): @@ -25,7 +25,7 @@ def get_user_associations_for_org(self, reporting_org: UUID) -> list[FineGrained raise NotImplementedError @abstractmethod - def get_user_role_for_org(self, user: UUID, org: UUID) -> FineGrainedAuthorisationRoleAssociation | None: + def get_user_roles_for_org(self, user: UUID, org: UUID) -> list[FineGrainedAuthorisationRoleAssociation]: """Returns a list of all the user's fine grained access roles for a specific organisation""" raise NotImplementedError @@ -65,3 +65,13 @@ def delete_all_fine_grained_authorisations_for_user(self, user: UUID) -> None: def delete_all_fine_grained_authorisations_for_org(self, org: UUID) -> None: """Deletes all fine grained role associations for an organisation""" raise NotImplementedError + + @abstractmethod + def get_all_tools(self) -> list[FineGrainedAuthorisationTool]: + """Get a list of all the tools stored in the database.""" + raise NotImplementedError + + @abstractmethod + def get_tools_for_user(self, user: UUID) -> list[FineGrainedAuthorisationTool]: + """Get a list of all the tools for which the user is an admin user.""" + raise NotImplementedError diff --git a/src/register_your_data_api/auth/fga/fga_provider_db.py b/src/register_your_data_api/auth/fga/fga_provider_db.py index 2f81632..6e27499 100644 --- a/src/register_your_data_api/auth/fga/fga_provider_db.py +++ b/src/register_your_data_api/auth/fga/fga_provider_db.py @@ -5,7 +5,7 @@ from sqlmodel import Field, Session, SQLModel, col, create_engine, select from .fga_provider import FineGrainedAuthorisationIntegrityError, FineGrainedAuthorisationProvider -from .models import FineGrainedAuthorisationRole, FineGrainedAuthorisationRoleAssociation +from .models import FineGrainedAuthorisationRole, FineGrainedAuthorisationRoleAssociation, FineGrainedAuthorisationTool class FineGrainedAuthorisationDbModel(SQLModel, table=True): @@ -25,6 +25,7 @@ class ToolDbModel(SQLModel, table=True): id: UUID = Field(primary_key=True, default_factory=lambda: uuid4()) name: str provider: str + client_id: str class ToolAuthorisationDbModel(SQLModel, table=True): @@ -57,8 +58,9 @@ def get_user_fine_grained_permissions(self, user: UUID) -> list[FineGrainedAutho ).all() providers_reporting_orgs = session.exec( - select(ToolAuthorisationDbModel.reporting_org, ToolAdminUserDbModel.user) + select(ToolAuthorisationDbModel.reporting_org, ToolAdminUserDbModel.user, ToolDbModel.id) .join(ToolAdminUserDbModel, col(ToolAdminUserDbModel.tool) == col(ToolAuthorisationDbModel.tool)) + .join(ToolDbModel, col(ToolDbModel.id) == col(ToolAuthorisationDbModel.tool)) .where(ToolAdminUserDbModel.user == user) ).all() @@ -68,7 +70,10 @@ def get_user_fine_grained_permissions(self, user: UUID) -> list[FineGrainedAutho associations = [FineGrainedAuthorisationRoleAssociation(**db_fga.model_dump()) for db_fga in user_db_fgas] associations += [ FineGrainedAuthorisationRoleAssociation( - reporting_org=x[0], user=x[1], role=FineGrainedAuthorisationRole.PROVIDER_ADMIN + reporting_org=x[0], + user=x[1], + role=FineGrainedAuthorisationRole.PROVIDER_ADMIN, + restricted_to_tool=x[2], ) for x in providers_reporting_orgs ] @@ -84,59 +89,94 @@ def get_user_associations_for_org(self, reporting_org: UUID) -> list[FineGrained ).all() tool_admin_users_for_org = session.exec( - select(ToolAuthorisationDbModel.reporting_org, ToolAdminUserDbModel.user) + select(ToolAuthorisationDbModel.reporting_org, ToolAdminUserDbModel.user, ToolDbModel.id) .join(ToolAdminUserDbModel, col(ToolAdminUserDbModel.tool) == col(ToolAuthorisationDbModel.tool)) + .join(ToolDbModel, col(ToolDbModel.id) == col(ToolAuthorisationDbModel.tool)) .where(ToolAuthorisationDbModel.reporting_org == reporting_org) ).all() - associations = [FineGrainedAuthorisationRoleAssociation(**db_fga.model_dump()) for db_fga in user_db_fgas] - associations += [ + regular_associations = [ + FineGrainedAuthorisationRoleAssociation(**db_fga.model_dump()) for db_fga in user_db_fgas + ] + if len(regular_associations) > 1: + if ( + collections.Counter([association.user for association in regular_associations]).most_common(1)[0][1] + > 1 + ): + raise FineGrainedAuthorisationIntegrityError( + "Reporting org has user(s) that have multiple reporting org roles" + ) + + # Check that provider admin users are unique for this reporting org - each user can only have provider + # admin by each tool (we cannot have two accesses via provider admin by the same tool). + provider_admin_associations = [ FineGrainedAuthorisationRoleAssociation( user=tool_admin_user_for_org[1], reporting_org=reporting_org, role=FineGrainedAuthorisationRole.PROVIDER_ADMIN, + restricted_to_tool=tool_admin_user_for_org[2], ) for tool_admin_user_for_org in tool_admin_users_for_org ] - - if len(associations) > 1: - if collections.Counter([association.user for association in associations]).most_common(1)[0][1] > 1: + if provider_admin_associations: + if ( + collections.Counter( + [(association.user, association.restricted_to_tool) for association in provider_admin_associations] + ).most_common(1)[0][1] + > 1 + ): raise FineGrainedAuthorisationIntegrityError( - "Reporting org has user(s) that have multiple conflicting roles" + "Reporting org has provider admins with multiple conflicting tool admin roles" ) - return associations + # Check that a provider admin is not in the list of regular associations. + if len(set([x.user for x in regular_associations]) & set([x.user for x in provider_admin_associations])) > 0: + raise FineGrainedAuthorisationIntegrityError( + "Reporting org has user(s) with both provider admin and reporting org roles" + ) + + return regular_associations + provider_admin_associations - def get_user_role_for_org(self, user: UUID, org: UUID) -> FineGrainedAuthorisationRoleAssociation | None: + def get_user_roles_for_org(self, user: UUID, org: UUID) -> list[FineGrainedAuthorisationRoleAssociation]: with Session(self._engine) as session: - user_role_for_org = session.exec( + user_roles_for_org = session.exec( select(FineGrainedAuthorisationDbModel).where( (FineGrainedAuthorisationDbModel.user == user) & (FineGrainedAuthorisationDbModel.reporting_org == org) ) - ).first() + ).all() - tool_admin_user_for_org = session.exec( - select(ToolAuthorisationDbModel.reporting_org, ToolAdminUserDbModel.user) + tool_admin_users_for_org = session.exec( + select(ToolAuthorisationDbModel.reporting_org, ToolAdminUserDbModel.user, ToolDbModel.id) .join(ToolAdminUserDbModel, col(ToolAdminUserDbModel.tool) == col(ToolAuthorisationDbModel.tool)) + .join(ToolDbModel, col(ToolDbModel.id) == col(ToolAuthorisationDbModel.tool)) .where((ToolAuthorisationDbModel.reporting_org == org) & (ToolAdminUserDbModel.user == user)) ).all() - if tool_admin_user_for_org and user_role_for_org: + if tool_admin_users_for_org and user_roles_for_org: raise FineGrainedAuthorisationIntegrityError("User has both reporting org role and a provider admin role") - if not user_role_for_org and not tool_admin_user_for_org: - return None + if not user_roles_for_org and not tool_admin_users_for_org: + return [] - if tool_admin_user_for_org: - association = FineGrainedAuthorisationRoleAssociation( - user=user, reporting_org=org, role=FineGrainedAuthorisationRole.PROVIDER_ADMIN - ) + if tool_admin_users_for_org: + associations = [ + FineGrainedAuthorisationRoleAssociation( + user=user, + reporting_org=org, + role=FineGrainedAuthorisationRole.PROVIDER_ADMIN, + restricted_to_tool=tool_admin_user_for_org[2], + ) + for tool_admin_user_for_org in tool_admin_users_for_org + ] - if user_role_for_org: - association = FineGrainedAuthorisationRoleAssociation(**user_role_for_org.model_dump()) + if len(user_roles_for_org) > 1: + raise FineGrainedAuthorisationIntegrityError("User has multiple roles for this reporting org") - return association + if user_roles_for_org: + associations = [FineGrainedAuthorisationRoleAssociation(**user_roles_for_org[0].model_dump())] + + return associations def get_admin_users_for_org(self, org: UUID) -> list[FineGrainedAuthorisationRoleAssociation]: with Session(self._engine) as session: @@ -206,3 +246,24 @@ def delete_all_fine_grained_authorisations_for_org(self, org: UUID) -> None: session.commit() return None + + def get_all_tools(self) -> list[FineGrainedAuthorisationTool]: + """Get a list of all the tools stored in the database.""" + with Session(self._engine) as session: + db_tools = session.exec(select(ToolDbModel)).all() + + return [FineGrainedAuthorisationTool(**db_tool.model_dump()) for db_tool in db_tools] + + return [] + + def get_tools_for_user(self, user: UUID) -> list[FineGrainedAuthorisationTool]: + """Get a list of all the tools for which the user is an admin user.""" + + with Session(self._engine) as session: + db_tools = session.exec( + select(ToolDbModel).join(ToolAdminUserDbModel).where(ToolAdminUserDbModel.user == user) + ).all() + + return [FineGrainedAuthorisationTool(**db_tool.model_dump()) for db_tool in db_tools] + + return [] diff --git a/src/register_your_data_api/auth/fga/models.py b/src/register_your_data_api/auth/fga/models.py index 440328f..e50ebfa 100644 --- a/src/register_your_data_api/auth/fga/models.py +++ b/src/register_your_data_api/auth/fga/models.py @@ -18,3 +18,11 @@ class FineGrainedAuthorisationRoleAssociation(pydantic.BaseModel): user: UUID reporting_org: UUID role: FineGrainedAuthorisationRole + restricted_to_tool: UUID | None = pydantic.Field(default=None) + + +class FineGrainedAuthorisationTool(pydantic.BaseModel): + id: UUID = pydantic.Field(default_factory=lambda: uuid4()) + name: str + provider: str + client_id: str diff --git a/src/register_your_data_api/routers/users.py b/src/register_your_data_api/routers/users.py index 8b3f01a..6a3e820 100644 --- a/src/register_your_data_api/routers/users.py +++ b/src/register_your_data_api/routers/users.py @@ -381,9 +381,9 @@ def update_user_role_in_reporting_org( ) # 7. Check whether there is an existing FGA entry for this user and reporting org - user_role_for_org = context.fine_grained_auth_provider.get_user_role_for_org(user_id, org_id) + user_roles_for_org = context.fine_grained_auth_provider.get_user_roles_for_org(user_id, org_id) - if user_role_for_org is None: + if user_roles_for_org is None: context.app_logger.error( f"Unexpected error: user with id: {user.user_id_crm} attempted to change user id: {user_id}'s role in " f"organisation with id: {org_id} but the user has no entry in the FGA DB for this organisation. " @@ -401,14 +401,14 @@ def update_user_role_in_reporting_org( return JSONResponse({"data": None, "error": None, "status": "success"}, 200) # Don't update if the role is the same - if str(user_role_for_org.role.name) == new_role.role.upper(): + if str(user_roles_for_org[0].role.name) == new_role.role.upper(): return JSONResponse({"data": None, "error": None, "status": "success"}, 200) # 8. Update the user's role in the FGA database # This is safe as new_role has been validated by Pydantic - user_role_for_org.role = get_fga_role_from_str(new_role.role) + user_roles_for_org[0].role = get_fga_role_from_str(new_role.role) - context.fine_grained_auth_provider.update_user_role_for_org(user_role_for_org) + context.fine_grained_auth_provider.update_user_role_for_org(user_roles_for_org[0]) context.audit_logger.info( format_log_msg( @@ -483,12 +483,12 @@ def remove_user_from_reporting_org( ), ) - user_role_for_org = context.fine_grained_auth_provider.get_user_role_for_org(user_id, org_id) + user_roles_for_org = context.fine_grained_auth_provider.get_user_roles_for_org(user_id, org_id) assert_precondition_met( user.user_id_crm, user.client_id, - condition_func=lambda: user_role_for_org is not None, + condition_func=lambda: len(user_roles_for_org) != 0, public_msg=f"User id: {user_id} has no role in organisation with id: {str(org_id)} in the Registry.", status_code=fastapi.status.HTTP_400_BAD_REQUEST, audit_log_msg=( @@ -522,7 +522,7 @@ def remove_user_from_reporting_org( user.user_id_crm, user.client_id, condition_func=lambda: not ( - user_role_for_org.role == FineGrainedAuthorisationRole.ADMIN and number_admins == 1 # type: ignore + user_roles_for_org[0].role == FineGrainedAuthorisationRole.ADMIN and number_admins == 1 # type: ignore ), public_msg=( f"User id: {user_id} is the last admin user associated with " @@ -537,7 +537,7 @@ def remove_user_from_reporting_org( # 5. Delete the user's role in the FGA database try: - context.fine_grained_auth_provider.delete_user_role_for_org(user_role_for_org) # type: ignore + context.fine_grained_auth_provider.delete_user_role_for_org(user_roles_for_org[0]) # type: ignore context.audit_logger.info( format_log_msg( From d4ff91e1b10975f2c155cf1786211851fd18ea5b Mon Sep 17 00:00:00 2001 From: Chris Arridge Date: Mon, 27 Apr 2026 12:38:54 +0100 Subject: [PATCH 03/16] test: FGA validator rejects provider_admin when client id doesn't match --- tests/unit/test_fga_validator.py | 111 ++++++++++++++++++++++++++++--- 1 file changed, 103 insertions(+), 8 deletions(-) diff --git a/tests/unit/test_fga_validator.py b/tests/unit/test_fga_validator.py index 9e0595e..9380c5f 100644 --- a/tests/unit/test_fga_validator.py +++ b/tests/unit/test_fga_validator.py @@ -4,23 +4,45 @@ from register_your_data_api.auth.fga.models import ( FineGrainedAuthorisationRole, FineGrainedAuthorisationRoleAssociation, + FineGrainedAuthorisationTool, ) +from ..helpers.utilities import association_lists_equal_ignore_id, gen_random_client_id + def test_provider_admin() -> None: """Test validator responses for a provider admin role""" + # This user has access to org1 via two tools, and no access to org 2. user, org1, org2 = uuid4(), uuid4(), uuid4() - + tool1 = FineGrainedAuthorisationTool( + id=uuid4(), name="Tool 1", provider="Tool Maker", client_id=gen_random_client_id() + ) + tool2 = FineGrainedAuthorisationTool( + id=uuid4(), name="Tool 1", provider="Tool Maker", client_id=gen_random_client_id() + ) users_fgas = [ FineGrainedAuthorisationRoleAssociation( user=user, reporting_org=org1, role=FineGrainedAuthorisationRole.PROVIDER_ADMIN, - ) + restricted_to_tool=tool1.id, + ), + FineGrainedAuthorisationRoleAssociation( + user=user, + reporting_org=org1, + role=FineGrainedAuthorisationRole.PROVIDER_ADMIN, + restricted_to_tool=tool2.id, + ), ] + + # Test validator when the end user is accessing RYD via the same client ID as tool 1. v = FineGrainedAuthorisationUserValidator( - user_id=user, fine_grained_authorisations=users_fgas, is_superadmin=False + user_id=user, + fine_grained_authorisations=users_fgas, + is_superadmin=False, + tools=[tool1, tool2], + client_id=tool1.client_id, ) assert v.get_user_role_for_reporting_org(org1) == FineGrainedAuthorisationRole.PROVIDER_ADMIN @@ -52,11 +74,84 @@ def test_provider_admin() -> None: assert v.user_can_read_users_reporting_orgs(user) assert not v.user_can_read_users_reporting_orgs(uuid4()) - associations = v.get_users_fine_grained_associations() - assert len(associations) == 1 - assert associations[0].user == user - assert associations[0].reporting_org == org1 - assert associations[0].role == FineGrainedAuthorisationRole.PROVIDER_ADMIN + # Test validator when the end user is accessing RYD via the same client ID as tool 2. + v = FineGrainedAuthorisationUserValidator( + user_id=user, + fine_grained_authorisations=users_fgas, + is_superadmin=False, + tools=[tool1, tool2], + client_id=tool2.client_id, + ) + + assert v.get_user_role_for_reporting_org(org1) == FineGrainedAuthorisationRole.PROVIDER_ADMIN + assert v.get_user_role_for_reporting_org(org2) is None + + assert v.user_can_create_reporting_org() + + assert v.user_can_update_reporting_org(org1) + assert not v.user_can_update_reporting_org(org2) + + assert not v.user_can_delete_reporting_org(org1) + assert not v.user_can_delete_reporting_org(org2) + + assert v.user_can_create_reporting_org_datasets(org1) + assert not v.user_can_create_reporting_org_datasets(org2) + + assert v.user_can_update_reporting_org_datasets(org1) + assert not v.user_can_update_reporting_org_datasets(org2) + + assert v.user_can_update_reporting_org_dataset_visibility(org1) + assert not v.user_can_update_reporting_org_dataset_visibility(org2) + + assert v.user_can_delete_reporting_org_datasets(org1) + assert not v.user_can_delete_reporting_org_datasets(org2) + + assert not v.user_can_modify_user_roles_for_reporting_org(org1) + assert not v.user_can_modify_user_roles_for_reporting_org(org2) + + assert v.user_can_read_users_reporting_orgs(user) + assert not v.user_can_read_users_reporting_orgs(uuid4()) + + # Test validator when the end user is accessing RYD via a client id that doesn't + # match either of the provider admin tools. + v = FineGrainedAuthorisationUserValidator( + user_id=user, + fine_grained_authorisations=users_fgas, + is_superadmin=False, + tools=[tool1, tool2], + client_id=gen_random_client_id(), + ) + + assert v.get_user_role_for_reporting_org(org1) is None + assert v.get_user_role_for_reporting_org(org2) is None + + assert v.user_can_create_reporting_org() + + assert not v.user_can_update_reporting_org(org1) + assert not v.user_can_update_reporting_org(org2) + + assert not v.user_can_delete_reporting_org(org1) + assert not v.user_can_delete_reporting_org(org2) + + assert not v.user_can_create_reporting_org_datasets(org1) + assert not v.user_can_create_reporting_org_datasets(org2) + + assert not v.user_can_update_reporting_org_datasets(org1) + assert not v.user_can_update_reporting_org_datasets(org2) + + assert not v.user_can_update_reporting_org_dataset_visibility(org1) + assert not v.user_can_update_reporting_org_dataset_visibility(org2) + + assert not v.user_can_delete_reporting_org_datasets(org1) + assert not v.user_can_delete_reporting_org_datasets(org2) + + assert not v.user_can_modify_user_roles_for_reporting_org(org1) + assert not v.user_can_modify_user_roles_for_reporting_org(org2) + + assert v.user_can_read_users_reporting_orgs(user) + assert not v.user_can_read_users_reporting_orgs(uuid4()) + + assert association_lists_equal_ignore_id(v.get_users_fine_grained_associations(), users_fgas) reporting_orgs = v.get_users_reporting_orgs() assert len(reporting_orgs) == 1 From 88e130409533e48b2d8fad69db065c27f4f8467b Mon Sep 17 00:00:00 2001 From: Chris Arridge Date: Mon, 27 Apr 2026 15:44:57 +0100 Subject: [PATCH 04/16] feat: FGA validator accepts client id and tools and validates provider_admin This commit changes the FGA validator to store the client ID of the caller and a list of tools as well as the fine grained authorisations. Internally the validator now parses provider_admin and checks if the tools being used for provider_admin match the client id and return provider_admin roles as appropriate. --- .../auth/fga/fga_validator.py | 54 ++++++++++++++++--- 1 file changed, 47 insertions(+), 7 deletions(-) diff --git a/src/register_your_data_api/auth/fga/fga_validator.py b/src/register_your_data_api/auth/fga/fga_validator.py index 044834e..af56f34 100644 --- a/src/register_your_data_api/auth/fga/fga_validator.py +++ b/src/register_your_data_api/auth/fga/fga_validator.py @@ -1,18 +1,36 @@ +import collections +import functools from uuid import UUID import pydantic -from .models import FineGrainedAuthorisationRole, FineGrainedAuthorisationRoleAssociation +from .models import FineGrainedAuthorisationRole, FineGrainedAuthorisationRoleAssociation, FineGrainedAuthorisationTool class FineGrainedAuthorisationUserValidator(pydantic.BaseModel): user_id: UUID + client_id: str + fine_grained_authorisations: list[FineGrainedAuthorisationRoleAssociation] | None is_superadmin: bool + tools: list[FineGrainedAuthorisationTool] + + @functools.cached_property + def _tool_id_map(self) -> dict[UUID, int]: + return {tool.id: index for index, tool in enumerate(self.tools)} + + def get_tool_by_id(self, id: UUID) -> FineGrainedAuthorisationTool: + return self.tools[self._tool_id_map[id]] + + def is_tool_same_as_client(self, tool_id: UUID) -> bool: + if self.get_tool_by_id(tool_id).client_id == self.client_id: + return True + return False + def get_permissions_for_role(self, user_role: FineGrainedAuthorisationRole) -> list[str]: permissions: dict[FineGrainedAuthorisationRole, list[str]] = { FineGrainedAuthorisationRole.CONTRIBUTOR_PENDING: [], @@ -42,17 +60,36 @@ def get_permissions_for_role(self, user_role: FineGrainedAuthorisationRole) -> l return permissions[user_role] def get_user_role_for_reporting_org(self, reporting_org_id: str | UUID) -> FineGrainedAuthorisationRole | None: - reporting_orgs = [] # type: list[FineGrainedAuthorisationRoleAssociation] + associations = [] # type: list[FineGrainedAuthorisationRoleAssociation] if self.fine_grained_authorisations is not None: id_as_uuid = reporting_org_id if isinstance(reporting_org_id, UUID) else UUID(reporting_org_id) - reporting_orgs = list(filter(lambda x: x.reporting_org == id_as_uuid, self.fine_grained_authorisations)) - - if len(reporting_orgs) >= 1: - return reporting_orgs[0].role + associations = list(filter(lambda x: x.reporting_org == id_as_uuid, self.fine_grained_authorisations)) if self.is_superadmin: return FineGrainedAuthorisationRole.SUPER_ADMIN + if len(associations) == 0: + return None + + if len(associations) == 1 and associations[0].role != FineGrainedAuthorisationRole.PROVIDER_ADMIN: + return associations[0].role + + # Remaining associations should all be provider_admin. Do a quick check that this is the case. + if collections.Counter([association.role for association in associations])[ + FineGrainedAuthorisationRole.PROVIDER_ADMIN + ] != len(associations): + raise RuntimeError("Validator in an unknown state") + + # If any role has a tool that matches the one the user is logged into then return provider admin. + if any( + [ + self.is_tool_same_as_client(association.restricted_to_tool) + for association in associations + if association.restricted_to_tool is not None + ] + ): + return FineGrainedAuthorisationRole.PROVIDER_ADMIN + return None def user_can_create_reporting_org(self) -> bool: @@ -193,4 +230,7 @@ def get_users_fine_grained_associations(self) -> list[FineGrainedAuthorisationRo def get_users_reporting_orgs(self) -> list[UUID]: if self.fine_grained_authorisations is None: return [] - return [fga.reporting_org for fga in self.fine_grained_authorisations] + return list(set([fga.reporting_org for fga in self.fine_grained_authorisations])) + + def get_users_provider_admin_tools(self) -> list[FineGrainedAuthorisationTool]: + return self.tools From a97a3ee002eb4fe28702c9e717c53ad3ee9d9929 Mon Sep 17 00:00:00 2001 From: Chris Arridge Date: Mon, 27 Apr 2026 16:57:42 +0100 Subject: [PATCH 05/16] feat: add client id and tools to FineGrainedAuthorisationUserValidator When initialising the FineGrainedAuthorisationUserValidator the user client ID and list of user tools is added to the validator. --- src/register_your_data_api/auth/authz.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/register_your_data_api/auth/authz.py b/src/register_your_data_api/auth/authz.py index af95aae..6408c58 100644 --- a/src/register_your_data_api/auth/authz.py +++ b/src/register_your_data_api/auth/authz.py @@ -25,6 +25,7 @@ async def get_user_authnz( try: users_fgas = context.fine_grained_auth_provider.get_user_fine_grained_permissions(current_user_id) + users_tools = context.fine_grained_auth_provider.get_tools_for_user(current_user_id) except FineGrainedAuthorisationIntegrityError as exc: trace_id: UUID = uuid4() raise RYDUserException( @@ -41,7 +42,11 @@ async def get_user_authnz( is_superadmin = context.fine_grained_auth_provider.is_user_a_superadmin(current_user_id) user.fga_user_validator = FineGrainedAuthorisationUserValidator( - user_id=current_user_id, fine_grained_authorisations=users_fgas, is_superadmin=is_superadmin + user_id=current_user_id, + fine_grained_authorisations=users_fgas, + is_superadmin=is_superadmin, + client_id=user.client_id, + tools=users_tools, ) return user From ed48a4212e4a4ef6a8ce8aa4a66b0a80a0655017 Mon Sep 17 00:00:00 2001 From: Chris Arridge Date: Mon, 27 Apr 2026 18:35:47 +0100 Subject: [PATCH 06/16] test: fix dataset and reporting org routes to work with client id --- tests/helpers/mocking.py | 33 ++++++- tests/integration/test_dataset_routes.py | 61 ++++++------ .../integration/test_reporting_org_routes.py | 97 ++++++++++--------- 3 files changed, 112 insertions(+), 79 deletions(-) diff --git a/tests/helpers/mocking.py b/tests/helpers/mocking.py index f22ee4c..515fa3a 100644 --- a/tests/helpers/mocking.py +++ b/tests/helpers/mocking.py @@ -185,6 +185,8 @@ class MockedAppAndContext: _mocked_tool_ids: list[UUID] = [] + _mocked_tool_client_ids: list[str] = [] + def __init__(self) -> None: # Generate two test keys self._JWKS_KEYS: dict[str, tuple[bytes, bytes, rsa.RSAPublicKey]] = {} @@ -208,6 +210,9 @@ def __init__(self) -> None: self._mocked_tool_ids.append(UUID("3a9d5496-77f4-497d-bb77-2bda91285111")) # Tool One self._mocked_tool_ids.append(UUID("6a2d1ca1-b9c2-4bd3-a2a5-099178d1358d")) # Tool Two + self._mocked_tool_client_ids.append("wUtu4EuLSlstjasC") # Tool One + self._mocked_tool_client_ids.append("f0fAYHMeLZv8WbfE") # Tool Two + self._mocked_context = make_test_context(self) # type: ignore def __del__(self) -> None: @@ -247,13 +252,15 @@ async def test_lifespan(app: FastAPI) -> AsyncIterator[None]: return self._app - def get_valid_authorization_header(self, test_user_num: int) -> dict[str, str]: + def get_valid_authorization_header(self, test_user_num: int, client_id: str = "some_client") -> dict[str, str]: """Generates an authorisation header for the user specified in test_user_num Parameters ---------- test_user_num : int Index of the mock test user. + client_id : str + Client ID to use in the claim. Returns ------- @@ -263,11 +270,13 @@ def get_valid_authorization_header(self, test_user_num: int) -> dict[str, str]: claims = make_access_token_payload( subject=str(self._mocked_user_ids[test_user_num]), + client_id=client_id, audience="iati_register_your_data", scopes=( "ryd ryd:reporting_org ryd:reporting_org:create ryd:reporting_org:update ryd:reporting_org:delete " - "ryd:reporting_org:user ryd:reporting_org:user:update ryd:dataset " - "ryd:dataset:update ryd:dataset:delete" + "ryd:reporting_org:user ryd:reporting_org:user:update " + "ryd:reporting_org:tool ryd:reporting_org:tool:update " + "ryd:dataset ryd:dataset:update ryd:dataset:delete" ), ) token = jwt.encode(claims, self._JWKS_KEYS["key1"][0], algorithm="RS256", headers={"kid": "key1"}) @@ -413,7 +422,14 @@ def make_test_context(app_and_context: MockedAppAndContext) -> util.Context: ) # Add tools. - session.add(ToolDbModel(id=app_and_context._mocked_tool_ids[0], name="Tool One", provider="Tool Maker")) + session.add( + ToolDbModel( + id=app_and_context._mocked_tool_ids[0], + name="Tool One", + provider="Tool Maker", + client_id=app_and_context._mocked_tool_client_ids[0], + ) + ) session.add( ToolAuthorisationDbModel( tool=app_and_context._mocked_tool_ids[0], reporting_org=app_and_context._mocked_reporting_org_ids[0] @@ -424,7 +440,14 @@ def make_test_context(app_and_context: MockedAppAndContext) -> util.Context: tool=app_and_context._mocked_tool_ids[0], reporting_org=app_and_context._mocked_reporting_org_ids[1] ) ) - session.add(ToolDbModel(id=app_and_context._mocked_tool_ids[1], name="Tool Two", provider="Tool Maker")) + session.add( + ToolDbModel( + id=app_and_context._mocked_tool_ids[1], + name="Tool Two", + provider="Tool Maker", + client_id=app_and_context._mocked_tool_client_ids[1], + ) + ) # Person Five is a user for both tools. session.add( diff --git a/tests/integration/test_dataset_routes.py b/tests/integration/test_dataset_routes.py index 8a5919f..adca3ef 100644 --- a/tests/integration/test_dataset_routes.py +++ b/tests/integration/test_dataset_routes.py @@ -107,17 +107,18 @@ def test_dataset_update_with_invalid_short_name(invalid_short_name: str) -> None @pytest.mark.parametrize( - "user,status_code", + "user,status_code,client_id", [ - (0, 403), # Editor - (1, 200), # Admin - (2, 200), # Superadmin - (3, 403), # Contributor - (4, 200), # Provider admin - (6, 500), # Failure case, user is both provider admin and has an org role + (0, 403, "some_client"), # Editor + (1, 200, "some_client"), # Admin + (2, 200, "some_client"), # Superadmin + (3, 403, "some_client"), # Contributor + (4, 200, "wUtu4EuLSlstjasC"), # Provider admin via Tool 1 + (4, 403, "some_client"), # Provider admin via another tool + (6, 500, "some_client"), # Failure case, user is both provider admin and has an org role ], ) -def test_change_dataset_visibility(user: int, status_code: int) -> None: +def test_change_dataset_visibility(user: int, status_code: int, client_id: str) -> None: appAndContext = MockedAppAndContext() @@ -129,7 +130,7 @@ def test_change_dataset_visibility(user: int, status_code: int) -> None: # artefacts, so here we try to change the visibility to private. response = client.patch( "/api/v1/datasets/52ac525f-6375-079b-977d-68ecf3be2868", - headers=appAndContext.get_valid_authorization_header(user), + headers=appAndContext.get_valid_authorization_header(user, client_id=client_id), content=json.dumps( { "human_readable_name": "Aid Agency - Dataset 01", @@ -146,18 +147,19 @@ def test_change_dataset_visibility(user: int, status_code: int) -> None: @pytest.mark.parametrize( - "user,status_code,organisation_id", + "user,status_code,organisation_id,client_id", [ - (0, 201, "552376ae-2aa7-98ab-d800-68daa9bfeb4a"), # Editor of org - (1, 201, "552376ae-2aa7-98ab-d800-68daa9bfeb4a"), # Admin of org - (2, 201, "552376ae-2aa7-98ab-d800-68daa9bfeb4a"), # Superadmin - (3, 201, "552376ae-2aa7-98ab-d800-68daa9bfeb4a"), # Contributor of org - (3, 403, "ab851a83-a384-3eb9-caf0-68db8125b067"), # Contributor pending in org - (4, 201, "552376ae-2aa7-98ab-d800-68daa9bfeb4a"), # Provider admin via Tool One - (6, 500, "552376ae-2aa7-98ab-d800-68daa9bfeb4a"), # Failure: has provider admin and org role + (0, 201, "552376ae-2aa7-98ab-d800-68daa9bfeb4a", "some_client"), # Editor of org + (1, 201, "552376ae-2aa7-98ab-d800-68daa9bfeb4a", "some_client"), # Admin of org + (2, 201, "552376ae-2aa7-98ab-d800-68daa9bfeb4a", "some_client"), # Superadmin + (3, 201, "552376ae-2aa7-98ab-d800-68daa9bfeb4a", "some_client"), # Contributor of org + (3, 403, "ab851a83-a384-3eb9-caf0-68db8125b067", "some_client"), # Contributor pending in org + (4, 201, "552376ae-2aa7-98ab-d800-68daa9bfeb4a", "wUtu4EuLSlstjasC"), # Provider admin via Tool One + (4, 403, "552376ae-2aa7-98ab-d800-68daa9bfeb4a", "some_client"), # Provider admin via another tool + (6, 500, "552376ae-2aa7-98ab-d800-68daa9bfeb4a", "some_client"), # Failure: has provider admin and org role ], ) -def test_create_dataset(user: int, status_code: int, organisation_id: str) -> None: +def test_create_dataset(user: int, status_code: int, organisation_id: str, client_id: str) -> None: appAndContext = MockedAppAndContext() @@ -167,7 +169,7 @@ def test_create_dataset(user: int, status_code: int, organisation_id: str) -> No response = client.post( "/api/v1/datasets", - headers=appAndContext.get_valid_authorization_header(user), + headers=appAndContext.get_valid_authorization_header(user, client_id=client_id), content=json.dumps( { "human_readable_name": "Aid Agency - Dataset 01", @@ -185,18 +187,19 @@ def test_create_dataset(user: int, status_code: int, organisation_id: str) -> No @pytest.mark.parametrize( - "user,status_code,dataset_id", + "user,status_code,dataset_id,client_id", [ - (0, 200, "52ac525f-6375-079b-977d-68ecf3be2868"), # Editor of org - (1, 200, "52ac525f-6375-079b-977d-68ecf3be2868"), # Admin of org - (2, 200, "52ac525f-6375-079b-977d-68ecf3be2868"), # Superadmin - (3, 403, "52ac525f-6375-079b-977d-68ecf3be2868"), # Contributor of org - (3, 403, "4ad2b196-aa31-983e-93d9-68ecf476a2c6"), # Contributor pending in org - (4, 200, "52ac525f-6375-079b-977d-68ecf3be2868"), # Provider admin via Tool One - (6, 500, "52ac525f-6375-079b-977d-68ecf3be2868"), # Failure: has provider admin and org role + (0, 200, "52ac525f-6375-079b-977d-68ecf3be2868", "some_client"), # Editor of org + (1, 200, "52ac525f-6375-079b-977d-68ecf3be2868", "some_client"), # Admin of org + (2, 200, "52ac525f-6375-079b-977d-68ecf3be2868", "some_client"), # Superadmin + (3, 403, "52ac525f-6375-079b-977d-68ecf3be2868", "some_client"), # Contributor of org + (3, 403, "4ad2b196-aa31-983e-93d9-68ecf476a2c6", "some_client"), # Contributor pending in org + (4, 200, "52ac525f-6375-079b-977d-68ecf3be2868", "wUtu4EuLSlstjasC"), # Provider admin via Tool One + (4, 403, "52ac525f-6375-079b-977d-68ecf3be2868", "some_client"), # Provider admin via another tool + (6, 500, "52ac525f-6375-079b-977d-68ecf3be2868", "some_client"), # Failure: has provider admin and org role ], ) -def test_delete_dataset(user: int, status_code: int, dataset_id: str) -> None: +def test_delete_dataset(user: int, status_code: int, dataset_id: str, client_id: str) -> None: appAndContext = MockedAppAndContext() @@ -206,7 +209,7 @@ def test_delete_dataset(user: int, status_code: int, dataset_id: str) -> None: response = client.delete( f"/api/v1/datasets/{dataset_id}", - headers=appAndContext.get_valid_authorization_header(user), + headers=appAndContext.get_valid_authorization_header(user, client_id=client_id), ) assert response.status_code == status_code diff --git a/tests/integration/test_reporting_org_routes.py b/tests/integration/test_reporting_org_routes.py index 48b2126..3514c6a 100644 --- a/tests/integration/test_reporting_org_routes.py +++ b/tests/integration/test_reporting_org_routes.py @@ -182,27 +182,31 @@ def test_reporting_org_detail_handles_non_uuid() -> None: @pytest.mark.parametrize( - "user,reporting_org_id,status_code", + "user,reporting_org_id,client_id,status_code", [ - (0, "552376ae-2aa7-98ab-d800-68daa9bfeb4a", 200), - (0, "ab851a83-a384-3eb9-caf0-68db8125b067", 200), - (0, "01234567-0000-1111-2222-0123456789ab", 403), - (1, "552376ae-2aa7-98ab-d800-68daa9bfeb4a", 200), - (1, "ab851a83-a384-3eb9-caf0-68db8125b067", 403), - (1, "01234567-0000-1111-2222-0123456789ab", 403), - (2, "552376ae-2aa7-98ab-d800-68daa9bfeb4a", 200), - (2, "ab851a83-a384-3eb9-caf0-68db8125b067", 200), - (2, "01234567-0000-1111-2222-0123456789ab", 404), - (4, "552376ae-2aa7-98ab-d800-68daa9bfeb4a", 200), - (4, "ab851a83-a384-3eb9-caf0-68db8125b067", 200), - (4, "da17734d-3926-47ef-8563-8a1b0247ed11", 403), - (4, "01234567-0000-1111-2222-0123456789ab", 403), - (6, "da17734d-3926-47ef-8563-8a1b0247ed11", 500), - (6, "ab851a83-a384-3eb9-caf0-68db8125b067", 500), - (6, "01234567-0000-1111-2222-0123456789ab", 500), + (0, "552376ae-2aa7-98ab-d800-68daa9bfeb4a", "some_client", 200), + (0, "ab851a83-a384-3eb9-caf0-68db8125b067", "some_client", 200), + (0, "01234567-0000-1111-2222-0123456789ab", "some_client", 403), + (1, "552376ae-2aa7-98ab-d800-68daa9bfeb4a", "some_client", 200), + (1, "ab851a83-a384-3eb9-caf0-68db8125b067", "some_client", 403), + (1, "01234567-0000-1111-2222-0123456789ab", "some_client", 403), + (2, "552376ae-2aa7-98ab-d800-68daa9bfeb4a", "some_client", 200), + (2, "ab851a83-a384-3eb9-caf0-68db8125b067", "some_client", 200), + (2, "01234567-0000-1111-2222-0123456789ab", "some_client", 404), + (4, "552376ae-2aa7-98ab-d800-68daa9bfeb4a", "wUtu4EuLSlstjasC", 200), + (4, "ab851a83-a384-3eb9-caf0-68db8125b067", "wUtu4EuLSlstjasC", 200), + (4, "552376ae-2aa7-98ab-d800-68daa9bfeb4a", "some_client", 403), + (4, "ab851a83-a384-3eb9-caf0-68db8125b067", "some_client", 403), + (4, "da17734d-3926-47ef-8563-8a1b0247ed11", "some_client", 403), + (4, "01234567-0000-1111-2222-0123456789ab", "some_client", 403), + (6, "da17734d-3926-47ef-8563-8a1b0247ed11", "some_client", 500), + (6, "ab851a83-a384-3eb9-caf0-68db8125b067", "some_client", 500), + (6, "01234567-0000-1111-2222-0123456789ab", "some_client", 500), ], ) -def test_reporting_org_detail_check_user_access(user: int, reporting_org_id: str, status_code: int) -> None: +def test_reporting_org_detail_check_user_access( + user: int, reporting_org_id: str, client_id: str, status_code: int +) -> None: """Tests the user's access to /reporting_orgs/{oid}""" appAndContext = MockedAppAndContext() @@ -211,7 +215,8 @@ def test_reporting_org_detail_check_user_access(user: int, reporting_org_id: str with TestClient(fastAPIapp) as client: response = client.get( - f"/api/v1/reporting-orgs/{reporting_org_id}", headers=appAndContext.get_valid_authorization_header(user) + f"/api/v1/reporting-orgs/{reporting_org_id}", + headers=appAndContext.get_valid_authorization_header(user, client_id=client_id), ) assert response.status_code == status_code @@ -268,7 +273,7 @@ def test_reporting_org_detail_check_values( with TestClient(fastAPIapp) as client: response = client.get( f"/api/v1/reporting-orgs/{reporting_org_id}", - headers=appAndContext.get_valid_authorization_header(user), + headers=appAndContext.get_valid_authorization_header(user, client_id="wUtu4EuLSlstjasC"), params={}, ) @@ -494,7 +499,7 @@ def test_reporting_org_update(user: int, status_code: int, organisation_id: str) response = client.patch( f"/api/v1/reporting-orgs/{organisation_id}", - headers=appAndContext.get_valid_authorization_header(user), + headers=appAndContext.get_valid_authorization_header(user, client_id="wUtu4EuLSlstjasC"), content=json.dumps(payload), ) @@ -508,38 +513,40 @@ def test_reporting_org_delete(user: int, status_code: int) -> None: with TestClient(fastAPIapp) as client: response = client.delete( "/api/v1/reporting-orgs/552376ae-2aa7-98ab-d800-68daa9bfeb4a", - headers=appAndContext.get_valid_authorization_header(user), + headers=appAndContext.get_valid_authorization_header(user, client_id="wUtu4EuLSlstjasC"), ) assert response.status_code == status_code @pytest.mark.parametrize( - "user,reporting_org_id,status,status_s,num_datasets", + "user,reporting_org_id,client_id,status,status_s,num_datasets", [ - (0, "552376ae-2aa7-98ab-d800-68daa9bfeb4a", 200, "success", 1), - (0, "ab851a83-a384-3eb9-caf0-68db8125b067", 200, "success", 2), - (0, "da17734d-3926-47ef-8563-8a1b0247ed11", 403, "failed", -1), - (0, "92f398c1-6163-f097-3ded-68e9138bb9c8", 403, "failed", -1), - (1, "552376ae-2aa7-98ab-d800-68daa9bfeb4a", 200, "success", 1), - (1, "ab851a83-a384-3eb9-caf0-68db8125b067", 403, "failed", -1), - (1, "da17734d-3926-47ef-8563-8a1b0247ed11", 200, "success", 0), - (2, "552376ae-2aa7-98ab-d800-68daa9bfeb4a", 200, "success", 1), - (2, "ab851a83-a384-3eb9-caf0-68db8125b067", 200, "success", 2), - (2, "92f398c1-6163-f097-3ded-68e9138bb9c8", 404, "failed", -1), - (2, "da17734d-3926-47ef-8563-8a1b0247ed11", 200, "success", 0), - (4, "552376ae-2aa7-98ab-d800-68daa9bfeb4a", 200, "success", 1), - (4, "ab851a83-a384-3eb9-caf0-68db8125b067", 200, "success", 2), - (4, "92f398c1-6163-f097-3ded-68e9138bb9c8", 403, "failed", -1), - (4, "da17734d-3926-47ef-8563-8a1b0247ed11", 403, "failed", -1), - (6, "da17734d-3926-47ef-8563-8a1b0247ed11", 500, "failed", -1), - (6, "ab851a83-a384-3eb9-caf0-68db8125b067", 500, "failed", -1), - (6, "552376ae-2aa7-98ab-d800-68daa9bfeb4a", 500, "failed", -1), - (6, "92f398c1-6163-f097-3ded-68e9138bb9c8", 500, "failed", -1), + (0, "552376ae-2aa7-98ab-d800-68daa9bfeb4a", "some_client", 200, "success", 1), + (0, "ab851a83-a384-3eb9-caf0-68db8125b067", "some_client", 200, "success", 2), + (0, "da17734d-3926-47ef-8563-8a1b0247ed11", "some_client", 403, "failed", -1), + (0, "92f398c1-6163-f097-3ded-68e9138bb9c8", "some_client", 403, "failed", -1), + (1, "552376ae-2aa7-98ab-d800-68daa9bfeb4a", "some_client", 200, "success", 1), + (1, "ab851a83-a384-3eb9-caf0-68db8125b067", "some_client", 403, "failed", -1), + (1, "da17734d-3926-47ef-8563-8a1b0247ed11", "some_client", 200, "success", 0), + (2, "552376ae-2aa7-98ab-d800-68daa9bfeb4a", "some_client", 200, "success", 1), + (2, "ab851a83-a384-3eb9-caf0-68db8125b067", "some_client", 200, "success", 2), + (2, "92f398c1-6163-f097-3ded-68e9138bb9c8", "some_client", 404, "failed", -1), + (2, "da17734d-3926-47ef-8563-8a1b0247ed11", "some_client", 200, "success", 0), + (4, "552376ae-2aa7-98ab-d800-68daa9bfeb4a", "wUtu4EuLSlstjasC", 200, "success", 1), + (4, "ab851a83-a384-3eb9-caf0-68db8125b067", "wUtu4EuLSlstjasC", 200, "success", 2), + (4, "552376ae-2aa7-98ab-d800-68daa9bfeb4a", "invalid_client", 403, "failed", -1), + (4, "ab851a83-a384-3eb9-caf0-68db8125b067", "invalid_client", 403, "failed", -1), + (4, "92f398c1-6163-f097-3ded-68e9138bb9c8", "some_client", 403, "failed", -1), + (4, "da17734d-3926-47ef-8563-8a1b0247ed11", "some_client", 403, "failed", -1), + (6, "da17734d-3926-47ef-8563-8a1b0247ed11", "some_client", 500, "failed", -1), + (6, "ab851a83-a384-3eb9-caf0-68db8125b067", "some_client", 500, "failed", -1), + (6, "552376ae-2aa7-98ab-d800-68daa9bfeb4a", "some_client", 500, "failed", -1), + (6, "92f398c1-6163-f097-3ded-68e9138bb9c8", "some_client", 500, "failed", -1), ], ) def test_reporting_org_list_datasets( - user: int, reporting_org_id: str, status: int, status_s: str, num_datasets: int + user: int, reporting_org_id: str, client_id: str, status: int, status_s: str, num_datasets: int ) -> None: appAndContext = MockedAppAndContext() @@ -550,7 +557,7 @@ def test_reporting_org_list_datasets( response = client.get( f"/api/v1/reporting-orgs/{reporting_org_id}/datasets", - headers=appAndContext.get_valid_authorization_header(user), + headers=appAndContext.get_valid_authorization_header(user, client_id=client_id), ) assert response.status_code == status @@ -725,7 +732,7 @@ def test_reporting_org_list_users( response = client.get( f"/api/v1/reporting-orgs/{reporting_org_id}/users", - headers=appAndContext.get_valid_authorization_header(user), + headers=appAndContext.get_valid_authorization_header(user, client_id="wUtu4EuLSlstjasC"), ) if expect_unauthorised: From 716bd40f536f0d724ecaa86305d827bb2762f3d9 Mon Sep 17 00:00:00 2001 From: Chris Arridge Date: Fri, 24 Apr 2026 10:47:32 +0100 Subject: [PATCH 07/16] test: add tools list endpoint test Add test of the new /tools endpoint to fetch a list of tools known to RYD. --- tests/integration/test_tools_list.py | 40 ++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 tests/integration/test_tools_list.py diff --git a/tests/integration/test_tools_list.py b/tests/integration/test_tools_list.py new file mode 100644 index 0000000..9a1bba5 --- /dev/null +++ b/tests/integration/test_tools_list.py @@ -0,0 +1,40 @@ +import pytest +from fastapi.testclient import TestClient + +from ..helpers.mocking import MockedAppAndContext + + +@pytest.mark.parametrize( + "logged_in_user_idx,expected_status_code", + [ + (0, 200), + (1, 200), + (2, 200), + (3, 200), + (4, 200), + (6, 500), + ], +) +def test_get_tool_list(logged_in_user_idx: int, expected_status_code: int) -> None: + appAndContext = MockedAppAndContext() + + fastAPIapp = appAndContext.get_test_app() + + with TestClient(fastAPIapp) as client: + response = client.get( + "/api/v1/tools", + headers=appAndContext.get_valid_authorization_header(logged_in_user_idx), + ) + response_as_object = response.json() + + assert response.status_code == expected_status_code + assert "data" in response_as_object + + if expected_status_code == 200: + assert sorted(response_as_object["data"], key=lambda x: x["id"]) == sorted( + [ + {"id": "3a9d5496-77f4-497d-bb77-2bda91285111", "name": "Tool One", "provider": "Tool Maker"}, + {"id": "6a2d1ca1-b9c2-4bd3-a2a5-099178d1358d", "name": "Tool Two", "provider": "Tool Maker"}, + ], + key=lambda x: x["id"], + ) From f65b0979b1a40e88efd95cc74efdc65c768ffff9 Mon Sep 17 00:00:00 2001 From: Chris Arridge Date: Sat, 25 Apr 2026 10:25:41 +0100 Subject: [PATCH 08/16] feat: implement tools list endpoint Implements the GET /tools list endpoint. --- .../data_handling/data_schemas.py | 12 ++++++++++++ src/register_your_data_api/routers/misc.py | 17 +++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/src/register_your_data_api/data_handling/data_schemas.py b/src/register_your_data_api/data_handling/data_schemas.py index 73c2198..a777c5a 100644 --- a/src/register_your_data_api/data_handling/data_schemas.py +++ b/src/register_your_data_api/data_handling/data_schemas.py @@ -237,3 +237,15 @@ class UserReportingOrgRelationSingleResponse(pydantic.BaseModel): data: UserReportingOrgRelation error: str | None = pydantic.Field(None) status: str + + +class ToolMetadata(pydantic.BaseModel): + id: uuid.UUID + name: str + provider: str + + +class ToolListResponse(pydantic.BaseModel): + data: list[ToolMetadata] + error: str | None + status: str diff --git a/src/register_your_data_api/routers/misc.py b/src/register_your_data_api/routers/misc.py index 569c3fe..7b2b1c5 100644 --- a/src/register_your_data_api/routers/misc.py +++ b/src/register_your_data_api/routers/misc.py @@ -4,8 +4,11 @@ from fastapi.responses import JSONResponse from fastapi.security import SecurityScopes +from ..auth import authz from ..auth.authn import parse_decoded_token from ..auth.models import UserAndCredentials +from ..data_handling.data_schemas import ToolListResponse, ToolMetadata +from ..util import Context router = fastapi.APIRouter() @@ -47,3 +50,17 @@ def get_licences() -> JSONResponse: status_code=fastapi.status.HTTP_501_NOT_IMPLEMENTED, detail="Not yet implemented", ) + + +@router.get("/api/v1/tools") +def get_tool_list( + request: starlette.requests.Request, user: UserAndCredentials = Security(authz.get_user_authnz, scopes=["ryd"]) +) -> ToolListResponse: + + context: Context = request.app.state.context + + db_tools = context.fine_grained_auth_provider.get_all_tools() + + return ToolListResponse( + status="success", error=None, data=[ToolMetadata(**db_tool.model_dump()) for db_tool in db_tools] + ) From bba75b4454033369168623b3eb98ca7395f26dc0 Mon Sep 17 00:00:00 2001 From: Chris Arridge Date: Fri, 24 Apr 2026 10:47:19 +0100 Subject: [PATCH 09/16] test: user roles and reporting org permissions endpoints Adds tests of the /user/{uid}/roles endpoint for returning a list of user roles, and a test for the (not yet implemented) /user/{uid}/roles/reporting-org-permissions/{oid} endpoint. --- tests/integration/test_users_routes.py | 170 +++++++++++++++++++++++-- 1 file changed, 160 insertions(+), 10 deletions(-) diff --git a/tests/integration/test_users_routes.py b/tests/integration/test_users_routes.py index 3cff67d..90316b8 100644 --- a/tests/integration/test_users_routes.py +++ b/tests/integration/test_users_routes.py @@ -211,12 +211,12 @@ def test_update_user_role_role_updated(new_role: str) -> None: assert response.status_code == 200 - role = appAndContext._mocked_context.fine_grained_auth_provider.get_user_role_for_org( + roles = appAndContext._mocked_context.fine_grained_auth_provider.get_user_roles_for_org( uuid.UUID("698e0c1f-4e80-faa9-6533-68de801d1735"), uuid.UUID("552376ae-2aa7-98ab-d800-68daa9bfeb4a") ) - assert role is not None - assert role.role.name.lower() == new_role + assert roles + assert roles[0].role.name.lower() == new_role def test_update_user_role_role_repaired() -> None: @@ -225,11 +225,11 @@ def test_update_user_role_role_repaired() -> None: fastAPIapp = appAndContext.get_test_app() - role = appAndContext._mocked_context.fine_grained_auth_provider.get_user_role_for_org( + roles = appAndContext._mocked_context.fine_grained_auth_provider.get_user_roles_for_org( uuid.UUID("8c51d9bf-46c2-4d1d-869b-d9e2dd63ff48"), uuid.UUID("552376ae-2aa7-98ab-d800-68daa9bfeb4a") ) - assert role is None # pre-check - ensure no role exists before the update attempt + assert len(roles) == 0 # pre-check - ensure no role exists before the update attempt with TestClient(fastAPIapp) as client: @@ -241,12 +241,11 @@ def test_update_user_role_role_repaired() -> None: assert response.status_code == 200 - role = appAndContext._mocked_context.fine_grained_auth_provider.get_user_role_for_org( + roles = appAndContext._mocked_context.fine_grained_auth_provider.get_user_roles_for_org( uuid.UUID("8c51d9bf-46c2-4d1d-869b-d9e2dd63ff48"), uuid.UUID("552376ae-2aa7-98ab-d800-68daa9bfeb4a") ) - assert role is not None - assert role.role.name.lower() == "editor" + assert roles[0].role.name.lower() == "editor" @pytest.mark.parametrize( @@ -372,8 +371,159 @@ def test_delete_user_role_role_delete() -> None: assert response.status_code == 200 - role = appAndContext._mocked_context.fine_grained_auth_provider.get_user_role_for_org( + roles = appAndContext._mocked_context.fine_grained_auth_provider.get_user_roles_for_org( uuid.UUID("698e0c1f-4e80-faa9-6533-68de801d1735"), uuid.UUID("552376ae-2aa7-98ab-d800-68daa9bfeb4a") ) - assert role is None + assert len(roles) == 0 + + +@pytest.mark.parametrize( + "logged_in_user_idx,user_id,expected_status_code", + [ + (0, "698e0c1f-4e80-faa9-6533-68de801d1735", 200), # Person One getting Person One roles + (0, "bea511d3-c7a7-4097-55ed-68de81e94921", 403), # Person One getting Person Two roles + (0, "a1b191ee-4c12-461c-cbe1-68de8173f628", 403), # Person One getting Person Three roles + (0, "7625122c-f752-40dc-a577-5cb49e13de2a", 403), # Person One getting Person Four roles + (0, "b46b88bd-05e6-4cb8-8b6a-a2c47fcd666d", 403), # Person One getting Person Five roles + (0, "5c633101-42be-47ac-81e7-43d6ecb503e3", 403), # Person One getting Person Seven roles + (0, "1d973500-9c23-4f63-914c-c7f1d78fb756", 403), # Person One getting non-existent user roles + (1, "698e0c1f-4e80-faa9-6533-68de801d1735", 403), # Person Two getting Person One roles + (1, "bea511d3-c7a7-4097-55ed-68de81e94921", 200), # Person Two getting Person Two roles + (1, "a1b191ee-4c12-461c-cbe1-68de8173f628", 403), # Person Two getting Person Three roles + (1, "7625122c-f752-40dc-a577-5cb49e13de2a", 403), # Person Two getting Person Four roles + (1, "b46b88bd-05e6-4cb8-8b6a-a2c47fcd666d", 403), # Person Two getting Person Five roles + (1, "5c633101-42be-47ac-81e7-43d6ecb503e3", 403), # Person Two getting Person Seven roles + (1, "1d973500-9c23-4f63-914c-c7f1d78fb756", 403), # Person Two getting non-existent user roles + (2, "698e0c1f-4e80-faa9-6533-68de801d1735", 200), # Person Three (s/admin) getting Person One roles + (2, "bea511d3-c7a7-4097-55ed-68de81e94921", 200), # Person Three (s/admin) getting Person Two roles + (2, "a1b191ee-4c12-461c-cbe1-68de8173f628", 200), # Person Three (s/admin) getting Person Three roles + (2, "7625122c-f752-40dc-a577-5cb49e13de2a", 200), # Person Three (s/admin) getting Person Four roles + (2, "b46b88bd-05e6-4cb8-8b6a-a2c47fcd666d", 200), # Person Three (s/admin) getting Person Five roles + (2, "5c633101-42be-47ac-81e7-43d6ecb503e3", 200), # Person Three (s/admin) getting Person Seven roles + (2, "1d973500-9c23-4f63-914c-c7f1d78fb756", 404), # Person Three (s/admin) getting non-existent user roles + (3, "698e0c1f-4e80-faa9-6533-68de801d1735", 403), # Person Four getting Person One roles + (3, "bea511d3-c7a7-4097-55ed-68de81e94921", 403), # Person Four getting Person Two roles + (3, "a1b191ee-4c12-461c-cbe1-68de8173f628", 403), # Person Four getting Person Three roles + (3, "7625122c-f752-40dc-a577-5cb49e13de2a", 200), # Person Four getting Person Four roles + (3, "b46b88bd-05e6-4cb8-8b6a-a2c47fcd666d", 403), # Person Four getting Person Five roles + (3, "5c633101-42be-47ac-81e7-43d6ecb503e3", 403), # Person Four getting Person Seven roles + (3, "1d973500-9c23-4f63-914c-c7f1d78fb756", 403), # Person Four getting non-existent user roles + (4, "698e0c1f-4e80-faa9-6533-68de801d1735", 403), # Person Five (p/admin) getting Person One roles + (4, "bea511d3-c7a7-4097-55ed-68de81e94921", 403), # Person Five (p/admin) getting Person Two roles + (4, "a1b191ee-4c12-461c-cbe1-68de8173f628", 403), # Person Five (p/admin) getting Person Three roles + (4, "7625122c-f752-40dc-a577-5cb49e13de2a", 403), # Person Five (p/admin) getting Person Four roles + (4, "b46b88bd-05e6-4cb8-8b6a-a2c47fcd666d", 200), # Person Five (p/admin) getting Person Five roles + (4, "5c633101-42be-47ac-81e7-43d6ecb503e3", 403), # Person Five (p/admin) getting Person Seven roles + (4, "1d973500-9c23-4f63-914c-c7f1d78fb756", 403), # Person Five (p/admin) getting non-existent user roles + (6, "698e0c1f-4e80-faa9-6533-68de801d1735", 500), # Person Seven (p/admin) getting Person One roles + (6, "bea511d3-c7a7-4097-55ed-68de81e94921", 500), # Person Seven (p/admin) getting Person Two roles + (6, "a1b191ee-4c12-461c-cbe1-68de8173f628", 500), # Person Seven (p/admin) getting Person Three roles + (6, "7625122c-f752-40dc-a577-5cb49e13de2a", 500), # Person Seven (p/admin) getting Person Four roles + (6, "b46b88bd-05e6-4cb8-8b6a-a2c47fcd666d", 500), # Person Seven (p/admin) getting Person Five roles + (6, "5c633101-42be-47ac-81e7-43d6ecb503e3", 500), # Person Seven (p/admin) getting Person Seven roles + (6, "1d973500-9c23-4f63-914c-c7f1d78fb756", 500), # Person Seven (p/admin) getting non-existent user roles + ], +) +def test_user_role_access_permissions(logged_in_user_idx: int, user_id: str, expected_status_code: int) -> None: + appAndContext = MockedAppAndContext() + + fastAPIapp = appAndContext.get_test_app() + + with TestClient(fastAPIapp) as client: + response = client.get( + f"/api/v1/users/{user_id}/roles", + headers=appAndContext.get_valid_authorization_header(logged_in_user_idx), + ) + + assert response.status_code == expected_status_code + + +@pytest.mark.parametrize( + "logged_in_user_idx,user_id,superadmin,tools,reporting_orgs", + [ + ( + 0, + "698e0c1f-4e80-faa9-6533-68de801d1735", + False, + [], + { + "552376ae-2aa7-98ab-d800-68daa9bfeb4a": "editor", + "ab851a83-a384-3eb9-caf0-68db8125b067": "admin", + }, + ), + ( + 1, + "bea511d3-c7a7-4097-55ed-68de81e94921", + False, + [], + { + "552376ae-2aa7-98ab-d800-68daa9bfeb4a": "admin", + "da17734d-3926-47ef-8563-8a1b0247ed11": "admin", + "0a3a9507-d674-480e-b625-7d190f4f3319": "admin", + }, + ), + (2, "a1b191ee-4c12-461c-cbe1-68de8173f628", True, [], {}), + ( + 3, + "7625122c-f752-40dc-a577-5cb49e13de2a", + False, + [], + { + "552376ae-2aa7-98ab-d800-68daa9bfeb4a": "contributor", + "ab851a83-a384-3eb9-caf0-68db8125b067": "contributor_pending", + "0a3a9507-d674-480e-b625-7d190f4f3319": "contributor_pending", + }, + ), + ( + 4, + "b46b88bd-05e6-4cb8-8b6a-a2c47fcd666d", + False, + ["3a9d5496-77f4-497d-bb77-2bda91285111", "6a2d1ca1-b9c2-4bd3-a2a5-099178d1358d"], + { + "552376ae-2aa7-98ab-d800-68daa9bfeb4a": "provider_admin", + "ab851a83-a384-3eb9-caf0-68db8125b067": "provider_admin", + }, + ), + ], +) +def test_user_roles_correctly_returned( + logged_in_user_idx: int, user_id: str, superadmin: bool, tools: list[str], reporting_orgs: dict[str, str] +) -> None: + appAndContext = MockedAppAndContext() + + fastAPIapp = appAndContext.get_test_app() + + with TestClient(fastAPIapp) as client: + response = client.get( + f"/api/v1/users/{user_id}/roles", + headers=appAndContext.get_valid_authorization_header(logged_in_user_idx), + ) + + assert response.status_code == 200 + + resp_as_object = response.json() + + assert resp_as_object["data"]["superadmin"] == superadmin + assert len(resp_as_object["data"]["tools"]) == len(tools) + assert sorted(resp_as_object["data"]["tools"]) == sorted(tools) + assert len(resp_as_object["data"]["reporting_orgs"]) == len(reporting_orgs) + assert resp_as_object["data"]["reporting_orgs"] == reporting_orgs + + +def test_user_role_reporting_org_permissions_not_implemented() -> None: + appAndContext = MockedAppAndContext() + + fastAPIapp = appAndContext.get_test_app() + + # When implemented, this call should return a list of permissions matching "editor". + with TestClient(fastAPIapp) as client: + response = client.get( + ( + "/api/v1/users/698e0c1f-4e80-faa9-6533-68de801d1735/roles/" + "reporting-org-permissions/552376ae-2aa7-98ab-d800-68daa9bfeb4a" + ), + headers=appAndContext.get_valid_authorization_header(0), + ) + + assert response.status_code == 501 From 58035f52952ba3d4a8331a695c709c9124749e79 Mon Sep 17 00:00:00 2001 From: Chris Arridge Date: Sat, 25 Apr 2026 11:50:04 +0100 Subject: [PATCH 10/16] feat: implement user roles endpoint and return 501 for permissions endpoint This commit implements the endpoint to return a list of user roles. It also adds a 501 response for the (not yet implemented) permissions endpoint. --- .../data_handling/data_schemas.py | 12 +++ src/register_your_data_api/routers/users.py | 76 ++++++++++++++++++- 2 files changed, 84 insertions(+), 4 deletions(-) diff --git a/src/register_your_data_api/data_handling/data_schemas.py b/src/register_your_data_api/data_handling/data_schemas.py index a777c5a..f0c57c7 100644 --- a/src/register_your_data_api/data_handling/data_schemas.py +++ b/src/register_your_data_api/data_handling/data_schemas.py @@ -249,3 +249,15 @@ class ToolListResponse(pydantic.BaseModel): data: list[ToolMetadata] error: str | None status: str + + +class UserRolesData(pydantic.BaseModel): + superadmin: bool + tools: list[uuid.UUID] + reporting_orgs: dict[uuid.UUID, str] + + +class UserRolesResponse(pydantic.BaseModel): + data: UserRolesData + error: str | None + status: str diff --git a/src/register_your_data_api/routers/users.py b/src/register_your_data_api/routers/users.py index 6a3e820..28634b8 100644 --- a/src/register_your_data_api/routers/users.py +++ b/src/register_your_data_api/routers/users.py @@ -22,12 +22,14 @@ from ..auth import authz from ..auth.models import UserAndCredentials -from ..data_handling.converters import get_fga_role_from_str +from ..data_handling.converters import get_fga_role_as_str, get_fga_role_from_str from ..data_handling.data_schemas import ( OrganisationId, PaginationQueryParams, UserReportingOrgDiscoverableMetadataRelation, UserReportingOrgRelation, + UserRolesData, + UserRolesResponse, UserRoleUpdateModel, ) from ..exception_handlers import format_log_msg @@ -383,7 +385,7 @@ def update_user_role_in_reporting_org( # 7. Check whether there is an existing FGA entry for this user and reporting org user_roles_for_org = context.fine_grained_auth_provider.get_user_roles_for_org(user_id, org_id) - if user_roles_for_org is None: + if len(user_roles_for_org) == 0: context.app_logger.error( f"Unexpected error: user with id: {user.user_id_crm} attempted to change user id: {user_id}'s role in " f"organisation with id: {org_id} but the user has no entry in the FGA DB for this organisation. " @@ -522,7 +524,7 @@ def remove_user_from_reporting_org( user.user_id_crm, user.client_id, condition_func=lambda: not ( - user_roles_for_org[0].role == FineGrainedAuthorisationRole.ADMIN and number_admins == 1 # type: ignore + user_roles_for_org[0].role == FineGrainedAuthorisationRole.ADMIN and number_admins == 1 ), public_msg=( f"User id: {user_id} is the last admin user associated with " @@ -537,7 +539,7 @@ def remove_user_from_reporting_org( # 5. Delete the user's role in the FGA database try: - context.fine_grained_auth_provider.delete_user_role_for_org(user_roles_for_org[0]) # type: ignore + context.fine_grained_auth_provider.delete_user_role_for_org(user_roles_for_org[0]) context.audit_logger.info( format_log_msg( @@ -563,3 +565,69 @@ def remove_user_from_reporting_org( ) return JSONResponse({"data": None, "error": None, "status": "success"}, 200) + + +@router.get("/{user_id}/roles") +def get_user_roles( + user_id: uuid.UUID, + request: starlette.requests.Request, + user: UserAndCredentials = Security(authz.get_user_authnz, scopes=["ryd"]), +) -> UserRolesResponse: + """Get a list of the roles a user has.""" + + context: Context = request.app.state.context + + crm: SuiteCRM = context.suitecrm_client_factory.get_client() + # import pdb + # breakpoint() + # 1. Verify that the logged in user is the same as the requested user, unless the caller is superadmin. + assert_precondition_met( + user.user_id_crm, + user.client_id, + condition_func=lambda: (str(user_id) == user.user_id_crm) or (user.validator.is_superadmin), + public_msg="Requesting user attempting to access roles for another user and is not superadmin.", + status_code=fastapi.status.HTTP_403_FORBIDDEN, + audit_log_msg=( + f"user id: {user.user_id_crm} - GET /users/{user_id}/roles - Request to read user roles " + f"user {user_id} but the user is not a superadmin." + ), + ) + + # 2. Check that the requested user exists in CRM + assert_precondition_met( + user.user_id_crm, + user.client_id, + condition_func=lambda: check_crm_record_exists(crm, "Contacts", str(user_id)), + public_msg=f"There is no user with ID {str(user_id)}.", + status_code=fastapi.status.HTTP_404_NOT_FOUND, + audit_log_msg=( + f"user id: {user.user_id_crm} - GET /users/{user_id}reporting-orgs - Request to read reporting orgs for " + f"user {user_id} but there is no such user in the CRM." + ), + ) + + user_roles = UserRolesData( + superadmin=user.validator.is_superadmin, + tools=[x.id for x in user.validator.get_users_provider_admin_tools()], + reporting_orgs={ + association.reporting_org: get_fga_role_as_str(association.role) + for association in user.validator.get_users_fine_grained_associations() + }, + ) + + return UserRolesResponse(error=None, status="success", data=user_roles) + + +@router.get("/{user_id}/roles/reporting-org-permissions/{org_id}") +def get_user_fga_for_reporting_org( + user_id: uuid.UUID, + org_id: uuid.UUID, + request: starlette.requests.Request, + user: UserAndCredentials = Security(authz.get_user_authnz, scopes=["ryd"]), +) -> JSONResponse: + """Get a list of the fine grained authorisations a user has for a given reporting org.""" + + raise fastapi.HTTPException( + status_code=fastapi.status.HTTP_501_NOT_IMPLEMENTED, + detail="Not yet implemented", + ) From afa889b1ec3d60e7ca3303ea50eb5df85ab8073a Mon Sep 17 00:00:00 2001 From: Chris Arridge Date: Fri, 24 Apr 2026 15:24:11 +0100 Subject: [PATCH 11/16] test: provider admin management reporting org endpoints return 501 --- .../integration/test_reporting_org_routes.py | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/tests/integration/test_reporting_org_routes.py b/tests/integration/test_reporting_org_routes.py index 3514c6a..54c92b2 100644 --- a/tests/integration/test_reporting_org_routes.py +++ b/tests/integration/test_reporting_org_routes.py @@ -745,3 +745,38 @@ def test_reporting_org_list_users( users_returned = {user["id"] for user in resp_json["data"]} assert users_returned == visible_users + + +def test_reporting_org_tool_management_not_implemented() -> None: + appAndContext = MockedAppAndContext() + + fastAPIapp = appAndContext.get_test_app() + + with TestClient(fastAPIapp) as client: + # When implemented, this call should return a list of tools that have permission to + # edit records for an organisation. + response = client.get( + "/api/v1/reporting-orgs/ab851a83-a384-3eb9-caf0-68db8125b067/tools", + headers=appAndContext.get_valid_authorization_header(0), + ) + + assert response.status_code == 501 + + # When implemented, this call should authorise a tool to have permission to edit + # records for this organisation. + response = client.post( + "/api/v1/reporting-orgs/ab851a83-a384-3eb9-caf0-68db8125b067/tools", + headers=appAndContext.get_valid_authorization_header(0), + json={"tid": "6a2d1ca1-b9c2-4bd3-a2a5-099178d1358d"}, + ) + + assert response.status_code == 501 + + # When implemented, this call should revoke permission for this tool to have + # permission to edit records for this organisation. + response = client.delete( + "/api/v1/reporting-orgs/ab851a83-a384-3eb9-caf0-68db8125b067/tools/6a2d1ca1-b9c2-4bd3-a2a5-099178d1358d", + headers=appAndContext.get_valid_authorization_header(0), + ) + + assert response.status_code == 501 From 8baa302a55e9c5aa22966c1a49e845077f577c30 Mon Sep 17 00:00:00 2001 From: Chris Arridge Date: Mon, 27 Apr 2026 18:35:03 +0100 Subject: [PATCH 12/16] feat: add 501 responses for reporting org tool managmeent endpoints --- .../routers/reporting_orgs.py | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/src/register_your_data_api/routers/reporting_orgs.py b/src/register_your_data_api/routers/reporting_orgs.py index 52ba59f..71b28a9 100644 --- a/src/register_your_data_api/routers/reporting_orgs.py +++ b/src/register_your_data_api/routers/reporting_orgs.py @@ -631,3 +631,47 @@ def get_reporting_org_datasets( def get_reporting_org_actions(crm: SuiteCRM, org_id: str) -> list[ReportingOrgAction]: # TODO: return [] + + +@router.get("/{org_id}/tools") +def get_reporting_org_tool_list( + org_id: uuid.UUID, + request: starlette.requests.Request, + user: auth_models.UserAndCredentials = Security(authz.get_user_authnz, scopes=["ryd", "ryd:reporting_org:tool"]), +) -> JSONResponse: + + raise fastapi.HTTPException( + status_code=fastapi.status.HTTP_501_NOT_IMPLEMENTED, + detail="Not yet implemented", + ) + + +@router.post("/{org_id}/tools") +def authorise_tool_permission_for_reporting_org( + org_id: uuid.UUID, + request: starlette.requests.Request, + user: auth_models.UserAndCredentials = Security( + authz.get_user_authnz, scopes=["ryd", "ryd:reporting_org:tool:update"] + ), +) -> JSONResponse: + + raise fastapi.HTTPException( + status_code=fastapi.status.HTTP_501_NOT_IMPLEMENTED, + detail="Not yet implemented", + ) + + +@router.delete("/{org_id}/tools/{tool_id}") +def revoke_tool_permission_for_reporting_org( + org_id: uuid.UUID, + tool_id: uuid.UUID, + request: starlette.requests.Request, + user: auth_models.UserAndCredentials = Security( + authz.get_user_authnz, scopes=["ryd", "ryd:reporting_org:tool:update"] + ), +) -> JSONResponse: + + raise fastapi.HTTPException( + status_code=fastapi.status.HTTP_501_NOT_IMPLEMENTED, + detail="Not yet implemented", + ) From aade789e365fc8ceb9a5e7d2ced00a9215bb238e Mon Sep 17 00:00:00 2001 From: Chris Arridge Date: Tue, 28 Apr 2026 18:01:45 +0100 Subject: [PATCH 13/16] fix: added guards to make sure tool admin users cannot be given org roles --- .../auth/fga/fga_provider.py | 5 ++ .../auth/fga/fga_provider_db.py | 3 + src/register_your_data_api/routers/users.py | 60 ++++++++++++++++--- tests/integration/test_fga_provider_pgdb.py | 38 ++++++++++++ 4 files changed, 98 insertions(+), 8 deletions(-) diff --git a/src/register_your_data_api/auth/fga/fga_provider.py b/src/register_your_data_api/auth/fga/fga_provider.py index c82a3c3..ba26620 100644 --- a/src/register_your_data_api/auth/fga/fga_provider.py +++ b/src/register_your_data_api/auth/fga/fga_provider.py @@ -75,3 +75,8 @@ def get_all_tools(self) -> list[FineGrainedAuthorisationTool]: def get_tools_for_user(self, user: UUID) -> list[FineGrainedAuthorisationTool]: """Get a list of all the tools for which the user is an admin user.""" raise NotImplementedError + + @abstractmethod + def is_user_a_tool_adminuser(self, user: UUID) -> bool: + """Returns True is user is an admin user for tools, else False""" + raise NotImplementedError diff --git a/src/register_your_data_api/auth/fga/fga_provider_db.py b/src/register_your_data_api/auth/fga/fga_provider_db.py index 6e27499..02bd48b 100644 --- a/src/register_your_data_api/auth/fga/fga_provider_db.py +++ b/src/register_your_data_api/auth/fga/fga_provider_db.py @@ -267,3 +267,6 @@ def get_tools_for_user(self, user: UUID) -> list[FineGrainedAuthorisationTool]: return [FineGrainedAuthorisationTool(**db_tool.model_dump()) for db_tool in db_tools] return [] + + def is_user_a_tool_adminuser(self, user: UUID) -> bool: + return len(self.get_tools_for_user(user)) > 0 diff --git a/src/register_your_data_api/routers/users.py b/src/register_your_data_api/routers/users.py index 28634b8..c65d710 100644 --- a/src/register_your_data_api/routers/users.py +++ b/src/register_your_data_api/routers/users.py @@ -86,6 +86,19 @@ def add_user_to_reporting_org( ), ) + # Tool provider_admin users should not be allowed to join any organisations. + assert_precondition_met( + user.user_id_crm, + user.client_id, + condition_func=lambda: not context.fine_grained_auth_provider.is_user_a_tool_adminuser(user_id), + public_msg=f"User id: {user_id} cannot be given a role in organisation with ID {payload.oid_str}.", + status_code=fastapi.status.HTTP_400_BAD_REQUEST, + audit_log_msg=( + f"Request by user with id: {user.user_id_crm} to add a tool admin user {user_id} to join " + f"reporting org id: {payload.oid_str} but provider_admin users cannot join organisations." + ), + ) + # Query SuiteCRM to check that the reporting_org exists crm: SuiteCRM = context.suitecrm_client_factory.get_client() @@ -357,7 +370,23 @@ def update_user_role_in_reporting_org( ), ) - # 6. Check that the target user and reporting org are related in CRM + # 6. Check that the user to be modified is not a provider_admin for a tool - this is important to do + # since, if the following code does not find a relationship between the target user and the reporting + # org in the CRM, or a role in the FGA database, it will try to create them, but this must not be + # allowed for superadmins + assert_precondition_met( + user.user_id_crm, + user.client_id, + condition_func=lambda: not context.fine_grained_auth_provider.is_user_a_tool_adminuser(user_id), + public_msg=f"User id: {user_id} cannot be given a role in no organisation with ID {str(org_id)}.", + status_code=fastapi.status.HTTP_400_BAD_REQUEST, + audit_log_msg=( + f"User with id: {user.user_id_crm} attempted to change user id: {user_id}'s role " + f"in organisation with id: {org_id} but user with id: {user_id} is a provider_admin tool user." + ), + ) + + # 7. Check that the target user and reporting org are related in CRM users_for_org_from_suitecrm = crm.get_relationship("Accounts", str(org_id), "Contacts") user_related_to_org_in_crm = find_item_in_suitecrm_response(users_for_org_from_suitecrm, str(user_id)) @@ -382,7 +411,7 @@ def update_user_role_in_reporting_org( detail="There was a problem processing your request. Please contact support.", ) - # 7. Check whether there is an existing FGA entry for this user and reporting org + # 8. Check whether there is an existing FGA entry for this user and reporting org user_roles_for_org = context.fine_grained_auth_provider.get_user_roles_for_org(user_id, org_id) if len(user_roles_for_org) == 0: @@ -406,7 +435,7 @@ def update_user_role_in_reporting_org( if str(user_roles_for_org[0].role.name) == new_role.role.upper(): return JSONResponse({"data": None, "error": None, "status": "success"}, 200) - # 8. Update the user's role in the FGA database + # 9. Update the user's role in the FGA database # This is safe as new_role has been validated by Pydantic user_roles_for_org[0].role = get_fga_role_from_str(new_role.role) @@ -485,6 +514,22 @@ def remove_user_from_reporting_org( ), ) + # 4. Check that this isn't a tool provider_admin user. + assert_precondition_met( + user.user_id_crm, + user.client_id, + condition_func=lambda: not context.fine_grained_auth_provider.is_user_a_tool_adminuser(user_id), + public_msg=( + f"User id: {user_id} is a tool admin user and should not have a role in " + f"organisation with ID {str(org_id)}." + ), + status_code=fastapi.status.HTTP_400_BAD_REQUEST, + audit_log_msg=( + f"Request by user: {user.user_id_crm} to remove user {user_id} from reporting org id: {org_id} " + f"but {user_id} is a tool admin user and should not have an organisation role." + ), + ) + user_roles_for_org = context.fine_grained_auth_provider.get_user_roles_for_org(user_id, org_id) assert_precondition_met( @@ -499,7 +544,7 @@ def remove_user_from_reporting_org( ), ) - # 4a. Check that the user isn't the last user in the organisation + # 5a. Check that the user isn't the last user in the organisation users_in_org = context.fine_grained_auth_provider.get_user_associations_for_org(org_id) assert_precondition_met( @@ -517,7 +562,7 @@ def remove_user_from_reporting_org( ), ) - # 4b. Check that the user isn't the last admin user for this organisation + # 5b. Check that the user isn't the last admin user for this organisation number_admins = len(list(filter(lambda x: x.role == FineGrainedAuthorisationRole.ADMIN, users_in_org))) assert_precondition_met( @@ -537,7 +582,7 @@ def remove_user_from_reporting_org( ), ) - # 5. Delete the user's role in the FGA database + # 6. Delete the user's role in the FGA database try: context.fine_grained_auth_provider.delete_user_role_for_org(user_roles_for_org[0]) @@ -578,8 +623,7 @@ def get_user_roles( context: Context = request.app.state.context crm: SuiteCRM = context.suitecrm_client_factory.get_client() - # import pdb - # breakpoint() + # 1. Verify that the logged in user is the same as the requested user, unless the caller is superadmin. assert_precondition_met( user.user_id_crm, diff --git a/tests/integration/test_fga_provider_pgdb.py b/tests/integration/test_fga_provider_pgdb.py index ee6fbf1..2e4fa8a 100644 --- a/tests/integration/test_fga_provider_pgdb.py +++ b/tests/integration/test_fga_provider_pgdb.py @@ -337,3 +337,41 @@ def test_tool_lists_correctly_fetched() -> None: assert tools[1] == FineGrainedAuthorisationTool( id=tool2, name="Tool 2", provider="Tool Maker", client_id="v4amDn43kirvBmB9" ) + + +def test_tool_admin_users_can_be_detected() -> None: + + setup_db("sqlite:///test.db") + + fga = FineGrainedAuthorisationProviderDb("sqlite:///test.db") + fga.setup() + + u_o1, u_t1, u_t2, u_bothtools, u_noperms = uuid4(), uuid4(), uuid4(), uuid4(), uuid4() + o1, o2 = uuid4(), uuid4() + t1, t2 = uuid4(), uuid4() + t1_clientid, t2_clientid = gen_random_client_id(), gen_random_client_id() + + with sqlmodel.Session(fga._engine) as session: + session.add( + FineGrainedAuthorisationDbModel( + user=u_o1, reporting_org=o1, role=FineGrainedAuthorisationRole.ADMIN, id=uuid4() + ) + ) + + session.add(ToolDbModel(id=t1, name="Tool 1", provider="Tool Maker", client_id=t1_clientid)) + session.add(ToolDbModel(id=t2, name="Tool 2", provider="Tool Maker", client_id=t2_clientid)) + session.add(ToolAuthorisationDbModel(tool=t1, reporting_org=o1, id=uuid4())) + session.add(ToolAuthorisationDbModel(tool=t2, reporting_org=o1, id=uuid4())) + session.add(ToolAuthorisationDbModel(tool=t2, reporting_org=o2, id=uuid4())) + session.add(ToolAdminUserDbModel(tool=t1, user=u_t1, id=uuid4())) + session.add(ToolAdminUserDbModel(tool=t2, user=u_t2, id=uuid4())) + session.add(ToolAdminUserDbModel(tool=t1, user=u_bothtools, id=uuid4())) + session.add(ToolAdminUserDbModel(tool=t2, user=u_bothtools, id=uuid4())) + + session.commit() + + assert not fga.is_user_a_tool_adminuser(u_o1) + assert fga.is_user_a_tool_adminuser(u_t1) + assert fga.is_user_a_tool_adminuser(u_t2) + assert fga.is_user_a_tool_adminuser(u_bothtools) + assert not fga.is_user_a_tool_adminuser(u_noperms) From 67859b456776d35089300605b606bf954c26fae4 Mon Sep 17 00:00:00 2001 From: Chris Arridge Date: Tue, 28 Apr 2026 18:40:23 +0100 Subject: [PATCH 14/16] refactor: small refactoring changes in fga provider DB --- .../auth/fga/fga_provider_db.py | 36 +++++++++---------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/src/register_your_data_api/auth/fga/fga_provider_db.py b/src/register_your_data_api/auth/fga/fga_provider_db.py index 02bd48b..18b6a98 100644 --- a/src/register_your_data_api/auth/fga/fga_provider_db.py +++ b/src/register_your_data_api/auth/fga/fga_provider_db.py @@ -1,4 +1,4 @@ -import collections +from collections import Counter from uuid import UUID, uuid4 from sqlalchemy import Engine, delete @@ -98,14 +98,10 @@ def get_user_associations_for_org(self, reporting_org: UUID) -> list[FineGrained regular_associations = [ FineGrainedAuthorisationRoleAssociation(**db_fga.model_dump()) for db_fga in user_db_fgas ] - if len(regular_associations) > 1: - if ( - collections.Counter([association.user for association in regular_associations]).most_common(1)[0][1] - > 1 - ): - raise FineGrainedAuthorisationIntegrityError( - "Reporting org has user(s) that have multiple reporting org roles" - ) + if max(Counter([association.user for association in regular_associations]).values(), default=0) > 1: + raise FineGrainedAuthorisationIntegrityError( + "Reporting org has user(s) that have multiple reporting org roles" + ) # Check that provider admin users are unique for this reporting org - each user can only have provider # admin by each tool (we cannot have two accesses via provider admin by the same tool). @@ -118,19 +114,21 @@ def get_user_associations_for_org(self, reporting_org: UUID) -> list[FineGrained ) for tool_admin_user_for_org in tool_admin_users_for_org ] - if provider_admin_associations: - if ( - collections.Counter( + if ( + max( + Counter( [(association.user, association.restricted_to_tool) for association in provider_admin_associations] - ).most_common(1)[0][1] - > 1 - ): - raise FineGrainedAuthorisationIntegrityError( - "Reporting org has provider admins with multiple conflicting tool admin roles" - ) + ).values(), + default=0, + ) + > 1 + ): + raise FineGrainedAuthorisationIntegrityError( + "Reporting org has provider admins with multiple conflicting tool admin roles" + ) # Check that a provider admin is not in the list of regular associations. - if len(set([x.user for x in regular_associations]) & set([x.user for x in provider_admin_associations])) > 0: + if set([x.user for x in regular_associations]) & set([x.user for x in provider_admin_associations]): raise FineGrainedAuthorisationIntegrityError( "Reporting org has user(s) with both provider admin and reporting org roles" ) From 1ebdcd6b1e80f825c744a178d338e6f4b893e76e Mon Sep 17 00:00:00 2001 From: Chris Arridge Date: Tue, 28 Apr 2026 09:44:31 +0100 Subject: [PATCH 15/16] bump: version bump --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 2eb14a0..5f5c7e6 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "register-your-data-api" -version = "0.3.4" +version = "0.3.5" requires-python = ">= 3.12.11" readme = "README.md" authors = [{name="IATI Secretariat", email="support@iatistandard.org"}] From 94046ec1d53b76c4a45fe3c32d57a4fbf856a1b8 Mon Sep 17 00:00:00 2001 From: Chris Arridge Date: Tue, 28 Apr 2026 09:54:11 +0100 Subject: [PATCH 16/16] docs: updated CHANGELOG --- CHANGELOG.md | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e72765f..0aefa18 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,25 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Security +## [0.3.5] - 2026-04-28 + +### Added + +- Tools list endpoint and associated test. +- User roles endpoint and associated test; and a placeholder for the related permissions endpoint. +- Provider admin management endpoint placeholders and tests. + +### Changed + +- FGA Provider now stores tool client IDs and can handle multiple provider_admin roles per user per reporting + orgs to accommodate multiple tools being able to access the same reporting org via provider_admin. +- FGA Validator now ingests a list of tools that the user is an admin user for and the client id of the calling + application. +- FGA Validator restricts provider_admin access to only the client id associated with that tool. +- Tests for the FGA provider DB and validator. +- Changes to the tests for dataset and reporting org routes so that calls via provider_admin can use the correct client id. + + ## [0.3.4] - 2026-04-20 ### Fixed