feat: Make authorization code pluggable #102
Conversation
jgadling
changed the title
| @@ -119,13 +117,13 @@ def convert_where_clauses_to_sql( | |||
| for join_field, join_info in all_joins.items(): | |||
| relationship = mapper.relationships[join_field] # type: ignore | |||
| related_cls = relationship.mapper.entity | |||
| cerbos_query = get_resource_query(principal, cerbos_client, action, related_cls) | |||
| secure_query = authz_client.get_resource_query(principal, action, related_cls) | |||
There was a problem hiding this comment.
KEY CHANGE HERE: get_resource_query isn't called directly anymore, because it was impossible/impractical to override that function call. Instead, we've made that a method of AuthzClient, which is passed in as a FastAPI dependency and can be overridden.
| raise PlatformicsError("Unauthorized: cannot update file") from None | ||
|
|
||
| # Fetch the entity if have access to it | ||
| entity_class, entity = await get_entity_by_id( |
There was a problem hiding this comment.
This call to get_entity_by_id checks to see whether the user has access to update the associated entity.
There was a problem hiding this comment.
If the get_resource_query() call below applies authz policies properly, we may not need this call!
| joins = [(db.Entity, db.File.entity_id == db.Entity.id)] # type: ignore | ||
| else: | ||
| # Send all non-relationship columns to cerbos to make decisions | ||
| for col in sqlalchemy_helpers.model_class_cols(model_cls): |
There was a problem hiding this comment.
should we use obj_to_dict or model_class_cols? Can we unify these???
Minor fixes.
| @@ -43,6 +43,8 @@ def run_migrations_offline() -> None: | |||
| ) | |||
|
|
|||
| with context.begin_transaction(): | |||
| context.get_context()._ensure_version_table() # pylint: disable=protected-access | |||
| connection.execute(sa.sql.text("LOCK TABLE alembic_version IN ACCESS EXCLUSIVE MODE")) | |||
There was a problem hiding this comment.
This ensures that if multiple processes (e.g. k8s pods) try to run migrations at the same time, only one of them will actually be able to run them.
| principal: Principal, | ||
| action: AuthzAction, | ||
| model_cls: type[db.Base], # type: ignore | ||
| ) -> Select: |
There was a problem hiding this comment.
how overridable do you think this is if we're not using CERBOS?
There was a problem hiding this comment.
It's TBD honestly. This hopefully gets us one step closer by putting most of the policy enforcement code in one place and allowing it to be overridden, but I'd love to hear more about what the interface for the authz systems you've been working with recently look like!
We expect that Cerbos is the main authorization tool used alongside platformics, but it was too deeply integrated into our system. This PR does a few things:
CerbosActiontoAuthzAction