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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
"""UN-3057: grant an OWNER row to custom tools left ownerless by the clone path.

The Prompt Studio clone helper never created the OWNER ``ResourceMembership``
that UN-2202 made authoritative, so every project cloned after
``0009_absorb_shared_users`` ran has no owner: visible (the clone copies the
parent's ``shared_to_org``) but unmanageable by anyone except an org admin.
The helper is fixed going forward; this repairs the rows already written.

Idempotent and non-destructive — only resources with zero OWNER rows are
touched, so it is safe to re-run and reverses to a no-op.
"""

from django.db import migrations
from tenant_account_v2.migrations._membership_backfill import (
repair_ownerless_owner_rows,
)

APP_LABEL = "prompt_studio_core_v2"
MODEL_NAME = "CustomTool"


def _forward(apps, schema_editor):
repair_ownerless_owner_rows(apps, APP_LABEL, MODEL_NAME)


class Migration(migrations.Migration):
dependencies = [
("prompt_studio_core_v2", "0010_customtool_custtool_org_modified_idx"),
("tenant_account_v2", "0005_resource_membership"),
]

operations = [
migrations.RunPython(_forward, migrations.RunPython.noop),
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
"""Repair of ownerless ``CustomTool`` rows (UN-3057).

The Prompt Studio clone path created projects without the OWNER
``ResourceMembership`` that UN-2202 made authoritative, so every project cloned
after the UN-2202 backfill ran is ownerless: visible, but unmanageable by anyone
except an org admin. Fixing the clone helper stops new breakage; these already
broken rows need a repair pass.

Exercises the migration helper against the real models (``django.apps.apps``
satisfies the ``apps.get_model`` interface the migration passes in), so the
behaviour is pinned without driving the migration executor.
"""

from __future__ import annotations

import secrets

from account_v2.models import Organization, User
from django.apps import apps as django_apps
from django.test import TestCase
from permissions.roles import ResourceRole
from tenant_account_v2.migrations._membership_backfill import (
repair_ownerless_owner_rows,
)

from prompt_studio.prompt_studio_core_v2.models import CustomTool

APP_LABEL = "prompt_studio_core_v2"
MODEL_NAME = "CustomTool"


def _make_user(email: str) -> User:
return User.objects.create_user(
username=email, email=email, password=secrets.token_urlsafe()
)


class RepairOwnerlessOwnerRowsTests(TestCase):
def setUp(self) -> None:
self.org = Organization.objects.create(
name="org-a", display_name="Org A", organization_id="org-a"
)
self.creator = _make_user("creator@example.com")
self.other = _make_user("other@example.com")

def _tool(self, name: str, creator: User | None) -> CustomTool:
return CustomTool.objects.create(
tool_name=name,
description="",
organization=self.org,
created_by=creator,
)

def _repair(self) -> int:
return repair_ownerless_owner_rows(django_apps, APP_LABEL, MODEL_NAME)

def _owner_ids(self, tool: CustomTool) -> set:
return set(
tool.memberships.filter(role=ResourceRole.OWNER).values_list(
"user_id", flat=True
)
)

def test_ownerless_tool_gets_an_owner_row_for_its_creator(self) -> None:
tool = self._tool("cloned-project", self.creator)
self.assertEqual(self._owner_ids(tool), set())

self._repair()

self.assertEqual(self._owner_ids(tool), {self.creator.id})

def test_tool_that_already_has_an_owner_is_left_alone(self) -> None:
"""A creator deliberately replaced by a co-owner must not be re-added."""
tool = self._tool("handed-over-project", self.creator)
tool.memberships.create(user=self.other, role=ResourceRole.OWNER)

self._repair()

self.assertEqual(self._owner_ids(tool), {self.other.id})

def test_tool_with_no_creator_is_skipped(self) -> None:
tool = self._tool("orphan-project", None)

self._repair()

self.assertEqual(self._owner_ids(tool), set())
54 changes: 54 additions & 0 deletions backend/tenant_account_v2/migrations/_membership_backfill.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,3 +68,57 @@ def backfill_memberships(apps, app_label: str, model_name: str) -> None:
skipped,
skipped_org,
)


def repair_ownerless_owner_rows(apps, app_label: str, model_name: str) -> int:
"""Give ``created_by`` an OWNER row on resources that have no owner at all.

UN-3057: the Prompt Studio clone path created ``CustomTool`` rows without
the OWNER row that UN-2202 made authoritative, so projects cloned after
:func:`backfill_memberships` ran are ownerless — visible (the clone copies
the parent's ``shared_to_org``), but unmanageable by anyone but an org
admin. This repairs what that backfill could not have seen.

Only resources with *zero* OWNER rows are touched, so a creator who was
deliberately replaced by a co-owner is not resurrected. Null creator or
null organization means there is nothing to grant, so those are skipped.
Idempotent: a second run finds no ownerless rows.
"""
Resource = apps.get_model(app_label, model_name) # NOSONAR
Membership = apps.get_model("tenant_account_v2", "ResourceMembership") # NOSONAR
ContentType = apps.get_model("contenttypes", "ContentType") # NOSONAR

content_type = ContentType.objects.get_for_model(Resource)
owned_ids = set(
Membership.objects.filter(content_type=content_type, role=OWNER).values_list(
"object_id", flat=True
)
)

# ``_base_manager``: several resources' default manager is org-scoped by
# ``UserContext`` (unset here → it would filter every row out and silently
# repair nothing). Same guard as ``tenant_account_v2.signals``.
repaired = skipped = 0
for resource in Resource._base_manager.exclude(created_by=None).iterator():
if resource.organization_id is None:
skipped += 1
continue
object_id = str(resource.pk)
if object_id in owned_ids:
continue
_, created = Membership.objects.get_or_create(
content_type=content_type,
object_id=object_id,
user_id=resource.created_by_id,
defaults={"role": OWNER, "organization_id": resource.organization_id},
)
Comment on lines +109 to +114

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Existing viewer remains ownerless

When an ownerless tool's creator already has a VIEWER membership, get_or_create returns that row without applying the OWNER default, causing ownership-gated PUT, PATCH, and DELETE operations to continue returning 403 after the repair.

Suggested change
_, created = Membership.objects.get_or_create(
content_type=content_type,
object_id=object_id,
user_id=resource.created_by_id,
defaults={"role": OWNER, "organization_id": resource.organization_id},
)
_, created = Membership.objects.update_or_create(
content_type=content_type,
object_id=object_id,
user_id=resource.created_by_id,
defaults={"role": OWNER, "organization_id": resource.organization_id},
)

Knowledge Base Used: Django Core Scaffold: Settings, Routing, Auth, Tenancy

Prompt To Fix With AI
This is a comment left during a code review.
Path: backend/tenant_account_v2/migrations/_membership_backfill.py
Line: 109-114

Comment:
**Existing viewer remains ownerless**

When an ownerless tool's creator already has a VIEWER membership, `get_or_create` returns that row without applying the OWNER default, causing ownership-gated PUT, PATCH, and DELETE operations to continue returning 403 after the repair.

```suggestion
        _, created = Membership.objects.update_or_create(
            content_type=content_type,
            object_id=object_id,
            user_id=resource.created_by_id,
            defaults={"role": OWNER, "organization_id": resource.organization_id},
        )
```

**Knowledge Base Used:** [Django Core Scaffold: Settings, Routing, Auth, Tenancy](https://app.greptile.com/zipstack/-/custom-context/knowledge-base/zipstack/unstract/-/docs/backend-django-core.md)

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Fix in Claude Code

repaired += int(created)

logger.info(
"%s.%s ownerless repair: owners granted=%s (skipped %s null-org)",
app_label,
model_name,
repaired,
skipped,
)
return repaired
Loading