Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
91a9e45
UN-3853 [FIX] Attribute platform-key-created resources to the key's c…
kirtimanmishrazipstack Sep 2, 2026
b818d85
UN-3853 [FIX] Name the real owner on the deployment and pipeline card…
kirtimanmishrazipstack Sep 2, 2026
f3aa84a
UN-3853 Merge branch 'main' into UN-3853-co-ownership-resources-with-…
kirtimanmishrazipstack Sep 10, 2026
136879d
UN-3853 [FIX] Cover platform-key resource ownership with tests
kirtimanmishrazipstack Sep 11, 2026
9194f53
UN-3853 [FIX] Do not grant ownership to a departed organization member
kirtimanmishrazipstack Sep 14, 2026
d3421ea
UN-3853 [FIX] Show the owner on a freshly created deployment, and cov…
kirtimanmishrazipstack Sep 14, 2026
8bb2f01
UN-3853 [FIX] Backfill existing platform-key resources and document t…
kirtimanmishrazipstack Sep 14, 2026
98d503c
UN-3853 [FIX] Move the owner grant onto HasMembersMixin
kirtimanmishrazipstack Sep 14, 2026
b73e1f8
UN-3853 [FIX] Apply the membership rule on the key-deletion and backf…
kirtimanmishrazipstack Sep 14, 2026
45183a3
UN-3853 [FIX] Name the backfill's model bindings like the rest of the…
kirtimanmishrazipstack Sep 15, 2026
b2d830e
UN-3853 [FIX] Cut the comment prose down to what stays true
kirtimanmishrazipstack Sep 15, 2026
a57f99b
UN-3853 [FIX] Keep the service account authorized and the audit trail…
kirtimanmishrazipstack Sep 16, 2026
8a18875
UN-3853 [FIX] Drop the now-unused list refresh handler
kirtimanmishrazipstack Sep 16, 2026
1615c57
UN-3853 [FIX] Share the platform-key ownership repair for cloud-only …
kirtimanmishrazipstack Sep 16, 2026
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
5 changes: 1 addition & 4 deletions backend/adapter_processor_v2/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
IsOwnerOrSharedUserOrSharedToOrg,
)
from permissions.resource_share_views import ResourceShareManagementMixin
from permissions.roles import ResourceRole
from plugins import get_plugin
from rest_framework import status
from rest_framework.decorators import action
Expand Down Expand Up @@ -271,9 +270,7 @@ def create(self, request: Any) -> Response:
instance = serializer.save(organization=UserContext.get_organization())
# ``created_by`` is audit-only; the creator's access flows through
# an OWNER membership row (UN-2202 co-owners).
instance.memberships.get_or_create(
user_id=request.user.id, defaults={"role": ResourceRole.OWNER}
)
instance.grant_owner(request.user)
organization_member = OrganizationMemberService.get_user_by_id(
request.user.id
)
Expand Down
12 changes: 6 additions & 6 deletions backend/api_v2/api_deployment_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
from permissions.membership_views import OwnerManagementMixin
from permissions.permission import IsOwner, IsOwnerOrSharedUserOrSharedToOrg
from permissions.resource_share_views import ResourceShareManagementMixin
from permissions.roles import ResourceRole
from platform_api.openapi_schema import PlatformKeyAutoSchema
from plugins import get_plugin
from prompt_studio.prompt_studio_registry_v2.models import PromptStudioRegistry
Expand Down Expand Up @@ -360,9 +359,7 @@ def create(
self.perform_create(serializer)
# ``created_by`` is audit-only; the creator's access flows through an
# OWNER membership row (UN-2202 co-owners).
serializer.instance.memberships.get_or_create(
user_id=request.user.id, defaults={"role": ResourceRole.OWNER}
)
serializer.instance.grant_owner(request.user)
api_key = DeploymentHelper.create_api_key(serializer=serializer, request=request)
response_serializer = DeploymentResponseSerializer(
{"api_key": api_key.api_key, **serializer.data}
Expand Down Expand Up @@ -406,8 +403,11 @@ def by_prompt_studio_tool(self, request: Request) -> Response:
# Get API deployments for these workflows the user can access —
# ``created_by`` is audit-only; access flows through memberships,
# sharing, and the admin/SA bypasses (UN-2202).
deployments = APIDeployment.objects.for_user(request.user).filter(
workflow_id__in=workflow_ids
deployments = (
APIDeployment.objects.for_user(request.user)
.select_related("created_by")
.prefetch_related("memberships__user")
.filter(workflow_id__in=workflow_ids)
)

serializer = APIDeploymentListSerializer(deployments, many=True)
Expand Down
8 changes: 8 additions & 0 deletions backend/api_v2/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -510,6 +510,7 @@ class APIDeploymentListSerializer(ModelSerializer):
last_run_time = SerializerMethodField()
is_owner = SerializerMethodField()
co_owners_count = SerializerMethodField()
owner_emails = SerializerMethodField()

class Meta:
model = APIDeployment
Expand All @@ -529,6 +530,7 @@ class Meta:
"last_run_time",
"is_owner",
"co_owners_count",
"owner_emails",
]

def get_created_by_email(self, obj) -> str | None:
Expand All @@ -542,6 +544,12 @@ def get_is_owner(self, obj) -> bool:
def get_co_owners_count(self, obj) -> int:
return obj.co_owners_count()

def get_owner_emails(self, obj) -> list[str]:
"""Email of each owner, earliest first. Empty if none is a person."""
# Published field: APIDeploymentSummary inherits it, so it also
# reaches platform-key callers.
return obj.owner_emails()

# Both read the list view's annotations when they are there, and fall back
# to a query for the callers that serialize a plain queryset. A deployment
# that has never run annotates to `None`, so absence is what decides, not
Expand Down
5 changes: 1 addition & 4 deletions backend/connector_v2/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
from permissions.membership_views import OwnerManagementMixin
from permissions.permission import IsOwner, IsOwnerOrSharedUserOrSharedToOrg
from permissions.resource_share_views import ResourceShareManagementMixin
from permissions.roles import ResourceRole
from plugins import get_plugin
from rest_framework import status, viewsets
from rest_framework.decorators import action
Expand Down Expand Up @@ -258,9 +257,7 @@ def create(self, request: Any) -> Response:
)
# ``created_by`` is audit-only; the creator's access flows through an
# OWNER membership row (UN-2202 co-owners).
serializer.instance.memberships.get_or_create(
user_id=request.user.id, defaults={"role": ResourceRole.OWNER}
)
serializer.instance.grant_owner(request.user)
headers = self.get_success_headers(serializer.data)
return Response(serializer.data, status=status.HTTP_201_CREATED, headers=headers)

Expand Down
9 changes: 9 additions & 0 deletions backend/permissions/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,3 +76,12 @@ def is_owner(self, user: Any) -> bool:
m.user_id == user.id and m.role == ResourceRole.OWNER
for m in self.memberships.all() # type: ignore[attr-defined]
)

def grant_owner(self, user: Any) -> None:
"""Grant OWNER on create, resolving a platform key to its creator."""
# Lazy: platform_api.services imports models at import time.
from platform_api.services import owner_user_for

self.memberships.get_or_create( # type: ignore[attr-defined]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[critical] grant_owner moves the OWNER row off the service account, but two authorization paths still authorize by OWNER membership and have no is_service_account bypass — so every workflow provisioned through a platform key starts failing at execution with PermissionDenied, and migration 0006 applies the same change to already-deployed installs.

Provision entirely through a platform API key — the flow this PR exists for: POST an AdapterInstance, then a Workflow, then a tool instance on it.

grant_owner now writes the OWNER ResourceMembership row to the key's human creator instead of the service account S. created_by is untouched, so Workflow.created_by is still S (AuditSerializer.create, backend/backend/serializers.py:12).

At run time WorkflowHelper.run_workflow -> validate_tool_instances_meta sets user = tool.workflow.created_by (= S, workflow_helper.py:289, reached from :321) and calls validate_adapter_access (tool_instance_v2/tool_instance_helper.py:520-551). That gate evaluates:

  • is_admin -> False (OrganizationMemberService.is_user_organization_admin early-returns False for service accounts, organization_member_service.py:40-41; create_api_user_for_key gives the account UserRole.USER)
  • adapter_instance.shared_to_org -> False (default, adapter_processor_v2/models.py:112-115)
  • _is_resource_owner(S, adapter) -> False after this PR (it consults only memberships, permissions/permission.py:79-84; pre-PR the row named S and this returned True)
  • _is_resource_viewer / has_group_access -> False

=> raise PermissionDenied on every execution. Both transports hit it: the in-backend Celery path and the workers path via tool_instance_v2/internal_views.py:352-366.

The same gap hits IsWorkflowOwnerOrShared (workflow_manager/workflow_v2/permissions.py:40-45) — no service-account short-circuit — so the key gets 403 on the file-history endpoints for workflows it created.

Reproduced live against Postgres. With the post-PR grant, validate_adapter_access(user=service_account) raises PermissionDenied; the control writing main's service-account OWNER row passes the identical gate. Over real HTTP with platform-key bearer auth: GET .../workflow/<id>/file-histories/ returns 403 permission_denied post-PR and 200 with main's row.

Note PromptStudioRegistry.objects.list_tools does have the is_service_account bypass (prompt_studio_registry_v2/models.py:32-35) — which is likely why this gap went unnoticed. And migration 0006 filters role=OWNER, user_id__in=<service accounts> with no content-type restriction, so already-deployed, currently-working platform-key pipelines start failing the moment this deploys.

This is not covered by the object-permission short-circuit in permission.py — neither call site is a DRF has_object_permission on a service-account request.

Suggested fix: Either add the same if getattr(user, "is_service_account", False): return short-circuit that every other surface has to ToolInstanceHelper.validate_adapter_access and IsWorkflowOwnerOrShared.has_permission; or have grant_owner write both rows (service account + resolved creator). The owner surfaces already filter service accounts out, so the extra row stays invisible in the UI while nothing that resolves ownership through the membership table regresses.

Worth a test that executes a key-provisioned workflow end to end — the new test files assert the create-time row but never an execution.

🤖 Unstract PR review kit (Claude Code) · review-pr-bot:60d76e591c9b

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@Deepak-Kesavan Confirmed and fixed in a57f99b.

Verified the whole chain rather than taking it on trust: validate_tool_instances_meta sets user = tool.workflow.created_by (the service account, since created_by is deliberately untouched), validate_adapter_access has no service-account bypass, is_user_organization_admin early-returns False for one, and _is_resource_owner reads only memberships — which after this PR names the human. So every gate term evaluates False. Same gap in IsWorkflowOwnerOrShared.has_permission.

Took your first option — the short-circuit — rather than writing both rows, because a second OWNER row would have to be kept in sync by every future create site and the migration, whereas the bypass is the pattern every other surface already uses (IsOwner, for_user, list_tools).

Added two regression tests. Worth noting the first one I wrote was vacuous: AdapterInstance.objects is org-scoped and UserContext was unset, so the gate looped over an empty queryset and passed with the fix reverted. Setting the org identifier makes it fail correctly without the bypass.

user=owner_user_for(user), defaults={"role": ResourceRole.OWNER}
)
6 changes: 6 additions & 0 deletions backend/pipeline_v2/serializers/crud.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ class PipelineSerializer(IntegrityErrorMixin, AuditSerializer):
next_run_time = SerializerMethodField()
is_owner = SerializerMethodField()
co_owners_count = SerializerMethodField()
owner_emails = SerializerMethodField()
# ``shared_groups`` is no longer an M2M on Pipeline — declare it
# explicitly so ``fields = "__all__"`` continues to expose it. Share
# mutations go through ``POST /pipeline/{id}/share/`` (UN-2977 plan §B).
Expand Down Expand Up @@ -224,6 +225,11 @@ def get_is_owner(self, obj) -> bool:
def get_co_owners_count(self, obj) -> int:
return obj.co_owners_count()

def get_owner_emails(self, obj) -> list[str]:
# Names the actual owner in "Owned By"; ``created_by`` is audit-only
# (UN-2202) and stays the service account on platform-key creates.
return obj.owner_emails()

def get_last_5_run_statuses(self, instance: Pipeline) -> list[dict]:
"""Fetch the last 5 execution statuses with timestamps for this pipeline."""
return WorkflowExecution.get_last_run_statuses(instance.id, limit=5)
Expand Down
5 changes: 1 addition & 4 deletions backend/pipeline_v2/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
from permissions.membership_views import OwnerManagementMixin
from permissions.permission import IsOwner, IsOwnerOrSharedUserOrSharedToOrg
from permissions.resource_share_views import ResourceShareManagementMixin
from permissions.roles import ResourceRole
from plugins import get_plugin
from rest_framework import serializers, status, viewsets
from rest_framework.decorators import action
Expand Down Expand Up @@ -158,9 +157,7 @@ def create(self, request: Request) -> Response:
pipeline_instance = serializer.save()
# Grant before the API key so the creator's access is committed
# with the row itself, matching api_deployment_views.create().
pipeline_instance.memberships.get_or_create(
user_id=request.user.id, defaults={"role": ResourceRole.OWNER}
)
pipeline_instance.grant_owner(request.user)
# Create API key using the created instance
KeyHelper.create_api_key(pipeline_instance, request)
except IntegrityError:
Expand Down
107 changes: 96 additions & 11 deletions backend/platform_api/services.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from __future__ import annotations

import logging
import re
import uuid as _uuid
from typing import TYPE_CHECKING
Expand All @@ -15,6 +16,15 @@

from platform_api.models import PlatformApiKey

logger = logging.getLogger(__name__)

# Distinguishes "caller said nothing" from an explicit None in transfer_ownership.
_SAME_AS_TO_USER: object = object()

# Reserved domain for service-account addresses. The frontend matches on it to
# label an ownerless resource "Platform key" instead of naming a machine.
SERVICE_ACCOUNT_EMAIL_DOMAIN = "platform.internal"

# Business app labels whose models may carry created_by / membership rows.
# Restricts transfer_ownership to avoid scanning Django built-in and third-party models.
_BUSINESS_APP_LABELS = {
Expand Down Expand Up @@ -45,7 +55,7 @@ def create_api_user_for_key(
name_slug = _slugify_for_email(platform_api_key.name)
user = User(
username=f"svc-{name_slug}-{uid[:8]}",
email=f"{name_slug}-{uid[:8]}@platform.internal",
email=f"{name_slug}-{uid[:8]}@{SERVICE_ACCOUNT_EMAIL_DOMAIN}",
user_id=uid,
is_service_account=True,
)
Expand All @@ -63,6 +73,63 @@ def create_api_user_for_key(
return user


def live_key_creator(platform_api_key: PlatformApiKey) -> User | None:
"""The key's creator if they still belong to the key's organization.

``_is_resource_owner`` grants on any surviving OWNER row without checking
live membership, which is why ``cleanup_user_org_access`` purges those rows
when a user leaves. Handing an ex-member a fresh row -- at create, on key
deletion, or in a backfill -- reopens that rejoin backdoor, so every path
that names a successor asks this one question.
"""
creator = platform_api_key.created_by
if creator is None:
return None
# ``_base_manager`` because the default manager is org-scoped by
# ``UserContext``, which is None outside a request — an empty result would
# silently strip every resource of its owner. The org is filtered here.
if not OrganizationMember._base_manager.filter(
user=creator, organization=platform_api_key.organization
).exists():
return None
return creator


def owner_user_for(user: User) -> User:
"""Resolve the human who should own a resource created by ``user``.

Service accounts are filtered out of every owner surface, so granting to
one leaves no human owner; attribute it to the key's live creator instead.
Returns ``user`` unchanged for a normal session, or when no live creator
can be named -- the UI then labels the resource "Platform key".
"""
if not getattr(user, "is_service_account", False):
return user

# Imported here so the module keeps its models import behind TYPE_CHECKING.
from platform_api.models import PlatformApiKey

key = (
PlatformApiKey.objects.filter(api_user=user)
.select_related("created_by", "organization")
.first()
)
if key is None:
logger.warning(
"Service account %s backs no platform key; resource gets no human owner",
user.id,
)
return user
creator = live_key_creator(key)
if creator is None:
logger.warning(
"Platform key %s has no live creator; resource gets no human owner",
key.id,
)
return user
return creator


def _get_user_fk_fields(model: type) -> list[str]:
"""Return names of all ForeignKey fields pointing to User."""
return [
Expand Down Expand Up @@ -150,7 +217,9 @@ def _transfer_membership_rows(from_user: User, to_user: User) -> None:
row.delete()


def transfer_ownership(from_user: User, to_user: User | None) -> None:
def transfer_ownership(
from_user: User, to_user: User | None, membership_to: User | None = _SAME_AS_TO_USER
) -> None:
"""Transfer all resource ownership from one user to another.

Replaces from_user with to_user across business models:
Expand All @@ -159,25 +228,41 @@ def transfer_ownership(from_user: User, to_user: User | None) -> None:
- OWNER/VIEWER membership rows (custom-through, UN-2202) — re-pointed,
reconciling by role precedence when to_user already holds a row so the
resource keeps an owner.

``membership_to`` splits the two halves. Audit fields may follow a user who
has left the org -- nulling them has no security value and breaks deletes
that dereference ``created_by`` -- while an OWNER row may not.
"""
if not to_user:
return
if membership_to is _SAME_AS_TO_USER:
membership_to = to_user

with transaction.atomic():
for model in apps.get_models():
if model._meta.app_label not in _BUSINESS_APP_LABELS:
continue
_transfer_model_ownership(model, from_user, to_user)
if to_user:
for model in apps.get_models():
if model._meta.app_label not in _BUSINESS_APP_LABELS:
continue
_transfer_model_ownership(model, from_user, to_user)
# Memberships live in one polymorphic table — transfer once, not per model.
_transfer_membership_rows(from_user, to_user)
if membership_to:
_transfer_membership_rows(from_user, membership_to)


def delete_api_user_for_key(platform_api_key: PlatformApiKey) -> None:
"""Transfer ownership to key creator, then delete the service account."""
"""Transfer ownership to the key's creator, then delete the service account.

Audit fields follow ``created_by`` even if they have left the org: deleting
the account is ``SET_NULL`` on those FKs, and a null ``created_by`` breaks
callers that dereference it. An OWNER row is a live-membership question, so
it goes only to a creator ``live_key_creator`` still admits.
"""
api_user = platform_api_key.api_user
if not api_user:
return

with transaction.atomic():
transfer_ownership(from_user=api_user, to_user=platform_api_key.created_by)
transfer_ownership(
from_user=api_user,
to_user=platform_api_key.created_by,
membership_to=live_key_creator(platform_api_key),
)
api_user.delete()
Loading
Loading