-
Notifications
You must be signed in to change notification settings - Fork 0
Provider admin #75
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Provider admin #75
Changes from all commits
5f1ace1
bcdc704
ee524c7
62474ab
7389fbd
b4b56fd
b5b1d40
2468501
7986bca
b1e2997
6e73df8
2788c16
be723b6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,63 @@ | ||
| """Added provider admin tables | ||
|
|
||
| Revision ID: 69dbf30b36a0 | ||
| Revises: ec6411d99e0b | ||
| Create Date: 2026-03-24 11:43:56.481757 | ||
|
|
||
| """ | ||
|
|
||
| from typing import Sequence, Union | ||
|
|
||
| import sqlalchemy as sa | ||
| import sqlmodel.sql.sqltypes | ||
| from alembic import op | ||
|
|
||
| # revision identifiers, used by Alembic. | ||
| revision: str = "69dbf30b36a0" | ||
| down_revision: Union[str, Sequence[str], None] = "ec6411d99e0b" | ||
| 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.create_table( | ||
| "tooldbmodel", | ||
| sa.Column("id", sa.Uuid(), nullable=False), | ||
| sa.Column("name", sqlmodel.sql.sqltypes.AutoString(), nullable=False), | ||
| sa.Column("provider", sqlmodel.sql.sqltypes.AutoString(), nullable=False), | ||
| sa.PrimaryKeyConstraint("id"), | ||
| ) | ||
| op.create_table( | ||
| "toolauthorisationdbmodel", | ||
| sa.Column("id", sa.Uuid(), nullable=False), | ||
| sa.Column("tool", sa.Uuid(), nullable=False), | ||
| sa.Column("reporting_org", sa.Uuid(), nullable=False), | ||
| sa.ForeignKeyConstraint( | ||
| ["tool"], | ||
| ["tooldbmodel.id"], | ||
| ), | ||
| sa.PrimaryKeyConstraint("id"), | ||
| ) | ||
| op.create_table( | ||
| "tooluserdbmodel", | ||
| sa.Column("id", sa.Uuid(), nullable=False), | ||
| sa.Column("tool", sa.Uuid(), nullable=False), | ||
| sa.Column("user", sa.Uuid(), nullable=False), | ||
| sa.ForeignKeyConstraint( | ||
| ["tool"], | ||
| ["tooldbmodel.id"], | ||
| ), | ||
| sa.PrimaryKeyConstraint("id"), | ||
| ) | ||
| # ### end Alembic commands ### | ||
|
|
||
|
|
||
| def downgrade() -> None: | ||
| """Downgrade schema.""" | ||
| # ### commands auto generated by Alembic - please adjust! ### | ||
| op.drop_table("tooluserdbmodel") | ||
| op.drop_table("toolauthorisationdbmodel") | ||
| op.drop_table("tooldbmodel") | ||
| # ### end Alembic commands ### |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,10 @@ | ||
| import collections | ||
| from uuid import UUID, uuid4 | ||
|
|
||
| from sqlalchemy import Engine, delete | ||
| from sqlmodel import Field, Session, SQLModel, create_engine, select | ||
| from sqlmodel import Field, Session, SQLModel, col, create_engine, select | ||
|
|
||
| from .fga_provider import FineGrainedAuthorisationProvider | ||
| from .fga_provider import FineGrainedAuthorisationIntegrityError, FineGrainedAuthorisationProvider | ||
| from .models import FineGrainedAuthorisationRole, FineGrainedAuthorisationRoleAssociation | ||
|
|
||
|
|
||
|
|
@@ -20,6 +21,24 @@ class SuperAdminUserDbModel(SQLModel, table=True): | |
| is_superadmin: bool | ||
|
|
||
|
|
||
| class ToolDbModel(SQLModel, table=True): | ||
| id: UUID = Field(primary_key=True, default_factory=lambda: uuid4()) | ||
| name: str | ||
| provider: str | ||
|
|
||
|
|
||
| class ToolAuthorisationDbModel(SQLModel, table=True): | ||
| id: UUID = Field(primary_key=True, default_factory=lambda: uuid4()) | ||
| tool: UUID = Field(foreign_key="tooldbmodel.id") | ||
| reporting_org: UUID | ||
|
|
||
|
|
||
| class ToolUserDbModel(SQLModel, table=True): | ||
| id: UUID = Field(primary_key=True, default_factory=lambda: uuid4()) | ||
| tool: UUID = Field(foreign_key="tooldbmodel.id") | ||
| user: UUID | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add index? |
||
|
|
||
|
|
||
| class FineGrainedAuthorisationProviderDb(FineGrainedAuthorisationProvider): | ||
|
|
||
| _connection_str: str | ||
|
|
@@ -37,7 +56,24 @@ def get_user_fine_grained_permissions(self, user: UUID) -> list[FineGrainedAutho | |
| select(FineGrainedAuthorisationDbModel).where(FineGrainedAuthorisationDbModel.user == user) | ||
| ).all() | ||
|
|
||
| return [FineGrainedAuthorisationRoleAssociation(**db_fga.model_dump()) for db_fga in user_db_fgas] | ||
| providers_reporting_orgs = session.exec( | ||
| select(ToolAuthorisationDbModel.reporting_org, ToolUserDbModel.user) | ||
| .join(ToolUserDbModel, col(ToolUserDbModel.tool) == col(ToolAuthorisationDbModel.tool)) | ||
| .where(ToolUserDbModel.user == user) | ||
| ).all() | ||
|
|
||
| if user_db_fgas and providers_reporting_orgs: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will block people from being both a provider admin for a tool, and also associated with a govern organisation in the normal way - is that correct? But some 3rd party tool providers publish datasets (whether or not they should or need to I don't know, but they do), so we might need to think that use-case through. |
||
| raise FineGrainedAuthorisationIntegrityError("User has both reporting org role(s) and is a tool user") | ||
|
|
||
| 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 | ||
| ) | ||
| for x in providers_reporting_orgs | ||
| ] | ||
|
|
||
| return associations | ||
|
|
||
| def get_user_associations_for_org(self, reporting_org: UUID) -> list[FineGrainedAuthorisationRoleAssociation]: | ||
| with Session(self._engine) as session: | ||
|
|
@@ -47,7 +83,29 @@ def get_user_associations_for_org(self, reporting_org: UUID) -> list[FineGrained | |
| ) | ||
| ).all() | ||
|
|
||
| return [FineGrainedAuthorisationRoleAssociation(**db_fga.model_dump()) for db_fga in user_db_fgas] | ||
| tool_users_for_org = session.exec( | ||
| select(ToolAuthorisationDbModel.reporting_org, ToolUserDbModel.user) | ||
| .join(ToolUserDbModel, col(ToolUserDbModel.tool) == col(ToolAuthorisationDbModel.tool)) | ||
| .where(ToolAuthorisationDbModel.reporting_org == reporting_org) | ||
| ).all() | ||
|
|
||
| associations = [FineGrainedAuthorisationRoleAssociation(**db_fga.model_dump()) for db_fga in user_db_fgas] | ||
| associations += [ | ||
| FineGrainedAuthorisationRoleAssociation( | ||
| user=tool_user_for_org[1], | ||
| reporting_org=reporting_org, | ||
| role=FineGrainedAuthorisationRole.PROVIDER_ADMIN, | ||
| ) | ||
| for tool_user_for_org in tool_users_for_org | ||
| ] | ||
|
|
||
| if len(associations) > 1: | ||
| if collections.Counter([association.user for association in associations]).most_common(1)[0][1] > 1: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This would also pick up if the FGA database gets into an error state where a user has two entries in there for the same org. I'm not suggesting this code needs to be changed, just flagging. |
||
| raise FineGrainedAuthorisationIntegrityError( | ||
| "Reporting org has user(s) that have both reporting org role and a provider admin role" | ||
| ) | ||
|
|
||
| return associations | ||
|
|
||
| def get_user_role_for_org(self, user: UUID, org: UUID) -> FineGrainedAuthorisationRoleAssociation | None: | ||
| with Session(self._engine) as session: | ||
|
|
@@ -58,10 +116,27 @@ def get_user_role_for_org(self, user: UUID, org: UUID) -> FineGrainedAuthorisati | |
| ) | ||
| ).first() | ||
|
|
||
| if not user_role_for_org: | ||
| tool_user_for_org = session.exec( | ||
| select(ToolAuthorisationDbModel.reporting_org, ToolUserDbModel.user) | ||
| .join(ToolUserDbModel, col(ToolUserDbModel.tool) == col(ToolAuthorisationDbModel.tool)) | ||
| .where((ToolAuthorisationDbModel.reporting_org == org) & (ToolUserDbModel.user == user)) | ||
| ).all() | ||
|
|
||
| if tool_user_for_org and user_role_for_org: | ||
| raise FineGrainedAuthorisationIntegrityError("User has both reporting org role and a provider admin role") | ||
|
|
||
| if not user_role_for_org and not tool_user_for_org: | ||
| return None | ||
|
|
||
| return FineGrainedAuthorisationRoleAssociation(**user_role_for_org.model_dump()) | ||
| if tool_user_for_org: | ||
| association = FineGrainedAuthorisationRoleAssociation( | ||
| user=user, reporting_org=org, role=FineGrainedAuthorisationRole.PROVIDER_ADMIN | ||
| ) | ||
|
|
||
| if user_role_for_org: | ||
| association = FineGrainedAuthorisationRoleAssociation(**user_role_for_org.model_dump()) | ||
|
|
||
| return association | ||
|
|
||
| def get_admin_users_for_org(self, org: UUID) -> list[FineGrainedAuthorisationRoleAssociation]: | ||
| with Session(self._engine) as session: | ||
|
|
@@ -92,12 +167,14 @@ def create_user_fine_grained_authorisation( | |
| with Session(self._engine) as session: | ||
| session.add(user_org_role_db) | ||
| session.commit() | ||
| # TODO: Check user doesn't have provider admin | ||
|
|
||
| def update_user_role_for_org(self, user_reporting_org_role: FineGrainedAuthorisationRoleAssociation) -> None: | ||
| user_org_role_db = FineGrainedAuthorisationDbModel(**user_reporting_org_role.model_dump()) | ||
| with Session(self._engine) as session: | ||
| session.merge(user_org_role_db) | ||
| session.commit() | ||
| # TODO: Check user doesn't have provider admin | ||
|
|
||
| def delete_user_role_for_org(self, user_reporting_org_role: FineGrainedAuthorisationRoleAssociation) -> None: | ||
| with Session(self._engine) as session: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,7 +24,9 @@ def get_permissions_for_role(self, user_role: FineGrainedAuthorisationRole) -> l | |
| "delete-dataset", | ||
| ] | ||
| permissions[FineGrainedAuthorisationRole.PROVIDER_ADMIN] = [ | ||
| *permissions[FineGrainedAuthorisationRole.EDITOR], | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this change in the requirements for provider admin permissions documented somewhere? |
||
| "read-org", | ||
| "read-dataset", | ||
| "update-dataset", | ||
| "update-dataset-visibility", | ||
| ] | ||
| permissions[FineGrainedAuthorisationRole.ADMIN] = [ | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This all looks good, my only thought so far is that
ToolUserDbModelmay (going forward) have the potential to be confusing, because the clients/customers of the 3rd party tools are also aptly described as "(3rd party) tool users". I wonder ifToolAdminUserDbModelmight be more explanatory? Maybe that was discounted for another reason?