From 91a9e45b615afccf80262c619e5b2ca772e833da Mon Sep 17 00:00:00 2001 From: kirtimanmishrazipstack Date: Wed, 2 Sep 2026 16:08:49 +0530 Subject: [PATCH 01/14] UN-3853 [FIX] Attribute platform-key-created resources to the key's creator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A platform API key authenticates as a service account, and every resource create path granted the OWNER membership row to that machine identity. Service accounts are filtered out of every owner surface (HasMembersMixin), so such a resource ended up with no human owner: invisible to its creator in list views, manageable only through the org-admin fallback, and rendered in "Owned By" as a synthetic @platform.internal address dressed up as a colleague. Record the key's creator as owner instead — the same successor delete_api_user_for_key already hands ownership to when a key is deleted, now applied at creation rather than only at deletion. The service account loses nothing: permission classes and for_user() short-circuit on is_service_account. Where no human can be named (the key's creator has since been deleted), the resource stays deliberately ownerless and the table labels it "Platform key" rather than naming a machine. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01JHnDZZWGhsevUdwgMyR2ai --- backend/adapter_processor_v2/views.py | 4 +- backend/api_v2/api_deployment_views.py | 4 +- backend/connector_v2/views.py | 4 +- backend/pipeline_v2/views.py | 4 +- backend/platform_api/services.py | 37 ++++++++++++++++++- .../prompt_studio_helper.py | 5 ++- .../prompt_studio_core_v2/views.py | 4 +- backend/workflow_manager/workflow_v2/views.py | 4 +- .../widgets/resource-table/ResourceTable.jsx | 22 +++++++++-- 9 files changed, 77 insertions(+), 11 deletions(-) diff --git a/backend/adapter_processor_v2/views.py b/backend/adapter_processor_v2/views.py index f2aef82b0c..2e6bd025fa 100644 --- a/backend/adapter_processor_v2/views.py +++ b/backend/adapter_processor_v2/views.py @@ -16,6 +16,7 @@ ) from permissions.resource_share_views import ResourceShareManagementMixin from permissions.roles import ResourceRole +from platform_api.services import owner_user_for from plugins import get_plugin from rest_framework import status from rest_framework.decorators import action @@ -272,7 +273,8 @@ 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). instance.memberships.get_or_create( - user_id=request.user.id, defaults={"role": ResourceRole.OWNER} + user_id=owner_user_for(request.user).id, + defaults={"role": ResourceRole.OWNER}, ) organization_member = OrganizationMemberService.get_user_by_id( request.user.id diff --git a/backend/api_v2/api_deployment_views.py b/backend/api_v2/api_deployment_views.py index 8f5d0763a9..e34948b9e6 100644 --- a/backend/api_v2/api_deployment_views.py +++ b/backend/api_v2/api_deployment_views.py @@ -9,6 +9,7 @@ from permissions.permission import IsOwner, IsOwnerOrSharedUserOrSharedToOrg from permissions.resource_share_views import ResourceShareManagementMixin from permissions.roles import ResourceRole +from platform_api.services import owner_user_for from plugins import get_plugin from prompt_studio.prompt_studio_registry_v2.models import PromptStudioRegistry from rest_framework import serializers, status, views, viewsets @@ -326,7 +327,8 @@ def create( # ``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} + user_id=owner_user_for(request.user).id, + defaults={"role": ResourceRole.OWNER}, ) api_key = DeploymentHelper.create_api_key(serializer=serializer, request=request) response_serializer = DeploymentResponseSerializer( diff --git a/backend/connector_v2/views.py b/backend/connector_v2/views.py index fd75b749db..a764aa932c 100644 --- a/backend/connector_v2/views.py +++ b/backend/connector_v2/views.py @@ -12,6 +12,7 @@ from permissions.permission import IsOwner, IsOwnerOrSharedUserOrSharedToOrg from permissions.resource_share_views import ResourceShareManagementMixin from permissions.roles import ResourceRole +from platform_api.services import owner_user_for from plugins import get_plugin from rest_framework import status, viewsets from rest_framework.decorators import action @@ -259,7 +260,8 @@ 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} + user_id=owner_user_for(request.user).id, + defaults={"role": ResourceRole.OWNER}, ) headers = self.get_success_headers(serializer.data) return Response(serializer.data, status=status.HTTP_201_CREATED, headers=headers) diff --git a/backend/pipeline_v2/views.py b/backend/pipeline_v2/views.py index ec7e7720f3..831727e684 100644 --- a/backend/pipeline_v2/views.py +++ b/backend/pipeline_v2/views.py @@ -13,6 +13,7 @@ from permissions.permission import IsOwner, IsOwnerOrSharedUserOrSharedToOrg from permissions.resource_share_views import ResourceShareManagementMixin from permissions.roles import ResourceRole +from platform_api.services import owner_user_for from plugins import get_plugin from rest_framework import serializers, status, viewsets from rest_framework.decorators import action @@ -159,7 +160,8 @@ def create(self, request: Request) -> Response: # 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} + user_id=owner_user_for(request.user).id, + defaults={"role": ResourceRole.OWNER}, ) # Create API key using the created instance KeyHelper.create_api_key(pipeline_instance, request) diff --git a/backend/platform_api/services.py b/backend/platform_api/services.py index 4087741227..203f337329 100644 --- a/backend/platform_api/services.py +++ b/backend/platform_api/services.py @@ -15,6 +15,10 @@ from platform_api.models import PlatformApiKey +# 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 = { @@ -45,7 +49,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, ) @@ -63,6 +67,37 @@ def create_api_user_for_key( return user +def owner_user_for(user: User) -> User: + """Resolve the human who should own a resource created by ``user``. + + A platform key authenticates as a service account, and service accounts are + filtered out of every owner surface (``HasMembersMixin``), so a resource + granted to one has no human owner: it is invisible to its creator and only + an org admin can manage it. Attribute it to the key's creator instead — the + same successor :func:`delete_api_user_for_key` already hands ownership to. + + Returns ``user`` unchanged for a normal session, and for the residual case + where the key's creator has since been deleted (``created_by`` is + ``SET_NULL``) — such a resource stays deliberately ownerless and the UI + labels it "Platform key". + + Org membership of the creator is deliberately not re-checked: a key can + outlive its creator's membership, and granting to an ex-member matches what + :func:`delete_api_user_for_key` already does. The row is inert until they + rejoin, which beats leaving the resource with no owner at all. + """ + 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").first() + ) + return key.created_by if key and key.created_by else user + + def _get_user_fk_fields(model: type) -> list[str]: """Return names of all ForeignKey fields pointing to User.""" return [ diff --git a/backend/prompt_studio/prompt_studio_core_v2/prompt_studio_helper.py b/backend/prompt_studio/prompt_studio_core_v2/prompt_studio_helper.py index 94f3850c62..2caeecec9d 100644 --- a/backend/prompt_studio/prompt_studio_core_v2/prompt_studio_helper.py +++ b/backend/prompt_studio/prompt_studio_core_v2/prompt_studio_helper.py @@ -18,6 +18,7 @@ has_group_access, ) from permissions.roles import ResourceRole +from platform_api.services import owner_user_for from plugins import get_plugin from rest_framework.exceptions import APIException from rest_framework.request import Request @@ -2919,7 +2920,9 @@ def create_tool_from_import_data( # created_by is audit-only; grant the creator an OWNER membership row so # access/ownership flows through it (UN-2202), as the viewset create does. - tool.memberships.get_or_create(user=user, defaults={"role": ResourceRole.OWNER}) + tool.memberships.get_or_create( + user=owner_user_for(user), defaults={"role": ResourceRole.OWNER} + ) return tool diff --git a/backend/prompt_studio/prompt_studio_core_v2/views.py b/backend/prompt_studio/prompt_studio_core_v2/views.py index 8990e870eb..ffbac48387 100644 --- a/backend/prompt_studio/prompt_studio_core_v2/views.py +++ b/backend/prompt_studio/prompt_studio_core_v2/views.py @@ -19,6 +19,7 @@ from permissions.permission import IsOwner, IsOwnerOrSharedUserOrSharedToOrg from permissions.resource_share_views import ResourceShareManagementMixin from permissions.roles import ResourceRole +from platform_api.services import owner_user_for from plugins import get_plugin from rest_framework import status, viewsets from rest_framework.decorators import action @@ -209,7 +210,8 @@ def create(self, request: HttpRequest) -> 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} + user_id=owner_user_for(request.user).id, + defaults={"role": ResourceRole.OWNER}, ) PromptStudioHelper.create_default_profile_manager( request.user, serializer.data["tool_id"] diff --git a/backend/workflow_manager/workflow_v2/views.py b/backend/workflow_manager/workflow_v2/views.py index fefba8c21a..9305800627 100644 --- a/backend/workflow_manager/workflow_v2/views.py +++ b/backend/workflow_manager/workflow_v2/views.py @@ -13,6 +13,7 @@ from permissions.roles import ResourceRole from pipeline_v2.models import Pipeline from pipeline_v2.pipeline_processor import PipelineProcessor +from platform_api.services import owner_user_for from plugins import get_plugin from rest_framework import serializers, status, viewsets from rest_framework.decorators import action, api_view @@ -162,7 +163,8 @@ def perform_create(self, serializer: WorkflowSerializer) -> Workflow: # ``created_by`` is audit-only; the creator's access flows through an # OWNER membership row (UN-2202 co-owners). workflow.memberships.get_or_create( - user_id=self.request.user.id, defaults={"role": ResourceRole.OWNER} + user_id=owner_user_for(self.request.user).id, + defaults={"role": ResourceRole.OWNER}, ) try: # Create empty WorkflowEndpoints for UI compatibility diff --git a/frontend/src/components/widgets/resource-table/ResourceTable.jsx b/frontend/src/components/widgets/resource-table/ResourceTable.jsx index c2a99ecb35..47d82f6515 100644 --- a/frontend/src/components/widgets/resource-table/ResourceTable.jsx +++ b/frontend/src/components/widgets/resource-table/ResourceTable.jsx @@ -24,6 +24,10 @@ import { Typography } from "@/components/ui/shims/antd-typography"; import { formattedDateTime, timeAgo } from "../../../helpers/GetStaticData"; import "./ResourceTable.css"; +// Service-account address minted by `create_api_user_for_key`. Its owner is a +// platform API key, not a person, so the cell is labelled rather than named. +const PLATFORM_KEY_EMAIL_DOMAIN = "@platform.internal"; + // Stable, distinct avatar swatch per owner (seeded on email/name) like the // design: a light pastel fill paired with a matching darker initial. const AVATAR_COLORS = [ @@ -219,16 +223,28 @@ function ResourceTable({ const renderOwner = (item) => { // owner_emails is earliest-first; [0] is the primary shown owner. // Fall back to created_by_email so rows with no live OWNER membership - // (platform API-key sessions, pre-backfill rows) don't render "Unknown". + // (pre-backfill rows) don't render "Unknown". const ownerEmails = item?.[ownerEmailsProp]; - const email = + const rawEmail = (Array.isArray(ownerEmails) ? ownerEmails[0] : undefined) ?? item?.created_by_email; + // Reached only when a platform key's creator has since been deleted, so no + // human can be named. Suppress the synthetic address rather than dress a + // machine identity up as a colleague. + const isPlatformKey = Boolean( + rawEmail?.endsWith(PLATFORM_KEY_EMAIL_DOMAIN), + ); + const email = isPlatformKey ? undefined : rawEmail; // "Me" must track the DISPLAYED owner, not the viewer's own membership — // else a co-owner sees "Me" over the primary owner's avatar/email. Match on // the shown email so the creator viewing their own resource still reads "Me". const isMe = Boolean(email) && email === sessionDetails?.email; - const name = isMe ? "Me" : email?.split("@")[0] || "Unknown"; + let name = email?.split("@")[0] || "Unknown"; + if (isPlatformKey) { + name = "Platform key"; + } else if (isMe) { + name = "Me"; + } const extra = item?.co_owners_count > 1 ? ` +${item.co_owners_count - 1}` : ""; const initials = (email || name).slice(0, 2).toUpperCase(); From b818d85937aa016f72584d672cc09c2c83fbd368 Mon Sep 17 00:00:00 2001 From: kirtimanmishrazipstack Date: Wed, 2 Sep 2026 16:17:25 +0530 Subject: [PATCH 02/14] UN-3853 [FIX] Name the real owner on the deployment and pipeline cards too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The ticket asks for Owned By to be correct on every resource type. The API deployment and ETL pipeline card views were still wrong: OwnerFieldRow read created_by_email only, and their serializers never exposed owner_emails — so those cards named the audit creator, which on a platform-key create is the service account. The backend fix alone could not reach them. Expose owner_emails on both serializers (their querysets already prefetch memberships__user, so it costs no extra query), and move the owner-label rule into one resolveOwnerDisplay helper shared by the table and the cards. The two had already drifted on both the source field and the "Me" rule — the card said "Me" to any owner, which is the co-owner bug the table's comment warns about. Cards now match the table: "Me" tracks the displayed owner. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01JHnDZZWGhsevUdwgMyR2ai --- backend/api_v2/serializers.py | 7 +++ backend/pipeline_v2/serializers/crud.py | 6 +++ .../card-grid-view/CardFieldComponents.jsx | 7 +-- .../src/components/widgets/owner-display.js | 44 +++++++++++++++++++ .../widgets/resource-table/ResourceTable.jsx | 34 +++----------- 5 files changed, 64 insertions(+), 34 deletions(-) create mode 100644 frontend/src/components/widgets/owner-display.js diff --git a/backend/api_v2/serializers.py b/backend/api_v2/serializers.py index a8703f01d1..b39e68d45c 100644 --- a/backend/api_v2/serializers.py +++ b/backend/api_v2/serializers.py @@ -512,6 +512,7 @@ class APIDeploymentListSerializer(ModelSerializer): last_run_time = SerializerMethodField() is_owner = SerializerMethodField() co_owners_count = SerializerMethodField() + owner_emails = SerializerMethodField() class Meta: model = APIDeployment @@ -531,6 +532,7 @@ class Meta: "last_run_time", "is_owner", "co_owners_count", + "owner_emails", ] def get_created_by_email(self, obj): @@ -544,6 +546,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_run_count(self, instance) -> int: """Get total execution count for this API deployment.""" return WorkflowExecution.objects.filter(pipeline_id=instance.id).count() diff --git a/backend/pipeline_v2/serializers/crud.py b/backend/pipeline_v2/serializers/crud.py index 956d9d3bf7..acc45a3336 100644 --- a/backend/pipeline_v2/serializers/crud.py +++ b/backend/pipeline_v2/serializers/crud.py @@ -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). @@ -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) diff --git a/frontend/src/components/widgets/card-grid-view/CardFieldComponents.jsx b/frontend/src/components/widgets/card-grid-view/CardFieldComponents.jsx index 646d2fbced..4ea6ae0906 100644 --- a/frontend/src/components/widgets/card-grid-view/CardFieldComponents.jsx +++ b/frontend/src/components/widgets/card-grid-view/CardFieldComponents.jsx @@ -27,6 +27,7 @@ import { formattedDateTime, shortenApiEndpoint, } from "../../../helpers/GetStaticData"; +import { resolveOwnerDisplay } from "../owner-display"; /** * Reusable action box with Edit, Share, Delete icons and kebab menu @@ -139,11 +140,7 @@ CardActionBox.propTypes = { * @return {JSX.Element} Rendered owner field row */ function OwnerFieldRow({ item, sessionDetails, onManageCoOwners }) { - const isOwner = item?.is_owner ?? item.created_by === sessionDetails?.userId; - const email = item.created_by_email; - const name = isOwner ? "Me" : email?.split("@")[0] || "Unknown"; - const extra = - item?.co_owners_count > 1 ? ` +${item.co_owners_count - 1}` : ""; + const { email, name, extra } = resolveOwnerDisplay(item, sessionDetails); const ownerDisplay = `${name}${extra}`; const ownerContent = ( diff --git a/frontend/src/components/widgets/owner-display.js b/frontend/src/components/widgets/owner-display.js new file mode 100644 index 0000000000..0900cbc237 --- /dev/null +++ b/frontend/src/components/widgets/owner-display.js @@ -0,0 +1,44 @@ +// Service-account address minted by `create_api_user_for_key`. Its owner is a +// platform API key, not a person, so the field is labelled rather than named. +const PLATFORM_KEY_EMAIL_DOMAIN = "@platform.internal"; + +/** + * Resolve the "Owned By" label for a resource row. + * + * Shared by the list table and the card views so the two cannot drift — they + * previously disagreed on both the source field and the "Me" rule. + * + * @param {object} item Resource row from a list endpoint. + * @param {object} sessionDetails Current session, for the "Me" comparison. + * @param {string} ownerEmailsProp Field holding the owner emails. + * @return {{email: string|undefined, name: string, extra: string}} + */ +function resolveOwnerDisplay(item, sessionDetails, ownerEmailsProp) { + // owner_emails is earliest-first; [0] is the primary shown owner. Fall back + // to created_by_email so rows with no live OWNER membership (pre-backfill + // rows) don't render "Unknown". + const ownerEmails = item?.[ownerEmailsProp ?? "owner_emails"]; + const rawEmail = + (Array.isArray(ownerEmails) ? ownerEmails[0] : undefined) ?? + item?.created_by_email; + // Reached only when a platform key's creator has since been deleted, so no + // human can be named. Suppress the synthetic address rather than dress a + // machine identity up as a colleague. + const isPlatformKey = Boolean(rawEmail?.endsWith(PLATFORM_KEY_EMAIL_DOMAIN)); + const email = isPlatformKey ? undefined : rawEmail; + // "Me" must track the DISPLAYED owner, not the viewer's own membership — + // else a co-owner sees "Me" over the primary owner's avatar/email. Match on + // the shown email so the creator viewing their own resource still reads "Me". + const isMe = Boolean(email) && email === sessionDetails?.email; + let name = email?.split("@")[0] || "Unknown"; + if (isPlatformKey) { + name = "Platform key"; + } else if (isMe) { + name = "Me"; + } + const extra = + item?.co_owners_count > 1 ? ` +${item.co_owners_count - 1}` : ""; + return { email, name, extra }; +} + +export { resolveOwnerDisplay }; diff --git a/frontend/src/components/widgets/resource-table/ResourceTable.jsx b/frontend/src/components/widgets/resource-table/ResourceTable.jsx index 47d82f6515..d218b51076 100644 --- a/frontend/src/components/widgets/resource-table/ResourceTable.jsx +++ b/frontend/src/components/widgets/resource-table/ResourceTable.jsx @@ -22,12 +22,9 @@ import { Table } from "@/components/ui/shims/antd-structure"; import { Typography } from "@/components/ui/shims/antd-typography"; import { formattedDateTime, timeAgo } from "../../../helpers/GetStaticData"; +import { resolveOwnerDisplay } from "../owner-display"; import "./ResourceTable.css"; -// Service-account address minted by `create_api_user_for_key`. Its owner is a -// platform API key, not a person, so the cell is labelled rather than named. -const PLATFORM_KEY_EMAIL_DOMAIN = "@platform.internal"; - // Stable, distinct avatar swatch per owner (seeded on email/name) like the // design: a light pastel fill paired with a matching darker initial. const AVATAR_COLORS = [ @@ -221,32 +218,11 @@ function ResourceTable({ }; const renderOwner = (item) => { - // owner_emails is earliest-first; [0] is the primary shown owner. - // Fall back to created_by_email so rows with no live OWNER membership - // (pre-backfill rows) don't render "Unknown". - const ownerEmails = item?.[ownerEmailsProp]; - const rawEmail = - (Array.isArray(ownerEmails) ? ownerEmails[0] : undefined) ?? - item?.created_by_email; - // Reached only when a platform key's creator has since been deleted, so no - // human can be named. Suppress the synthetic address rather than dress a - // machine identity up as a colleague. - const isPlatformKey = Boolean( - rawEmail?.endsWith(PLATFORM_KEY_EMAIL_DOMAIN), + const { email, name, extra } = resolveOwnerDisplay( + item, + sessionDetails, + ownerEmailsProp, ); - const email = isPlatformKey ? undefined : rawEmail; - // "Me" must track the DISPLAYED owner, not the viewer's own membership — - // else a co-owner sees "Me" over the primary owner's avatar/email. Match on - // the shown email so the creator viewing their own resource still reads "Me". - const isMe = Boolean(email) && email === sessionDetails?.email; - let name = email?.split("@")[0] || "Unknown"; - if (isPlatformKey) { - name = "Platform key"; - } else if (isMe) { - name = "Me"; - } - const extra = - item?.co_owners_count > 1 ? ` +${item.co_owners_count - 1}` : ""; const initials = (email || name).slice(0, 2).toUpperCase(); const swatch = colorForSeed(email || name); From 136879d9239a0f26f75487e1852bc4e4e65198e0 Mon Sep 17 00:00:00 2001 From: kirtimanmishrazipstack Date: Fri, 11 Sep 2026 10:35:06 +0530 Subject: [PATCH 03/14] UN-3853 [FIX] Cover platform-key resource ownership with tests `owner_user_for` had no coverage. Adds the resolver's own branches (normal user early-returns with no query, service account resolves to the key's creator, a deleted creator or a missing key leaves the resource ownerless) and one case per OSS resource that grants an OWNER row on create: workflow, prompt studio, ETL pipeline, API deployment, connector, adapter. The resource cases drive the real URLconf and middleware chain with a key minted in the test, so the service-account swap that caused the bug is exercised rather than simulated. Verified by mutation: reverting each call site to the raw request user fails the matching case. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01BAfubrF8kR2rKewMSjXagg --- .../platform_api/tests/test_owner_user_for.py | 74 ++++++ .../test_platform_key_resource_ownership.py | 225 ++++++++++++++++++ 2 files changed, 299 insertions(+) create mode 100644 backend/platform_api/tests/test_owner_user_for.py create mode 100644 backend/platform_api/tests/test_platform_key_resource_ownership.py diff --git a/backend/platform_api/tests/test_owner_user_for.py b/backend/platform_api/tests/test_owner_user_for.py new file mode 100644 index 0000000000..7844e06fe5 --- /dev/null +++ b/backend/platform_api/tests/test_owner_user_for.py @@ -0,0 +1,74 @@ +"""The `owner_user_for` resolver in isolation. + +End-to-end coverage of the create sites that call it lives in +`test_platform_key_resource_ownership.py`. +""" + +import secrets +import uuid + +from account_v2.models import Organization, User +from django.db import connection +from django.test.utils import CaptureQueriesContext +from platform_api.models import ApiKeyPermission, PlatformApiKey +from platform_api.services import create_api_user_for_key, owner_user_for +from rest_framework.test import APITestCase + +ORG = "org-owner-test" + + +def _make_user() -> User: + email = f"user-{uuid.uuid4().hex[:8]}@example.com" + return User.objects.create_user( + username=email, email=email, password=secrets.token_urlsafe() + ) + + +class OwnerUserForTest(APITestCase): + """The resolver in isolation.""" + + def setUp(self) -> None: + self.org = Organization.objects.create( + name=ORG, display_name="Owner Test", organization_id=ORG + ) + + def _make_key(self, created_by: User | None) -> PlatformApiKey: + key = PlatformApiKey.objects.create( + name=f"key-{uuid.uuid4().hex[:8]}", + description="test key", + organization=self.org, + permission=ApiKeyPermission.FULL_ACCESS, + created_by=created_by, + ) + # The minting path, for the `is_service_account` flag it sets. + create_api_user_for_key(key, self.org) + key.refresh_from_db() + return key + + def test_a_normal_user_is_returned_unchanged(self) -> None: + user = _make_user() + self.assertEqual(owner_user_for(user), user) + + def test_a_normal_user_costs_no_query(self) -> None: + """The early return is the hot path — every create site calls this.""" + user = _make_user() + with CaptureQueriesContext(connection) as queries: + owner_user_for(user) + self.assertEqual(len(queries), 0) + + def test_a_service_account_resolves_to_the_keys_creator(self) -> None: + creator = _make_user() + key = self._make_key(created_by=creator) + self.assertEqual(owner_user_for(key.api_user), creator) + + def test_a_deleted_creator_leaves_the_resource_ownerless(self) -> None: + """`created_by` is SET_NULL, so the key can outlive its creator.""" + key = self._make_key(created_by=_make_user()) + PlatformApiKey.objects.filter(pk=key.pk).update(created_by=None) + self.assertEqual(owner_user_for(key.api_user), key.api_user) + + def test_a_service_account_with_no_key_stays_itself(self) -> None: + key = self._make_key(created_by=_make_user()) + service_account = key.api_user + PlatformApiKey.objects.filter(pk=key.pk).delete() + self.assertEqual(owner_user_for(service_account), service_account) diff --git a/backend/platform_api/tests/test_platform_key_resource_ownership.py b/backend/platform_api/tests/test_platform_key_resource_ownership.py new file mode 100644 index 0000000000..40b2b1eebc --- /dev/null +++ b/backend/platform_api/tests/test_platform_key_resource_ownership.py @@ -0,0 +1,225 @@ +"""A resource created through a platform key is owned by the key's creator. + +One case per resource that grants an OWNER row on create. They live together +rather than in each owning app because the behaviour under test is one +resolver's (`owner_user_for`), and the interesting part is the same everywhere: +the middleware swaps `request.user` for a service account, and service accounts +are filtered out of every owner surface, so granting to one leaves the resource +with no human owner at all. + +Each case drives the real URLconf and middleware chain with a key minted here, +so the swap is exercised rather than simulated. Side effects that are not part +of the grant -- cron scheduling, API-key minting, adapter encryption -- are +patched out; what is asserted is only who ends up on the OWNER row. +""" + +import secrets +import uuid +from unittest.mock import patch + +from account_v2.models import Organization, User +from django.conf import settings +from django.test import override_settings +from permissions.roles import ResourceRole +from platform_api.models import ApiKeyPermission, PlatformApiKey +from platform_api.services import create_api_user_for_key +from rest_framework.test import APITestCase +from utils.user_context import UserContext +from workflow_manager.workflow_v2.models.workflow import Workflow + +ORG = "org-ownership" + +# Trimmed from the production chain, preserving its relative order. Pinning it +# keeps the suite behaving the same under the OSS and cloud test settings. +_MIDDLEWARE = [ + "middleware.request_id.CustomRequestIDMiddleware", + settings.TENANT_MIDDLEWARE, + "django.contrib.sessions.middleware.SessionMiddleware", + "django.middleware.csrf.CsrfViewMiddleware", + "django.contrib.auth.middleware.AuthenticationMiddleware", + settings.CUSTOM_AUTH_MIDDLEWARE, +] + + +@override_settings(MIDDLEWARE=_MIDDLEWARE) +class PlatformKeyResourceOwnershipTest(APITestCase): + def setUp(self) -> None: + self.org = Organization.objects.create( + name=ORG, display_name="Ownership", organization_id=ORG + ) + email = f"creator-{uuid.uuid4().hex[:8]}@example.com" + self.creator = User.objects.create_user( + username=email, email=email, password=secrets.token_urlsafe() + ) + self.key = PlatformApiKey.objects.create( + name=f"key-{uuid.uuid4().hex[:8]}", + description="test key", + organization=self.org, + permission=ApiKeyPermission.FULL_ACCESS, + created_by=self.creator, + ) + # The minting path, for the `is_service_account` flag it sets. + create_api_user_for_key(self.key, self.org) + + def tearDown(self) -> None: + # Left set, this thread-local scopes the managers in whatever runs next. + UserContext.set_organization_identifier(None) + + # -- helpers --------------------------------------------------------- + + def _post(self, path: str, payload: dict): + return self.client.post( + f"/{settings.PATH_PREFIX}/unstract/{ORG}/{path}", + payload, + format="json", + HTTP_AUTHORIZATION=f"Bearer {self.key.key}", + ) + + def _assert_owned_by_creator(self, instance) -> None: + """The OWNER row names the human who made the key, not the machine.""" + membership = instance.memberships.get(role=ResourceRole.OWNER) + self.assertEqual(membership.user, self.creator) + self.assertFalse( + membership.user.is_service_account, + "the OWNER row went to the key's service account", + ) + + def _make_workflow(self, *, api_endpoints: bool = False) -> Workflow: + UserContext.set_organization_identifier(ORG) + workflow = Workflow.objects.create( + workflow_name=f"wf-{uuid.uuid4().hex[:8]}", + organization=self.org, + created_by=self.creator, + ) + if api_endpoints: + # An API deployment is refused unless both endpoints exist and + # carry a connection type; API ones need no connector instance. + from workflow_manager.endpoint_v2.models import WorkflowEndpoint + + for endpoint_type in ( + WorkflowEndpoint.EndpointType.SOURCE, + WorkflowEndpoint.EndpointType.DESTINATION, + ): + WorkflowEndpoint.objects.update_or_create( + workflow=workflow, + endpoint_type=endpoint_type, + defaults={ + "connection_type": WorkflowEndpoint.ConnectionType.API + }, + ) + return workflow + + def _fetch(self, model, pk): + # Managers are org-scoped off a thread-local the request cleared. + UserContext.set_organization_identifier(ORG) + return model.objects.get(pk=pk) + + # -- resources ------------------------------------------------------- + + def test_workflow(self) -> None: + response = self._post( + "workflow/", {"workflow_name": f"wf-{uuid.uuid4().hex[:8]}"} + ) + self.assertEqual(response.status_code, 201, response.content) + self._assert_owned_by_creator(self._fetch(Workflow, response.json()["id"])) + + def test_prompt_studio_project(self) -> None: + from prompt_studio.prompt_studio_core_v2.models import CustomTool + + with patch( + "prompt_studio.prompt_studio_core_v2.views.PromptStudioHelper." + "create_default_profile_manager" + ): + response = self._post( + "prompt-studio/", + { + "tool_name": f"ps-{uuid.uuid4().hex[:8]}", + "description": "owned by the key's creator", + "author": "tester", + }, + ) + self.assertEqual(response.status_code, 201, response.content) + self._assert_owned_by_creator( + self._fetch(CustomTool, response.json()["tool_id"]) + ) + + def test_etl_pipeline(self) -> None: + from pipeline_v2.models import Pipeline + + workflow = self._make_workflow() + with patch("pipeline_v2.views.KeyHelper.create_api_key"): + response = self._post( + "pipeline/", + { + "pipeline_name": f"etl-{uuid.uuid4().hex[:8]}", + "workflow": str(workflow.id), + "pipeline_type": "ETL", + }, + ) + self.assertEqual(response.status_code, 201, response.content) + self._assert_owned_by_creator(self._fetch(Pipeline, response.json()["id"])) + + def test_api_deployment(self) -> None: + from api_v2.models import APIDeployment + + workflow = self._make_workflow(api_endpoints=True) + with ( + patch("api_v2.api_deployment_views.DeploymentHelper.create_api_key"), + patch("api_v2.api_deployment_views.notify_hubspot_event"), + ): + response = self._post( + "api/deployment/", + { + "display_name": f"api-{uuid.uuid4().hex[:8]}", + "api_name": f"api-{uuid.uuid4().hex[:8]}", + "description": "owned by the key's creator", + "workflow": str(workflow.id), + }, + ) + self.assertEqual(response.status_code, 201, response.content) + self._assert_owned_by_creator( + self._fetch(APIDeployment, response.json()["id"]) + ) + + def test_connector(self) -> None: + from connector_v2.models import ConnectorInstance + + workflow = self._make_workflow() + response = self._post( + "connector/", + { + "connector_name": f"conn-{uuid.uuid4().hex[:8]}", + # Must resolve in the connector registry, which is keyed by + # this exact string -- see ConnectorProcessor. + "connector_id": "minio|c799f6e3-2b57-434e-aaac-b5daa415da19", + "workflow": str(workflow.id), + "connector_mode": "FILESYSTEM", + "connector_metadata": { + "key": "test", + "secret": "test", + "endpoint_url": "http://localhost:9000", + "bucket": "test", + }, + }, + ) + self.assertEqual(response.status_code, 201, response.content) + self._assert_owned_by_creator( + self._fetch(ConnectorInstance, response.json()["id"]) + ) + + def test_adapter(self) -> None: + from adapter_processor_v2.models import AdapterInstance + + response = self._post( + "adapter/", + { + "adapter_name": f"adapter-{uuid.uuid4().hex[:8]}", + "adapter_id": "openai|502ecf49-e47c-445c-9907-6d4b90c5cd17", + "adapter_type": "LLM", + "adapter_metadata": {"adapter_name": "test", "api_key": "sk-test"}, + }, + ) + self.assertEqual(response.status_code, 201, response.content) + self._assert_owned_by_creator( + self._fetch(AdapterInstance, response.json()["id"]) + ) From 9194f5389efaa179f92b7382a453ba34247c2b33 Mon Sep 17 00:00:00 2001 From: kirtimanmishrazipstack Date: Mon, 14 Sep 2026 23:24:26 +0530 Subject: [PATCH 04/14] UN-3853 [FIX] Do not grant ownership to a departed organization member F1: owner_user_for granted an OWNER membership row to the key's creator without checking they are still a member of the key's organization. _is_resource_owner grants on any surviving OWNER row with no live-membership check, which is why cleanup_user_org_access purges those rows on departure; minting a new one afterwards reopened that rejoin backdoor for the key's lifetime. AddOwnerSerializer already refuses both non-members and service accounts, so these sites were the only OWNER writes without the gate. The check reads _base_manager: the default manager is org-scoped by UserContext, which is unset outside a request, so the plain manager would return empty and silently strip every resource of its owner. F6: the fallback arms now log, naming the key id (not the key itself). F7, F8: the docstring claimed service accounts are filtered from "every" owner surface and that the residual resource "stays deliberately ownerless". Neither held -- owner_memberships() and is_owner() apply no such filter, and a real OWNER row is written that the last-owner guard counts as live. Tests: the fixtures created a key creator with no OrganizationMember row, a user who could never have created a key through IsOrganizationAdmin. They now build the member production guarantees, and two cases pin the new guard (creator left the org; creator belongs to a different org). Both fail when the guard is neutered. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Tm83ikxv1euWWQQGLMBcjC --- backend/platform_api/services.py | 57 +++++++++++++------ .../platform_api/tests/test_owner_user_for.py | 50 +++++++++++++++- .../test_platform_key_resource_ownership.py | 8 +++ 3 files changed, 95 insertions(+), 20 deletions(-) diff --git a/backend/platform_api/services.py b/backend/platform_api/services.py index 203f337329..8b23161fd9 100644 --- a/backend/platform_api/services.py +++ b/backend/platform_api/services.py @@ -1,5 +1,6 @@ from __future__ import annotations +import logging import re import uuid as _uuid from typing import TYPE_CHECKING @@ -15,6 +16,8 @@ from platform_api.models import PlatformApiKey +logger = logging.getLogger(__name__) + # 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" @@ -70,21 +73,22 @@ def create_api_user_for_key( def owner_user_for(user: User) -> User: """Resolve the human who should own a resource created by ``user``. - A platform key authenticates as a service account, and service accounts are - filtered out of every owner surface (``HasMembersMixin``), so a resource - granted to one has no human owner: it is invisible to its creator and only - an org admin can manage it. Attribute it to the key's creator instead — the - same successor :func:`delete_api_user_for_key` already hands ownership to. - - Returns ``user`` unchanged for a normal session, and for the residual case - where the key's creator has since been deleted (``created_by`` is - ``SET_NULL``) — such a resource stays deliberately ownerless and the UI - labels it "Platform key". - - Org membership of the creator is deliberately not re-checked: a key can - outlive its creator's membership, and granting to an ex-member matches what - :func:`delete_api_user_for_key` already does. The row is inert until they - rejoin, which beats leaving the resource with no owner at all. + A platform key authenticates as a service account, which ``owners()``, + ``owner_email()`` and ``owner_emails()`` all filter out, so a resource + granted to one names no human owner and only an org admin can manage it. + Attribute it to the key's creator instead — the successor + :func:`delete_api_user_for_key` already hands ownership to. + + Returns ``user`` unchanged for a normal session, and whenever no live + creator can be named: no key row, a creator deleted since (``created_by`` + is ``SET_NULL``), or a creator who has left the organization. The OWNER row + then goes to the service account, which the owner surfaces filter out, and + the UI labels the resource "Platform key". + + The membership check is required, not optional: ``_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. + Minting a fresh one for an ex-member would reopen that rejoin backdoor. """ if not getattr(user, "is_service_account", False): return user @@ -93,9 +97,28 @@ def owner_user_for(user: User) -> User: from platform_api.models import PlatformApiKey key = ( - PlatformApiKey.objects.filter(api_user=user).select_related("created_by").first() + PlatformApiKey.objects.filter(api_user=user) + .select_related("created_by", "organization") + .first() ) - return key.created_by if key and key.created_by else user + if not (key and key.created_by): + logger.warning( + "Platform key %s has no creator; resource gets no human owner", + key.id if key else None, + ) + return user + # ``_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=key.created_by, organization=key.organization + ).exists(): + logger.warning( + "Creator of platform key %s has left the org; resource gets no human owner", + key.id, + ) + return user + return key.created_by def _get_user_fk_fields(model: type) -> list[str]: diff --git a/backend/platform_api/tests/test_owner_user_for.py b/backend/platform_api/tests/test_owner_user_for.py index 7844e06fe5..3e8e100a2a 100644 --- a/backend/platform_api/tests/test_owner_user_for.py +++ b/backend/platform_api/tests/test_owner_user_for.py @@ -7,12 +7,14 @@ import secrets import uuid +from account_v2.enums import UserRole from account_v2.models import Organization, User from django.db import connection from django.test.utils import CaptureQueriesContext from platform_api.models import ApiKeyPermission, PlatformApiKey from platform_api.services import create_api_user_for_key, owner_user_for from rest_framework.test import APITestCase +from tenant_account_v2.models import OrganizationMember ORG = "org-owner-test" @@ -32,6 +34,19 @@ def setUp(self) -> None: name=ORG, display_name="Owner Test", organization_id=ORG ) + def _make_member(self) -> User: + """A key creator as production guarantees one. + + ``IsOrganizationAdmin`` resolves the caller's ``OrganizationMember`` + before allowing key creation, so a key's ``created_by`` is always a + member of that org at mint time. + """ + user = _make_user() + OrganizationMember.objects.create( + user=user, organization=self.org, role=UserRole.ADMIN.value + ) + return user + def _make_key(self, created_by: User | None) -> PlatformApiKey: key = PlatformApiKey.objects.create( name=f"key-{uuid.uuid4().hex[:8]}", @@ -57,18 +72,47 @@ def test_a_normal_user_costs_no_query(self) -> None: self.assertEqual(len(queries), 0) def test_a_service_account_resolves_to_the_keys_creator(self) -> None: - creator = _make_user() + creator = self._make_member() key = self._make_key(created_by=creator) self.assertEqual(owner_user_for(key.api_user), creator) def test_a_deleted_creator_leaves_the_resource_ownerless(self) -> None: """`created_by` is SET_NULL, so the key can outlive its creator.""" - key = self._make_key(created_by=_make_user()) + key = self._make_key(created_by=self._make_member()) PlatformApiKey.objects.filter(pk=key.pk).update(created_by=None) self.assertEqual(owner_user_for(key.api_user), key.api_user) def test_a_service_account_with_no_key_stays_itself(self) -> None: - key = self._make_key(created_by=_make_user()) + key = self._make_key(created_by=self._make_member()) service_account = key.api_user PlatformApiKey.objects.filter(pk=key.pk).delete() self.assertEqual(owner_user_for(service_account), service_account) + + def test_a_creator_who_left_the_org_leaves_the_resource_ownerless(self) -> None: + """Granting OWNER to an ex-member would reopen the rejoin backdoor. + + ``cleanup_user_org_access`` purges a departing user's OWNER rows + because ``_is_resource_owner`` grants on any surviving row without + checking live membership. Minting a new one after that purge would + hand co-ownership back on re-invite. + """ + creator = self._make_member() + key = self._make_key(created_by=creator) + # ``_base_manager``: the default manager is org-scoped by UserContext, + # which is unset here, so a plain delete would match nothing. + OrganizationMember._base_manager.filter( + user=creator, organization=self.org + ).delete() + self.assertEqual(owner_user_for(key.api_user), key.api_user) + + def test_a_creator_in_a_different_org_is_not_granted(self) -> None: + """Membership is checked against the key's org, not any org.""" + creator = _make_user() + other_org = Organization.objects.create( + name="other", display_name="Other", organization_id="org-other" + ) + OrganizationMember.objects.create( + user=creator, organization=other_org, role=UserRole.ADMIN.value + ) + key = self._make_key(created_by=creator) + self.assertEqual(owner_user_for(key.api_user), key.api_user) diff --git a/backend/platform_api/tests/test_platform_key_resource_ownership.py b/backend/platform_api/tests/test_platform_key_resource_ownership.py index 40b2b1eebc..27fa32349f 100644 --- a/backend/platform_api/tests/test_platform_key_resource_ownership.py +++ b/backend/platform_api/tests/test_platform_key_resource_ownership.py @@ -17,6 +17,7 @@ import uuid from unittest.mock import patch +from account_v2.enums import UserRole from account_v2.models import Organization, User from django.conf import settings from django.test import override_settings @@ -24,6 +25,7 @@ from platform_api.models import ApiKeyPermission, PlatformApiKey from platform_api.services import create_api_user_for_key from rest_framework.test import APITestCase +from tenant_account_v2.models import OrganizationMember from utils.user_context import UserContext from workflow_manager.workflow_v2.models.workflow import Workflow @@ -51,6 +53,12 @@ def setUp(self) -> None: self.creator = User.objects.create_user( username=email, email=email, password=secrets.token_urlsafe() ) + # ``IsOrganizationAdmin`` resolves the caller's membership before + # allowing key creation, so a key's creator is always a member of its + # org -- and ``owner_user_for`` re-checks that before granting. + OrganizationMember.objects.create( + user=self.creator, organization=self.org, role=UserRole.ADMIN.value + ) self.key = PlatformApiKey.objects.create( name=f"key-{uuid.uuid4().hex[:8]}", description="test key", From d3421ea73e32faf2edd5e19ec2150fab2f6de715 Mon Sep 17 00:00:00 2001 From: kirtimanmishrazipstack Date: Mon, 14 Sep 2026 23:37:22 +0530 Subject: [PATCH 05/14] UN-3853 [FIX] Show the owner on a freshly created deployment, and cover the gaps F5: the create response is a seven-field summary carrying none of the owner fields the list renders, and the modal spliced it straight into the table, so a just-created API deployment showed no owner until the next fetch. It now refetches; the response is still handed to the code modal, which is the only thing that needs the API key. The edit path already merged over the existing row, so it kept its owner fields and is unchanged. F3: reverting the seventh grant site (create_tool_from_import_data, reached by project-transfer/ and sync-prompts/ with create_copy) left the whole backend suite green -- six cases covered seven sites. A case now pins it, and fails when that site is reverted. F9: the comment called the "Platform key" branch reachable only via a deleted creator. With no backfill it is also every resource created through a key before this change; the accurate statement already sits on the fallback above. F11: resolveOwnerDisplay decides the Owned By cell on every list and card and had no test -- the only suite rendering it asserts nothing about that cell. Six cases now cover its four branches; they fail when the platform-key suppression or the "Me" rule is broken. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Tm83ikxv1euWWQQGLMBcjC --- .../test_platform_key_resource_ownership.py | 29 +++++++++ .../api-deployment/ApiDeployment.jsx | 1 + .../CreateApiDeploymentModal.jsx | 9 ++- .../src/components/widgets/owner-display.js | 5 +- .../components/widgets/owner-display.test.js | 64 +++++++++++++++++++ 5 files changed, 103 insertions(+), 5 deletions(-) create mode 100644 frontend/src/components/widgets/owner-display.test.js diff --git a/backend/platform_api/tests/test_platform_key_resource_ownership.py b/backend/platform_api/tests/test_platform_key_resource_ownership.py index 27fa32349f..fc181752a9 100644 --- a/backend/platform_api/tests/test_platform_key_resource_ownership.py +++ b/backend/platform_api/tests/test_platform_key_resource_ownership.py @@ -215,6 +215,35 @@ def test_connector(self) -> None: self._fetch(ConnectorInstance, response.json()["id"]) ) + def test_prompt_studio_project_import(self) -> None: + """The import path grants its own OWNER row, separately from create. + + ``create_tool_from_import_data`` is reached by ``project-transfer/`` + and by ``sync-prompts/`` with ``create_copy``, and takes ``request.user`` + directly. Called here rather than over HTTP because the endpoint's other + work -- profile managers, prompt import, adapter validation -- is not + what is under test, and stubbing it would pin the stubs. + """ + from prompt_studio.prompt_studio_core_v2.models import CustomTool + from prompt_studio.prompt_studio_core_v2.prompt_studio_helper import ( + PromptStudioHelper, + ) + + tool = PromptStudioHelper.create_tool_from_import_data( + { + "tool_metadata": { + "tool_name": f"imported-{uuid.uuid4().hex[:8]}", + "author": "test", + "description": "imported through a platform key", + }, + "tool_settings": {}, + }, + f"imported-{uuid.uuid4().hex[:8]}", + self.org, + self.key.api_user, + ) + self._assert_owned_by_creator(self._fetch(CustomTool, tool.pk)) + def test_adapter(self) -> None: from adapter_processor_v2.models import AdapterInstance diff --git a/frontend/src/components/deployments/api-deployment/ApiDeployment.jsx b/frontend/src/components/deployments/api-deployment/ApiDeployment.jsx index b1e5bd0708..0de1b53faf 100644 --- a/frontend/src/components/deployments/api-deployment/ApiDeployment.jsx +++ b/frontend/src/components/deployments/api-deployment/ApiDeployment.jsx @@ -360,6 +360,7 @@ function ApiDeployment() { openCodeModal={setOpenCodeModal} setSelectedRow={setSelectedRow} workflowEndpointList={workflowEndpointList} + refreshList={() => fetchRef.current?.()} /> )} { const workflowStore = useWorkflowStore(); const { updateWorkflow } = workflowStore; @@ -109,8 +110,11 @@ const CreateApiDeploymentModal = ({ onDeploymentCreated(); } } else { - // Add new deployment to list - setTableData((prev) => [res?.data, ...prev]); + // Refetch rather than splice the create response: it is a summary + // carrying none of the owner fields the list renders, so a spliced + // row shows no owner until the next fetch. The response is still + // what the code modal needs -- only it carries the API key. + refreshList?.(); setSelectedRow(res?.data); openCodeModal(true); } @@ -294,6 +298,7 @@ CreateApiDeploymentModal.propTypes = { workflowEndpointList: PropTypes.object, setDeploymentName: PropTypes.func, onDeploymentCreated: PropTypes.func, + refreshList: PropTypes.func, }; export { CreateApiDeploymentModal }; diff --git a/frontend/src/components/widgets/owner-display.js b/frontend/src/components/widgets/owner-display.js index 0900cbc237..4ca0a784ee 100644 --- a/frontend/src/components/widgets/owner-display.js +++ b/frontend/src/components/widgets/owner-display.js @@ -21,9 +21,8 @@ function resolveOwnerDisplay(item, sessionDetails, ownerEmailsProp) { const rawEmail = (Array.isArray(ownerEmails) ? ownerEmails[0] : undefined) ?? item?.created_by_email; - // Reached only when a platform key's creator has since been deleted, so no - // human can be named. Suppress the synthetic address rather than dress a - // machine identity up as a colleague. + // Suppress the synthetic address rather than dress a machine identity up as + // a colleague. Reachability is stated at the fallback above. const isPlatformKey = Boolean(rawEmail?.endsWith(PLATFORM_KEY_EMAIL_DOMAIN)); const email = isPlatformKey ? undefined : rawEmail; // "Me" must track the DISPLAYED owner, not the viewer's own membership — diff --git a/frontend/src/components/widgets/owner-display.test.js b/frontend/src/components/widgets/owner-display.test.js new file mode 100644 index 0000000000..41bb4b83c2 --- /dev/null +++ b/frontend/src/components/widgets/owner-display.test.js @@ -0,0 +1,64 @@ +import { describe, expect, it } from "vitest"; + +import { resolveOwnerDisplay } from "./owner-display"; + +const SESSION = { email: "me@example.com" }; + +describe("resolveOwnerDisplay", () => { + it("prefers the first owner email over created_by_email", () => { + const { email, name } = resolveOwnerDisplay( + { + owner_emails: ["owner@example.com"], + created_by_email: "creator@example.com", + }, + SESSION, + ); + expect(email).toBe("owner@example.com"); + expect(name).toBe("owner"); + }); + + it("falls back to created_by_email when owner_emails is absent or empty", () => { + for (const item of [ + { created_by_email: "creator@example.com" }, + { owner_emails: [], created_by_email: "creator@example.com" }, + ]) { + expect(resolveOwnerDisplay(item, SESSION).email).toBe( + "creator@example.com", + ); + } + }); + + it("labels a service-account address 'Platform key' and shows no email", () => { + const { email, name } = resolveOwnerDisplay( + { created_by_email: "svc-key-1a2b3c4d@platform.internal" }, + SESSION, + ); + expect(name).toBe("Platform key"); + expect(email).toBeUndefined(); + }); + + it("reads 'Me' only when the DISPLAYED owner is the viewer", () => { + expect( + resolveOwnerDisplay({ owner_emails: ["me@example.com"] }, SESSION).name, + ).toBe("Me"); + // A co-owner viewing a resource someone else owns must not read "Me". + expect( + resolveOwnerDisplay({ owner_emails: ["other@example.com"] }, SESSION) + .name, + ).toBe("other"); + }); + + it("suffixes the extra co-owners, and nothing when there is one owner", () => { + const item = { owner_emails: ["a@example.com"] }; + expect( + resolveOwnerDisplay({ ...item, co_owners_count: 3 }, SESSION).extra, + ).toBe(" +2"); + expect( + resolveOwnerDisplay({ ...item, co_owners_count: 1 }, SESSION).extra, + ).toBe(""); + }); + + it("renders 'Unknown' when the row carries no owner field at all", () => { + expect(resolveOwnerDisplay({}, SESSION).name).toBe("Unknown"); + }); +}); From 8bb2f01e747053f6175af185948e6e07f3e85c80 Mon Sep 17 00:00:00 2001 From: kirtimanmishrazipstack Date: Mon, 14 Sep 2026 23:45:23 +0530 Subject: [PATCH 06/14] UN-3853 [FIX] Backfill existing platform-key resources and document the field F10: resources created through a platform key before this change hold an OWNER row naming the service account, which every owner surface filters out, so they show no owner and only an org admin can manage them. Nothing moved them -- the only re-pointer runs from delete_api_user_for_key. A data migration now hands each to the same successor owner_user_for picks, under the same membership rule: a creator who has left the org is skipped rather than handed a fresh OWNER row, which would reopen the rejoin backdoor. Where the creator already holds a row on that resource the stronger role is kept and the service account's is dropped, so the unique triple is never violated. Verified on a disposable database seeded with all three cases -- live creator, departed creator, and a creator already holding a VIEWER row: forward applies, reverse unapplies, and a re-apply is a no-op. F4: APIDeploymentSummary subclasses the list serializer and declares no fields of its own, so owner_emails reached the published platform-key contract with no description. Stripping it there would only desynchronise the spec from the response, and the UI reads the same endpoint, so the field stays and the docstring now documents what a caller receives. Spec regenerated. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Tm83ikxv1euWWQQGLMBcjC --- backend/api_v2/serializers.py | 7 +- .../0006_backfill_platform_key_ownership.py | 67 +++++++++++++++++++ specs/docstudio-oss.json | 1 + 3 files changed, 73 insertions(+), 2 deletions(-) create mode 100644 backend/tenant_account_v2/migrations/0006_backfill_platform_key_ownership.py diff --git a/backend/api_v2/serializers.py b/backend/api_v2/serializers.py index 080fcb84ef..bf98a0c755 100644 --- a/backend/api_v2/serializers.py +++ b/backend/api_v2/serializers.py @@ -545,8 +545,11 @@ 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. + """Email of each owner, earliest first. Empty if none is a person.""" + # This docstring is the published description: APIDeploymentSummary + # subclasses this serializer, so the field reaches platform-key callers + # beside the created_by_email they already received. Kept deliberately + # -- the UI reads the same endpoint and needs it to name the owner. return obj.owner_emails() # Both read the list view's annotations when they are there, and fall back diff --git a/backend/tenant_account_v2/migrations/0006_backfill_platform_key_ownership.py b/backend/tenant_account_v2/migrations/0006_backfill_platform_key_ownership.py new file mode 100644 index 0000000000..51f5569c48 --- /dev/null +++ b/backend/tenant_account_v2/migrations/0006_backfill_platform_key_ownership.py @@ -0,0 +1,67 @@ +"""UN-3853: re-point service-account OWNER rows to the key's creator. + +Rows written before ``owner_user_for`` existed name the key's service account. +Every owner surface filters those out, so the resource shows no owner and only +an org admin can manage it. This hands each one to the same successor the +resolver picks, under the same membership rule -- a creator who has left the +org is skipped, since granting them a fresh OWNER row would reopen the rejoin +backdoor ``cleanup_user_org_access`` exists to close. + +Idempotent: a second run finds no service-account OWNER rows left to move. +""" + +from django.db import migrations + +OWNER = "owner" + + +def _forward(apps, schema_editor): + ResourceMembership = apps.get_model("tenant_account_v2", "ResourceMembership") + OrganizationMember = apps.get_model("tenant_account_v2", "OrganizationMember") + PlatformApiKey = apps.get_model("platform_api", "PlatformApiKey") + + # Successor per service account. Keyed off the key rows rather than + # ``is_service_account`` so only accounts that actually back a key move. + successor: dict[int, int] = {} + for key in PlatformApiKey.objects.exclude(api_user_id=None).exclude( + created_by_id=None + ): + if OrganizationMember.objects.filter( + user_id=key.created_by_id, organization_id=key.organization_id + ).exists(): + successor[key.api_user_id] = key.created_by_id + + if not successor: + return + + rows = ResourceMembership.objects.filter(role=OWNER, user_id__in=successor.keys()) + for row in rows.iterator(): + new_user_id = successor[row.user_id] + clash = ResourceMembership.objects.filter( + user_id=new_user_id, + content_type_id=row.content_type_id, + object_id=row.object_id, + ).first() + if clash is None: + row.user_id = new_user_id + row.save(update_fields=["user"]) + continue + # The creator already holds a row on this resource. Keep the stronger + # role and drop the service account's, so (user, content_type, + # object_id) is never violated -- mirrors _transfer_membership_rows. + if clash.role != OWNER: + clash.role = OWNER + clash.save(update_fields=["role"]) + row.delete() + + +class Migration(migrations.Migration): + dependencies = [ + ("tenant_account_v2", "0005_resource_membership"), + ("platform_api", "0004_alter_platformapikey_organization"), + ] + + # Irreversible in substance: which rows were the service account's is not + # recoverable afterwards. Reversing is a no-op so the migration can still + # be unapplied without blocking a rollback. + operations = [migrations.RunPython(_forward, migrations.RunPython.noop)] diff --git a/specs/docstudio-oss.json b/specs/docstudio-oss.json index 3a9dbcf419..36a7ffe203 100644 --- a/specs/docstudio-oss.json +++ b/specs/docstudio-oss.json @@ -66,6 +66,7 @@ "type": "string" }, "owner_emails": { + "description": "Email of each owner, earliest first. Empty if none is a person.", "items": { "type": "string" }, From 98d503cb550615d70ddc28a610cd6032d314a5d7 Mon Sep 17 00:00:00 2001 From: kirtimanmishrazipstack Date: Mon, 14 Sep 2026 23:52:45 +0530 Subject: [PATCH 07/14] UN-3853 [FIX] Move the owner grant onto HasMembersMixin F12: resolving the acting user was a rule every create site had to remember, applied at seven sites here and four in cloud. That is how the agentic sample-project site came to be missed. HasMembersMixin is already on every one of these models, so grant_owner() lives there and each site calls it -- the rule now has one enforcement point, and all seven ownership cases fail when it is broken, instead of one case per site. Also drops seven imports of platform_api from the business apps, for an operation permissions/ owns; the resolver import is now lazy and in one place. Out of scope, checked: sharing_helpers grants VIEWER rather than create-time OWNER, and platform_admin's two onboarding grants take the human from signup rather than request.user, so no bearer session reaches them. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Tm83ikxv1euWWQQGLMBcjC --- backend/adapter_processor_v2/views.py | 7 +------ backend/api_v2/api_deployment_views.py | 7 +------ backend/connector_v2/views.py | 7 +------ backend/permissions/models.py | 15 +++++++++++++++ backend/pipeline_v2/views.py | 7 +------ .../prompt_studio_core_v2/prompt_studio_helper.py | 6 +----- .../prompt_studio/prompt_studio_core_v2/views.py | 7 +------ backend/workflow_manager/workflow_v2/views.py | 7 +------ 8 files changed, 22 insertions(+), 41 deletions(-) diff --git a/backend/adapter_processor_v2/views.py b/backend/adapter_processor_v2/views.py index 2e6bd025fa..79092acadf 100644 --- a/backend/adapter_processor_v2/views.py +++ b/backend/adapter_processor_v2/views.py @@ -15,8 +15,6 @@ IsOwnerOrSharedUserOrSharedToOrg, ) from permissions.resource_share_views import ResourceShareManagementMixin -from permissions.roles import ResourceRole -from platform_api.services import owner_user_for from plugins import get_plugin from rest_framework import status from rest_framework.decorators import action @@ -272,10 +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=owner_user_for(request.user).id, - defaults={"role": ResourceRole.OWNER}, - ) + instance.grant_owner(request.user) organization_member = OrganizationMemberService.get_user_by_id( request.user.id ) diff --git a/backend/api_v2/api_deployment_views.py b/backend/api_v2/api_deployment_views.py index 2132b38128..6511e49376 100644 --- a/backend/api_v2/api_deployment_views.py +++ b/backend/api_v2/api_deployment_views.py @@ -9,9 +9,7 @@ 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 platform_api.services import owner_user_for from plugins import get_plugin from prompt_studio.prompt_studio_registry_v2.models import PromptStudioRegistry from rest_framework import serializers, status, views, viewsets @@ -361,10 +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=owner_user_for(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} diff --git a/backend/connector_v2/views.py b/backend/connector_v2/views.py index a764aa932c..28f8038213 100644 --- a/backend/connector_v2/views.py +++ b/backend/connector_v2/views.py @@ -11,8 +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 platform_api.services import owner_user_for from plugins import get_plugin from rest_framework import status, viewsets from rest_framework.decorators import action @@ -259,10 +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=owner_user_for(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) diff --git a/backend/permissions/models.py b/backend/permissions/models.py index 7f5ad12087..e1dd47d045 100644 --- a/backend/permissions/models.py +++ b/backend/permissions/models.py @@ -76,3 +76,18 @@ 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, to the person behind ``user``. + + ``user`` is a platform key's service account on the bearer path, and + granting to one leaves the resource with no human owner. Resolving it + here rather than at each create site keeps that from being a rule every + new resource has to remember -- the reason a site was missed before. + """ + # Imported lazily: platform_api.services reads permissions.roles. + from platform_api.services import owner_user_for + + self.memberships.get_or_create( # type: ignore[attr-defined] + user=owner_user_for(user), defaults={"role": ResourceRole.OWNER} + ) diff --git a/backend/pipeline_v2/views.py b/backend/pipeline_v2/views.py index 831727e684..2bf502c904 100644 --- a/backend/pipeline_v2/views.py +++ b/backend/pipeline_v2/views.py @@ -12,8 +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 platform_api.services import owner_user_for from plugins import get_plugin from rest_framework import serializers, status, viewsets from rest_framework.decorators import action @@ -159,10 +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=owner_user_for(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: diff --git a/backend/prompt_studio/prompt_studio_core_v2/prompt_studio_helper.py b/backend/prompt_studio/prompt_studio_core_v2/prompt_studio_helper.py index a3d59966a4..a1d1cfb543 100644 --- a/backend/prompt_studio/prompt_studio_core_v2/prompt_studio_helper.py +++ b/backend/prompt_studio/prompt_studio_core_v2/prompt_studio_helper.py @@ -17,8 +17,6 @@ _is_resource_viewer, has_group_access, ) -from permissions.roles import ResourceRole -from platform_api.services import owner_user_for from plugins import get_plugin from rest_framework.exceptions import APIException from rest_framework.request import Request @@ -2924,9 +2922,7 @@ def create_tool_from_import_data( # created_by is audit-only; grant the creator an OWNER membership row so # access/ownership flows through it (UN-2202), as the viewset create does. - tool.memberships.get_or_create( - user=owner_user_for(user), defaults={"role": ResourceRole.OWNER} - ) + tool.grant_owner(user) return tool diff --git a/backend/prompt_studio/prompt_studio_core_v2/views.py b/backend/prompt_studio/prompt_studio_core_v2/views.py index 67bd4b1585..86a739efc8 100644 --- a/backend/prompt_studio/prompt_studio_core_v2/views.py +++ b/backend/prompt_studio/prompt_studio_core_v2/views.py @@ -19,8 +19,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.services import owner_user_for from plugins import get_plugin from rest_framework import status, viewsets from rest_framework.decorators import action @@ -212,10 +210,7 @@ def create(self, request: HttpRequest) -> 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=owner_user_for(request.user).id, - defaults={"role": ResourceRole.OWNER}, - ) + serializer.instance.grant_owner(request.user) PromptStudioHelper.create_default_profile_manager( request.user, serializer.data["tool_id"] ) diff --git a/backend/workflow_manager/workflow_v2/views.py b/backend/workflow_manager/workflow_v2/views.py index e00491fad3..e8f6d22723 100644 --- a/backend/workflow_manager/workflow_v2/views.py +++ b/backend/workflow_manager/workflow_v2/views.py @@ -10,10 +10,8 @@ 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 pipeline_v2.models import Pipeline from pipeline_v2.pipeline_processor import PipelineProcessor -from platform_api.services import owner_user_for from plugins import get_plugin from rest_framework import serializers, status, viewsets from rest_framework.decorators import action, api_view @@ -162,10 +160,7 @@ def perform_create(self, serializer: WorkflowSerializer) -> Workflow: ) # ``created_by`` is audit-only; the creator's access flows through an # OWNER membership row (UN-2202 co-owners). - workflow.memberships.get_or_create( - user_id=owner_user_for(self.request.user).id, - defaults={"role": ResourceRole.OWNER}, - ) + workflow.grant_owner(self.request.user) try: # Create empty WorkflowEndpoints for UI compatibility # ConnectorInstances will be created when users actually configure connectors From b73e1f8aa3cf9524f817c84fab93c049b51446d8 Mon Sep 17 00:00:00 2001 From: kirtimanmishrazipstack Date: Tue, 15 Sep 2026 00:40:52 +0530 Subject: [PATCH 08/14] UN-3853 [FIX] Apply the membership rule on the key-deletion and backfill paths Found by adversarial verification of the previous commits. F21: closing the create path left delete_api_user_for_key still handing every OWNER row to the key's creator with no membership check, so deleting a key re-granted a departed creator the rows the create path had just refused -- the same rejoin backdoor, one step later. The membership question is now live_key_creator(), asked by both paths instead of stated twice, and transfer_ownership already short-circuits on None. Two cases pin it. F22: migration 0006 declared no edge to UN-2202's *_absorb_shared_users backfills, which write an OWNER row from created_by -- the service account on a key-created resource. Six of the eight sorted before 0006 by alphabetical accident and workflow_v2.0022 sorted after it, so workflows would have been backfilled ownerless and left that way. The edges are now declared; workflow_v2.0022 moves from plan index 289 to 183, immediately before 0006. F23: the create-modal refresh called the fetch with its own defaults, so it dropped any active search and reset to page 1 -- and a never-run deployment sorts into the tail, so it was usually not on the page it landed on. It now uses handleListRefresh, which the hook documents for exactly this and which the sibling delete and co-owner callbacks already use. Also corrects two comments this branch introduced: the owner-display pointer named a fallback that states something else, and the lazy-import note named a cycle that does not exist (the real constraint is import-time model loading). Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Tm83ikxv1euWWQQGLMBcjC --- backend/permissions/models.py | 3 +- backend/platform_api/services.py | 54 +++++++++++++------ .../platform_api/tests/test_owner_user_for.py | 53 ++++++++++++++++-- .../0006_backfill_platform_key_ownership.py | 11 ++++ .../api-deployment/ApiDeployment.jsx | 3 +- .../src/components/widgets/owner-display.js | 2 +- 6 files changed, 102 insertions(+), 24 deletions(-) diff --git a/backend/permissions/models.py b/backend/permissions/models.py index e1dd47d045..d17496a204 100644 --- a/backend/permissions/models.py +++ b/backend/permissions/models.py @@ -85,7 +85,8 @@ def grant_owner(self, user: Any) -> None: here rather than at each create site keeps that from being a rule every new resource has to remember -- the reason a site was missed before. """ - # Imported lazily: platform_api.services reads permissions.roles. + # Imported lazily: this module is imported while models load, and + # platform_api.services pulls in models at import time. from platform_api.services import owner_user_for self.memberships.get_or_create( # type: ignore[attr-defined] diff --git a/backend/platform_api/services.py b/backend/platform_api/services.py index 8b23161fd9..0e7eef4d51 100644 --- a/backend/platform_api/services.py +++ b/backend/platform_api/services.py @@ -70,6 +70,28 @@ 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``. @@ -85,10 +107,7 @@ def owner_user_for(user: User) -> User: then goes to the service account, which the owner surfaces filter out, and the UI labels the resource "Platform key". - The membership check is required, not optional: ``_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. - Minting a fresh one for an ex-member would reopen that rejoin backdoor. + Naming a successor is :func:`live_key_creator`'s question, not this one's. """ if not getattr(user, "is_service_account", False): return user @@ -101,24 +120,20 @@ def owner_user_for(user: User) -> User: .select_related("created_by", "organization") .first() ) - if not (key and key.created_by): + if key is None: logger.warning( - "Platform key %s has no creator; resource gets no human owner", - key.id if key else None, + "Service account %s backs no platform key; resource gets no human owner", + user.id, ) return user - # ``_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=key.created_by, organization=key.organization - ).exists(): + creator = live_key_creator(key) + if creator is None: logger.warning( - "Creator of platform key %s has left the org; resource gets no human owner", + "Platform key %s has no live creator; resource gets no human owner", key.id, ) return user - return key.created_by + return creator def _get_user_fk_fields(model: type) -> list[str]: @@ -231,11 +246,16 @@ def transfer_ownership(from_user: User, to_user: User | None) -> None: 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. + + A creator who has left the org is not a successor -- ``transfer_ownership`` + short-circuits on ``None`` and the rows are dropped with the account, which + is what ``cleanup_user_org_access`` would have done to them anyway. + """ 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=live_key_creator(platform_api_key)) api_user.delete() diff --git a/backend/platform_api/tests/test_owner_user_for.py b/backend/platform_api/tests/test_owner_user_for.py index 3e8e100a2a..caae2ac6ea 100644 --- a/backend/platform_api/tests/test_owner_user_for.py +++ b/backend/platform_api/tests/test_owner_user_for.py @@ -1,6 +1,6 @@ -"""The `owner_user_for` resolver in isolation. +"""The ownership resolvers in isolation. -End-to-end coverage of the create sites that call it lives in +End-to-end coverage of the create sites that call them lives in `test_platform_key_resource_ownership.py`. """ @@ -11,6 +11,7 @@ from account_v2.models import Organization, User from django.db import connection from django.test.utils import CaptureQueriesContext +from permissions.roles import ResourceRole from platform_api.models import ApiKeyPermission, PlatformApiKey from platform_api.services import create_api_user_for_key, owner_user_for from rest_framework.test import APITestCase @@ -26,8 +27,8 @@ def _make_user() -> User: ) -class OwnerUserForTest(APITestCase): - """The resolver in isolation.""" +class _KeyFixture: + """Org, a member who mints keys, and the minting path itself.""" def setUp(self) -> None: self.org = Organization.objects.create( @@ -60,6 +61,10 @@ def _make_key(self, created_by: User | None) -> PlatformApiKey: key.refresh_from_db() return key + +class OwnerUserForTest(_KeyFixture, APITestCase): + """The resolver in isolation.""" + def test_a_normal_user_is_returned_unchanged(self) -> None: user = _make_user() self.assertEqual(owner_user_for(user), user) @@ -116,3 +121,43 @@ def test_a_creator_in_a_different_org_is_not_granted(self) -> None: ) key = self._make_key(created_by=creator) self.assertEqual(owner_user_for(key.api_user), key.api_user) + + +class KeyDeletionSuccessorTest(_KeyFixture, APITestCase): + """Deleting a key must not hand its rows to a departed creator. + + ``delete_api_user_for_key`` re-points the service account's rows to the + key's creator. That is the same grant ``owner_user_for`` refuses at create + time, so it asks the same question -- otherwise deleting a key reopens the + rejoin backdoor the resolver closes. + """ + + def _owner_row_users(self, resource): + return {m.user_id for m in resource.memberships.filter(role=ResourceRole.OWNER)} + + def _key_owned_workflow(self, creator): + from workflow_manager.workflow_v2.models.workflow import Workflow + + key = self._make_key(created_by=creator) + workflow = Workflow.objects.create( + workflow_name=f"wf-{uuid.uuid4().hex[:8]}", organization=self.org + ) + workflow.memberships.create( + user=key.api_user, role=ResourceRole.OWNER, organization=self.org + ) + return key, workflow + + def test_a_live_creator_inherits_the_rows(self) -> None: + creator = self._make_member() + key, workflow = self._key_owned_workflow(creator) + key.delete() + self.assertEqual(self._owner_row_users(workflow), {creator.id}) + + def test_a_departed_creator_inherits_nothing(self) -> None: + creator = self._make_member() + key, workflow = self._key_owned_workflow(creator) + OrganizationMember._base_manager.filter( + user=creator, organization=self.org + ).delete() + key.delete() + self.assertNotIn(creator.id, self._owner_row_users(workflow)) diff --git a/backend/tenant_account_v2/migrations/0006_backfill_platform_key_ownership.py b/backend/tenant_account_v2/migrations/0006_backfill_platform_key_ownership.py index 51f5569c48..6a87f7afc0 100644 --- a/backend/tenant_account_v2/migrations/0006_backfill_platform_key_ownership.py +++ b/backend/tenant_account_v2/migrations/0006_backfill_platform_key_ownership.py @@ -59,6 +59,17 @@ class Migration(migrations.Migration): dependencies = [ ("tenant_account_v2", "0005_resource_membership"), ("platform_api", "0004_alter_platformapikey_organization"), + # UN-2202's backfills write an OWNER row from ``created_by``, which is + # the service account on a key-created resource. They must land before + # this one or their rows are written after the repair and stay + # ownerless -- ordering that was otherwise alphabetical accident, and + # already wrong for workflow_v2. + ("adapter_processor_v2", "0005_absorb_shared_users"), + ("api_v2", "0005_absorb_shared_users"), + ("connector_v2", "0007_absorb_shared_users"), + ("pipeline_v2", "0005_absorb_shared_users"), + ("prompt_studio_core_v2", "0009_absorb_shared_users"), + ("workflow_v2", "0022_absorb_shared_users"), ] # Irreversible in substance: which rows were the service account's is not diff --git a/frontend/src/components/deployments/api-deployment/ApiDeployment.jsx b/frontend/src/components/deployments/api-deployment/ApiDeployment.jsx index 0de1b53faf..62867bacbf 100644 --- a/frontend/src/components/deployments/api-deployment/ApiDeployment.jsx +++ b/frontend/src/components/deployments/api-deployment/ApiDeployment.jsx @@ -85,6 +85,7 @@ function ApiDeployment() { fetchRef, handlePaginationChange, handleSearch, + handleListRefresh, } = usePaginatedList(); const { scrollRestoreId, activateScrollRestore, clearPendingScroll } = @@ -360,7 +361,7 @@ function ApiDeployment() { openCodeModal={setOpenCodeModal} setSelectedRow={setSelectedRow} workflowEndpointList={workflowEndpointList} - refreshList={() => fetchRef.current?.()} + refreshList={handleListRefresh} /> )} Date: Tue, 15 Sep 2026 19:16:55 +0530 Subject: [PATCH 09/14] UN-3853 [FIX] Name the backfill's model bindings like the rest of the tree Sonar flagged all three `apps.get_model` bindings in the backfill (python:S117). `0005_add_reconciliation_task` already uses the `*_model` form, so this matches the migration the repo most recently reviewed rather than the older PascalCase one in `0004_pg_periodic_tasks`. The quoted model names are untouched -- those are lookup keys, not identifiers. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01BAfubrF8kR2rKewMSjXagg --- .../0006_backfill_platform_key_ownership.py | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/backend/tenant_account_v2/migrations/0006_backfill_platform_key_ownership.py b/backend/tenant_account_v2/migrations/0006_backfill_platform_key_ownership.py index 6a87f7afc0..7a13365d81 100644 --- a/backend/tenant_account_v2/migrations/0006_backfill_platform_key_ownership.py +++ b/backend/tenant_account_v2/migrations/0006_backfill_platform_key_ownership.py @@ -16,17 +16,17 @@ def _forward(apps, schema_editor): - ResourceMembership = apps.get_model("tenant_account_v2", "ResourceMembership") - OrganizationMember = apps.get_model("tenant_account_v2", "OrganizationMember") - PlatformApiKey = apps.get_model("platform_api", "PlatformApiKey") + resource_membership_model = apps.get_model("tenant_account_v2", "ResourceMembership") + organization_member_model = apps.get_model("tenant_account_v2", "OrganizationMember") + platform_api_key_model = apps.get_model("platform_api", "PlatformApiKey") # Successor per service account. Keyed off the key rows rather than # ``is_service_account`` so only accounts that actually back a key move. successor: dict[int, int] = {} - for key in PlatformApiKey.objects.exclude(api_user_id=None).exclude( + for key in platform_api_key_model.objects.exclude(api_user_id=None).exclude( created_by_id=None ): - if OrganizationMember.objects.filter( + if organization_member_model.objects.filter( user_id=key.created_by_id, organization_id=key.organization_id ).exists(): successor[key.api_user_id] = key.created_by_id @@ -34,10 +34,12 @@ def _forward(apps, schema_editor): if not successor: return - rows = ResourceMembership.objects.filter(role=OWNER, user_id__in=successor.keys()) + rows = resource_membership_model.objects.filter( + role=OWNER, user_id__in=successor.keys() + ) for row in rows.iterator(): new_user_id = successor[row.user_id] - clash = ResourceMembership.objects.filter( + clash = resource_membership_model.objects.filter( user_id=new_user_id, content_type_id=row.content_type_id, object_id=row.object_id, From b2d830ed470e0b3047dc68bd98eae8ffe02fccde Mon Sep 17 00:00:00 2001 From: kirtimanmishrazipstack Date: Tue, 15 Sep 2026 19:24:40 +0530 Subject: [PATCH 10/14] UN-3853 [FIX] Cut the comment prose down to what stays true The comments had grown into mechanism retellings and change history -- why a site was missed, what the two views used to disagree on, which surface filters what. That rots as soon as the code moves and costs every later reader. Keeps the contract (what a function returns, why a lazy import, why _base_manager) and drops the narration. No code changed. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01BAfubrF8kR2rKewMSjXagg --- backend/api_v2/serializers.py | 6 ++---- backend/permissions/models.py | 11 ++-------- backend/platform_api/services.py | 17 ++++----------- .../platform_api/tests/test_owner_user_for.py | 19 +++-------------- .../test_platform_key_resource_ownership.py | 14 +++---------- .../0006_backfill_platform_key_ownership.py | 21 +++++++------------ .../CreateApiDeploymentModal.jsx | 6 ++---- .../src/components/widgets/owner-display.js | 19 +++++------------ 8 files changed, 28 insertions(+), 85 deletions(-) diff --git a/backend/api_v2/serializers.py b/backend/api_v2/serializers.py index bf98a0c755..d9d8c9638a 100644 --- a/backend/api_v2/serializers.py +++ b/backend/api_v2/serializers.py @@ -546,10 +546,8 @@ def get_co_owners_count(self, obj) -> int: def get_owner_emails(self, obj) -> list[str]: """Email of each owner, earliest first. Empty if none is a person.""" - # This docstring is the published description: APIDeploymentSummary - # subclasses this serializer, so the field reaches platform-key callers - # beside the created_by_email they already received. Kept deliberately - # -- the UI reads the same endpoint and needs it to name the owner. + # 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 diff --git a/backend/permissions/models.py b/backend/permissions/models.py index d17496a204..80b2d5030b 100644 --- a/backend/permissions/models.py +++ b/backend/permissions/models.py @@ -78,15 +78,8 @@ def is_owner(self, user: Any) -> bool: ) def grant_owner(self, user: Any) -> None: - """Grant OWNER on create, to the person behind ``user``. - - ``user`` is a platform key's service account on the bearer path, and - granting to one leaves the resource with no human owner. Resolving it - here rather than at each create site keeps that from being a rule every - new resource has to remember -- the reason a site was missed before. - """ - # Imported lazily: this module is imported while models load, and - # platform_api.services pulls in models at import time. + """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] diff --git a/backend/platform_api/services.py b/backend/platform_api/services.py index 0e7eef4d51..7fe5fa7bce 100644 --- a/backend/platform_api/services.py +++ b/backend/platform_api/services.py @@ -95,19 +95,10 @@ def live_key_creator(platform_api_key: PlatformApiKey) -> User | None: def owner_user_for(user: User) -> User: """Resolve the human who should own a resource created by ``user``. - A platform key authenticates as a service account, which ``owners()``, - ``owner_email()`` and ``owner_emails()`` all filter out, so a resource - granted to one names no human owner and only an org admin can manage it. - Attribute it to the key's creator instead — the successor - :func:`delete_api_user_for_key` already hands ownership to. - - Returns ``user`` unchanged for a normal session, and whenever no live - creator can be named: no key row, a creator deleted since (``created_by`` - is ``SET_NULL``), or a creator who has left the organization. The OWNER row - then goes to the service account, which the owner surfaces filter out, and - the UI labels the resource "Platform key". - - Naming a successor is :func:`live_key_creator`'s question, not this one's. + 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 diff --git a/backend/platform_api/tests/test_owner_user_for.py b/backend/platform_api/tests/test_owner_user_for.py index caae2ac6ea..84914fb558 100644 --- a/backend/platform_api/tests/test_owner_user_for.py +++ b/backend/platform_api/tests/test_owner_user_for.py @@ -94,17 +94,10 @@ def test_a_service_account_with_no_key_stays_itself(self) -> None: self.assertEqual(owner_user_for(service_account), service_account) def test_a_creator_who_left_the_org_leaves_the_resource_ownerless(self) -> None: - """Granting OWNER to an ex-member would reopen the rejoin backdoor. - - ``cleanup_user_org_access`` purges a departing user's OWNER rows - because ``_is_resource_owner`` grants on any surviving row without - checking live membership. Minting a new one after that purge would - hand co-ownership back on re-invite. - """ + """An ex-member must not be granted OWNER; it would survive re-invite.""" creator = self._make_member() key = self._make_key(created_by=creator) - # ``_base_manager``: the default manager is org-scoped by UserContext, - # which is unset here, so a plain delete would match nothing. + # _base_manager: the default one is org-scoped and UserContext is unset. OrganizationMember._base_manager.filter( user=creator, organization=self.org ).delete() @@ -124,13 +117,7 @@ def test_a_creator_in_a_different_org_is_not_granted(self) -> None: class KeyDeletionSuccessorTest(_KeyFixture, APITestCase): - """Deleting a key must not hand its rows to a departed creator. - - ``delete_api_user_for_key`` re-points the service account's rows to the - key's creator. That is the same grant ``owner_user_for`` refuses at create - time, so it asks the same question -- otherwise deleting a key reopens the - rejoin backdoor the resolver closes. - """ + """Deleting a key must not hand its rows to a departed creator.""" def _owner_row_users(self, resource): return {m.user_id for m in resource.memberships.filter(role=ResourceRole.OWNER)} diff --git a/backend/platform_api/tests/test_platform_key_resource_ownership.py b/backend/platform_api/tests/test_platform_key_resource_ownership.py index fc181752a9..1b03dac208 100644 --- a/backend/platform_api/tests/test_platform_key_resource_ownership.py +++ b/backend/platform_api/tests/test_platform_key_resource_ownership.py @@ -1,16 +1,8 @@ """A resource created through a platform key is owned by the key's creator. -One case per resource that grants an OWNER row on create. They live together -rather than in each owning app because the behaviour under test is one -resolver's (`owner_user_for`), and the interesting part is the same everywhere: -the middleware swaps `request.user` for a service account, and service accounts -are filtered out of every owner surface, so granting to one leaves the resource -with no human owner at all. - -Each case drives the real URLconf and middleware chain with a key minted here, -so the swap is exercised rather than simulated. Side effects that are not part -of the grant -- cron scheduling, API-key minting, adapter encryption -- are -patched out; what is asserted is only who ends up on the OWNER row. +One case per resource that grants an OWNER row on create, kept together since +they all exercise one resolver. Each drives the real URLconf and middleware +with a key minted here; side effects unrelated to the grant are patched out. """ import secrets diff --git a/backend/tenant_account_v2/migrations/0006_backfill_platform_key_ownership.py b/backend/tenant_account_v2/migrations/0006_backfill_platform_key_ownership.py index 7a13365d81..4fae3178e3 100644 --- a/backend/tenant_account_v2/migrations/0006_backfill_platform_key_ownership.py +++ b/backend/tenant_account_v2/migrations/0006_backfill_platform_key_ownership.py @@ -1,13 +1,8 @@ -"""UN-3853: re-point service-account OWNER rows to the key's creator. +"""Re-point service-account OWNER rows to the key's live creator. -Rows written before ``owner_user_for`` existed name the key's service account. -Every owner surface filters those out, so the resource shows no owner and only -an org admin can manage it. This hands each one to the same successor the -resolver picks, under the same membership rule -- a creator who has left the -org is skipped, since granting them a fresh OWNER row would reopen the rejoin -backdoor ``cleanup_user_org_access`` exists to close. - -Idempotent: a second run finds no service-account OWNER rows left to move. +Rows written before ``owner_user_for`` existed name the service account, which +every owner surface filters out. Skips a creator who has left the org, matching +the resolver. Idempotent. """ from django.db import migrations @@ -20,8 +15,7 @@ def _forward(apps, schema_editor): organization_member_model = apps.get_model("tenant_account_v2", "OrganizationMember") platform_api_key_model = apps.get_model("platform_api", "PlatformApiKey") - # Successor per service account. Keyed off the key rows rather than - # ``is_service_account`` so only accounts that actually back a key move. + # Keyed off key rows so only accounts actually backing a key move. successor: dict[int, int] = {} for key in platform_api_key_model.objects.exclude(api_user_id=None).exclude( created_by_id=None @@ -48,9 +42,8 @@ def _forward(apps, schema_editor): row.user_id = new_user_id row.save(update_fields=["user"]) continue - # The creator already holds a row on this resource. Keep the stronger - # role and drop the service account's, so (user, content_type, - # object_id) is never violated -- mirrors _transfer_membership_rows. + # Creator already holds a row here: keep the stronger role, drop the + # service account's, so the uniqueness constraint holds. if clash.role != OWNER: clash.role = OWNER clash.save(update_fields=["role"]) diff --git a/frontend/src/components/deployments/create-api-deployment-modal/CreateApiDeploymentModal.jsx b/frontend/src/components/deployments/create-api-deployment-modal/CreateApiDeploymentModal.jsx index 56fd1a51a2..cd781569ca 100644 --- a/frontend/src/components/deployments/create-api-deployment-modal/CreateApiDeploymentModal.jsx +++ b/frontend/src/components/deployments/create-api-deployment-modal/CreateApiDeploymentModal.jsx @@ -110,10 +110,8 @@ const CreateApiDeploymentModal = ({ onDeploymentCreated(); } } else { - // Refetch rather than splice the create response: it is a summary - // carrying none of the owner fields the list renders, so a spliced - // row shows no owner until the next fetch. The response is still - // what the code modal needs -- only it carries the API key. + // Refetch: the create response is a summary without the owner + // fields the list renders. refreshList?.(); setSelectedRow(res?.data); openCodeModal(true); diff --git a/frontend/src/components/widgets/owner-display.js b/frontend/src/components/widgets/owner-display.js index fb5a98d08d..a8cdb93f76 100644 --- a/frontend/src/components/widgets/owner-display.js +++ b/frontend/src/components/widgets/owner-display.js @@ -1,12 +1,9 @@ -// Service-account address minted by `create_api_user_for_key`. Its owner is a -// platform API key, not a person, so the field is labelled rather than named. +// Service accounts live in this domain; label them rather than name them. const PLATFORM_KEY_EMAIL_DOMAIN = "@platform.internal"; /** - * Resolve the "Owned By" label for a resource row. - * - * Shared by the list table and the card views so the two cannot drift — they - * previously disagreed on both the source field and the "Me" rule. + * Resolve the "Owned By" label for a resource row. Shared by the table and + * card views so the two cannot drift. * * @param {object} item Resource row from a list endpoint. * @param {object} sessionDetails Current session, for the "Me" comparison. @@ -14,20 +11,14 @@ const PLATFORM_KEY_EMAIL_DOMAIN = "@platform.internal"; * @return {{email: string|undefined, name: string, extra: string}} */ function resolveOwnerDisplay(item, sessionDetails, ownerEmailsProp) { - // owner_emails is earliest-first; [0] is the primary shown owner. Fall back - // to created_by_email so rows with no live OWNER membership (pre-backfill - // rows) don't render "Unknown". + // Earliest owner first; created_by_email covers rows with no OWNER row. const ownerEmails = item?.[ownerEmailsProp ?? "owner_emails"]; const rawEmail = (Array.isArray(ownerEmails) ? ownerEmails[0] : undefined) ?? item?.created_by_email; - // Suppress the synthetic address rather than dress a machine identity up as - // a colleague. const isPlatformKey = Boolean(rawEmail?.endsWith(PLATFORM_KEY_EMAIL_DOMAIN)); const email = isPlatformKey ? undefined : rawEmail; - // "Me" must track the DISPLAYED owner, not the viewer's own membership — - // else a co-owner sees "Me" over the primary owner's avatar/email. Match on - // the shown email so the creator viewing their own resource still reads "Me". + // Tracks the displayed owner, not the viewer's own membership. const isMe = Boolean(email) && email === sessionDetails?.email; let name = email?.split("@")[0] || "Unknown"; if (isPlatformKey) { From a57f99b3bc329726663c70b3e0a07048b4b61611 Mon Sep 17 00:00:00 2001 From: kirtimanmishrazipstack Date: Wed, 16 Sep 2026 12:18:36 +0530 Subject: [PATCH 11/14] UN-3853 [FIX] Keep the service account authorized and the audit trail intact Review findings from #2274. Moving the OWNER row off the service account broke two gates that authorize by OWNER membership and have no service-account bypass, so a workflow provisioned through a platform key failed its own execution: `validate_adapter_access` and `IsWorkflowOwnerOrShared.has_permission`. Both now short-circuit on a service account, matching every other access surface. On key deletion the audit half and the ownership half are now separate. `created_by`/`modified_by` follow the key's creator even if they have left the org -- deleting the account is SET_NULL on those FKs, and a null `created_by` breaks `CustomTool.delete()`. The OWNER row still goes only to a live member. Also: eager-load `by_prompt_studio_tool` so `owner_emails` does not fan out, write `modified_at` in the backfill (historical models skip BaseModel.save), and return to page 1 after creating a deployment, since never-run rows sort last and the new one could land off-page. Three regression tests, each verified by reverting its fix. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01BAfubrF8kR2rKewMSjXagg --- backend/api_v2/api_deployment_views.py | 7 +- backend/platform_api/services.py | 40 +++++++---- .../platform_api/tests/test_owner_user_for.py | 70 ++++++++++++++++++- .../0006_backfill_platform_key_ownership.py | 8 ++- .../tool_instance_v2/tool_instance_helper.py | 6 ++ .../workflow_v2/permissions.py | 4 +- .../api-deployment/ApiDeployment.jsx | 14 +++- 7 files changed, 129 insertions(+), 20 deletions(-) diff --git a/backend/api_v2/api_deployment_views.py b/backend/api_v2/api_deployment_views.py index 6511e49376..33d7844b9d 100644 --- a/backend/api_v2/api_deployment_views.py +++ b/backend/api_v2/api_deployment_views.py @@ -403,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) diff --git a/backend/platform_api/services.py b/backend/platform_api/services.py index 7fe5fa7bce..0fce45cb32 100644 --- a/backend/platform_api/services.py +++ b/backend/platform_api/services.py @@ -18,6 +18,9 @@ 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" @@ -214,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: @@ -223,30 +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 the key's creator, then delete the service account. - A creator who has left the org is not a successor -- ``transfer_ownership`` - short-circuits on ``None`` and the rows are dropped with the account, which - is what ``cleanup_user_org_access`` would have done to them anyway. + 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=live_key_creator(platform_api_key)) + transfer_ownership( + from_user=api_user, + to_user=platform_api_key.created_by, + membership_to=live_key_creator(platform_api_key), + ) api_user.delete() diff --git a/backend/platform_api/tests/test_owner_user_for.py b/backend/platform_api/tests/test_owner_user_for.py index 84914fb558..b40956a517 100644 --- a/backend/platform_api/tests/test_owner_user_for.py +++ b/backend/platform_api/tests/test_owner_user_for.py @@ -6,6 +6,7 @@ import secrets import uuid +from types import SimpleNamespace from account_v2.enums import UserRole from account_v2.models import Organization, User @@ -14,8 +15,9 @@ from permissions.roles import ResourceRole from platform_api.models import ApiKeyPermission, PlatformApiKey from platform_api.services import create_api_user_for_key, owner_user_for -from rest_framework.test import APITestCase +from rest_framework.test import APIRequestFactory, APITestCase from tenant_account_v2.models import OrganizationMember +from utils.user_context import UserContext ORG = "org-owner-test" @@ -148,3 +150,69 @@ def test_a_departed_creator_inherits_nothing(self) -> None: ).delete() key.delete() self.assertNotIn(creator.id, self._owner_row_users(workflow)) + + def test_a_departed_creator_still_keeps_the_audit_trail(self) -> None: + """Deleting the account is SET_NULL on created_by; a null breaks deletes.""" + creator = self._make_member() + key, workflow = self._key_owned_workflow(creator) + workflow.created_by = key.api_user + workflow.save(update_fields=["created_by"]) + OrganizationMember._base_manager.filter( + user=creator, organization=self.org + ).delete() + + key.delete() + + workflow.refresh_from_db() + self.assertEqual(workflow.created_by_id, creator.id) + + +class ServiceAccountStaysAuthorizedTest(_KeyFixture, APITestCase): + """Ownership no longer names the service account, so the gates that read + ownership need their own bypass -- otherwise a key cannot use what it made. + """ + + def test_adapter_access_admits_a_service_account(self) -> None: + from adapter_processor_v2.models import AdapterInstance + from tool_instance_v2.tool_instance_helper import ToolInstanceHelper + + creator = self._make_member() + key = self._make_key(created_by=creator) + adapter = AdapterInstance.objects.create( + adapter_name=f"a-{uuid.uuid4().hex[:8]}", + adapter_id=f"llm|{uuid.uuid4()}", + adapter_type="LLM", + adapter_metadata={}, + organization=self.org, + created_by=creator, + ) + adapter.grant_owner(creator) + # AdapterInstance.objects is org-scoped; without this the gate sees an + # empty queryset and the assertion proves nothing. + UserContext.set_organization_identifier(ORG) + self.addCleanup(UserContext.set_organization_identifier, None) + + # Raises PermissionDenied without the bypass. + ToolInstanceHelper.validate_adapter_access( + user=key.api_user, adapter_ids={str(adapter.id)} + ) + + def test_workflow_permission_admits_a_service_account(self) -> None: + from workflow_manager.workflow_v2.models.workflow import Workflow + from workflow_manager.workflow_v2.permissions import IsWorkflowOwnerOrShared + + creator = self._make_member() + key = self._make_key(created_by=creator) + workflow = Workflow.objects.create( + workflow_name=f"wf-{uuid.uuid4().hex[:8]}", organization=self.org + ) + workflow.grant_owner(creator) + + request = APIRequestFactory().get("/") + request.user = key.api_user + view = SimpleNamespace(kwargs={"workflow_id": str(workflow.id)}) + # The permission resolves the workflow through the org-scoped manager. + UserContext.set_organization_identifier(ORG) + self.addCleanup(UserContext.set_organization_identifier, None) + + self.assertTrue(IsWorkflowOwnerOrShared().has_permission(request, view)) diff --git a/backend/tenant_account_v2/migrations/0006_backfill_platform_key_ownership.py b/backend/tenant_account_v2/migrations/0006_backfill_platform_key_ownership.py index 4fae3178e3..9e88b2a05c 100644 --- a/backend/tenant_account_v2/migrations/0006_backfill_platform_key_ownership.py +++ b/backend/tenant_account_v2/migrations/0006_backfill_platform_key_ownership.py @@ -6,6 +6,7 @@ """ from django.db import migrations +from django.utils import timezone OWNER = "owner" @@ -40,13 +41,16 @@ def _forward(apps, schema_editor): ).first() if clash is None: row.user_id = new_user_id - row.save(update_fields=["user"]) + # Historical models skip BaseModel.save's modified_at injection. + row.modified_at = timezone.now() + row.save(update_fields=["user", "modified_at"]) continue # Creator already holds a row here: keep the stronger role, drop the # service account's, so the uniqueness constraint holds. if clash.role != OWNER: clash.role = OWNER - clash.save(update_fields=["role"]) + clash.modified_at = timezone.now() + clash.save(update_fields=["role", "modified_at"]) row.delete() diff --git a/backend/tool_instance_v2/tool_instance_helper.py b/backend/tool_instance_v2/tool_instance_helper.py index 138e48b5b2..0669048a3b 100644 --- a/backend/tool_instance_v2/tool_instance_helper.py +++ b/backend/tool_instance_v2/tool_instance_helper.py @@ -523,6 +523,11 @@ def validate_adapter_access( ) -> None: adapter_instances = AdapterInstance.objects.filter(id__in=adapter_ids).all() is_admin = OrganizationMemberService.is_user_organization_admin(user) + # A platform key's service account is trusted across its own org, the + # same bypass every other access surface applies. Ownership no longer + # names it (UN-3853), so without this a key-provisioned workflow would + # fail its own execution. + is_service_account = getattr(user, "is_service_account", False) for adapter_instance in adapter_instances: if not adapter_instance.is_usable: @@ -536,6 +541,7 @@ def validate_adapter_access( if not ( is_admin + or is_service_account or adapter_instance.shared_to_org or _is_resource_owner(user, adapter_instance) or _is_resource_viewer(user, adapter_instance) diff --git a/backend/workflow_manager/workflow_v2/permissions.py b/backend/workflow_manager/workflow_v2/permissions.py index 01e0cb0ca0..d133ed4411 100644 --- a/backend/workflow_manager/workflow_v2/permissions.py +++ b/backend/workflow_manager/workflow_v2/permissions.py @@ -38,7 +38,9 @@ def has_permission(self, request, view): user = request.user has_access = ( - _is_resource_owner(user, workflow) + # Trusted across its own org; ownership no longer names it (UN-3853). + getattr(user, "is_service_account", False) + or _is_resource_owner(user, workflow) or _is_resource_viewer(user, workflow) or (workflow.shared_to_org and workflow.organization == user.organization) or OrganizationMemberService.is_user_organization_admin(user) diff --git a/frontend/src/components/deployments/api-deployment/ApiDeployment.jsx b/frontend/src/components/deployments/api-deployment/ApiDeployment.jsx index 62867bacbf..4b50bbb813 100644 --- a/frontend/src/components/deployments/api-deployment/ApiDeployment.jsx +++ b/frontend/src/components/deployments/api-deployment/ApiDeployment.jsx @@ -1,4 +1,4 @@ -import { useEffect, useMemo, useRef, useState } from "react"; +import { useCallback, useEffect, useMemo, useRef, useState } from "react"; import { useLocation } from "react-router-dom"; import { deploymentApiTypes, displayURL } from "../../../helpers/GetStaticData"; @@ -83,11 +83,21 @@ function ApiDeployment() { setSearchTerm, // The hook owns the fetch ref; assigned below (avoids declaration ordering). fetchRef, + sort, + requestList, handlePaginationChange, handleSearch, handleListRefresh, } = usePaginatedList(); + // A new deployment has never run, and the list sorts never-run rows last, so + // refreshing the current page can leave it out of view. Go back to page 1. + const handleCreatedRefresh = useCallback( + () => + requestList(1, pagination.pageSize, searchTerm, sort.sortBy, sort.order), + [pagination.pageSize, searchTerm, sort.sortBy, sort.order], + ); + const { scrollRestoreId, activateScrollRestore, clearPendingScroll } = useScrollRestoration({ location, @@ -361,7 +371,7 @@ function ApiDeployment() { openCodeModal={setOpenCodeModal} setSelectedRow={setSelectedRow} workflowEndpointList={workflowEndpointList} - refreshList={handleListRefresh} + refreshList={handleCreatedRefresh} /> )} Date: Wed, 16 Sep 2026 12:23:19 +0530 Subject: [PATCH 12/14] UN-3853 [FIX] Drop the now-unused list refresh handler Rewiring the create modal to the page-1 refresh left `handleListRefresh` destructured but unread here. The hook still exports it and Workflows.jsx still uses it. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01BAfubrF8kR2rKewMSjXagg --- .../src/components/deployments/api-deployment/ApiDeployment.jsx | 1 - 1 file changed, 1 deletion(-) diff --git a/frontend/src/components/deployments/api-deployment/ApiDeployment.jsx b/frontend/src/components/deployments/api-deployment/ApiDeployment.jsx index 4b50bbb813..dfd788efc5 100644 --- a/frontend/src/components/deployments/api-deployment/ApiDeployment.jsx +++ b/frontend/src/components/deployments/api-deployment/ApiDeployment.jsx @@ -87,7 +87,6 @@ function ApiDeployment() { requestList, handlePaginationChange, handleSearch, - handleListRefresh, } = usePaginatedList(); // A new deployment has never run, and the list sorts never-run rows last, so From 1615c57b365440c6c117e42afdde2a0414466155 Mon Sep 17 00:00:00 2001 From: kirtimanmishrazipstack Date: Wed, 16 Sep 2026 13:04:11 +0530 Subject: [PATCH 13/14] UN-3853 [FIX] Share the platform-key ownership repair for cloud-only apps tenant_account_v2's backfill migration can't depend on cloud-only apps' absorb_shared_users migrations, so their service-account OWNER rows could land after the repair and never get revisited. Extract the repair into _membership_backfill so a cloud migration can import and re-run it. Co-Authored-By: Claude Sonnet 5 --- .../0006_backfill_platform_key_ownership.py | 50 +++------------ .../migrations/_membership_backfill.py | 64 ++++++++++++++++++- 2 files changed, 69 insertions(+), 45 deletions(-) diff --git a/backend/tenant_account_v2/migrations/0006_backfill_platform_key_ownership.py b/backend/tenant_account_v2/migrations/0006_backfill_platform_key_ownership.py index 9e88b2a05c..11cfa3b74d 100644 --- a/backend/tenant_account_v2/migrations/0006_backfill_platform_key_ownership.py +++ b/backend/tenant_account_v2/migrations/0006_backfill_platform_key_ownership.py @@ -3,55 +3,21 @@ Rows written before ``owner_user_for`` existed name the service account, which every owner surface filters out. Skips a creator who has left the org, matching the resolver. Idempotent. + +The repair itself lives in ``_membership_backfill.repair_platform_key_ownership`` +so a cloud-only app -- which this migration can't declare a dependency on -- +can import and re-run it after its own absorb-shared-users migration. """ from django.db import migrations -from django.utils import timezone -OWNER = "owner" +from tenant_account_v2.migrations._membership_backfill import ( + repair_platform_key_ownership, +) def _forward(apps, schema_editor): - resource_membership_model = apps.get_model("tenant_account_v2", "ResourceMembership") - organization_member_model = apps.get_model("tenant_account_v2", "OrganizationMember") - platform_api_key_model = apps.get_model("platform_api", "PlatformApiKey") - - # Keyed off key rows so only accounts actually backing a key move. - successor: dict[int, int] = {} - for key in platform_api_key_model.objects.exclude(api_user_id=None).exclude( - created_by_id=None - ): - if organization_member_model.objects.filter( - user_id=key.created_by_id, organization_id=key.organization_id - ).exists(): - successor[key.api_user_id] = key.created_by_id - - if not successor: - return - - rows = resource_membership_model.objects.filter( - role=OWNER, user_id__in=successor.keys() - ) - for row in rows.iterator(): - new_user_id = successor[row.user_id] - clash = resource_membership_model.objects.filter( - user_id=new_user_id, - content_type_id=row.content_type_id, - object_id=row.object_id, - ).first() - if clash is None: - row.user_id = new_user_id - # Historical models skip BaseModel.save's modified_at injection. - row.modified_at = timezone.now() - row.save(update_fields=["user", "modified_at"]) - continue - # Creator already holds a row here: keep the stronger role, drop the - # service account's, so the uniqueness constraint holds. - if clash.role != OWNER: - clash.role = OWNER - clash.modified_at = timezone.now() - clash.save(update_fields=["role", "modified_at"]) - row.delete() + repair_platform_key_ownership(apps) class Migration(migrations.Migration): diff --git a/backend/tenant_account_v2/migrations/_membership_backfill.py b/backend/tenant_account_v2/migrations/_membership_backfill.py index 00235b60f6..b9368db908 100644 --- a/backend/tenant_account_v2/migrations/_membership_backfill.py +++ b/backend/tenant_account_v2/migrations/_membership_backfill.py @@ -1,9 +1,9 @@ -"""Shared backfill for the UN-2202 single-table membership migration. +"""Shared backfills for the UN-2202 single-table membership migration. Lives inside the migrations package with a ``_`` prefix so Django's migration loader skips it (it only treats non-``_``/``~`` modules as migrations), while -the per-app membership migrations can still import it and stay thin instead of -each carrying its own copy. +the per-app membership migrations -- OSS and cloud alike -- can still import +it and stay thin instead of each carrying its own copy. Idempotent: ``get_or_create`` is keyed on the unique ``(user, content_type, object_id)`` triple, so re-runs and the creator-is-also-a-shared-user overlap @@ -12,6 +12,8 @@ import logging +from django.utils import timezone + logger = logging.getLogger(__name__) OWNER = "owner" @@ -68,3 +70,59 @@ def backfill_memberships(apps, app_label: str, model_name: str) -> None: skipped, skipped_org, ) + + +def repair_platform_key_ownership(apps) -> None: + """Re-point service-account OWNER rows to the key's live creator. + + Rows written before ``owner_user_for`` existed name the service account, + which every owner surface filters out. Skips a creator who has left the + org, matching the resolver. Safe to re-run: a resource type whose OWNER + rows are written by a migration that lands after this one (cloud-only + apps this module's app can't declare a dependency on) needs this called + again from a migration that depends on both. + """ + resource_membership_model = apps.get_model( + "tenant_account_v2", "ResourceMembership" + ) # NOSONAR + organization_member_model = apps.get_model( + "tenant_account_v2", "OrganizationMember" + ) # NOSONAR + platform_api_key_model = apps.get_model("platform_api", "PlatformApiKey") # NOSONAR + + # Keyed off key rows so only accounts actually backing a key move. + successor: dict[int, int] = {} + for key in platform_api_key_model.objects.exclude(api_user_id=None).exclude( + created_by_id=None + ): + if organization_member_model.objects.filter( + user_id=key.created_by_id, organization_id=key.organization_id + ).exists(): + successor[key.api_user_id] = key.created_by_id + + if not successor: + return + + rows = resource_membership_model.objects.filter( + role=OWNER, user_id__in=successor.keys() + ) + for row in rows.iterator(): + new_user_id = successor[row.user_id] + clash = resource_membership_model.objects.filter( + user_id=new_user_id, + content_type_id=row.content_type_id, + object_id=row.object_id, + ).first() + if clash is None: + row.user_id = new_user_id + # Historical models skip BaseModel.save's modified_at injection. + row.modified_at = timezone.now() + row.save(update_fields=["user", "modified_at"]) + continue + # Creator already holds a row here: keep the stronger role, drop the + # service account's, so the uniqueness constraint holds. + if clash.role != OWNER: + clash.role = OWNER + clash.modified_at = timezone.now() + clash.save(update_fields=["role", "modified_at"]) + row.delete() From 040f20546d6673744f4d77866319bee6ce183fb5 Mon Sep 17 00:00:00 2001 From: kirtimanmishrazipstack Date: Wed, 16 Sep 2026 15:47:23 +0530 Subject: [PATCH 14/14] UN-3853 [FIX] Cover the third ownership gate, and stop jumping to page 1 Re-review findings from #2274. `IsFrictionLessAdapterDelete` is a third gate that authorizes by OWNER membership with no service-account bypass, so a full_access key could no longer delete an adapter it had just created. Every sibling in the file has the bypass, including `IsFrictionLessAdapter` directly above it. Swept the rest rather than wait for a fourth: all 13 `_is_resource_owner` call sites across both repos now either carry the bypass or are unreachable by a service account -- `prompt_studio_helper._adapter_accessible_by` is guarded by an `is_service_account` early return at its only caller. The page-1 refresh is reverted. The ordering is `last_run_time DESC NULLS LAST`, so a never-run deployment sorts to the END -- page 1 is the wrong target, and jumping there also threw the user off the page the row was actually on. `handleListRefresh` at least keeps them there. Making the new row visible needs an ordering change, which is a product decision, not this PR. Regression test for the adapter delete gate, verified by reverting the bypass. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01BAfubrF8kR2rKewMSjXagg --- backend/permissions/permission.py | 2 ++ .../platform_api/tests/test_owner_user_for.py | 26 +++++++++++++++++++ .../api-deployment/ApiDeployment.jsx | 15 +++-------- 3 files changed, 31 insertions(+), 12 deletions(-) diff --git a/backend/permissions/permission.py b/backend/permissions/permission.py index 33d4dd5079..18db6827ad 100644 --- a/backend/permissions/permission.py +++ b/backend/permissions/permission.py @@ -282,6 +282,8 @@ def has_object_permission( ) -> bool: if obj.is_friction_less: return True + if _is_service_account(request): + return True if _is_resource_owner(request.user, obj): return True return _is_organization_admin(request) diff --git a/backend/platform_api/tests/test_owner_user_for.py b/backend/platform_api/tests/test_owner_user_for.py index b40956a517..3408de4f98 100644 --- a/backend/platform_api/tests/test_owner_user_for.py +++ b/backend/platform_api/tests/test_owner_user_for.py @@ -197,6 +197,32 @@ def test_adapter_access_admits_a_service_account(self) -> None: user=key.api_user, adapter_ids={str(adapter.id)} ) + def test_adapter_delete_admits_a_service_account(self) -> None: + """A key must be able to delete the adapter it created.""" + from adapter_processor_v2.models import AdapterInstance + from permissions.permission import IsFrictionLessAdapterDelete + + creator = self._make_member() + key = self._make_key(created_by=creator) + adapter = AdapterInstance.objects.create( + adapter_name=f"a-{uuid.uuid4().hex[:8]}", + adapter_id=f"llm|{uuid.uuid4()}", + adapter_type="LLM", + adapter_metadata={}, + organization=self.org, + created_by=creator, + ) + adapter.grant_owner(creator) + + request = APIRequestFactory().delete("/") + request.user = key.api_user + + self.assertTrue( + IsFrictionLessAdapterDelete().has_object_permission( + request, view=None, obj=adapter + ) + ) + def test_workflow_permission_admits_a_service_account(self) -> None: from workflow_manager.workflow_v2.models.workflow import Workflow from workflow_manager.workflow_v2.permissions import IsWorkflowOwnerOrShared diff --git a/frontend/src/components/deployments/api-deployment/ApiDeployment.jsx b/frontend/src/components/deployments/api-deployment/ApiDeployment.jsx index dfd788efc5..62867bacbf 100644 --- a/frontend/src/components/deployments/api-deployment/ApiDeployment.jsx +++ b/frontend/src/components/deployments/api-deployment/ApiDeployment.jsx @@ -1,4 +1,4 @@ -import { useCallback, useEffect, useMemo, useRef, useState } from "react"; +import { useEffect, useMemo, useRef, useState } from "react"; import { useLocation } from "react-router-dom"; import { deploymentApiTypes, displayURL } from "../../../helpers/GetStaticData"; @@ -83,20 +83,11 @@ function ApiDeployment() { setSearchTerm, // The hook owns the fetch ref; assigned below (avoids declaration ordering). fetchRef, - sort, - requestList, handlePaginationChange, handleSearch, + handleListRefresh, } = usePaginatedList(); - // A new deployment has never run, and the list sorts never-run rows last, so - // refreshing the current page can leave it out of view. Go back to page 1. - const handleCreatedRefresh = useCallback( - () => - requestList(1, pagination.pageSize, searchTerm, sort.sortBy, sort.order), - [pagination.pageSize, searchTerm, sort.sortBy, sort.order], - ); - const { scrollRestoreId, activateScrollRestore, clearPendingScroll } = useScrollRestoration({ location, @@ -370,7 +361,7 @@ function ApiDeployment() { openCodeModal={setOpenCodeModal} setSelectedRow={setSelectedRow} workflowEndpointList={workflowEndpointList} - refreshList={handleCreatedRefresh} + refreshList={handleListRefresh} /> )}