Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
"""Sync RBAC enum values with ORM serialization."""

from alembic import op

revision = "rbacenumfix1"
down_revision = "cm0001jsonbgin"
branch_labels = None
depends_on = None


def upgrade():
for value in ("HIERARCHY", "ROLE", "ROLE_ASSIGNMENT", "ROLE_SCOPE"):
op.execute(f"ALTER TYPE entitytype ADD VALUE IF NOT EXISTS '{value}'")


def downgrade():
pass
5 changes: 4 additions & 1 deletion datajunction-server/datajunction_server/api/groups.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,10 @@ async def enforce_group_administration(access_checker: AccessChecker) -> None:
privileged, cross-cutting operation rather than a per-namespace one.
"""
access_checker.add_scope(ResourceType.NAMESPACE, "*", ResourceAction.MANAGE)
await access_checker.check(on_denied=AccessDenialMode.RAISE)
await access_checker.check(
on_denied=AccessDenialMode.RAISE,
require_explicit_grant=True,
)


@router.post("/groups/", response_model=GroupOutput, status_code=201)
Expand Down
17 changes: 14 additions & 3 deletions datajunction-server/datajunction_server/api/rbac.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,10 @@ async def enforce_scope_management(
scope.scope_value,
ResourceAction.MANAGE,
)
await access_checker.check(on_denied=AccessDenialMode.RAISE)
await access_checker.check(
on_denied=AccessDenialMode.RAISE,
require_explicit_grant=True,
)


async def log_activity(
Expand Down Expand Up @@ -463,8 +466,16 @@ async def delete_scope_from_role(
include_deleted=False,
)

access_checker.add_scope(scope_type, scope_value, ResourceAction.MANAGE)
await access_checker.check(on_denied=AccessDenialMode.RAISE)
await enforce_scope_management(
access_checker,
[
RoleScopeInput(
action=action,
scope_type=scope_type,
scope_value=scope_value,
),
],
)

# Find the scope by composite key
delete_stmt = (
Expand Down
13 changes: 11 additions & 2 deletions datajunction-server/datajunction_server/database/rbac.py
Original file line number Diff line number Diff line change
Expand Up @@ -220,9 +220,18 @@ class RoleScope(Base):
nullable=False,
)

action: Mapped[ResourceAction] = mapped_column(Enum(ResourceAction), nullable=False)
action: Mapped[ResourceAction] = mapped_column(
Enum(
ResourceAction,
values_callable=lambda enum: [item.value for item in enum],
),
nullable=False,
)
scope_type: Mapped[ResourceType] = mapped_column(
Enum(ResourceType),
Enum(
ResourceType,
values_callable=lambda enum: [item.value for item in enum],
),
nullable=False,
)
scope_value: Mapped[str] = mapped_column(String(500), nullable=False)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,21 @@ def authorize(
The same list of requests with approved=True/False set on each
"""

def authorize_explicit_grants(
self,
auth_context: AuthContext,
requests: list[ResourceRequest],
) -> list[AccessDecision]:
"""Deny control-plane access until a provider explicitly supports it."""
return [
AccessDecision(
request=request,
approved=False,
reason="explicit_grant_provider_required",
)
for request in requests
]


class RBACAuthorizationService(AuthorizationService):
"""
Expand Down Expand Up @@ -142,6 +157,19 @@ def authorize(
]
return [self._make_decision(auth_context, request) for request in requests]

def authorize_explicit_grants(
self,
auth_context: AuthContext,
requests: list[ResourceRequest],
) -> list[AccessDecision]:
"""Authorize from assigned roles without policy fallback."""
if auth_context.is_admin:
return self.authorize(auth_context, requests)
return [
self._make_explicit_grant_decision(auth_context, request)
for request in requests
]

def _make_decision(
self,
auth_context: AuthContext,
Expand All @@ -161,6 +189,33 @@ def _make_decision(
approved=(has_grant or settings.default_access_policy == "permissive"),
)

def _make_explicit_grant_decision(
self,
auth_context: AuthContext,
request: ResourceRequest,
) -> AccessDecision:
"""Resolve a request from assigned roles only."""
has_grant = (
self.has_scope_permission(
assignments=auth_context.role_assignments,
action=request.verb,
scope_type=request.access_object.resource_type,
scope_value=request.access_object.name,
)
if request.scope_target
else self.has_permission(
assignments=auth_context.role_assignments,
action=request.verb,
resource_type=request.access_object.resource_type,
resource_name=request.access_object.name,
)
)
return AccessDecision(
request=request,
approved=has_grant,
reason="explicit_grant" if has_grant else "explicit_grant_required",
)

@classmethod
def resource_matches_pattern(cls, resource_name: str, pattern: str) -> bool:
"""
Expand Down Expand Up @@ -235,6 +290,87 @@ def has_permission(

return False

@classmethod
def has_scope_permission(
cls,
assignments: List,
action: ResourceAction,
scope_type: ResourceType,
scope_value: str,
) -> bool:
"""Return whether an assigned scope contains the requested scope."""
for assignment in assignments:
if assignment.expires_at and assignment.expires_at < datetime.now(
timezone.utc,
):
continue
for granted_scope in assignment.role.scopes:
granted_actions = cls.PERMISSION_HIERARCHY.get(
granted_scope.action,
{granted_scope.action},
)
if action in granted_actions and cls.scope_contains_scope(
granted_scope.scope_type,
granted_scope.scope_value,
scope_type,
scope_value,
):
return True
return False

@classmethod
def scope_contains_scope(
cls,
granted_type: ResourceType,
granted_value: str,
delegated_type: ResourceType,
delegated_value: str,
) -> bool:
"""Return whether one supported scope pattern contains another."""
granted_pattern = cls._parse_scope_pattern(granted_value)
delegated_pattern = cls._parse_scope_pattern(delegated_value)
if granted_pattern is None or delegated_pattern is None:
return False

if granted_type != delegated_type and not (
granted_type == ResourceType.NAMESPACE
and delegated_type == ResourceType.NODE
and granted_pattern[0] != "global"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might be looking at this wrong, but does the logic here mean that a global namespace grant contains fewer scopes than a narrower one? E.g., namespace:finance.* correctly contains node:finance.revenue, but namespace:* (global) is excluded by the != "global" clause, so a global namespace manager cannot delegate any node scope, while a subtree manager can. Is this a case where we just need to bring the next block up so that this block doesn't prematurely deny?

        if granted_kind == "global":
            return True

):
return False

granted_kind, granted_prefix = granted_pattern
delegated_kind, delegated_prefix = delegated_pattern
if granted_kind == "global":
return True
if delegated_kind == "global":
return False
if granted_kind == "exact":
return delegated_kind == "exact" and granted_prefix == delegated_prefix
if delegated_kind == "exact":
return delegated_prefix.startswith(granted_prefix + SEPARATOR)
return delegated_prefix == granted_prefix or delegated_prefix.startswith(
granted_prefix + SEPARATOR,
)

@staticmethod
def _parse_scope_pattern(value: str) -> tuple[str, str] | None:
"""Parse exact, trailing-wildcard, and global scope patterns."""
if not value or value == "*":
return ("global", "")
if (
value.startswith(SEPARATOR)
or value.endswith(SEPARATOR)
or SEPARATOR * 2 in value
):
return None
if "*" not in value:
return ("exact", value)
if value.count("*") != 1 or not value.endswith(f"{SEPARATOR}*"):
return None
prefix = value[: -len(f"{SEPARATOR}*")]
return ("subtree", prefix) if prefix else None

@classmethod
def _scope_grants_permission(
cls,
Expand Down Expand Up @@ -293,6 +429,14 @@ def authorize(
"""Approve all requests without checks (sync)."""
return [AccessDecision(request=request, approved=True) for request in requests]

def authorize_explicit_grants(
self,
auth_context: AuthContext,
requests: list[ResourceRequest],
) -> list[AccessDecision]:
"""Approve control-plane checks when authorization is disabled."""
return self.authorize(auth_context, requests)


@lru_cache(maxsize=None)
def get_authorization_service() -> AuthorizationService:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,12 +128,14 @@ def add_scope(
name=scope_value or "*",
resource_type=scope_type,
),
scope_target=True,
),
)

async def check(
self,
on_denied: AccessDenialMode = AccessDenialMode.FILTER,
require_explicit_grant: bool = False,
) -> list[AccessDecision]:
"""
Validate all requests using AuthorizationService.
Expand All @@ -143,9 +145,14 @@ async def check(
- FILTER: Return only approved (default)
- RAISE: Raise exception if any denied
- RETURN_ALL: Return all with approved field set
require_explicit_grant: Ignore provider fallback authorization
"""
auth_service = get_authorization_service()
access_decisions = auth_service.authorize(self.auth_context, self.requests)
access_decisions = (
auth_service.authorize_explicit_grants(self.auth_context, self.requests)
if require_explicit_grant
else auth_service.authorize(self.auth_context, self.requests)
)

if on_denied == AccessDenialMode.RETURN:
return access_decisions
Expand Down
12 changes: 10 additions & 2 deletions datajunction-server/datajunction_server/models/access.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,16 +62,24 @@ class ResourceRequest:
"""
Resource Requests provide the information
that is available to grant access to a resource

``scope_target`` marks a delegated scope pattern so authorization can
prove containment instead of treating it as one concrete resource.
"""

verb: ResourceAction
access_object: Resource
scope_target: bool = False

def __hash__(self) -> int:
return hash((self.verb, self.access_object))
return hash((self.verb, self.access_object, self.scope_target))

def __eq__(self, other) -> bool:
return self.verb == other.verb and self.access_object == other.access_object
return (
self.verb == other.verb
and self.access_object == other.access_object
and self.scope_target == other.scope_target
)

def __str__(self) -> str:
return (
Expand Down
26 changes: 18 additions & 8 deletions datajunction-server/tests/api/graphql/find_nodes_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -1976,15 +1976,25 @@ async def _setup_team_ownership(client: AsyncClient) -> None:
- default.repair_order_details.owners == ['team-analytics'] (group only)
- default.repair_orders_fact still has 'dj' as an owner
"""
# Register the group and add 'dj' as a member.
resp = await client.post("/groups/", params={"username": "team-analytics"})
assert resp.status_code == 201, resp.text

resp = await client.post(
"/groups/team-analytics/members/",
params={"member_username": "dj"},
# Register the group and add 'dj' as a member. Group administration now
# requires an explicit MANAGE grant, so authorize this setup as passthrough.
from datajunction_server.internal.access.authorization import (
PassthroughAuthorizationService,
)
assert resp.status_code == 201, resp.text

with mock.patch(
"datajunction_server.internal.access.authorization."
"validator.get_authorization_service",
return_value=PassthroughAuthorizationService(),
):
resp = await client.post("/groups/", params={"username": "team-analytics"})
assert resp.status_code == 201, resp.text

resp = await client.post(
"/groups/team-analytics/members/",
params={"member_username": "dj"},
)
assert resp.status_code == 201, resp.text

# Re-assign a node so the group is the sole owner (dj no longer owns it).
resp = await client.patch(
Expand Down
Loading
Loading