diff --git a/platform/internal/handlers/admin_test_token.go b/platform/internal/handlers/admin_test_token.go index 6a2bb9c6..34372a51 100644 --- a/platform/internal/handlers/admin_test_token.go +++ b/platform/internal/handlers/admin_test_token.go @@ -75,14 +75,17 @@ func (h *AdminTestTokenHandler) GetTestToken(c *gin.Context) { return } - token, err := wsauth.IssueToken(c.Request.Context(), db.DB, workspaceID) + // #684: issue an admin token so E2E test scripts can reach AdminAuth-gated + // routes (/bundles/export, /events, /org/import, etc.). Workspace tokens + // (token_type='workspace') are now rejected by ValidateAnyToken. + token, err := wsauth.IssueAdminToken(c.Request.Context(), db.DB) if err != nil { c.JSON(http.StatusInternalServerError, gin.H{"error": "token issue failed"}) return } // INFO log — never include the token itself. - log.Printf("admin: issued test token for workspace %s", workspaceID) + log.Printf("admin: issued test admin token (for workspace %s)", workspaceID) c.JSON(http.StatusOK, gin.H{ "auth_token": token, diff --git a/platform/internal/handlers/admin_test_token_test.go b/platform/internal/handlers/admin_test_token_test.go index a6d537a1..47766a99 100644 --- a/platform/internal/handlers/admin_test_token_test.go +++ b/platform/internal/handlers/admin_test_token_test.go @@ -80,10 +80,10 @@ func TestAdminTestToken_HappyPath_TokenValidates(t *testing.T) { WithArgs("ws-1"). WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow("ws-1")) - // Capture the hash inserted by IssueToken so we can replay it on Validate. - var capturedHash []byte + // #684: IssueAdminToken inserts with NULL workspace_id, so only hash + prefix + // are positional args. token_type = 'admin' is a literal in the SQL. mock.ExpectExec("INSERT INTO workspace_auth_tokens"). - WithArgs("ws-1", sqlmock.AnyArg(), sqlmock.AnyArg()). + WithArgs(sqlmock.AnyArg(), sqlmock.AnyArg()). WillReturnResult(sqlmock.NewResult(0, 1)) h := NewAdminTestTokenHandler() @@ -111,20 +111,16 @@ func TestAdminTestToken_HappyPath_TokenValidates(t *testing.T) { t.Errorf("token looks too short: %d chars", len(resp.AuthToken)) } - // Now simulate ValidateToken lookup using the same DB — prove the token - // can be validated by feeding its sha256 back through ExpectedArgs. - // (We stub the SELECT rather than re-reading capturedHash since sqlmock - // doesn't capture live args; the important invariant is that the issued - // token passes ValidateToken given a matching hash row exists.) - _ = capturedHash - mock.ExpectQuery("SELECT id, workspace_id\\s+FROM workspace_auth_tokens"). + // Prove the issued admin token passes ValidateAnyToken (AdminAuth path). + // Stub the SELECT so sqlmock returns a matching row with token_type='admin'. + mock.ExpectQuery("SELECT id.*FROM workspace_auth_tokens.*token_type = 'admin'"). WithArgs(sqlmock.AnyArg()). - WillReturnRows(sqlmock.NewRows([]string{"id", "workspace_id"}).AddRow("tok-1", "ws-1")) + WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow("tok-1")) mock.ExpectExec("UPDATE workspace_auth_tokens SET last_used_at"). WillReturnResult(sqlmock.NewResult(0, 1)) - if err := wsauth.ValidateToken(c.Request.Context(), db.DB, "ws-1", resp.AuthToken); err != nil { - t.Errorf("issued token failed to validate: %v", err) + if err := wsauth.ValidateAnyToken(c.Request.Context(), db.DB, resp.AuthToken); err != nil { + t.Errorf("issued admin token failed ValidateAnyToken: %v", err) } } diff --git a/platform/internal/middleware/wsauth_middleware_test.go b/platform/internal/middleware/wsauth_middleware_test.go index a38e960e..fcc1704f 100644 --- a/platform/internal/middleware/wsauth_middleware_test.go +++ b/platform/internal/middleware/wsauth_middleware_test.go @@ -26,8 +26,9 @@ const hasAnyLiveTokenGlobalQuery = "SELECT COUNT.*FROM workspace_auth_tokens" const validateTokenSelectQuery = "SELECT id, workspace_id.*FROM workspace_auth_tokens.*token_hash" // validateAnyTokenQuery is matched for ValidateAnyToken (SELECT). -// The query now JOINs workspaces to enforce w.status != 'removed' (#682 defense-in-depth). -const validateAnyTokenSelectQuery = "SELECT t\\.id.*FROM workspace_auth_tokens t.*JOIN workspaces" +// #684: the query now filters token_type = 'admin' so workspace tokens cannot +// satisfy AdminAuth. No workspace JOIN needed (admin tokens have NULL workspace_id). +const validateAnyTokenSelectQuery = "SELECT id.*FROM workspace_auth_tokens.*token_type = 'admin'" // validateTokenUpdateQuery is matched for the best-effort last_used_at UPDATE. const validateTokenUpdateQuery = "UPDATE workspace_auth_tokens SET last_used_at" diff --git a/platform/internal/wsauth/tokens.go b/platform/internal/wsauth/tokens.go index ea30d268..cecb7410 100644 --- a/platform/internal/wsauth/tokens.go +++ b/platform/internal/wsauth/tokens.go @@ -38,6 +38,21 @@ const tokenPrefixLen = 8 // was known. var ErrInvalidToken = errors.New("invalid or revoked workspace token") +// Token type constants — recorded in the token_type column (migration 029). +// +// TokenTypeWorkspace — issued to workspace agents via IssueToken. Scoped to +// a single workspace. Accepted by WorkspaceAuth and the A2A layer, but +// rejected by AdminAuth (ValidateAnyToken). This is the safe default. +// +// TokenTypeAdmin — issued for platform-wide operations via IssueAdminToken. +// Not scoped to any specific workspace. The ONLY type that satisfies +// AdminAuth. Should be issued to operators, CI pipelines, and the E2E +// test-token endpoint — never to workspace agents at runtime. +const ( + TokenTypeWorkspace = "workspace" + TokenTypeAdmin = "admin" +) + // IssueToken mints a fresh token, stores its hash + prefix against the // given workspace, and returns the plaintext to show the caller exactly // once. The plaintext is never recoverable from the database afterwards. @@ -56,8 +71,8 @@ func IssueToken(ctx context.Context, db *sql.DB, workspaceID string) (string, er prefix := plaintext[:tokenPrefixLen] _, err := db.ExecContext(ctx, ` - INSERT INTO workspace_auth_tokens (workspace_id, token_hash, prefix) - VALUES ($1, $2, $3) + INSERT INTO workspace_auth_tokens (workspace_id, token_hash, prefix, token_type) + VALUES ($1, $2, $3, 'workspace') `, workspaceID, hash[:], prefix) if err != nil { return "", fmt.Errorf("wsauth: persist token: %w", err) @@ -65,6 +80,34 @@ func IssueToken(ctx context.Context, db *sql.DB, workspaceID string) (string, er return plaintext, nil } +// IssueAdminToken mints a platform-wide admin token that is NOT scoped to any +// specific workspace. Only admin tokens satisfy AdminAuth — regular workspace +// tokens are rejected by ValidateAnyToken (#684). +// +// Use this for: E2E test-token endpoint (dev/CI), molecule-controlplane +// provisioner, operator tooling. Never issue admin tokens to workspace agents +// at runtime. +func IssueAdminToken(ctx context.Context, db *sql.DB) (string, error) { + buf := make([]byte, tokenPayloadBytes) + if _, err := rand.Read(buf); err != nil { + return "", fmt.Errorf("wsauth: generate admin token: %w", err) + } + plaintext := base64.RawURLEncoding.EncodeToString(buf) + + hash := sha256.Sum256([]byte(plaintext)) + prefix := plaintext[:tokenPrefixLen] + + // workspace_id is NULL for admin tokens — they are platform-wide. + _, err := db.ExecContext(ctx, ` + INSERT INTO workspace_auth_tokens (workspace_id, token_hash, prefix, token_type) + VALUES (NULL, $1, $2, 'admin') + `, hash[:], prefix) + if err != nil { + return "", fmt.Errorf("wsauth: persist admin token: %w", err) + } + return plaintext, nil +} + // ValidateToken confirms the presented plaintext matches a live row whose // workspace_id equals expectedWorkspaceID. On success it refreshes // last_used_at (best-effort — failure to update is logged by the caller, @@ -166,13 +209,19 @@ func BearerTokenFromHeader(h string) string { return strings.TrimSpace(h[len(prefix):]) } -// HasAnyLiveTokenGlobal reports whether ANY workspace has at least one live -// (non-revoked) token on file. Used by AdminAuth to decide whether to enforce -// auth on global/admin routes — fresh installs with no tokens fail open. +// HasAnyLiveTokenGlobal reports whether ANY admin token (token_type='admin') +// exists and is live (non-revoked). Used by AdminAuth for the lazy-bootstrap +// decision: fresh installs with no admin tokens fail open so operators can +// reach admin routes to issue the first token. Once an admin token exists the +// gate is permanently enforced — workspace tokens can never satisfy AdminAuth. +// +// #684: counts only admin tokens (not workspace tokens). Workspace tokens +// existing on the platform do NOT trigger enforcement — only admin tokens do. func HasAnyLiveTokenGlobal(ctx context.Context, db *sql.DB) (bool, error) { var n int err := db.QueryRowContext(ctx, ` - SELECT COUNT(*) FROM workspace_auth_tokens WHERE revoked_at IS NULL + SELECT COUNT(*) FROM workspace_auth_tokens + WHERE token_type = 'admin' AND revoked_at IS NULL `).Scan(&n) if err != nil { return false, err @@ -180,16 +229,12 @@ func HasAnyLiveTokenGlobal(ctx context.Context, db *sql.DB) (bool, error) { return n > 0, nil } -// ValidateAnyToken confirms the presented plaintext matches any live workspace -// token (not scoped to a specific workspace). Used for admin/global routes -// where workspace-scoped auth is not applicable — any authenticated agent may -// access platform-wide settings. +// ValidateAnyToken confirms the presented plaintext matches a live admin token +// (token_type='admin'). Used exclusively by AdminAuth — workspace bearer +// tokens are unconditionally rejected here (#684). // -// Defense-in-depth (#682): the JOIN against workspaces ensures that even if a -// token revocation was delayed (e.g. DB error between workspace status='removed' -// and the token UPDATE), the token still fails validation once the workspace row -// is marked removed. This closes the theoretical race window in the Delete -// handler without relying solely on revoked_at being set atomically. +// Admin tokens are not scoped to a workspace (workspace_id IS NULL), so no +// workspace JOIN is needed. The type filter is the sole privilege boundary. func ValidateAnyToken(ctx context.Context, db *sql.DB, plaintext string) error { if plaintext == "" { return ErrInvalidToken @@ -198,12 +243,11 @@ func ValidateAnyToken(ctx context.Context, db *sql.DB, plaintext string) error { var tokenID string err := db.QueryRowContext(ctx, ` - SELECT t.id - FROM workspace_auth_tokens t - JOIN workspaces w ON w.id = t.workspace_id - WHERE t.token_hash = $1 - AND t.revoked_at IS NULL - AND w.status != 'removed' + SELECT id + FROM workspace_auth_tokens + WHERE token_hash = $1 + AND token_type = 'admin' + AND revoked_at IS NULL `, hash[:]).Scan(&tokenID) if err != nil { return ErrInvalidToken diff --git a/platform/internal/wsauth/tokens_test.go b/platform/internal/wsauth/tokens_test.go index fa311c18..c3074ae9 100644 --- a/platform/internal/wsauth/tokens_test.go +++ b/platform/internal/wsauth/tokens_test.go @@ -231,14 +231,15 @@ func TestHasAnyLiveTokenGlobal(t *testing.T) { count int want bool }{ - {"no tokens anywhere", 0, false}, - {"one live token", 1, true}, - {"many live tokens", 5, true}, + {"no admin tokens", 0, false}, + {"one admin token", 1, true}, + {"many admin tokens", 5, true}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { db, mock := setupMock(t) - mock.ExpectQuery(`SELECT COUNT\(\*\) FROM workspace_auth_tokens`). + // #684: must filter by token_type = 'admin' + mock.ExpectQuery(`SELECT COUNT\(\*\) FROM workspace_auth_tokens\s+WHERE token_type = 'admin'`). WillReturnRows(sqlmock.NewRows([]string{"count"}).AddRow(tc.count)) got, err := HasAnyLiveTokenGlobal(context.Background(), db) @@ -256,19 +257,22 @@ func TestHasAnyLiveTokenGlobal(t *testing.T) { // ValidateAnyToken // ------------------------------------------------------------ +// validateAnyTokenQuery is the regexp matched by sqlmock for ValidateAnyToken. +// #684: must filter by token_type = 'admin' (no workspace JOIN — admin tokens have NULL workspace_id). +const validateAnyTokenQuery = `SELECT id\s+FROM workspace_auth_tokens\s+WHERE.*token_type = 'admin'` + func TestValidateAnyToken_HappyPath(t *testing.T) { db, mock := setupMock(t) - // Issue a token for some workspace. + // Issue an admin token. mock.ExpectExec(`INSERT INTO workspace_auth_tokens`).WillReturnResult(sqlmock.NewResult(1, 1)) - tok, err := IssueToken(context.Background(), db, "ws-admin") + tok, err := IssueAdminToken(context.Background(), db) if err != nil { - t.Fatalf("IssueToken: %v", err) + t.Fatalf("IssueAdminToken: %v", err) } - // ValidateAnyToken: lookup by hash with JOIN against workspaces to ensure - // the workspace is not 'removed' (#682 defense-in-depth). - mock.ExpectQuery(`SELECT t\.id\s+FROM workspace_auth_tokens t\s+JOIN workspaces`). + // ValidateAnyToken: lookup by hash, must filter token_type = 'admin'. + mock.ExpectQuery(validateAnyTokenQuery). WithArgs(sqlmock.AnyArg()). WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow("tok-id-global")) // Best-effort last_used_at update. @@ -277,16 +281,31 @@ func TestValidateAnyToken_HappyPath(t *testing.T) { WillReturnResult(sqlmock.NewResult(0, 1)) if err := ValidateAnyToken(context.Background(), db, tok); err != nil { - t.Errorf("expected valid token, got error: %v", err) + t.Errorf("expected valid admin token, got error: %v", err) } if err := mock.ExpectationsWereMet(); err != nil { t.Errorf("unmet expectations: %v", err) } } +// TestValidateAnyToken_WorkspaceTokenRejected verifies the #684 fix: a +// workspace bearer token (token_type='workspace') must NOT satisfy ValidateAnyToken. +// The DB returns no rows because the admin filter excludes workspace tokens. +func TestValidateAnyToken_WorkspaceTokenRejected(t *testing.T) { + db, mock := setupMock(t) + + // DB returns no rows — simulates a workspace token not matching the admin filter. + mock.ExpectQuery(validateAnyTokenQuery). + WillReturnError(sql.ErrNoRows) + + if err := ValidateAnyToken(context.Background(), db, "workspace-bearer-token"); err != ErrInvalidToken { + t.Errorf("#684 regression: workspace token should be rejected, got %v", err) + } +} + func TestValidateAnyToken_UnknownTokenRejected(t *testing.T) { db, mock := setupMock(t) - mock.ExpectQuery(`SELECT t\.id\s+FROM workspace_auth_tokens t\s+JOIN workspaces`). + mock.ExpectQuery(validateAnyTokenQuery). WillReturnError(sql.ErrNoRows) if err := ValidateAnyToken(context.Background(), db, "not-a-real-token"); err != ErrInvalidToken { @@ -301,19 +320,57 @@ func TestValidateAnyToken_EmptyTokenRejected(t *testing.T) { } } -// TestValidateAnyToken_RemovedWorkspaceRejected verifies defense-in-depth (#682): -// even if revoked_at was not set (e.g. a race between workspace deletion and token -// revocation), the JOIN against workspaces.status ensures tokens from 'removed' -// workspaces never authenticate. -func TestValidateAnyToken_RemovedWorkspaceRejected(t *testing.T) { - db, mock := setupMock(t) - // The JOIN filters out status='removed', so the query returns no rows. - mock.ExpectQuery(`SELECT t\.id\s+FROM workspace_auth_tokens t\s+JOIN workspaces`). - WithArgs(sqlmock.AnyArg()). - WillReturnError(sql.ErrNoRows) +// ------------------------------------------------------------ +// IssueAdminToken +// ------------------------------------------------------------ - if err := ValidateAnyToken(context.Background(), db, "token-for-deleted-workspace"); err != ErrInvalidToken { - t.Errorf("expected ErrInvalidToken for removed workspace, got %v", err) +func TestIssueAdminToken_PersistsAdminType(t *testing.T) { + db, mock := setupMock(t) + + // Admin tokens have NULL workspace_id and token_type='admin'. + mock.ExpectExec(`INSERT INTO workspace_auth_tokens`). + WithArgs( + sqlmock.AnyArg(), // hash (bytea) + sqlmock.AnyArg(), // prefix + ). + WillReturnResult(sqlmock.NewResult(1, 1)) + + tok, err := IssueAdminToken(context.Background(), db) + if err != nil { + t.Fatalf("IssueAdminToken: %v", err) + } + if len(tok) < 40 { + t.Errorf("admin token looks too short: len=%d", len(tok)) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("unmet expectations: %v", err) + } +} + +func TestIssueAdminToken_UniqueAcrossCalls(t *testing.T) { + db, mock := setupMock(t) + mock.ExpectExec(`INSERT INTO workspace_auth_tokens`).WillReturnResult(sqlmock.NewResult(1, 1)) + mock.ExpectExec(`INSERT INTO workspace_auth_tokens`).WillReturnResult(sqlmock.NewResult(1, 1)) + + a, _ := IssueAdminToken(context.Background(), db) + b, _ := IssueAdminToken(context.Background(), db) + if a == b { + t.Errorf("expected unique admin tokens, got %q twice", a) + } +} + +// TestValidateAnyToken_RevokedAdminTokenRejected verifies that a revoked admin +// token is correctly rejected. The revoked_at filter in the query excludes it, +// returning no rows. +func TestValidateAnyToken_RevokedAdminTokenRejected(t *testing.T) { + db, mock := setupMock(t) + // Revoked token: query returns no rows (revoked_at IS NULL filter excludes it). + mock.ExpectQuery(validateAnyTokenQuery). + WithArgs(sqlmock.AnyArg()). + WillReturnError(sql.ErrNoRows) + + if err := ValidateAnyToken(context.Background(), db, "revoked-admin-token"); err != ErrInvalidToken { + t.Errorf("expected ErrInvalidToken for revoked admin token, got %v", err) } if err := mock.ExpectationsWereMet(); err != nil { t.Errorf("unmet expectations: %v", err) diff --git a/platform/migrations/029_token_type.down.sql b/platform/migrations/029_token_type.down.sql new file mode 100644 index 00000000..416831ef --- /dev/null +++ b/platform/migrations/029_token_type.down.sql @@ -0,0 +1,5 @@ +ALTER TABLE workspace_auth_tokens DROP CONSTRAINT IF EXISTS workspace_auth_tokens_scope_check; +ALTER TABLE workspace_auth_tokens DROP CONSTRAINT IF EXISTS workspace_auth_tokens_token_type_check; +ALTER TABLE workspace_auth_tokens DROP COLUMN IF EXISTS token_type; +-- Note: we cannot safely re-add NOT NULL to workspace_id if admin rows (NULL) exist. +-- Operators should purge admin tokens before rolling back this migration. diff --git a/platform/migrations/029_token_type.up.sql b/platform/migrations/029_token_type.up.sql new file mode 100644 index 00000000..fa12a46a --- /dev/null +++ b/platform/migrations/029_token_type.up.sql @@ -0,0 +1,53 @@ +-- #684 — token type distinction: 'workspace' vs 'admin' +-- +-- Before this migration AdminAuth called ValidateAnyToken, which accepted ANY +-- live token regardless of which workspace it was issued to. That meant a +-- workspace agent bearer could hit /bundles/import, /events, /org/import, etc. +-- by presenting its own workspace token. +-- +-- Fix: introduce a token_type column. IssueToken continues to produce +-- 'workspace' tokens (scoped to an agent). IssueAdminToken produces 'admin' +-- tokens (platform-wide, not scoped to a workspace). ValidateAnyToken (used +-- by AdminAuth) now filters WHERE token_type = 'admin', so workspace bearers +-- are unconditionally rejected on admin routes. +-- +-- Existing rows default to 'workspace'. Any token issued before this migration +-- by the test-token endpoint (dev/CI only) must be re-issued — the endpoint +-- was updated to call IssueAdminToken instead. + +-- Make workspace_id nullable so admin tokens (not bound to any workspace) can +-- be stored in the same table. The NOT NULL constraint on existing 'workspace' +-- rows is preserved by the CHECK constraint below. +ALTER TABLE workspace_auth_tokens + ALTER COLUMN workspace_id DROP NOT NULL; + +ALTER TABLE workspace_auth_tokens + ADD COLUMN IF NOT EXISTS token_type TEXT NOT NULL DEFAULT 'workspace'; + +-- CHECK constraint validates accepted values and enforces that workspace tokens +-- always carry a workspace_id while admin tokens must have workspace_id = NULL. +DO $$ +BEGIN + IF NOT EXISTS ( + SELECT 1 FROM pg_constraint + WHERE conname = 'workspace_auth_tokens_token_type_check' + AND conrelid = 'workspace_auth_tokens'::regclass + ) THEN + ALTER TABLE workspace_auth_tokens + ADD CONSTRAINT workspace_auth_tokens_token_type_check + CHECK (token_type IN ('workspace', 'admin')); + END IF; + -- workspace tokens MUST have a workspace_id; admin tokens MUST NOT. + IF NOT EXISTS ( + SELECT 1 FROM pg_constraint + WHERE conname = 'workspace_auth_tokens_scope_check' + AND conrelid = 'workspace_auth_tokens'::regclass + ) THEN + ALTER TABLE workspace_auth_tokens + ADD CONSTRAINT workspace_auth_tokens_scope_check + CHECK ( + (token_type = 'workspace' AND workspace_id IS NOT NULL) OR + (token_type = 'admin' AND workspace_id IS NULL) + ); + END IF; +END $$;