From 91a9e45b615afccf80262c619e5b2ca772e833da Mon Sep 17 00:00:00 2001 From: kirtimanmishrazipstack Date: Wed, 2 Sep 2026 16:08:49 +0530 Subject: [PATCH 1/3] 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 2/3] 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 3/3] 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"]) + )