feat(api): Sweep remaining single-org assumptions in OSS#4675
feat(api): Sweep remaining single-org assumptions in OSS#4675jp-agenta wants to merge 4 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
dosubot
Bot
added
the
size:L
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
api/oss/src/middlewares/auth.py (1)
626-691:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winEnforce explicit-scope membership checks in OSS too.
The
project_member_exists/workspace_member_existsgate still only runs underis_ee(). After removing OSS singleton assumptions, any authenticated OSS user can pass another organization'sproject_idorworkspace_idin the query string and this middleware will mint a scoped secret token without proving membership in that resource.api/oss/src/routers/organization_router.py (1)
76-110:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPopulate
workspaces_by_orgbefore serializing the EE response.This branch initializes
workspaces_by_org = {}and never fills it, so every EE organization now returnsworkspaces: []. Any client code that relies onOrganization.workspacesto keep org/workspace selection in sync loses that relationship.api/oss/src/routers/workspace_router.py (1)
164-168:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse the route's
workspace_idfor OSS removals.The OSS branch ignores the path parameter and forwards
request.state.project_idtodb_manager.remove_user_from_workspace(). After this PR, callers can target a different workspace than their ambient auth scope, so/workspaces/{workspace_id}/users/can remove memberships/invitations from the wrong workspace context.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: fda5621b-f8ce-4759-949b-2ed3c92035e4
📒 Files selected for processing (7)
api/ee/src/services/db_manager_ee.pyapi/oss/src/core/accounts/service.pyapi/oss/src/middlewares/auth.pyapi/oss/src/routers/organization_router.pyapi/oss/src/routers/workspace_router.pyapi/oss/src/services/db_manager.pyweb/oss/src/components/Sidebar/components/ListOfOrgs.tsx
💤 Files with no reviewable changes (1)
- api/oss/src/core/accounts/service.py
| organization_workspaces = [ | ||
| workspace_db | ||
| for workspace_db in await db_manager.get_workspaces() | ||
| if str(workspace_db.organization_id) == str(organization_id) | ||
| ] | ||
| active_workspace = next(iter(organization_workspaces), None) | ||
| if not active_workspace: | ||
| return {} | ||
|
|
||
| organization_owner = await db_manager.get_organization_owner( | ||
| organization_id=organization_id | ||
| ) | ||
| project_invitations = await db_manager.get_project_invitations( | ||
| project_id=request.state.project_id | ||
|
|
||
| default_project = await db_manager.get_default_project_by_organization_id( | ||
| organization_id=organization_id | ||
| ) | ||
| project_invitations = ( | ||
| await db_manager.get_project_invitations(project_id=str(default_project.id)) | ||
| if default_project | ||
| else [] | ||
| ) | ||
|
|
||
| organization_db = await db_manager.get_organization_by_id( | ||
| organization_id=organization_id | ||
| ) |
There was a problem hiding this comment.
Scope org-details reads to the caller's memberships.
fetch_organization_details() loads workspaces, owner metadata, and invitations for whatever organization_id is in the path, but it never verifies that request.state.user_id belongs to that organization. In OSS multi-org mode this lets any authenticated user read another organization's owner and pending-invite data if they know the UUID.
| owner_membership = next( | ||
| (membership for membership in memberships if membership.role == "owner"), | ||
| None, | ||
| ) | ||
| if owner_membership is not None: | ||
| return str(owner_membership.workspace_id) | ||
|
|
||
| return str(workspaces[0].id) | ||
| memberships.sort( | ||
| key=lambda membership: ( | ||
| membership.created_at or datetime.min.replace(tzinfo=timezone.utc), | ||
| str(membership.workspace_id), | ||
| ) | ||
| ) | ||
| return str(memberships[0].workspace_id) |
There was a problem hiding this comment.
Make owner selection deterministic.
memberships is unordered here, so next(... role == "owner") returns whichever owner row the database happens to emit first. Once a user owns multiple workspaces, their default workspace can flip between requests even though the fallback path is explicitly sorted. Sort owner memberships with the same key before picking one, or push the owner-first ordering into SQL and select a single row.
| items.push({ | ||
| key: "create-organization", | ||
| label: ( | ||
| <div className="flex items-center gap-2"> | ||
| <span className="text-gray-900">+ New organization</span> | ||
| </div> | ||
| ), | ||
| }) | ||
| items.push({type: "divider", key: "organizations-actions-divider"}) |
There was a problem hiding this comment.
Honor organizationSelectionEnabled for the create action too.
The prop contract says org items should stay visible but non-actionable when selection is disabled, with logout as the only actionable escape hatch. create-organization remains enabled here, so pages that intentionally freeze org interactions can still mutate org state.
Railway Preview Environment
|
junaway
force-pushed
the
feat/oss-singleton-sweep
branch
from
ab96500 to
96c348b
Compare
junaway
force-pushed
the
feat/oss-org-creation
branch
from
042bffd to
c4da60a
Compare
junaway
force-pushed
the
feat/oss-singleton-sweep
branch
from
b6d4a7d to
dcb998f
Compare
junaway
force-pushed
the
feat/oss-org-creation
branch
from
c4da60a to
8492cc4
Compare
junaway
force-pushed
the
feat/oss-singleton-sweep
branch
from
dcb998f to
91d979d
Compare
junaway
force-pushed
the
feat/oss-org-creation
branch
from
58a009f to
8492cc4
Compare
junaway
force-pushed
the
feat/oss-singleton-sweep
branch
from
4356253 to
91d979d
Compare
Default workspace resolution becomes membership-based in both editions (get_default_workspace_id moves OSS-ward; the OSS oldest-workspace fallback and get_oss_organization are gone). Organization and workspace listings are scoped to the requesting user's memberships. The admin singleton org/workspace delete guards and the admin_create_organization ON CONFLICT branch are removed. Web: org switcher, New Organization, and owner submenu are enabled in OSS.
…d-trip The workspace-resolution unit tests move to the OSS suite (the function moved OSS-ward; patching db_manager_ee's engine no longer reaches it). The admin org create/delete round-trip returns to the OSS acceptance suite now that the singleton slug-collapse and delete guard are gone.
OSS no longer needs the owner-signup + invite + re-login dance: any user can sign up directly and gets their own organization, same as EE. global-setup drops the license fork and inviteOssUser; AGENTA_TEST_OSS_OWNER_EMAIL stays as an optional fixed-account login for persistent CI deployments.
junaway
force-pushed
the
feat/oss-org-creation
branch
from
8492cc4 to
83c2f59
Compare
junaway
force-pushed
the
feat/oss-singleton-sweep
branch
from
91d979d to
13535b9
Compare
The api-key auth path sets request.state.user_id to the user's DB id (api_key.created_by_id), but get_user only matched UserDB.id when is_ee(). In OSS the new user-facing POST /organizations/ resolved the api-key user via get_user and 404'd. Apply the id fallback in both editions, guarded so a non-UUID SuperTokens uid still matches on uid only. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 participant
Context
After the org-creation PR (#4673), OSS can have multiple organizations, but several code paths still assume exactly one: bare-API-key auth resolves "the" workspace by picking the oldest row, organization/workspace listings return everything in the database instead of the user's memberships, the admin API refuses to delete the legacy
oss-defaultorg, and the web sidebar keeps the org switcher disabled outside EE. Sequencing step 4 of the convergence plan.Changes
API:
get_default_workspace_id(user_id)(workspace owned by the user, falling back to their oldest membership) moves fromdb_manager_eeto OSSdb_manager; theis_ee()fork in the auth middleware's bare-key path collapses to one call. The OSS-onlyget_default_workspace_id_oss()(oldest workspace in the whole DB) andget_oss_organization()are deleted.GET /organizations/(OSS branch) now lists the requesting user's organizations via the membership join, with each org's workspaces attached, instead of all orgs plus the first workspace in the database.GET /organizations/{id}scopes its workspace and invitation lookups to the requested org rather thanrequest.state.project_id.GET /workspaces/returns the user's workspaces instead of all of them.oss-defaultsingleton guards are gone (the legacy org is a normal org now; deleting it follows EE semantics), andadmin_create_organizationalways inserts a new row instead of collapsing onto the singleton slug in OSS.Web (
ListOfOrgs.tsx): organization selection is enabled in OSS, the "New organization" item and the owner submenu (transfer/rename/delete) are no longer EE-gated, and the label shows the selected organization's name instead of the hard-coded "Agenta". The Organization settings tab stays EE-gated (its content is domain verification and SSO, which remain EE features).Tests / notes
ruff formatandruff checkpass; the edited component passes prettier and introduces no tsc errors.get_default_workspace_idunit tests move tooss/tests/pytest/unit/services/test_db_manager.py(the function moved OSS-ward, so patchingdb_manager_ee's engine no longer reached it). The admin org create/delete round-trip returns to the OSS acceptance suite now that the singleton delete guard is gone.AGENTA_TEST_OSS_OWNER_EMAILremains as an optional fixed-account login for persistent CI deployments.What to QA
🤖 Generated with Claude Code