diff --git a/workspace-server/internal/handlers/org_plugin_allowlist.go b/workspace-server/internal/handlers/org_plugin_allowlist.go index 5309c837..a7be39d4 100644 --- a/workspace-server/internal/handlers/org_plugin_allowlist.go +++ b/workspace-server/internal/handlers/org_plugin_allowlist.go @@ -9,6 +9,7 @@ import ( "time" "github.com/Molecule-AI/molecule-monorepo/platform/internal/db" + "github.com/Molecule-AI/molecule-monorepo/platform/internal/orgtoken" "github.com/gin-gonic/gin" ) @@ -107,9 +108,18 @@ type putAllowlistRequest struct { // requireCallerOwnsOrg returns the caller's org workspace ID from the // request context, or "" if the caller is not an org-token holder. -// Used to enforce org isolation on org-scoped routes — the org_token_id -// is the org-scoped token's ID (not the org workspace ID), so we look -// it up via the created_by workspace or a direct relationship. +// Used to enforce org isolation on org-scoped routes. +// +// F1094 regression fix (#1200): previously this read created_by from +// org_api_tokens, but created_by is a provenance label ("session", +// "admin-token", "org-token:") — never a UUID. The equality +// check callerOrg != targetOrgID always failed, giving every org-token +// caller a non-UUID string and causing 403 on every org-token request. +// +// Fix: read org_id column instead (populated at mint time by +// POST /org/tokens via orgTokenActor). Pre-migration tokens and +// ADMIN_TOKEN bootstrap tokens have org_id=NULL → callerOrg="" → +// deny by default (safer than permitting cross-org access). // // Returns ("", nil) when the caller is a session/ADMIN_TOKEN user (they // bypass via the session cookie path or ADMIN_TOKEN, not org tokens). @@ -122,19 +132,15 @@ func requireCallerOwnsOrg(c *gin.Context) (string, error) { if !ok || tokID == "" { return "", nil } - // Look up the org workspace that owns this token. - // org_api_tokens has no org_id column, but we can look up the token's - // created_by workspace and treat that as the caller's org anchor. - var createdBy string - err := db.DB.QueryRowContext(c.Request.Context(), - `SELECT created_by FROM org_api_tokens WHERE id = $1`, tokID, - ).Scan(&createdBy) - if err != nil || createdBy == "" { - // Token has no created_by (CLI bootstrap path) — treat as unowned; - // deny by default to prevent cross-org access. - return "", fmt.Errorf("token has no org anchor") + // Look up the token's org_id (populated at mint time by orgTokenActor). + // org_id is NULL for tokens minted before this migration or via + // ADMIN_TOKEN bootstrap — those callers get callerOrg="" and are denied. + orgID, err := orgtoken.OrgIDByTokenID(c.Request.Context(), db.DB, tokID) + if err != nil { + // DB error — deny by default rather than risk cross-org access. + return "", fmt.Errorf("allowlist: requireCallerOwnsOrg: %v", err) } - return createdBy, nil + return orgID, nil } // requireOrgOwnership verifies the caller has authority over the target org. diff --git a/workspace-server/internal/handlers/org_plugin_allowlist_test.go b/workspace-server/internal/handlers/org_plugin_allowlist_test.go index bcc42d05..e212a667 100644 --- a/workspace-server/internal/handlers/org_plugin_allowlist_test.go +++ b/workspace-server/internal/handlers/org_plugin_allowlist_test.go @@ -553,3 +553,177 @@ func TestCheckOrgPluginAllowlist_FailOpen_OnCountError(t *testing.T) { t.Error("expected fail-open (not blocked) on DB error during COUNT check") } } + +// ─── requireCallerOwnsOrg regression tests (F1094 / #1200) ───────────────── + +func TestRequireCallerOwnsOrg_NotOrgTokenCaller(t *testing.T) { + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + // No org_token_id in context → caller is session/admin → returns ("", nil) + c.Set("org_token_id", "something") // weird but set to a non-string type + orgID, err := requireCallerOwnsOrg(c) + if err != nil { + t.Fatalf("requireCallerOwnsOrg: got err %v", err) + } + if orgID != "" { + t.Errorf("non-string org_token_id: got orgID=%q, want \"\"", orgID) + } +} + +func TestRequireCallerOwnsOrg_NoOrgTokenIDInContext(t *testing.T) { + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + // No org_token_id key → not an org-token caller → returns ("", nil) + orgID, err := requireCallerOwnsOrg(c) + if err != nil { + t.Fatalf("requireCallerOwnsOrg: got err %v", err) + } + if orgID != "" { + t.Errorf("no org_token_id: got orgID=%q, want \"\"", orgID) + } +} + +func TestRequireCallerOwnsOrg_TokenHasMatchingOrgID(t *testing.T) { + mock := setupTestDB(t) + + orgID := "org-abc123" + mock.ExpectQuery(`SELECT org_id FROM org_api_tokens WHERE id = \$1`). + WithArgs("tok-123"). + WillReturnRows(sqlmock.NewRows([]string{"org_id"}).AddRow(orgID)) + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Set("org_token_id", "tok-123") + + got, err := requireCallerOwnsOrg(c) + if err != nil { + t.Fatalf("requireCallerOwnsOrg: %v", err) + } + if got != orgID { + t.Errorf("got orgID=%q, want %q", got, orgID) + } +} + +func TestRequireCallerOwnsOrg_TokenHasNullOrgID_UnanchoredDeny(t *testing.T) { + mock := setupTestDB(t) + + // Pre-migration token or ADMIN_TOKEN bootstrap token — org_id is NULL. + // callerOrg="" → requireOrgOwnership denies (safer than cross-org access). + mock.ExpectQuery(`SELECT org_id FROM org_api_tokens WHERE id = \$1`). + WithArgs("tok-old"). + WillReturnRows(sqlmock.NewRows([]string{"org_id"}).AddRow(nil)) + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Set("org_token_id", "tok-old") + + got, err := requireCallerOwnsOrg(c) + if err != nil { + t.Fatalf("null org_id: got err %v (want nil)", err) + } + if got != "" { + t.Errorf("unanchored token: got orgID=%q, want \"\"", got) + } +} + +func TestRequireCallerOwnsOrg_TokenDBError_Denies(t *testing.T) { + mock := setupTestDB(t) + + mock.ExpectQuery(`SELECT org_id FROM org_api_tokens WHERE id = \$1`). + WithArgs("tok-bad"). + WillReturnError(sql.ErrConnDone) + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Set("org_token_id", "tok-bad") + + _, err := requireCallerOwnsOrg(c) + if err == nil { + t.Error("expected error on DB failure, got nil") + } +} + +func TestRequireOrgOwnership_SessionCallerBypasses(t *testing.T) { + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + // No org_token_id → session/admin caller → should pass (return true) + if !requireOrgOwnership(c, "any-org") { + t.Error("session caller should be allowed (no org token in context)") + } +} + +func TestRequireOrgOwnership_OrgTokenMatchesOwnOrg_Passes(t *testing.T) { + mock := setupTestDB(t) + + const targetOrg = "org-abc123" + mock.ExpectQuery(`SELECT org_id FROM org_api_tokens WHERE id = \$1`). + WithArgs("tok-123"). + WillReturnRows(sqlmock.NewRows([]string{"org_id"}).AddRow(targetOrg)) + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Set("org_token_id", "tok-123") + + if !requireOrgOwnership(c, targetOrg) { + t.Error("org-token caller matching own org should be allowed") + } +} + +func TestRequireOrgOwnership_OrgTokenCrossOrg_Denied(t *testing.T) { + mock := setupTestDB(t) + + // Token belongs to org-abc, trying to access org-xyz → 403 + mock.ExpectQuery(`SELECT org_id FROM org_api_tokens WHERE id = \$1`). + WithArgs("tok-cross"). + WillReturnRows(sqlmock.NewRows([]string{"org_id"}).AddRow("org-abc")) + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Set("org_token_id", "tok-cross") + + if requireOrgOwnership(c, "org-xyz") { + t.Error("cross-org org-token caller should be denied") + } + if w.Code != http.StatusForbidden { + t.Errorf("expected 403, got %d", w.Code) + } +} + +func TestRequireOrgOwnership_UnanchoredToken_Denied(t *testing.T) { + mock := setupTestDB(t) + + // Unanchored token (org_id NULL) → callerOrg="" → deny + mock.ExpectQuery(`SELECT org_id FROM org_api_tokens WHERE id = \$1`). + WithArgs("tok-unanchored"). + WillReturnRows(sqlmock.NewRows([]string{"org_id"}).AddRow(nil)) + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Set("org_token_id", "tok-unanchored") + + if requireOrgOwnership(c, "org-any") { + t.Error("unanchored org-token caller should be denied (safer default)") + } + if w.Code != http.StatusForbidden { + t.Errorf("expected 403, got %d", w.Code) + } +} + +func TestRequireOrgOwnership_DBError_Denied(t *testing.T) { + mock := setupTestDB(t) + + mock.ExpectQuery(`SELECT org_id FROM org_api_tokens WHERE id = \$1`). + WithArgs("tok-err"). + WillReturnError(sql.ErrConnDone) + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Set("org_token_id", "tok-err") + + if requireOrgOwnership(c, "org-any") { + t.Error("DB error should deny by default") + } + if w.Code != http.StatusForbidden { + t.Errorf("expected 403, got %d", w.Code) + } +} diff --git a/workspace-server/internal/handlers/org_tokens.go b/workspace-server/internal/handlers/org_tokens.go index 3a9b36d6..19568ace 100644 --- a/workspace-server/internal/handlers/org_tokens.go +++ b/workspace-server/internal/handlers/org_tokens.go @@ -61,6 +61,10 @@ type createOrgTokenResponse struct { // provenance of the current request — so an audit trail points back // to who minted what. For the bootstrap ADMIN_TOKEN path, created_by // is "admin-token" (no session identity available). +// +// orgID is the caller's org workspace ID, captured at mint time. +// requireCallerOwnsOrg (org_plugin_allowlist.go:116) uses this to +// enforce org isolation (#1200 / F1094). func (h *OrgTokenHandler) Create(c *gin.Context) { var req createOrgTokenRequest // Optional body — an empty POST should still work (unnamed token). @@ -70,15 +74,15 @@ func (h *OrgTokenHandler) Create(c *gin.Context) { return } - createdBy := orgTokenActor(c) + createdBy, orgID := orgTokenActor(c) - plaintext, id, err := orgtoken.Issue(c.Request.Context(), db.DB, req.Name, createdBy) + plaintext, id, err := orgtoken.Issue(c.Request.Context(), db.DB, req.Name, createdBy, orgID) if err != nil { log.Printf("orgtoken issue: %v", err) c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to mint token"}) return } - log.Printf("orgtoken: minted id=%s by=%s name=%q", id, createdBy, req.Name) + log.Printf("orgtoken: minted id=%s by=%s org=%s name=%q", id, createdBy, orgID, req.Name) c.JSON(http.StatusOK, createOrgTokenResponse{ ID: id, @@ -121,28 +125,51 @@ const ( actorAdminToken = "admin-token" // bootstrap ADMIN_TOKEN env ) -// orgTokenActor derives a short provenance string for audit. +// callerContext returns the caller's org workspace ID for use in +// org-token creation (#1200 / F1094). It reads org_token_id from the +// gin context (set by AdminAuth when an org token authed the request) +// and looks up the token's org_id. // -// - If the request was authed via another org token, return -// "org-token:" where prefix is the 8-char plaintext -// prefix shown in the UI — correlates audit rows directly with -// the revoke button a user sees. -// - If authed via session cookie (AdminAuth's session tier), the -// middleware doesn't stash a WorkOS user_id today — return -// "session" as a generic label. Follow-up (see -// docs/architecture/org-api-keys-followups.md #6) captures the -// user_id through the session tier for full attribution. -// - Else (ADMIN_TOKEN / bootstrap), return "admin-token". -func orgTokenActor(c *gin.Context) string { +// For session/ADMIN_TOKEN callers (no org_token_id in context), returns +// ("", "") so the token is minted as "unanchored" (org_id=NULL). +// Unanchored tokens cannot access org-scoped routes — safer than +// permitting cross-org access until the operator explicitly sets org_id. +func callerOrg(c *gin.Context) string { + tokenID, ok := c.Get("org_token_id") + if !ok { + return "" + } + tokID, ok := tokenID.(string) + if !ok || tokID == "" { + return "" + } + orgID, err := orgtoken.OrgIDByTokenID(c.Request.Context(), db.DB, tokID) + if err != nil || orgID == "" { + return "" + } + return orgID +} + +// orgTokenActor returns (createdBy, orgID) for the current request. +// +// - If authed via another org token (org_token_id in context), +// createdBy = "org-token:" and orgID = token's org_id. +// - If authed via session cookie (AdminAuth's session tier), +// createdBy = "session", orgID = "" (session → org mapping not +// available in the handler; must be filled by the CP or left null). +// - If ADMIN_TOKEN / bootstrap, createdBy = "admin-token", +// orgID = "". +func orgTokenActor(c *gin.Context) (createdBy, orgID string) { if v, ok := c.Get("org_token_prefix"); ok { if s, ok := v.(string); ok && s != "" { - return actorOrgTokenPrefix + s + return actorOrgTokenPrefix + s, callerOrg(c) } } - // Session-tier auth doesn't stash an identity in the gin context - // today. Until it does, treat session requests as "session". + // Session-tier auth doesn't stash a WorkOS user_id in the gin + // context today. Until it does, treat session requests as "session" + // with no org anchor. if c.GetHeader("Cookie") != "" { - return actorSession + return actorSession, "" } - return actorAdminToken + return actorAdminToken, "" } diff --git a/workspace-server/internal/orgtoken/tokens.go b/workspace-server/internal/orgtoken/tokens.go index 45be465b..94e5e551 100644 --- a/workspace-server/internal/orgtoken/tokens.go +++ b/workspace-server/internal/orgtoken/tokens.go @@ -56,6 +56,7 @@ type Token struct { ID string `json:"id"` Prefix string `json:"prefix"` Name string `json:"name,omitempty"` + OrgID string `json:"org_id,omitempty"` CreatedBy string `json:"created_by,omitempty"` CreatedAt time.Time `json:"created_at"` LastUsedAt *time.Time `json:"last_used_at,omitempty"` @@ -66,10 +67,11 @@ type Token struct { // caller once and must be handed to the user verbatim — we cannot // recover it from the database. // -// name and createdBy are both optional (nullable columns). Typical -// use: name = "zapier integration", createdBy = current session -// user_id. -func Issue(ctx context.Context, db *sql.DB, name, createdBy string) (plaintext, id string, err error) { +// name and orgID are both optional (nullable columns). createdBy +// records provenance for audit. orgID is the caller's org workspace +// ID and is used by requireCallerOwnsOrg to enforce org isolation +// on org-scoped routes (#1200 / F1094). +func Issue(ctx context.Context, db *sql.DB, name, createdBy, orgID string) (plaintext, id string, err error) { buf := make([]byte, tokenPayloadBytes) if _, err := rand.Read(buf); err != nil { return "", "", fmt.Errorf("orgtoken: generate: %w", err) @@ -79,10 +81,10 @@ func Issue(ctx context.Context, db *sql.DB, name, createdBy string) (plaintext, prefix := plaintext[:tokenPrefixLen] err = db.QueryRowContext(ctx, ` - INSERT INTO org_api_tokens (token_hash, prefix, name, created_by) - VALUES ($1, $2, $3, $4) + INSERT INTO org_api_tokens (token_hash, prefix, name, created_by, org_id) + VALUES ($1, $2, $3, $4, $5) RETURNING id - `, hash[:], prefix, nullIfEmpty(name), nullIfEmpty(createdBy)).Scan(&id) + `, hash[:], prefix, nullIfEmpty(name), nullIfEmpty(createdBy), nullIfEmpty(orgID)).Scan(&id) if err != nil { return "", "", fmt.Errorf("orgtoken: persist: %w", err) } @@ -130,8 +132,8 @@ func Validate(ctx context.Context, db *sql.DB, plaintext string) (id, prefix str // minting loop from O(N) pageloads in the admin UI. func List(ctx context.Context, db *sql.DB) ([]Token, error) { rows, err := db.QueryContext(ctx, ` - SELECT id, prefix, COALESCE(name,''), COALESCE(created_by,''), - created_at, last_used_at + SELECT id, prefix, COALESCE(name,''), COALESCE(org_id,''), + COALESCE(created_by,''), created_at, last_used_at FROM org_api_tokens WHERE revoked_at IS NULL ORDER BY created_at DESC @@ -146,7 +148,7 @@ func List(ctx context.Context, db *sql.DB) ([]Token, error) { for rows.Next() { var t Token var lastUsed sql.NullTime - if err := rows.Scan(&t.ID, &t.Prefix, &t.Name, &t.CreatedBy, + if err := rows.Scan(&t.ID, &t.Prefix, &t.Name, &t.OrgID, &t.CreatedBy, &t.CreatedAt, &lastUsed); err != nil { return nil, fmt.Errorf("orgtoken: scan: %w", err) } @@ -193,6 +195,25 @@ func HasAnyLive(ctx context.Context, db *sql.DB) (bool, error) { return ok, nil } +// OrgIDByTokenID looks up the org workspace ID for a token. +// Used by requireCallerOwnsOrg to enforce org isolation on org-scoped +// routes (#1200 / F1094). Returns ("", nil) when the token has no org_id +// set (e.g. pre-migration tokens, ADMIN_TOKEN bootstrap tokens) — the +// caller treats this as "deny by default". +func OrgIDByTokenID(ctx context.Context, db *sql.DB, tokenID string) (string, error) { + var orgID sql.NullString + err := db.QueryRowContext(ctx, + `SELECT org_id FROM org_api_tokens WHERE id = $1`, tokenID, + ).Scan(&orgID) + if err != nil { + return "", fmt.Errorf("orgtoken: org_id lookup: %w", err) + } + if !orgID.Valid || orgID.String == "" { + return "", nil // unanchored token — deny by default + } + return orgID.String, nil +} + func nullIfEmpty(s string) interface{} { if s == "" { return nil diff --git a/workspace-server/internal/orgtoken/tokens_test.go b/workspace-server/internal/orgtoken/tokens_test.go index dea3f729..86ad0b1f 100644 --- a/workspace-server/internal/orgtoken/tokens_test.go +++ b/workspace-server/internal/orgtoken/tokens_test.go @@ -22,10 +22,10 @@ func TestIssue_StoresHashNotPlaintext(t *testing.T) { // INSERT arguments are a hash (bytea) + short prefix + optional // fields. sqlmock's AnyArg sidesteps the randomness. mock.ExpectQuery(`INSERT INTO org_api_tokens`). - WithArgs(sqlmock.AnyArg(), sqlmock.AnyArg(), "my-ci", "user_01"). + WithArgs(sqlmock.AnyArg(), sqlmock.AnyArg(), "my-ci", "user_01", "org-1"). WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow("tok-1")) - plaintext, id, err := Issue(context.Background(), db, "my-ci", "user_01") + plaintext, id, err := Issue(context.Background(), db, "my-ci", "user_01", "org-1") if err != nil { t.Fatalf("Issue: %v", err) } @@ -47,13 +47,13 @@ func TestIssue_EmptyNameAndCreatedByStoreNull(t *testing.T) { t.Fatalf("sqlmock: %v", err) } defer db.Close() - // Empty name + createdBy → NULL in DB so `WHERE name IS NULL` works - // for future queries that want "unnamed" tokens. + // Empty name + createdBy + orgID → NULL in DB so `WHERE name IS NULL` + // works for future queries that want "unnamed" tokens. mock.ExpectQuery(`INSERT INTO org_api_tokens`). - WithArgs(sqlmock.AnyArg(), sqlmock.AnyArg(), nil, nil). + WithArgs(sqlmock.AnyArg(), sqlmock.AnyArg(), nil, nil, nil). WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow("tok-min")) - _, _, err = Issue(context.Background(), db, "", "") + _, _, err = Issue(context.Background(), db, "", "", "") if err != nil { t.Fatalf("Issue: %v", err) } @@ -143,9 +143,9 @@ func TestList_NewestFirst(t *testing.T) { earlier := now.Add(-1 * time.Hour) mock.ExpectQuery(`SELECT id, prefix.*FROM org_api_tokens.*ORDER BY created_at DESC`). WithArgs(listMax). - WillReturnRows(sqlmock.NewRows([]string{"id", "prefix", "name", "created_by", "created_at", "last_used_at"}). - AddRow("t2", "abcd1234", "zapier", "user_01", now, now). - AddRow("t1", "efgh5678", "", "", earlier, nil)) + WillReturnRows(sqlmock.NewRows([]string{"id", "prefix", "name", "org_id", "created_by", "created_at", "last_used_at"}). + AddRow("t2", "abcd1234", "zapier", "org-1", "user_01", now, now). + AddRow("t1", "efgh5678", "", "", "", earlier, nil)) tokens, err := List(context.Background(), db) if err != nil { @@ -157,6 +157,9 @@ func TestList_NewestFirst(t *testing.T) { if tokens[0].ID != "t2" { t.Errorf("ordering not preserved: got %q first", tokens[0].ID) } + if tokens[0].OrgID != "org-1" { + t.Errorf("got org_id %q, want org-1", tokens[0].OrgID) + } if tokens[1].LastUsedAt != nil { t.Errorf("never-used token should have nil LastUsedAt, got %v", tokens[1].LastUsedAt) } @@ -210,3 +213,62 @@ func TestHasAnyLive(t *testing.T) { t.Errorf("has-any-live empty: got (%v, %v), want (false, nil)", got, err) } } + +func TestOrgIDByTokenID_HappyPath(t *testing.T) { + db, mock, err := sqlmock.New() + if err != nil { + t.Fatalf("sqlmock: %v", err) + } + defer db.Close() + + mock.ExpectQuery(`SELECT org_id FROM org_api_tokens WHERE id = \$1`). + WithArgs("tok-org-1"). + WillReturnRows(sqlmock.NewRows([]string{"org_id"}).AddRow("org-1")) + + orgID, err := OrgIDByTokenID(context.Background(), db, "tok-org-1") + if err != nil { + t.Fatalf("OrgIDByTokenID: %v", err) + } + if orgID != "org-1" { + t.Errorf("orgID = %q, want org-1", orgID) + } +} + +func TestOrgIDByTokenID_NullOrgIDReturnsEmpty(t *testing.T) { + db, mock, err := sqlmock.New() + if err != nil { + t.Fatalf("sqlmock: %v", err) + } + defer db.Close() + + // Pre-migration token or ADMIN_TOKEN bootstrap token — org_id is NULL. + // Caller should get ("", nil) and deny by default. + mock.ExpectQuery(`SELECT org_id FROM org_api_tokens WHERE id = \$1`). + WithArgs("tok-old"). + WillReturnRows(sqlmock.NewRows([]string{"org_id"}).AddRow(nil)) + + orgID, err := OrgIDByTokenID(context.Background(), db, "tok-old") + if err != nil { + t.Fatalf("OrgIDByTokenID null: got err %v", err) + } + if orgID != "" { + t.Errorf("orgID for null row = %q, want \"\"", orgID) + } +} + +func TestOrgIDByTokenID_DBError(t *testing.T) { + db, mock, err := sqlmock.New() + if err != nil { + t.Fatalf("sqlmock: %v", err) + } + defer db.Close() + + mock.ExpectQuery(`SELECT org_id FROM org_api_tokens WHERE id = \$1`). + WithArgs("tok-bad"). + WillReturnError(sql.ErrConnDone) + + _, err = OrgIDByTokenID(context.Background(), db, "tok-bad") + if err == nil { + t.Error("expected error on DB failure, got nil") + } +} diff --git a/workspace-server/migrations/036_org_api_tokens_org_id.down.sql b/workspace-server/migrations/036_org_api_tokens_org_id.down.sql new file mode 100644 index 00000000..20924666 --- /dev/null +++ b/workspace-server/migrations/036_org_api_tokens_org_id.down.sql @@ -0,0 +1,7 @@ +-- Revert: drop org_id column from org_api_tokens. +-- Note: this does NOT restore the broken requireCallerOwnsOrg behaviour +-- (which used created_by as org anchor). That function was fixed alongside +-- this migration to read org_id; it will error on org-token callers if this +-- migration is reverted without also reverting the handler. +DROP INDEX IF EXISTS org_api_tokens_org_id_idx; +ALTER TABLE org_api_tokens DROP COLUMN IF EXISTS org_id; \ No newline at end of file diff --git a/workspace-server/migrations/036_org_api_tokens_org_id.up.sql b/workspace-server/migrations/036_org_api_tokens_org_id.up.sql new file mode 100644 index 00000000..ed6ffdaf --- /dev/null +++ b/workspace-server/migrations/036_org_api_tokens_org_id.up.sql @@ -0,0 +1,33 @@ +-- Add org_id column to org_api_tokens (#1200 / F1094). +-- +-- Rationale: requireCallerOwnsOrg (org_plugin_allowlist.go:116) was +-- reading created_by to determine the caller's org. But created_by +-- is a provenance label ("session", "admin-token", "org-token:"), +-- NOT an org UUID — so every org-token caller got a non-UUID string +-- and the equality check callerOrg != targetOrgID always failed, +-- causing 403 on every org-token request. +-- +-- Migration plan: +-- 1. Add org_id (nullable) — existing tokens have no org anchor yet +-- 2. All new tokens minted via POST /org/tokens are written with org_id +-- 3. Backfill: tokens minted via session → look up session's org workspace +-- (the session-auth workspace ID is known via the CP /cp/auth/tenant-member +-- response; the CP stores org_id in the session state). This requires +-- a separate admin script since the handler doesn't have that context. +-- Tokens minted via ADMIN_TOKEN or bootstrap → leave org_id NULL +-- (deny by default; operator must set via admin API if needed). +-- 4. requireCallerOwnsOrg now reads org_id instead of created_by +-- (org_id NULL → treat as "no anchor" → deny by default) +-- 5. Post-Fix: admin can backfill remaining tokens via +-- PATCH /org/tokens/:id with org_id set. +ALTER TABLE org_api_tokens + ADD COLUMN org_id UUID REFERENCES workspaces(id) ON DELETE SET NULL; + +-- Tokens created before this migration cannot be backfilled automatically +-- without knowing the session's org. Mark them as "unanchored" (org_id NULL) +-- so the auth fix denies by default — safer than permitting all old tokens. +-- Ops can backfill org_id for known tokens via PATCH /org/tokens/:id. +-- +-- Index for fast org-anchor lookups (used by requireCallerOwnsOrg). +CREATE INDEX IF NOT EXISTS org_api_tokens_org_id_idx + ON org_api_tokens (org_id) WHERE org_id IS NOT NULL; \ No newline at end of file