From 2d07934cf09bda9e28b0267a5a3f825dd07393bd Mon Sep 17 00:00:00 2001 From: Philip Weiss Date: Fri, 5 Jun 2026 16:22:03 -0700 Subject: [PATCH 1/5] 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. Co-authored-by: Cursor --- .../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 5c954be62..468e6de15 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 6abd28f12..73aabf408 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 5e2b61b92..f4495d922 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 6da138a3cd1aa64ea8d228d379ffff1936fe8580 Mon Sep 17 00:00:00 2001 From: Philip Weiss Date: Fri, 5 Jun 2026 16:27:51 -0700 Subject: [PATCH 2/5] 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. Co-authored-by: Cursor --- .../datajunction_server/config.py | 6 + .../internal/access/authorization/context.py | 27 ++- .../internal/access/authorization/service.py | 46 ++++- .../tests/internal/authorization_test.py | 188 ++++++++++++++++++ 4 files changed, 258 insertions(+), 9 deletions(-) diff --git a/datajunction-server/datajunction_server/config.py b/datajunction-server/datajunction_server/config.py index 9dbdb8ef0..ee22e25b4 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 468e6de15..1a17810c7 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 73aabf408..495b7b1b3 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 f4495d922..aeef3337c 100644 --- a/datajunction-server/tests/internal/authorization_test.py +++ b/datajunction-server/tests/internal/authorization_test.py @@ -673,6 +673,194 @@ 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 == [] + + @pytest.mark.asyncio class TestGroupBasedPermissions: """Tests for group-based role assignments.""" From d11ea0bf50914d11c500d1340b6638d6ba98d281 Mon Sep 17 00:00:00 2001 From: Philip Weiss Date: Fri, 5 Jun 2026 17:09:54 -0700 Subject: [PATCH 3/5] Add per-action, per-namespace restrictive scopes Add a RESTRICTIVE_SCOPES setting (entries like "write:namespace:finance.*") so enforcement can be scoped to specific actions and namespaces: a matching request is denied unless an explicit grant allows it, while everything else follows the permissive baseline. This enables write governance on one namespace at a time (writes restrictive there, reads and other namespaces permissive) without a global deny flip. Extracts resource matching into a shared helper and applies the restrictive check in the decision fallback, after explicit grants. No-op when restrictive_scopes is empty. Co-authored-by: Cursor --- .../datajunction_server/config.py | 10 ++ .../internal/access/authorization/service.py | 91 ++++++++++-- .../tests/internal/authorization_test.py | 131 ++++++++++++++++++ 3 files changed, 219 insertions(+), 13 deletions(-) diff --git a/datajunction-server/datajunction_server/config.py b/datajunction-server/datajunction_server/config.py index ee22e25b4..78a6dc0cc 100644 --- a/datajunction-server/datajunction_server/config.py +++ b/datajunction-server/datajunction_server/config.py @@ -219,6 +219,16 @@ class Settings(BaseSettings): # pragma: no cover # permissive. Applied before the default_access_policy fallback. default_access_role: Optional[str] = None + # Scoped restrictive policy for per-action, per-namespace enforcement. + # Each entry is "action:scope_type:scope_value", e.g. "write:namespace:finance.*". + # A request matching any entry is denied unless an explicit grant allows it, + # regardless of default_access_policy. Anything not matched follows + # default_access_policy. This enables write governance scoped to specific + # namespaces (e.g. restrict write/delete/manage on one namespace while reads + # and all other namespaces stay permissive). List each mutating action + # explicitly; the permission hierarchy is not applied to these entries. + restrictive_scopes: List[str] = [] + # 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/service.py b/datajunction-server/datajunction_server/internal/access/authorization/service.py index 495b7b1b3..b53c492e1 100644 --- a/datajunction-server/datajunction_server/internal/access/authorization/service.py +++ b/datajunction-server/datajunction_server/internal/access/authorization/service.py @@ -174,8 +174,9 @@ def _make_decision( 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. + and approves if any grants the request. With no explicit grant, a + request under a configured restrictive scope is denied; otherwise it + falls back to the configured default_access_policy. """ granted = any( self._scope_grants_permission( @@ -188,6 +189,12 @@ def _make_decision( ) if granted: return AccessDecision(request=request, approved=True) + if self.request_is_restricted(request): + return AccessDecision( + request=request, + approved=False, + reason="restrictive scope (explicit grant required)", + ) return AccessDecision( request=request, approved=(settings.default_access_policy == "permissive"), @@ -289,26 +296,84 @@ def _scope_grants_permission( if action not in granted_actions: return False + return cls._resource_in_scope( + scope.scope_type, + scope.scope_value, + resource_type, + resource_name, + ) + + @classmethod + def _resource_in_scope( + cls, + scope_type: ResourceType, + scope_value: str, + resource_type: ResourceType, + resource_name: str, + ) -> bool: + """ + Check if a resource falls within a (scope_type, scope_value) boundary, + ignoring actions. Handles global ("*"/empty), same-type pattern matching, + and the namespace-covers-node cross-type case. + """ # Handle global access (empty string, None, or "*" scope_value) - if not scope.scope_value or scope.scope_value == "" or scope.scope_value == "*": - # Global scope matches any resource of the same type - return scope.scope_type == resource_type + if not scope_value or scope_value == "*": + return scope_type == resource_type # Same resource type - use pattern matching - if scope.scope_type == resource_type: - return cls.resource_matches_pattern(resource_name, scope.scope_value) + if scope_type == resource_type: + return cls.resource_matches_pattern(resource_name, scope_value) # Cross-resource-type: namespace scope can cover nodes - if ( - scope.scope_type == ResourceType.NAMESPACE - and resource_type == ResourceType.NODE - ): - # Check if node name matches the namespace pattern - return cls.resource_matches_pattern(resource_name, scope.scope_value) + if scope_type == ResourceType.NAMESPACE and resource_type == ResourceType.NODE: + return cls.resource_matches_pattern(resource_name, scope_value) # No match return False + @classmethod + def _restrictive_rules(cls) -> List[tuple]: + """ + Parse settings.restrictive_scopes into (action, scope_type, scope_value) + tuples. Malformed entries are ignored. + """ + rules: List[tuple] = [] + for raw in getattr(settings, "restrictive_scopes", []) or []: + parts = raw.split(":") + if len(parts) != 3: # pragma: no cover + continue + action_str, type_str, scope_value = parts + try: + rules.append( + ( + ResourceAction(action_str), + ResourceType(type_str), + scope_value, + ), + ) + except ValueError: # pragma: no cover + continue + return rules + + @classmethod + def request_is_restricted(cls, request: ResourceRequest) -> bool: + """ + Whether a request falls under a configured restrictive scope, meaning it + is denied by default (requires an explicit grant). Action match is exact + here (no hierarchy), so each restricted action must be listed explicitly. + """ + for action, scope_type, scope_value in cls._restrictive_rules(): + if action != request.verb: + continue + if cls._resource_in_scope( + scope_type, + scope_value, + request.access_object.resource_type, + request.access_object.name, + ): + return True + return False + class PassthroughAuthorizationService(AuthorizationService): """ diff --git a/datajunction-server/tests/internal/authorization_test.py b/datajunction-server/tests/internal/authorization_test.py index aeef3337c..f8d061f67 100644 --- a/datajunction-server/tests/internal/authorization_test.py +++ b/datajunction-server/tests/internal/authorization_test.py @@ -861,6 +861,137 @@ async def test_missing_default_role_falls_through( assert scopes == [] +@pytest.mark.asyncio +class TestRestrictiveScopes: + """Tests for per-action, per-namespace restrictive scopes (write governance).""" + + SERVICE_SETTINGS = ( + "datajunction_server.internal.access.authorization.service.settings" + ) + + async def test_restrictive_write_scope_denies_ungranted_writes( + self, + default_user: User, + session: AsyncSession, + mocker, + ): + """Writes under a restrictive scope are denied; reads and other namespaces stay open.""" + svc = mocker.patch(self.SERVICE_SETTINGS) + svc.authorization_provider = "rbac" + svc.default_access_policy = "permissive" + svc.restrictive_scopes = ["write:namespace:finance.*"] + + 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.NODE, + ), + ), + ResourceRequest( + verb=ResourceAction.READ, + access_object=Resource( + name="finance.revenue", + resource_type=ResourceType.NODE, + ), + ), + ResourceRequest( + verb=ResourceAction.WRITE, + access_object=Resource( + name="growth.signups", + resource_type=ResourceType.NODE, + ), + ), + ], + ) + results = await access_checker.check(on_denied=AccessDenialMode.RETURN) + assert results[0].approved is False # write on finance.* restricted + assert results[1].approved is True # read not restricted (permissive) + assert results[2].approved is True # write on a non-restricted namespace + + async def test_explicit_grant_overrides_restrictive_scope( + self, + default_user: User, + session: AsyncSession, + mocker, + ): + """An explicit grant still allows a write inside a restrictive scope.""" + role = Role(name="finance-writer", created_by_id=default_user.id) + session.add(role) + await session.flush() + session.add( + RoleScope( + role_id=role.id, + action=ResourceAction.WRITE, + scope_type=ResourceType.NAMESPACE, + scope_value="finance.*", + ), + ) + session.add( + RoleAssignment( + principal_id=default_user.id, + role_id=role.id, + granted_by_id=default_user.id, + ), + ) + await session.commit() + + svc = mocker.patch(self.SERVICE_SETTINGS) + svc.authorization_provider = "rbac" + svc.default_access_policy = "permissive" + svc.restrictive_scopes = ["write:namespace:finance.*"] + + 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.WRITE, + access_object=Resource( + name="finance.revenue", + resource_type=ResourceType.NODE, + ), + ), + ) + results = await access_checker.check(on_denied=AccessDenialMode.RETURN) + assert results[0].approved is True + + async def test_no_restrictive_scopes_is_permissive( + self, + default_user: User, + session: AsyncSession, + mocker, + ): + """With no restrictive scopes configured, permissive baseline applies.""" + svc = mocker.patch(self.SERVICE_SETTINGS) + svc.authorization_provider = "rbac" + svc.default_access_policy = "permissive" + svc.restrictive_scopes = [] + + 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.WRITE, + access_object=Resource( + name="finance.revenue", + resource_type=ResourceType.NODE, + ), + ), + ) + results = await access_checker.check(on_denied=AccessDenialMode.RETURN) + assert results[0].approved is True + + @pytest.mark.asyncio class TestGroupBasedPermissions: """Tests for group-based role assignments.""" From 6b3a66f80613b5032c346fe9b9c85bb7710121cc Mon Sep 17 00:00:00 2001 From: pweiss Date: Tue, 23 Jun 2026 22:15:58 +0000 Subject: [PATCH 4/5] Fix restrictive scope default role precedence Co-authored-by: Cursor --- .../internal/access/authorization/service.py | 57 ++++-- .../tests/internal/authorization_test.py | 179 ++++++++++++++++++ 2 files changed, 216 insertions(+), 20 deletions(-) diff --git a/datajunction-server/datajunction_server/internal/access/authorization/service.py b/datajunction-server/datajunction_server/internal/access/authorization/service.py index b53c492e1..33a7ef3f4 100644 --- a/datajunction-server/datajunction_server/internal/access/authorization/service.py +++ b/datajunction-server/datajunction_server/internal/access/authorization/service.py @@ -146,15 +146,14 @@ def authorize( return [self._make_decision(auth_context, request) for request in requests] @classmethod - def candidate_scopes(cls, auth_context: AuthContext) -> List["RoleScope"]: + def explicit_scopes(cls, auth_context: AuthContext) -> List["RoleScope"]: """ - Collect every scope that could grant a request for this context. + Collect explicit principal scopes 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. + This includes the principal's own non-expired role scopes and group + role scopes flattened into the auth context. Default-access role scopes + are deliberately excluded because restrictive scopes must require an + explicit principal/group/service-account grant. """ scopes: List["RoleScope"] = [] now = datetime.now(timezone.utc) @@ -162,7 +161,6 @@ def candidate_scopes(cls, auth_context: AuthContext) -> List["RoleScope"]: 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( @@ -173,21 +171,21 @@ def _make_decision( """ Convert ResourceRequest to AccessDecision. - Gathers all applicable scopes (explicit grants + default-access role) - and approves if any grants the request. With no explicit grant, a - request under a configured restrictive scope is denied; otherwise it - falls back to the configured default_access_policy. + Explicit grants are evaluated first. With no explicit grant, a request + under a configured restrictive scope is denied. Default-access role + scopes are evaluated only after the restrictive check, so they cannot + bypass governance for a restricted mutating action. """ - granted = any( + explicitly_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) + for scope in self.explicit_scopes(auth_context) ) - if granted: + if explicitly_granted: return AccessDecision(request=request, approved=True) if self.request_is_restricted(request): return AccessDecision( @@ -195,6 +193,17 @@ def _make_decision( approved=False, reason="restrictive scope (explicit grant required)", ) + default_granted = any( + self._scope_grants_permission( + scope, + request.verb, + request.access_object.resource_type, + request.access_object.name, + ) + for scope in auth_context.default_scopes + ) + if default_granted: + return AccessDecision(request=request, approved=True) return AccessDecision( request=request, approved=(settings.default_access_policy == "permissive"), @@ -335,13 +344,17 @@ def _resource_in_scope( def _restrictive_rules(cls) -> List[tuple]: """ Parse settings.restrictive_scopes into (action, scope_type, scope_value) - tuples. Malformed entries are ignored. + tuples. Invalid entries raise to avoid silently leaving a namespace + permissive because of a typo. """ rules: List[tuple] = [] for raw in getattr(settings, "restrictive_scopes", []) or []: parts = raw.split(":") - if len(parts) != 3: # pragma: no cover - continue + if len(parts) != 3: + raise ValueError( + "Invalid restrictive scope " + f"`{raw}`. Expected `action:scope_type:scope_value`.", + ) action_str, type_str, scope_value = parts try: rules.append( @@ -351,8 +364,12 @@ def _restrictive_rules(cls) -> List[tuple]: scope_value, ), ) - except ValueError: # pragma: no cover - continue + except ValueError as exc: + raise ValueError( + "Invalid restrictive scope " + f"`{raw}`. Expected `action:scope_type:scope_value` " + "with a valid ResourceAction and ResourceType.", + ) from exc return rules @classmethod diff --git a/datajunction-server/tests/internal/authorization_test.py b/datajunction-server/tests/internal/authorization_test.py index f8d061f67..2a9715605 100644 --- a/datajunction-server/tests/internal/authorization_test.py +++ b/datajunction-server/tests/internal/authorization_test.py @@ -865,10 +865,28 @@ async def test_missing_default_role_falls_through( class TestRestrictiveScopes: """Tests for per-action, per-namespace restrictive scopes (write governance).""" + 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_restrictive_write_scope_denies_ungranted_writes( self, default_user: User, @@ -963,6 +981,167 @@ async def test_explicit_grant_overrides_restrictive_scope( results = await access_checker.check(on_denied=AccessDenialMode.RETURN) assert results[0].approved is True + async def test_default_role_does_not_bypass_restrictive_scope( + self, + default_user: User, + session: AsyncSession, + mocker, + ): + """A default role cannot satisfy a write under a restrictive scope.""" + await self._make_role( + session, + default_user, + "global-writer", + ResourceAction.WRITE, + "*", + ) + + ctx_settings = mocker.patch(self.CONTEXT_SETTINGS) + ctx_settings.default_access_role = "global-writer" + svc = mocker.patch(self.SERVICE_SETTINGS) + svc.authorization_provider = "rbac" + svc.default_access_policy = "permissive" + svc.restrictive_scopes = ["write:namespace:finance.*"] + + 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.NODE, + ), + ), + ResourceRequest( + verb=ResourceAction.WRITE, + access_object=Resource( + name="growth.signups", + resource_type=ResourceType.NODE, + ), + ), + ], + ) + + results = await access_checker.check(on_denied=AccessDenialMode.RETURN) + assert results[0].approved is False + assert ( + results[0].reason + == "restrictive scope (explicit grant required)" + ) + assert results[1].approved is True + + async def test_default_read_role_still_applies_outside_restricted_action( + self, + default_user: User, + session: AsyncSession, + mocker, + ): + """Read access from the default role still applies when writes are restricted.""" + await self._make_role( + session, + default_user, + "restrictive-global-reader", + ResourceAction.READ, + "*", + ) + + ctx_settings = mocker.patch(self.CONTEXT_SETTINGS) + ctx_settings.default_access_role = "restrictive-global-reader" + svc = mocker.patch(self.SERVICE_SETTINGS) + svc.authorization_provider = "rbac" + svc.default_access_policy = "restrictive" + svc.restrictive_scopes = ["write:namespace:finance.*"] + + 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.NODE, + ), + ), + ) + + results = await access_checker.check(on_denied=AccessDenialMode.RETURN) + assert results[0].approved is True + + async def test_explicit_grant_allows_restricted_write_with_default_role( + self, + default_user: User, + session: AsyncSession, + mocker, + ): + """Explicit grants still approve restricted writes.""" + await self._make_role( + session, + default_user, + "global-reader-with-writer", + ResourceAction.READ, + "*", + ) + writer_role = await self._make_role( + session, + default_user, + "explicit-finance-writer", + ResourceAction.WRITE, + "finance.*", + ) + session.add( + RoleAssignment( + principal_id=default_user.id, + role_id=writer_role.id, + granted_by_id=default_user.id, + ), + ) + await session.commit() + + ctx_settings = mocker.patch(self.CONTEXT_SETTINGS) + ctx_settings.default_access_role = "global-reader-with-writer" + svc = mocker.patch(self.SERVICE_SETTINGS) + svc.authorization_provider = "rbac" + svc.default_access_policy = "permissive" + svc.restrictive_scopes = ["write:namespace:finance.*"] + + 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.WRITE, + access_object=Resource( + name="finance.revenue", + resource_type=ResourceType.NODE, + ), + ), + ) + + results = await access_checker.check(on_denied=AccessDenialMode.RETURN) + assert results[0].approved is True + + def test_malformed_restrictive_scope_raises(self, mocker): + """Bad restrictive scope config fails closed instead of being ignored.""" + svc = mocker.patch(self.SERVICE_SETTINGS) + svc.restrictive_scopes = ["writes:namespace:finance.*"] + request = ResourceRequest( + verb=ResourceAction.WRITE, + access_object=Resource( + name="finance.revenue", + resource_type=ResourceType.NODE, + ), + ) + + with pytest.raises(ValueError, match="Invalid restrictive scope"): + RBACAuthorizationService.request_is_restricted(request) + async def test_no_restrictive_scopes_is_permissive( self, default_user: User, From ecfde3f9a67157d437cdbc0483ae7d76f4801188 Mon Sep 17 00:00:00 2001 From: pweiss Date: Tue, 23 Jun 2026 22:22:23 +0000 Subject: [PATCH 5/5] Apply ruff formatting to RBAC tests Co-authored-by: Cursor --- datajunction-server/tests/internal/authorization_test.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/datajunction-server/tests/internal/authorization_test.py b/datajunction-server/tests/internal/authorization_test.py index 2a9715605..983b70fea 100644 --- a/datajunction-server/tests/internal/authorization_test.py +++ b/datajunction-server/tests/internal/authorization_test.py @@ -1028,10 +1028,7 @@ async def test_default_role_does_not_bypass_restrictive_scope( results = await access_checker.check(on_denied=AccessDenialMode.RETURN) assert results[0].approved is False - assert ( - results[0].reason - == "restrictive scope (explicit grant required)" - ) + assert results[0].reason == "restrictive scope (explicit grant required)" assert results[1].approved is True async def test_default_read_role_still_applies_outside_restricted_action(