From bfeb57e84f5771d267c4a6bef6f90f4f9e4a62cf Mon Sep 17 00:00:00 2001 From: Philip Weiss Date: Fri, 5 Jun 2026 16:22:03 -0700 Subject: [PATCH 1/3] Add admin break-glass bypass to RBAC authorization Thread is_admin into AuthContext and short-circuit RBAC authorization for admins, approving all requests. The bypass is a single explicit check and is logged for audit, so it is easy to find and to later scope down if admins should still respect some constraints. --- .../internal/access/authorization/context.py | 2 + .../internal/access/authorization/service.py | 19 ++++ .../tests/internal/authorization_test.py | 89 +++++++++++++++++++ 3 files changed, 110 insertions(+) diff --git a/datajunction-server/datajunction_server/internal/access/authorization/context.py b/datajunction-server/datajunction_server/internal/access/authorization/context.py index 5c954be62c..468e6de152 100644 --- a/datajunction-server/datajunction_server/internal/access/authorization/context.py +++ b/datajunction-server/datajunction_server/internal/access/authorization/context.py @@ -46,6 +46,7 @@ class AuthContext: username: str oauth_provider: Optional[str] role_assignments: List[RoleAssignment] # Direct + groups, flattened + is_admin: bool = False @classmethod async def from_user( @@ -76,6 +77,7 @@ async def from_user( username=user.username, oauth_provider=user.oauth_provider, role_assignments=assignments, + is_admin=bool(user.is_admin), ) @classmethod diff --git a/datajunction-server/datajunction_server/internal/access/authorization/service.py b/datajunction-server/datajunction_server/internal/access/authorization/service.py index 6abd28f128..73aabf4082 100644 --- a/datajunction-server/datajunction_server/internal/access/authorization/service.py +++ b/datajunction-server/datajunction_server/internal/access/authorization/service.py @@ -2,6 +2,7 @@ Authorization service implementations for access control. """ +import logging from abc import ABC, abstractmethod from datetime import datetime, timezone from functools import lru_cache @@ -22,6 +23,8 @@ get_settings, ) +logger = logging.getLogger(__name__) + settings = get_settings() @@ -121,6 +124,22 @@ def authorize( Returns: Same list of requests with approved=True/False set """ + # Break-glass: admins bypass all RBAC checks. Kept as a single explicit + # check (and logged for audit) so the bypass is easy to find and, if + # ever needed, to scope down to "admin bypasses grants but still + # respects X". + if auth_context.is_admin: + logger.info( + "Admin access bypass: user=%s (id=%s) approved %d request(s): %s", + auth_context.username, + auth_context.user_id, + len(requests), + ", ".join(str(request) for request in requests), + ) + return [ + AccessDecision(request=request, approved=True, reason="admin") + for request in requests + ] return [self._make_decision(auth_context, request) for request in requests] def _make_decision( diff --git a/datajunction-server/tests/internal/authorization_test.py b/datajunction-server/tests/internal/authorization_test.py index 5e2b61b92e..f4495d922a 100644 --- a/datajunction-server/tests/internal/authorization_test.py +++ b/datajunction-server/tests/internal/authorization_test.py @@ -584,6 +584,95 @@ async def test_get_authorization_service_factory(self, mocker): assert "passthrough" in str(exc_info.value).lower() +@pytest.mark.asyncio +class TestAdminBypass: + """Tests for the admin break-glass bypass in RBACAuthorizationService.""" + + async def test_admin_bypasses_restrictive_policy(self, mocker): + """An admin is approved for everything, even under restrictive policy.""" + mock_settings = mocker.patch( + "datajunction_server.internal.access.authorization.service.settings", + ) + mock_settings.default_access_policy = "restrictive" + + service = RBACAuthorizationService() + auth_context = AuthContext( + user_id=1, + username="root", + oauth_provider="basic", + role_assignments=[], + is_admin=True, + ) + requests = [ + ResourceRequest( + verb=ResourceAction.MANAGE, + access_object=Resource( + name="anything.at.all", + resource_type=ResourceType.NODE, + ), + ), + ResourceRequest( + verb=ResourceAction.WRITE, + access_object=Resource( + name="finance", + resource_type=ResourceType.NAMESPACE, + ), + ), + ] + decisions = service.authorize(auth_context, requests) + assert all(decision.approved for decision in decisions) + assert all(decision.reason == "admin" for decision in decisions) + + async def test_non_admin_denied_under_restrictive(self, mocker): + """A non-admin with no grants is denied under restrictive policy.""" + mock_settings = mocker.patch( + "datajunction_server.internal.access.authorization.service.settings", + ) + mock_settings.default_access_policy = "restrictive" + + service = RBACAuthorizationService() + auth_context = AuthContext( + user_id=2, + username="bob", + oauth_provider="basic", + role_assignments=[], + is_admin=False, + ) + requests = [ + ResourceRequest( + verb=ResourceAction.WRITE, + access_object=Resource( + name="finance.revenue", + resource_type=ResourceType.NODE, + ), + ), + ] + decisions = service.authorize(auth_context, requests) + assert decisions[0].approved is False + + async def test_auth_context_from_user_carries_is_admin( + self, + default_user: User, + session: AsyncSession, + ): + """AuthContext.from_user reflects the user's is_admin flag.""" + admin = User( + username="admin-user", + kind=PrincipalKind.USER, + oauth_provider="basic", + is_admin=True, + ) + session.add(admin) + await session.commit() + + admin = await get_user(username="admin-user", session=session) + auth_context = await AuthContext.from_user(session, admin) + assert auth_context.is_admin is True + + non_admin_context = await AuthContext.from_user(session, default_user) + assert non_admin_context.is_admin is False + + @pytest.mark.asyncio class TestGroupBasedPermissions: """Tests for group-based role assignments.""" From 64822f3ba80ae45f705d671119217a409e098d83 Mon Sep 17 00:00:00 2001 From: Philip Weiss Date: Fri, 5 Jun 2026 16:27:51 -0700 Subject: [PATCH 2/3] Add configurable default-access role to RBAC Add a DEFAULT_ACCESS_ROLE setting whose scopes are evaluated as a fallback when no explicit grant matches, so deployments can express graceful defaults (e.g. read on *) without flipping the whole policy to permissive. The default role's scopes are pre-loaded into AuthContext and evaluated together with the principal's own scopes: _make_decision now gathers all candidate scopes (explicit grants + default role) and resolves them in one step, then falls back to default_access_policy. Collecting candidates before deciding (rather than a nested allow-ladder) keeps the door open for future deny/precedence rules. --- .../datajunction_server/config.py | 6 + .../internal/access/authorization/context.py | 27 +- .../internal/access/authorization/service.py | 46 +++- .../tests/internal/authorization_test.py | 237 ++++++++++++++++++ 4 files changed, 307 insertions(+), 9 deletions(-) diff --git a/datajunction-server/datajunction_server/config.py b/datajunction-server/datajunction_server/config.py index 9dbdb8ef05..ee22e25b47 100644 --- a/datajunction-server/datajunction_server/config.py +++ b/datajunction-server/datajunction_server/config.py @@ -213,6 +213,12 @@ class Settings(BaseSettings): # pragma: no cover # - "restrictive": Deny by default default_access_policy: str = "permissive" # or "restrictive" + # Optional role name whose scopes are evaluated as a fallback when no + # explicit grant matches. Lets a deployment express graceful defaults such + # as "everyone gets read on *" without flipping the whole policy to + # permissive. Applied before the default_access_policy fallback. + default_access_role: Optional[str] = None + # Interval in seconds with which to expire caching of any indexes index_cache_expire: int = 60 diff --git a/datajunction-server/datajunction_server/internal/access/authorization/context.py b/datajunction-server/datajunction_server/internal/access/authorization/context.py index 468e6de152..1a17810c79 100644 --- a/datajunction-server/datajunction_server/internal/access/authorization/context.py +++ b/datajunction-server/datajunction_server/internal/access/authorization/context.py @@ -3,7 +3,7 @@ """ from fastapi import Depends -from dataclasses import dataclass +from dataclasses import dataclass, field from typing import List, Optional from sqlalchemy import select @@ -14,7 +14,7 @@ from datajunction_server.internal.access.group_membership import ( get_group_membership_service, ) -from datajunction_server.database.rbac import RoleAssignment, Role +from datajunction_server.database.rbac import RoleAssignment, Role, RoleScope from datajunction_server.database.user import User from datajunction_server.utils import ( @@ -47,6 +47,9 @@ class AuthContext: oauth_provider: Optional[str] role_assignments: List[RoleAssignment] # Direct + groups, flattened is_admin: bool = False + # Scopes from the configured default-access role, evaluated as a fallback + # alongside the user's own grants. + default_scopes: List[RoleScope] = field(default_factory=list) @classmethod async def from_user( @@ -71,6 +74,7 @@ async def from_user( session=session, user=user, ) + default_scopes = await cls.get_default_scopes(session=session) return cls( user_id=user.id, @@ -78,8 +82,27 @@ async def from_user( oauth_provider=user.oauth_provider, role_assignments=assignments, is_admin=bool(user.is_admin), + default_scopes=default_scopes, ) + @classmethod + async def get_default_scopes( + cls, + session: AsyncSession, + ) -> List[RoleScope]: + """ + Load the scopes of the configured default-access role, if any. + + Returns an empty list when no default role is configured or the named + role does not exist, so authorization simply falls through to the + default_access_policy. + """ + role_name = settings.default_access_role + if not role_name: + return [] + default_role = await Role.get_by_name(session, role_name) + return list(default_role.scopes) if default_role else [] + @classmethod async def get_effective_assignments( cls, diff --git a/datajunction-server/datajunction_server/internal/access/authorization/service.py b/datajunction-server/datajunction_server/internal/access/authorization/service.py index 73aabf4082..495b7b1b3f 100644 --- a/datajunction-server/datajunction_server/internal/access/authorization/service.py +++ b/datajunction-server/datajunction_server/internal/access/authorization/service.py @@ -6,7 +6,10 @@ from abc import ABC, abstractmethod from datetime import datetime, timezone from functools import lru_cache -from typing import List +from typing import List, TYPE_CHECKING + +if TYPE_CHECKING: + from datajunction_server.database.rbac import RoleScope from datajunction_server.models.access import ( @@ -142,6 +145,26 @@ def authorize( ] return [self._make_decision(auth_context, request) for request in requests] + @classmethod + def candidate_scopes(cls, auth_context: AuthContext) -> List["RoleScope"]: + """ + Collect every scope that could grant a request for this context. + + This is the union of the principal's own (non-expired) role scopes and + the configured default-access role's scopes. Collecting all candidates + up front (rather than short-circuiting source by source) keeps the + decision a single resolve step, leaving room for future deny/precedence + rules without restructuring. + """ + scopes: List["RoleScope"] = [] + now = datetime.now(timezone.utc) + for assignment in auth_context.role_assignments: + if assignment.expires_at and assignment.expires_at < now: + continue + scopes.extend(assignment.role.scopes) + scopes.extend(auth_context.default_scopes) + return scopes + def _make_decision( self, auth_context: AuthContext, @@ -149,16 +172,25 @@ def _make_decision( ) -> AccessDecision: """ Convert ResourceRequest to AccessDecision. + + Gathers all applicable scopes (explicit grants + default-access role) + and approves if any grants the request. Otherwise falls back to the + configured default_access_policy. """ - has_grant = self.has_permission( - assignments=auth_context.role_assignments, - action=request.verb, - resource_type=request.access_object.resource_type, - resource_name=request.access_object.name, + granted = any( + self._scope_grants_permission( + scope, + request.verb, + request.access_object.resource_type, + request.access_object.name, + ) + for scope in self.candidate_scopes(auth_context) ) + if granted: + return AccessDecision(request=request, approved=True) return AccessDecision( request=request, - approved=(has_grant or settings.default_access_policy == "permissive"), + approved=(settings.default_access_policy == "permissive"), ) @classmethod diff --git a/datajunction-server/tests/internal/authorization_test.py b/datajunction-server/tests/internal/authorization_test.py index f4495d922a..4cf79bd194 100644 --- a/datajunction-server/tests/internal/authorization_test.py +++ b/datajunction-server/tests/internal/authorization_test.py @@ -673,6 +673,243 @@ async def test_auth_context_from_user_carries_is_admin( assert non_admin_context.is_admin is False +@pytest.mark.asyncio +class TestDefaultAccessRole: + """Tests for the configurable default-access role fallback.""" + + CONTEXT_SETTINGS = ( + "datajunction_server.internal.access.authorization.context.settings" + ) + SERVICE_SETTINGS = ( + "datajunction_server.internal.access.authorization.service.settings" + ) + + async def _make_role(self, session, default_user, name, action, scope_value): + role = Role(name=name, created_by_id=default_user.id) + session.add(role) + await session.flush() + session.add( + RoleScope( + role_id=role.id, + action=action, + scope_type=ResourceType.NAMESPACE, + scope_value=scope_value, + ), + ) + await session.commit() + return role + + async def test_default_role_grants_fallback_access( + self, + default_user: User, + session: AsyncSession, + mocker, + ): + """Default role scopes grant access when there is no explicit grant.""" + await self._make_role( + session, + default_user, + "global-viewer", + ResourceAction.READ, + "*", + ) + + ctx_settings = mocker.patch(self.CONTEXT_SETTINGS) + ctx_settings.default_access_role = "global-viewer" + svc_settings = mocker.patch(self.SERVICE_SETTINGS) + svc_settings.authorization_provider = "rbac" + svc_settings.default_access_policy = "restrictive" + + user = await get_user(username=default_user.username, session=session) + access_checker = AccessChecker( + auth_context=await AuthContext.from_user(user=user, session=session), + ) + access_checker.add_requests( + [ + ResourceRequest( + verb=ResourceAction.READ, + access_object=Resource( + name="finance.revenue", + resource_type=ResourceType.NAMESPACE, + ), + ), + ResourceRequest( + verb=ResourceAction.WRITE, + access_object=Resource( + name="finance.revenue", + resource_type=ResourceType.NAMESPACE, + ), + ), + ], + ) + results = await access_checker.check(on_denied=AccessDenialMode.RETURN) + assert results[0].approved is True # read granted by default role + assert results[1].approved is False # write not in default role, restrictive + + async def test_no_default_role_restrictive_denies( + self, + default_user: User, + session: AsyncSession, + mocker, + ): + """With no default role and restrictive policy, ungranted access is denied.""" + ctx_settings = mocker.patch(self.CONTEXT_SETTINGS) + ctx_settings.default_access_role = None + svc_settings = mocker.patch(self.SERVICE_SETTINGS) + svc_settings.authorization_provider = "rbac" + svc_settings.default_access_policy = "restrictive" + + user = await get_user(username=default_user.username, session=session) + access_checker = AccessChecker( + auth_context=await AuthContext.from_user(user=user, session=session), + ) + access_checker.add_request( + ResourceRequest( + verb=ResourceAction.READ, + access_object=Resource( + name="finance.revenue", + resource_type=ResourceType.NAMESPACE, + ), + ), + ) + results = await access_checker.check(on_denied=AccessDenialMode.RETURN) + assert results[0].approved is False + + async def test_default_role_unions_with_explicit_grants( + self, + default_user: User, + session: AsyncSession, + mocker, + ): + """Explicit grants and default-role scopes both apply.""" + # Default role: read on everything + await self._make_role( + session, + default_user, + "viewer", + ResourceAction.READ, + "*", + ) + # Explicit grant: write on finance.* + write_role = await self._make_role( + session, + default_user, + "finance-writer", + ResourceAction.WRITE, + "finance.*", + ) + session.add( + RoleAssignment( + principal_id=default_user.id, + role_id=write_role.id, + granted_by_id=default_user.id, + ), + ) + await session.commit() + + ctx_settings = mocker.patch(self.CONTEXT_SETTINGS) + ctx_settings.default_access_role = "viewer" + svc_settings = mocker.patch(self.SERVICE_SETTINGS) + svc_settings.authorization_provider = "rbac" + svc_settings.default_access_policy = "restrictive" + + user = await get_user(username=default_user.username, session=session) + access_checker = AccessChecker( + auth_context=await AuthContext.from_user(user=user, session=session), + ) + access_checker.add_requests( + [ + ResourceRequest( + verb=ResourceAction.WRITE, + access_object=Resource( + name="finance.revenue", + resource_type=ResourceType.NAMESPACE, + ), + ), + ResourceRequest( + verb=ResourceAction.WRITE, + access_object=Resource( + name="growth.signups", + resource_type=ResourceType.NAMESPACE, + ), + ), + ResourceRequest( + verb=ResourceAction.READ, + access_object=Resource( + name="growth.signups", + resource_type=ResourceType.NAMESPACE, + ), + ), + ], + ) + results = await access_checker.check(on_denied=AccessDenialMode.RETURN) + assert results[0].approved is True # explicit write on finance.* + assert results[1].approved is False # no write on growth.* + assert results[2].approved is True # read via default role + + async def test_missing_default_role_falls_through( + self, + default_user: User, + session: AsyncSession, + mocker, + ): + """A configured-but-nonexistent default role loads no scopes.""" + ctx_settings = mocker.patch(self.CONTEXT_SETTINGS) + ctx_settings.default_access_role = "does-not-exist" + + scopes = await AuthContext.get_default_scopes(session=session) + assert scopes == [] + + async def test_expired_assignment_excluded_from_candidate_scopes( + self, + default_user: User, + session: AsyncSession, + mocker, + ): + """An expired assignment contributes no scopes to the candidate set.""" + role = await self._make_role( + session, + default_user, + "temp-writer", + ResourceAction.WRITE, + "finance.*", + ) + session.add( + RoleAssignment( + principal_id=default_user.id, + role_id=role.id, + granted_by_id=default_user.id, + expires_at=datetime.now(timezone.utc) - timedelta(hours=1), + ), + ) + await session.commit() + + ctx_settings = mocker.patch(self.CONTEXT_SETTINGS) + ctx_settings.default_access_role = None + svc_settings = mocker.patch(self.SERVICE_SETTINGS) + svc_settings.authorization_provider = "rbac" + svc_settings.default_access_policy = "restrictive" + + user = await get_user(username=default_user.username, session=session) + auth_context = await AuthContext.from_user(user=user, session=session) + + # The expired assignment is skipped, so no scopes are collected. + assert RBACAuthorizationService.candidate_scopes(auth_context) == [] + + access_checker = AccessChecker(auth_context=auth_context) + access_checker.add_request( + ResourceRequest( + verb=ResourceAction.WRITE, + access_object=Resource( + name="finance.revenue", + resource_type=ResourceType.NAMESPACE, + ), + ), + ) + results = await access_checker.check(on_denied=AccessDenialMode.RETURN) + assert results[0].approved is False + + @pytest.mark.asyncio class TestGroupBasedPermissions: """Tests for group-based role assignments.""" From d112f6210610997beea8a7d20bd512aa8d89dc2c Mon Sep 17 00:00:00 2001 From: Philip Weiss Date: Fri, 17 Jul 2026 17:27:47 -0700 Subject: [PATCH 3/3] Collect candidate scopes once per authorize() call Hoist the explicit-grant + default-access-role scope collection out of the per-request decision path so authorizing N resources no longer rebuilds the same candidate list N times. Co-authored-by: Cursor --- .../internal/access/authorization/service.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/datajunction-server/datajunction_server/internal/access/authorization/service.py b/datajunction-server/datajunction_server/internal/access/authorization/service.py index 495b7b1b3f..21f402b6c7 100644 --- a/datajunction-server/datajunction_server/internal/access/authorization/service.py +++ b/datajunction-server/datajunction_server/internal/access/authorization/service.py @@ -143,7 +143,8 @@ def authorize( AccessDecision(request=request, approved=True, reason="admin") for request in requests ] - return [self._make_decision(auth_context, request) for request in requests] + candidate_scopes = self.candidate_scopes(auth_context) + return [self._make_decision(request, candidate_scopes) for request in requests] @classmethod def candidate_scopes(cls, auth_context: AuthContext) -> List["RoleScope"]: @@ -167,15 +168,15 @@ def candidate_scopes(cls, auth_context: AuthContext) -> List["RoleScope"]: def _make_decision( self, - auth_context: AuthContext, request: ResourceRequest, + candidate_scopes: List["RoleScope"], ) -> AccessDecision: """ Convert ResourceRequest to AccessDecision. - Gathers all applicable scopes (explicit grants + default-access role) - and approves if any grants the request. Otherwise falls back to the - configured default_access_policy. + Evaluates the candidate scopes (explicit grants + default-access role) + collected once per authorize() call and approves if any grants the + request. Otherwise falls back to the configured default_access_policy. """ granted = any( self._scope_grants_permission( @@ -184,7 +185,7 @@ def _make_decision( request.access_object.resource_type, request.access_object.name, ) - for scope in self.candidate_scopes(auth_context) + for scope in candidate_scopes ) if granted: return AccessDecision(request=request, approved=True)