From 6e854fdebfedbb72f95a315ad371337e623e4346 Mon Sep 17 00:00:00 2001 From: alex-dembele Date: Thu, 1 Oct 2026 11:16:51 +0100 Subject: [PATCH 1/4] feat(auth): bind personal access tokens to the tenant they were minted in (#782) A PAT had no tenant of its own. The middleware resolved the owner's default organization on every request, so changing that default moved every token the person held, and listing or revoking was filtered on user_id alone. The token now stores tenant_id. The middleware resolves the session with the org resolver for that tenant, which refuses an owner who is no longer an active member there. List, revoke and the last-used stamp filter on tenant_id and user_id; the hash lookup stays unscoped because it is the authentication step and the row it returns names the tenant. Existing rows take users.default_org_id, the tenant they already acted in. Rows whose owner has no default organization never authenticated anything and are deleted. This runs before AutoMigrate and again in migration 0067. New tokens read orsk_<8-hex>_ so a leaked one is recognisable; the older <8-hex>_ form still validates. A "*" scope now means the owner's own permissions, where before it intersected to nothing for any member who was not an administrator. Signed-off-by: alex-dembele --- backend/cmd/server/main.go | 7 +- backend/internal/auth/pat.go | 83 +++++--- backend/internal/auth/pat_test.go | 196 ++++++++++++++++++ backend/internal/domain/membership.go | 8 +- backend/internal/handler/auth/pat_handler.go | 23 +- .../infrastructure/database/prepare.go | 42 ++++ .../database/prepare_pat_pg_test.go | 92 ++++++++ .../gorm_personal_access_token_repository.go | 36 ++-- ...ersonal_access_token_repository_pg_test.go | 186 +++++++++++++++++ .../infrastructure/repository/interfaces.go | 12 +- backend/internal/middleware/pat_e2e_test.go | 161 ++++++++++++++ backend/internal/middleware/pat_middleware.go | 56 +++-- .../middleware/pat_middleware_test.go | 6 +- .../internal/security/isolation/registry.go | 2 +- ...067_personal_access_tokens_tenant.down.sql | 12 ++ .../0067_personal_access_tokens_tenant.up.sql | 42 ++++ 16 files changed, 872 insertions(+), 92 deletions(-) create mode 100644 backend/internal/auth/pat_test.go create mode 100644 backend/internal/infrastructure/database/prepare_pat_pg_test.go create mode 100644 backend/internal/infrastructure/repository/gorm_personal_access_token_repository_pg_test.go create mode 100644 backend/internal/middleware/pat_e2e_test.go create mode 100644 migrations/0067_personal_access_tokens_tenant.down.sql create mode 100644 migrations/0067_personal_access_tokens_tenant.up.sql diff --git a/backend/cmd/server/main.go b/backend/cmd/server/main.go index fd47317e..696d288d 100644 --- a/backend/cmd/server/main.go +++ b/backend/cmd/server/main.go @@ -592,8 +592,9 @@ func main() { tokenManager.SetOrgSessionResolver(resolveSessionForOrg) // L5 — Personal Access Tokens. DB-backed service (survives restarts, scoped), - // its auth middleware, and a management handler. The same resolveSession gives a - // PAT the owner's tenant + permissions (narrowed to the token's scopes). + // its auth middleware, and a management handler. resolveSessionForOrg gives a + // PAT its owner's permissions in the token's own tenant (narrowed to the + // token's scopes), and refuses once the owner is no longer a member there. patService := coreauth.NewPersonalAccessTokenService(repository.NewGormPersonalAccessTokenRepository(database.DB)) // L7 — full-fidelity auth audit trail (auth_audit_logs: IP, UA, geo, device @@ -969,7 +970,7 @@ func main() { // L5 — PAT authentication runs BEFORE the JWT gate: it authenticates PAT-shaped // bearers and is a no-op for JWTs (which the RS256 middleware then handles). The // JWT middleware skips when a PAT already authenticated the request. - api.Use(middleware.PATMiddleware(patService, resolveSession)) + api.Use(middleware.PATMiddleware(patService, resolveSessionForOrg)) protected := api.Use(middleware.Protected(rsaKeys, jtiBlacklistChecker)) // Response-cache invalidation (#337). Mounted here, right after the gate that diff --git a/backend/internal/auth/pat.go b/backend/internal/auth/pat.go index 098f4899..558b4a1e 100644 --- a/backend/internal/auth/pat.go +++ b/backend/internal/auth/pat.go @@ -21,6 +21,11 @@ import ( "github.com/opendefender/openrisk/internal/infrastructure/repository" ) +// PATValuePrefix opens every token value this service mints, so a leaked token +// is recognisable on sight and by secret scanners: "orsk_<8-hex>_<64-hex>". +// Tokens minted before #782 have no such prefix and still validate. +const PATValuePrefix = "orsk_" + // PersonalAccessTokenService handles PAT operations type PersonalAccessTokenService struct { repo repository.PersonalAccessTokenRepository @@ -55,8 +60,13 @@ func (s *PersonalAccessTokenService) generateTokenPrefix() (string, error) { return hex.EncodeToString(bytes)[:8], nil } -// CreateToken creates a new personal access token -func (s *PersonalAccessTokenService) CreateToken(ctx context.Context, userID uuid.UUID, name, description string, scopes []string, expiresAt *time.Time) (*domain.PersonalAccessToken, string, error) { +// CreateToken creates a personal access token for userID in tenantID. The token +// can only ever act in that tenant. The raw value is returned once and is never +// stored; only its SHA-256 hash is. +func (s *PersonalAccessTokenService) CreateToken(ctx context.Context, userID, tenantID uuid.UUID, name, description string, scopes []string, expiresAt *time.Time) (*domain.PersonalAccessToken, string, error) { + if userID == uuid.Nil || tenantID == uuid.Nil { + return nil, "", fmt.Errorf("%w: token owner and tenant are required", domain.ErrValidation) + } // Generate secure token token, err := s.generateSecureToken() if err != nil { @@ -78,6 +88,7 @@ func (s *PersonalAccessTokenService) CreateToken(ctx context.Context, userID uui pat := &domain.PersonalAccessToken{ UserID: userID, + TenantID: tenantID, Name: name, Description: description, TokenHash: tokenHash, @@ -93,24 +104,25 @@ func (s *PersonalAccessTokenService) CreateToken(ctx context.Context, userID uui return nil, "", fmt.Errorf("failed to save token: %w", err) } - // Return the GitHub-style "_" value. ValidateToken splits on - // "_" and requires an 8-char prefix; previously CreateToken returned the bare - // secret with no prefix, so every token this service minted failed validation. - return pat, tokenPrefix + "_" + token, nil + return pat, PATValuePrefix + tokenPrefix + "_" + token, nil } -// ValidateToken validates a token and returns the PAT if valid -func (s *PersonalAccessTokenService) ValidateToken(ctx context.Context, token string) (*domain.PersonalAccessToken, error) { - // Extract prefix and validate format - parts := strings.Split(token, "_") - if len(parts) != 2 { - return nil, fmt.Errorf("invalid token format") +// SplitPATValue parses "orsk_<8-hex>_" or the pre-#782 form +// "<8-hex>_" into its lookup prefix and secret. ok is false for +// anything else, including a JWT. +func SplitPATValue(raw string) (prefix, secret string, ok bool) { + parts := strings.Split(strings.TrimPrefix(raw, PATValuePrefix), "_") + if len(parts) != 2 || len(parts[0]) != 8 || parts[1] == "" { + return "", "", false } - prefix := parts[0] - tokenValue := parts[1] + return parts[0], parts[1], true +} - if len(prefix) != 8 { - return nil, fmt.Errorf("invalid token prefix") +// ValidateToken validates a token and returns the PAT if valid +func (s *PersonalAccessTokenService) ValidateToken(ctx context.Context, token string) (*domain.PersonalAccessToken, error) { + prefix, tokenValue, ok := SplitPATValue(token) + if !ok { + return nil, fmt.Errorf("%w: invalid token format", domain.ErrUnauthorized) } // Hash the token value @@ -119,21 +131,26 @@ func (s *PersonalAccessTokenService) ValidateToken(ctx context.Context, token st // Find token by hash pat, err := s.repo.GetByTokenHash(ctx, tokenHash) if err != nil { - return nil, fmt.Errorf("token not found") + return nil, fmt.Errorf("%w: unknown token", domain.ErrUnauthorized) } // Check if token belongs to the correct prefix if pat.TokenPrefix != prefix { - return nil, fmt.Errorf("invalid token") + return nil, fmt.Errorf("%w: invalid token", domain.ErrUnauthorized) } // Check if token is expired if pat.IsExpired() { - return nil, fmt.Errorf("token expired") + return nil, fmt.Errorf("%w: token expired", domain.ErrUnauthorized) + } + + // A row from before #782 that the backfill could not attribute is unusable. + if pat.TenantID == uuid.Nil { + return nil, fmt.Errorf("%w: token has no tenant", domain.ErrUnauthorized) } // Update last used timestamp - if err := s.repo.UpdateLastUsed(ctx, pat.ID); err != nil { + if err := s.repo.UpdateLastUsed(ctx, pat.TenantID, pat.ID); err != nil { // Log error but don't fail validation fmt.Printf("Failed to update last used timestamp: %v\n", err) } @@ -175,22 +192,22 @@ func (s *PersonalAccessTokenService) HasScope(pat *domain.PersonalAccessToken, r return false } -// ListUserTokens gets all tokens for a user -func (s *PersonalAccessTokenService) ListUserTokens(ctx context.Context, userID uuid.UUID) ([]*domain.PersonalAccessToken, error) { - return s.repo.GetByUserID(ctx, userID) +// ListUserTokens lists the tokens userID minted in tenantID. Tokens the same +// person minted in another organization are not shown. +func (s *PersonalAccessTokenService) ListUserTokens(ctx context.Context, tenantID, userID uuid.UUID) ([]*domain.PersonalAccessToken, error) { + return s.repo.ListByOwner(ctx, tenantID, userID) } -// RevokeToken revokes a token -func (s *PersonalAccessTokenService) RevokeToken(ctx context.Context, tokenID uuid.UUID, userID uuid.UUID) error { - // First verify the token belongs to the user - pat, err := s.repo.GetByID(ctx, tokenID) +// RevokeToken deletes a token userID owns in tenantID. Someone else's token, or +// the caller's own token from another organization, is ErrNotFound: whether it +// exists is not the caller's business. +func (s *PersonalAccessTokenService) RevokeToken(ctx context.Context, tenantID, tokenID, userID uuid.UUID) error { + removed, err := s.repo.DeleteByOwner(ctx, tenantID, userID, tokenID) if err != nil { - return fmt.Errorf("token not found") + return err } - - if pat.UserID != userID { - return fmt.Errorf("unauthorized") + if !removed { + return domain.ErrNotFound } - - return s.repo.Delete(ctx, tokenID) + return nil } diff --git a/backend/internal/auth/pat_test.go b/backend/internal/auth/pat_test.go new file mode 100644 index 00000000..e8a01230 --- /dev/null +++ b/backend/internal/auth/pat_test.go @@ -0,0 +1,196 @@ +// Copyright (c) 2026 OpenDefender Contributors +// SPDX-License-Identifier: AGPL-3.0-only +// This program is free software: you can redistribute it and/or modify it under +// the terms of the GNU Affero General Public License v3.0 (see LICENSE). + +package auth + +import ( + "context" + "errors" + "strings" + "testing" + "time" + + "github.com/google/uuid" + "gorm.io/gorm" + + "github.com/opendefender/openrisk/internal/domain" +) + +// memPATRepo is an in-memory PersonalAccessTokenRepository that applies the +// same tenant and owner predicates as the GORM one, so a service test fails if +// the service stops passing them. +type memPATRepo struct { + rows map[uuid.UUID]*domain.PersonalAccessToken +} + +func newMemPATRepo() *memPATRepo { + return &memPATRepo{rows: map[uuid.UUID]*domain.PersonalAccessToken{}} +} + +func (r *memPATRepo) Create(_ context.Context, t *domain.PersonalAccessToken) error { + t.ID = uuid.New() + cp := *t + r.rows[t.ID] = &cp + return nil +} + +func (r *memPATRepo) GetByTokenHash(_ context.Context, hash string) (*domain.PersonalAccessToken, error) { + for _, t := range r.rows { + if t.TokenHash == hash { + cp := *t + return &cp, nil + } + } + return nil, gorm.ErrRecordNotFound +} + +func (r *memPATRepo) ListByOwner(_ context.Context, tenantID, userID uuid.UUID) ([]*domain.PersonalAccessToken, error) { + var out []*domain.PersonalAccessToken + for _, t := range r.rows { + if t.TenantID == tenantID && t.UserID == userID { + cp := *t + out = append(out, &cp) + } + } + return out, nil +} + +func (r *memPATRepo) UpdateLastUsed(_ context.Context, tenantID, id uuid.UUID) error { + if t, ok := r.rows[id]; ok && t.TenantID == tenantID { + now := time.Now() + t.LastUsedAt = &now + } + return nil +} + +func (r *memPATRepo) DeleteByOwner(_ context.Context, tenantID, userID, id uuid.UUID) (bool, error) { + t, ok := r.rows[id] + if !ok || t.TenantID != tenantID || t.UserID != userID { + return false, nil + } + delete(r.rows, id) + return true, nil +} + +func TestCreateToken_Success(t *testing.T) { + ctx := context.Background() + svc := NewPersonalAccessTokenService(newMemPATRepo()) + user, tenant := uuid.New(), uuid.New() + + pat, raw, err := svc.CreateToken(ctx, user, tenant, "CI/CD", "", []string{"*"}, nil) + if err != nil { + t.Fatalf("create: %v", err) + } + if !strings.HasPrefix(raw, PATValuePrefix) { + t.Fatalf("value %q does not start with %q", raw, PATValuePrefix) + } + if pat.TenantID != tenant || pat.UserID != user { + t.Fatalf("token stored for user %s tenant %s, want %s %s", pat.UserID, pat.TenantID, user, tenant) + } + if strings.Contains(pat.TokenHash, strings.TrimPrefix(raw, PATValuePrefix)) { + t.Fatal("the stored hash contains the raw secret") + } + + got, err := svc.ValidateToken(ctx, raw) + if err != nil { + t.Fatalf("the value CreateToken returned does not validate: %v", err) + } + if got.ID != pat.ID || got.TenantID != tenant { + t.Fatalf("validated the wrong token: %+v", got) + } +} + +func TestCreateToken_RequiresTenant(t *testing.T) { + svc := NewPersonalAccessTokenService(newMemPATRepo()) + _, _, err := svc.CreateToken(context.Background(), uuid.New(), uuid.Nil, "x", "", []string{"*"}, nil) + if !errors.Is(err, domain.ErrValidation) { + t.Fatalf("got %v, want ErrValidation", err) + } +} + +// A token minted before #782 has no "orsk_" prefix and must keep working. +func TestValidateToken_LegacyFormat(t *testing.T) { + ctx := context.Background() + svc := NewPersonalAccessTokenService(newMemPATRepo()) + _, raw, err := svc.CreateToken(ctx, uuid.New(), uuid.New(), "old", "", []string{"*"}, nil) + if err != nil { + t.Fatal(err) + } + if _, err := svc.ValidateToken(ctx, strings.TrimPrefix(raw, PATValuePrefix)); err != nil { + t.Fatalf("legacy form rejected: %v", err) + } +} + +func TestValidateToken_Unauthorized(t *testing.T) { + ctx := context.Background() + repo := newMemPATRepo() + svc := NewPersonalAccessTokenService(repo) + pat, raw, err := svc.CreateToken(ctx, uuid.New(), uuid.New(), "x", "", []string{"*"}, nil) + if err != nil { + t.Fatal(err) + } + prefix, _, _ := SplitPATValue(raw) + + past := time.Now().Add(-time.Minute) + repo.rows[pat.ID].ExpiresAt = &past + expired := raw + + for name, value := range map[string]string{ + "empty": "", + "jwt": "eyJhbGciOi.J9.sig", + "unknown": PATValuePrefix + prefix + "_" + strings.Repeat("0", 64), + "wrong prefix": PATValuePrefix + "deadbeef_" + strings.Split(raw, "_")[2], + "expired": expired, + "missing parts": PATValuePrefix + prefix, + } { + if _, err := svc.ValidateToken(ctx, value); !errors.Is(err, domain.ErrUnauthorized) { + t.Errorf("%s: got %v, want ErrUnauthorized", name, err) + } + } +} + +func TestRevokeToken_Success(t *testing.T) { + ctx := context.Background() + svc := NewPersonalAccessTokenService(newMemPATRepo()) + user, tenant := uuid.New(), uuid.New() + pat, raw, _ := svc.CreateToken(ctx, user, tenant, "x", "", []string{"*"}, nil) + + if err := svc.RevokeToken(ctx, tenant, pat.ID, user); err != nil { + t.Fatalf("revoke: %v", err) + } + if _, err := svc.ValidateToken(ctx, raw); err == nil { + t.Fatal("a revoked token still authenticates") + } +} + +func TestRevokeToken_NotFound(t *testing.T) { + svc := NewPersonalAccessTokenService(newMemPATRepo()) + err := svc.RevokeToken(context.Background(), uuid.New(), uuid.New(), uuid.New()) + if !errors.Is(err, domain.ErrNotFound) { + t.Fatalf("got %v, want ErrNotFound", err) + } +} + +// Another user, or the owner acting from another organization, cannot revoke +// the token, cannot list it, and learns nothing: the answer is ErrNotFound. +func TestRevokeToken_Unauthorized(t *testing.T) { + ctx := context.Background() + svc := NewPersonalAccessTokenService(newMemPATRepo()) + owner, tenantA, tenantB := uuid.New(), uuid.New(), uuid.New() + pat, raw, _ := svc.CreateToken(ctx, owner, tenantA, "x", "", []string{"*"}, nil) + + if err := svc.RevokeToken(ctx, tenantA, pat.ID, uuid.New()); !errors.Is(err, domain.ErrNotFound) { + t.Errorf("other user: got %v, want ErrNotFound", err) + } + if err := svc.RevokeToken(ctx, tenantB, pat.ID, owner); !errors.Is(err, domain.ErrNotFound) { + t.Errorf("owner from tenant B: got %v, want ErrNotFound", err) + } + if list, _ := svc.ListUserTokens(ctx, tenantB, owner); len(list) != 0 { + t.Errorf("tenant B lists %d of tenant A's tokens", len(list)) + } + if _, err := svc.ValidateToken(ctx, raw); err != nil { + t.Fatalf("the token should still work after refused revokes: %v", err) + } +} diff --git a/backend/internal/domain/membership.go b/backend/internal/domain/membership.go index 9ab84c82..5721e6da 100644 --- a/backend/internal/domain/membership.go +++ b/backend/internal/domain/membership.go @@ -148,8 +148,12 @@ func (AuthAuditLog) TableName() string { // PersonalAccessToken represents a PAT for API access type PersonalAccessToken struct { - ID uuid.UUID `gorm:"type:uuid;primaryKey;default:gen_random_uuid()" json:"id"` - UserID uuid.UUID `gorm:"type:uuid;index;not null" json:"user_id"` + ID uuid.UUID `gorm:"type:uuid;primaryKey;default:gen_random_uuid()" json:"id"` + UserID uuid.UUID `gorm:"type:uuid;index;not null" json:"user_id"` + // TenantID is the organization the token was minted in and the only one it + // can act in (#782). Before it existed a PAT followed its owner's default + // organization, so changing that default silently moved every token. + TenantID uuid.UUID `gorm:"type:uuid;index;not null" json:"tenant_id"` Name string `gorm:"not null" json:"name"` Description string `json:"description"` TokenHash string `gorm:"type:varchar(64);uniqueIndex;not null" json:"-"` // SHA256 hash diff --git a/backend/internal/handler/auth/pat_handler.go b/backend/internal/handler/auth/pat_handler.go index f11af064..3064f8a5 100644 --- a/backend/internal/handler/auth/pat_handler.go +++ b/backend/internal/handler/auth/pat_handler.go @@ -6,12 +6,14 @@ package auth import ( + "errors" "time" "github.com/gofiber/fiber/v2" "github.com/google/uuid" coreauth "github.com/opendefender/openrisk/internal/auth" + "github.com/opendefender/openrisk/internal/domain" ) // PATHandler exposes DB-backed Personal Access Token management (L5): create, list, @@ -38,7 +40,7 @@ type createPATRequest struct { func (h *PATHandler) Create(c *fiber.Ctx) error { userID := ctxUUID(c, "user_id") tenantID := ctxUUID(c, "tenant_id") - if userID == uuid.Nil { + if userID == uuid.Nil || tenantID == uuid.Nil { return c.Status(fiber.StatusUnauthorized).JSON(fiber.Map{"error": "not authenticated"}) } if c.Locals("is_pat") == true { @@ -62,7 +64,7 @@ func (h *PATHandler) Create(c *fiber.Ctx) error { expiresAt = &t } - pat, raw, err := h.svc.CreateToken(c.UserContext(), userID, req.Name, req.Description, req.Scopes, expiresAt) + pat, raw, err := h.svc.CreateToken(c.UserContext(), userID, tenantID, req.Name, req.Description, req.Scopes, expiresAt) if err != nil { return c.Status(fiber.StatusInternalServerError).JSON(fiber.Map{"error": "failed to create token"}) } @@ -83,13 +85,15 @@ func (h *PATHandler) Create(c *fiber.Ctx) error { }) } -// List returns the caller's tokens (metadata only, never the secret). +// List returns the tokens the caller minted in the current organization +// (metadata only, never the secret). func (h *PATHandler) List(c *fiber.Ctx) error { userID := ctxUUID(c, "user_id") - if userID == uuid.Nil { + tenantID := ctxUUID(c, "tenant_id") + if userID == uuid.Nil || tenantID == uuid.Nil { return c.Status(fiber.StatusUnauthorized).JSON(fiber.Map{"error": "not authenticated"}) } - tokens, err := h.svc.ListUserTokens(c.UserContext(), userID) + tokens, err := h.svc.ListUserTokens(c.UserContext(), tenantID, userID) if err != nil { return c.Status(fiber.StatusInternalServerError).JSON(fiber.Map{"error": "failed to list tokens"}) } @@ -100,15 +104,18 @@ func (h *PATHandler) List(c *fiber.Ctx) error { func (h *PATHandler) Revoke(c *fiber.Ctx) error { userID := ctxUUID(c, "user_id") tenantID := ctxUUID(c, "tenant_id") - if userID == uuid.Nil { + if userID == uuid.Nil || tenantID == uuid.Nil { return c.Status(fiber.StatusUnauthorized).JSON(fiber.Map{"error": "not authenticated"}) } tokenID, err := uuid.Parse(c.Params("id")) if err != nil { return c.Status(fiber.StatusBadRequest).JSON(fiber.Map{"error": "invalid token id"}) } - if err := h.svc.RevokeToken(c.UserContext(), tokenID, userID); err != nil { - return c.Status(fiber.StatusNotFound).JSON(fiber.Map{"error": "token not found"}) + if err := h.svc.RevokeToken(c.UserContext(), tenantID, tokenID, userID); err != nil { + if errors.Is(err, domain.ErrNotFound) { + return c.Status(fiber.StatusNotFound).JSON(fiber.Map{"error": "token not found"}) + } + return c.Status(fiber.StatusInternalServerError).JSON(fiber.Map{"error": "failed to revoke token"}) } if h.audit != nil { uid, tid := userID, tenantID diff --git a/backend/internal/infrastructure/database/prepare.go b/backend/internal/infrastructure/database/prepare.go index 7393aac0..437dccb0 100644 --- a/backend/internal/infrastructure/database/prepare.go +++ b/backend/internal/infrastructure/database/prepare.go @@ -45,9 +45,51 @@ func PrepareForAutoMigrate(db *gorm.DB) error { if err := prepareOnboardingProgressPerOrg(db); err != nil { return fmt.Errorf("onboarding_progress: %w", err) } + if err := backfillPersonalAccessTokenTenant(db); err != nil { + return fmt.Errorf("personal_access_tokens: %w", err) + } return nil } +// backfillPersonalAccessTokenTenant gives every pre-existing personal access +// token the tenant it already acted in, before AutoMigrate makes tenant_id NOT +// NULL (#782). +// +// Before tenant_id existed, the PAT middleware resolved a token's tenant from +// its owner's default organization on every request. Copying +// users.default_org_id therefore changes no token's behaviour. A token whose +// owner has no default organization never authenticated a single request (the +// resolver refused it), so it is deleted rather than guessed at: a credential +// pinned to the wrong tenant is worse than one that is gone. +// Mirrors migrations/0067_personal_access_tokens_tenant.up.sql. Idempotent; a +// no-op outside Postgres and on a fresh database. +func backfillPersonalAccessTokenTenant(db *gorm.DB) error { + if db.Dialector.Name() != "postgres" { + return nil + } + if !db.Migrator().HasTable("personal_access_tokens") { + return nil + } + stmts := []string{ + `ALTER TABLE personal_access_tokens ADD COLUMN IF NOT EXISTS tenant_id uuid`, + `UPDATE personal_access_tokens p + SET tenant_id = u.default_org_id + FROM users u + WHERE p.user_id = u.id + AND p.tenant_id IS NULL + AND u.default_org_id IS NOT NULL`, + `DELETE FROM personal_access_tokens WHERE tenant_id IS NULL`, + } + return db.Transaction(func(tx *gorm.DB) error { + for _, s := range stmts { + if err := tx.Exec(s).Error; err != nil { + return err + } + } + return nil + }) +} + // prepareOnboardingProgressPerOrg moves onboarding_progress from one row per // user to one row per (organization, user) (#735). The old unique index on // user_id made a member of two organizations share a single row, which moved diff --git a/backend/internal/infrastructure/database/prepare_pat_pg_test.go b/backend/internal/infrastructure/database/prepare_pat_pg_test.go new file mode 100644 index 00000000..7c16c906 --- /dev/null +++ b/backend/internal/infrastructure/database/prepare_pat_pg_test.go @@ -0,0 +1,92 @@ +// Copyright (c) 2026 OpenDefender Contributors +// SPDX-License-Identifier: AGPL-3.0-only +// This program is free software: you can redistribute it and/or modify it under +// the terms of the GNU Affero General Public License v3.0 (see LICENSE). + +package database + +import ( + "os" + "strings" + "testing" + + "github.com/google/uuid" + "gorm.io/driver/postgres" + "gorm.io/gorm" + "gorm.io/gorm/logger" +) + +// A database from before #782 has personal_access_tokens without tenant_id. +// The backfill must give each token the tenant it already acted in (its +// owner's default organization), drop the tokens that never authenticated +// anything, and run twice without harm. Runs in a private schema. +func TestBackfillPersonalAccessTokenTenant_Postgres(t *testing.T) { + dsn := os.Getenv("DATABASE_URL") + if dsn == "" { + t.Skip("DATABASE_URL not set") + } + cfg := &gorm.Config{Logger: logger.Default.LogMode(logger.Silent)} + admin, err := gorm.Open(postgres.Open(dsn), cfg) + if err != nil { + t.Fatalf("open: %v", err) + } + schema := "patfill782_" + uuid.NewString()[:8] + if err := admin.Exec("CREATE SCHEMA " + schema).Error; err != nil { + t.Fatalf("create schema: %v", err) + } + t.Cleanup(func() { admin.Exec("DROP SCHEMA " + schema + " CASCADE") }) + + sep := "?" + if strings.Contains(dsn, "?") { + sep = "&" + } + scoped := dsn + sep + "search_path=" + schema + if !strings.Contains(dsn, "://") { + scoped = dsn + " search_path=" + schema + } + db, err := gorm.Open(postgres.Open(scoped), cfg) + if err != nil { + t.Fatalf("open schema: %v", err) + } + t.Cleanup(func() { + if s, err := db.DB(); err == nil { + _ = s.Close() + } + }) + + exec := func(sql string, args ...interface{}) { + t.Helper() + if err := db.Exec(sql, args...).Error; err != nil { + t.Fatalf("%s: %v", sql, err) + } + } + // The pre-#782 shape: no tenant_id. + exec(`CREATE TABLE users (id uuid PRIMARY KEY, default_org_id uuid)`) + exec(`CREATE TABLE personal_access_tokens (id uuid PRIMARY KEY, user_id uuid NOT NULL, token_hash varchar(64) NOT NULL)`) + + org := uuid.New() + withOrg, noOrg := uuid.New(), uuid.New() + exec(`INSERT INTO users VALUES (?, ?), (?, NULL)`, withOrg, org, noOrg) + kept, dropped := uuid.New(), uuid.New() + exec(`INSERT INTO personal_access_tokens VALUES (?, ?, 'a'), (?, ?, 'b')`, kept, withOrg, dropped, noOrg) + + for i := 0; i < 2; i++ { // idempotent + if err := backfillPersonalAccessTokenTenant(db); err != nil { + t.Fatalf("run %d: %v", i+1, err) + } + } + + var rows []struct { + ID uuid.UUID + TenantID *uuid.UUID + } + if err := db.Raw(`SELECT id, tenant_id FROM personal_access_tokens`).Scan(&rows).Error; err != nil { + t.Fatal(err) + } + if len(rows) != 1 || rows[0].ID != kept { + t.Fatalf("rows after backfill = %+v, want only %s", rows, kept) + } + if rows[0].TenantID == nil || *rows[0].TenantID != org { + t.Fatalf("tenant_id = %v, want the owner's default org %s", rows[0].TenantID, org) + } +} diff --git a/backend/internal/infrastructure/repository/gorm_personal_access_token_repository.go b/backend/internal/infrastructure/repository/gorm_personal_access_token_repository.go index ec20d6f1..824ec8e6 100644 --- a/backend/internal/infrastructure/repository/gorm_personal_access_token_repository.go +++ b/backend/internal/infrastructure/repository/gorm_personal_access_token_repository.go @@ -30,17 +30,9 @@ func (r *GormPersonalAccessTokenRepository) Create(ctx context.Context, token *d return r.db.WithContext(ctx).Create(token).Error } -// GetByID gets a PAT by ID -func (r *GormPersonalAccessTokenRepository) GetByID(ctx context.Context, id uuid.UUID) (*domain.PersonalAccessToken, error) { - var token domain.PersonalAccessToken - err := r.db.WithContext(ctx).First(&token, id).Error - if err != nil { - return nil, err - } - return &token, nil -} - -// GetByTokenHash gets a PAT by its hash (for validation) +// GetByTokenHash gets a PAT by its hash. This is the authentication lookup: it +// runs before any tenant is known, and the row it returns names the only tenant +// the token may act in. Every other method below filters on tenant_id. func (r *GormPersonalAccessTokenRepository) GetByTokenHash(ctx context.Context, hash string) (*domain.PersonalAccessToken, error) { var token domain.PersonalAccessToken err := r.db.WithContext(ctx). @@ -52,28 +44,32 @@ func (r *GormPersonalAccessTokenRepository) GetByTokenHash(ctx context.Context, return &token, nil } -// GetByUserID gets all PATs for a user -func (r *GormPersonalAccessTokenRepository) GetByUserID(ctx context.Context, userID uuid.UUID) ([]*domain.PersonalAccessToken, error) { +// ListByOwner lists the tokens userID minted in tenantID, newest first. +func (r *GormPersonalAccessTokenRepository) ListByOwner(ctx context.Context, tenantID, userID uuid.UUID) ([]*domain.PersonalAccessToken, error) { var tokens []*domain.PersonalAccessToken err := r.db.WithContext(ctx). - Where("user_id = ?", userID). + Where("tenant_id = ? AND user_id = ?", tenantID, userID). Order("created_at DESC"). Find(&tokens).Error return tokens, err } -// UpdateLastUsed updates the last used timestamp -func (r *GormPersonalAccessTokenRepository) UpdateLastUsed(ctx context.Context, id uuid.UUID) error { +// UpdateLastUsed stamps the token's last use. +func (r *GormPersonalAccessTokenRepository) UpdateLastUsed(ctx context.Context, tenantID, id uuid.UUID) error { return r.db.WithContext(ctx). Model(&domain.PersonalAccessToken{}). - Where("id = ?", id). + Where("tenant_id = ? AND id = ?", tenantID, id). Updates(map[string]interface{}{ "last_used_at": time.Now(), "updated_at": time.Now(), }).Error } -// Delete deletes a PAT -func (r *GormPersonalAccessTokenRepository) Delete(ctx context.Context, id uuid.UUID) error { - return r.db.WithContext(ctx).Delete(&domain.PersonalAccessToken{}, id).Error +// DeleteByOwner deletes the token only when userID owns it in tenantID. A +// foreign id, user or tenant matches no row and reports false. +func (r *GormPersonalAccessTokenRepository) DeleteByOwner(ctx context.Context, tenantID, userID, id uuid.UUID) (bool, error) { + res := r.db.WithContext(ctx). + Where("tenant_id = ? AND user_id = ? AND id = ?", tenantID, userID, id). + Delete(&domain.PersonalAccessToken{}) + return res.RowsAffected > 0, res.Error } diff --git a/backend/internal/infrastructure/repository/gorm_personal_access_token_repository_pg_test.go b/backend/internal/infrastructure/repository/gorm_personal_access_token_repository_pg_test.go new file mode 100644 index 00000000..3eece563 --- /dev/null +++ b/backend/internal/infrastructure/repository/gorm_personal_access_token_repository_pg_test.go @@ -0,0 +1,186 @@ +// Copyright (c) 2026 OpenDefender Contributors +// SPDX-License-Identifier: AGPL-3.0-only +// This program is free software: you can redistribute it and/or modify it under +// the terms of the GNU Affero General Public License v3.0 (see LICENSE). + +package repository + +import ( + "context" + "fmt" + "os" + "strings" + "testing" + + "github.com/google/uuid" + "gorm.io/driver/postgres" + "gorm.io/gorm" + "gorm.io/gorm/logger" + + "github.com/opendefender/openrisk/internal/domain" +) + +// patPGSchema creates a private schema holding the users, organizations and +// personal_access_tokens tables, and returns a function that opens a NEW +// connection pool on it. Each pool is a separate *gorm.DB, so a value written +// through one and read through another went through PostgreSQL, not through +// any process memory. The schema is dropped when the test ends. +func patPGSchema(t *testing.T) func() *gorm.DB { + t.Helper() + dsn := os.Getenv("DATABASE_URL") + if dsn == "" { + t.Skip("DATABASE_URL not set") + } + schema := "pat782_" + strings.ReplaceAll(uuid.NewString()[:8], "-", "") + cfg := &gorm.Config{Logger: logger.Default.LogMode(logger.Silent)} + + admin, err := gorm.Open(postgres.Open(dsn), cfg) + if err != nil { + t.Fatalf("open: %v", err) + } + if err := admin.Exec("CREATE SCHEMA " + schema).Error; err != nil { + t.Fatalf("create schema: %v", err) + } + t.Cleanup(func() { + admin.Exec("DROP SCHEMA " + schema + " CASCADE") + if sqlDB, err := admin.DB(); err == nil { + _ = sqlDB.Close() + } + }) + + sep := "?" + if strings.Contains(dsn, "?") { + sep = "&" + } + scoped := dsn + sep + "search_path=" + schema + if !strings.Contains(dsn, "://") { // key=value DSN + scoped = dsn + " search_path=" + schema + } + + open := func() *gorm.DB { + t.Helper() + db, err := gorm.Open(postgres.Open(scoped), cfg) + if err != nil { + t.Fatalf("open %s: %v", schema, err) + } + t.Cleanup(func() { + if sqlDB, err := db.DB(); err == nil { + _ = sqlDB.Close() + } + }) + return db + } + + if err := open().AutoMigrate(&domain.Organization{}, &domain.User{}, &domain.PersonalAccessToken{}); err != nil { + t.Fatalf("migrate: %v", err) + } + return open +} + +func seedPATOwner(t *testing.T, db *gorm.DB) (userID uuid.UUID) { + t.Helper() + userID = uuid.New() + err := db.Exec(`INSERT INTO users (id, email, username, password, full_name, created_at, updated_at) + VALUES (?, ?, ?, 'x', 'Owner', now(), now())`, + userID, userID.String()+"@example.test", userID.String()).Error + if err != nil { + t.Fatalf("seed user: %v", err) + } + return userID +} + +func newPATRow(userID, tenantID uuid.UUID, hash string) *domain.PersonalAccessToken { + return &domain.PersonalAccessToken{ + UserID: userID, + TenantID: tenantID, + Name: "CI/CD", + TokenHash: hash, + TokenPrefix: hash[:8], + Scopes: []byte(`["*"]`), + } +} + +// Criterion 2 of #782: a token written through one connection pool is found by +// its hash through a pool opened afterwards — a restarted server. +func TestPATRepo_Postgres_SurvivesRestart(t *testing.T) { + open := patPGSchema(t) + ctx := context.Background() + + before := open() + user, tenant := seedPATOwner(t, before), uuid.New() + hash := fmt.Sprintf("%064x", uuid.New().ID()) + row := newPATRow(user, tenant, hash) + if err := NewGormPersonalAccessTokenRepository(before).Create(ctx, row); err != nil { + t.Fatalf("create: %v", err) + } + if row.ID == uuid.Nil { + t.Fatal("Postgres did not assign an id") + } + sqlDB, _ := before.DB() + _ = sqlDB.Close() + + after := NewGormPersonalAccessTokenRepository(open()) + got, err := after.GetByTokenHash(ctx, hash) + if err != nil { + t.Fatalf("token lost across the reconnect: %v", err) + } + if got.ID != row.ID || got.TenantID != tenant || got.UserID != user { + t.Fatalf("read back %+v, want id %s tenant %s user %s", got, row.ID, tenant, user) + } + if err := after.UpdateLastUsed(ctx, tenant, got.ID); err != nil { + t.Fatalf("update last used: %v", err) + } + again, _ := after.GetByTokenHash(ctx, hash) + if again.LastUsedAt == nil { + t.Error("last_used_at was not stamped") + } +} + +// Criterion 4 of #782, at the SQL level: the same person's tokens in two +// organizations never cross. Listing, revoking and stamping from the other +// tenant all match zero rows. +func TestPATRepo_TenantIsolation(t *testing.T) { + open := patPGSchema(t) + ctx := context.Background() + db := open() + repo := NewGormPersonalAccessTokenRepository(db) + + owner, other := seedPATOwner(t, db), seedPATOwner(t, db) + tenantA, tenantB := uuid.New(), uuid.New() + inA := newPATRow(owner, tenantA, fmt.Sprintf("%064x", 0xA)) + inB := newPATRow(owner, tenantB, fmt.Sprintf("%064x", 0xB)) + for _, r := range []*domain.PersonalAccessToken{inA, inB} { + if err := repo.Create(ctx, r); err != nil { + t.Fatalf("create: %v", err) + } + } + + listA, err := repo.ListByOwner(ctx, tenantA, owner) + if err != nil || len(listA) != 1 || listA[0].ID != inA.ID { + t.Fatalf("tenant A lists %v (err %v), want only its own token", listA, err) + } + if list, _ := repo.ListByOwner(ctx, tenantA, other); len(list) != 0 { + t.Errorf("another user in tenant A lists %d tokens", len(list)) + } + + if removed, err := repo.DeleteByOwner(ctx, tenantB, owner, inA.ID); err != nil || removed { + t.Errorf("revoking A's token from B: removed=%v err=%v, want false", removed, err) + } + if removed, err := repo.DeleteByOwner(ctx, tenantA, other, inA.ID); err != nil || removed { + t.Errorf("revoking A's token as another user: removed=%v err=%v, want false", removed, err) + } + + if err := repo.UpdateLastUsed(ctx, tenantB, inA.ID); err != nil { + t.Fatal(err) + } + if got, _ := repo.GetByTokenHash(ctx, inA.TokenHash); got.LastUsedAt != nil { + t.Error("a stamp issued from tenant B touched tenant A's token") + } + + if removed, err := repo.DeleteByOwner(ctx, tenantA, owner, inA.ID); err != nil || !removed { + t.Fatalf("owner revoking in A: removed=%v err=%v, want true", removed, err) + } + if list, _ := repo.ListByOwner(ctx, tenantB, owner); len(list) != 1 { + t.Errorf("revoking in A removed tenant B's token too: %d left", len(list)) + } +} diff --git a/backend/internal/infrastructure/repository/interfaces.go b/backend/internal/infrastructure/repository/interfaces.go index 25c62983..6bc42e5b 100644 --- a/backend/internal/infrastructure/repository/interfaces.go +++ b/backend/internal/infrastructure/repository/interfaces.go @@ -20,9 +20,13 @@ type AuthAuditLogRepository interface { type PersonalAccessTokenRepository interface { Create(ctx context.Context, token *domain.PersonalAccessToken) error - GetByID(ctx context.Context, id uuid.UUID) (*domain.PersonalAccessToken, error) + // GetByTokenHash is the authentication lookup and is deliberately not + // tenant-filtered: no tenant is known until the token is found, and the row + // it returns carries the one tenant the token may act in. GetByTokenHash(ctx context.Context, hash string) (*domain.PersonalAccessToken, error) - GetByUserID(ctx context.Context, userID uuid.UUID) ([]*domain.PersonalAccessToken, error) - UpdateLastUsed(ctx context.Context, id uuid.UUID) error - Delete(ctx context.Context, id uuid.UUID) error + ListByOwner(ctx context.Context, tenantID, userID uuid.UUID) ([]*domain.PersonalAccessToken, error) + UpdateLastUsed(ctx context.Context, tenantID, id uuid.UUID) error + // DeleteByOwner removes the token only when it belongs to userID in + // tenantID, and reports whether a row was removed. + DeleteByOwner(ctx context.Context, tenantID, userID, id uuid.UUID) (bool, error) } diff --git a/backend/internal/middleware/pat_e2e_test.go b/backend/internal/middleware/pat_e2e_test.go new file mode 100644 index 00000000..d2e8412e --- /dev/null +++ b/backend/internal/middleware/pat_e2e_test.go @@ -0,0 +1,161 @@ +// Copyright (c) 2026 OpenDefender Contributors +// SPDX-License-Identifier: AGPL-3.0-only +// This program is free software: you can redistribute it and/or modify it under +// the terms of the GNU Affero General Public License v3.0 (see LICENSE). + +package middleware + +import ( + "context" + "crypto/rand" + "crypto/rsa" + "encoding/json" + "fmt" + "io" + "net/http/httptest" + "testing" + + "github.com/gofiber/fiber/v2" + "github.com/google/uuid" + "gorm.io/gorm" + + "github.com/opendefender/openrisk/internal/auth" + "github.com/opendefender/openrisk/internal/domain" + authpkg "github.com/opendefender/openrisk/pkg/auth" +) + +// patRepoFake stores tokens in memory with the GORM repository's predicates. +type patRepoFake struct{ rows []*domain.PersonalAccessToken } + +func (r *patRepoFake) Create(_ context.Context, t *domain.PersonalAccessToken) error { + t.ID = uuid.New() + r.rows = append(r.rows, t) + return nil +} +func (r *patRepoFake) GetByTokenHash(_ context.Context, h string) (*domain.PersonalAccessToken, error) { + for _, t := range r.rows { + if t.TokenHash == h { + return t, nil + } + } + return nil, gorm.ErrRecordNotFound +} +func (r *patRepoFake) ListByOwner(context.Context, uuid.UUID, uuid.UUID) ([]*domain.PersonalAccessToken, error) { + return nil, nil +} +func (r *patRepoFake) UpdateLastUsed(context.Context, uuid.UUID, uuid.UUID) error { return nil } +func (r *patRepoFake) DeleteByOwner(context.Context, uuid.UUID, uuid.UUID, uuid.UUID) (bool, error) { + return false, nil +} + +// patTestServer mounts the production chain: PATMiddleware, then the RS256 JWT +// gate, then a protected route that echoes the resolved tenant and permissions. +// members maps user → the organizations they are an active member of, with +// their permissions there; the resolver refuses any other pair, as +// resolveSessionClaimsForOrg does. +func patTestServer(t *testing.T, svc *auth.PersonalAccessTokenService, members map[uuid.UUID]map[uuid.UUID][]string) *fiber.App { + t.Helper() + pk, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatal(err) + } + keys := &authpkg.RSAKeys{PrivateKey: pk, PublicKey: &pk.PublicKey} + + resolve := func(_ context.Context, uid, orgID uuid.UUID) (*auth.SessionClaims, error) { + perms, ok := members[uid][orgID] + if !ok { + return nil, fmt.Errorf("not an active member") + } + return &auth.SessionClaims{TenantID: orgID, Permissions: perms}, nil + } + + app := fiber.New() + api := app.Group("/api/v1") + api.Use(PATMiddleware(svc, resolve)) + api.Use(Protected(keys, nil)) + api.Get("/risks", func(c *fiber.Ctx) error { + return c.JSON(fiber.Map{"tenant_id": c.Locals("tenant_id"), "permissions": c.Locals("permissions")}) + }) + return app +} + +func callRisks(t *testing.T, app *fiber.App, bearer string) (int, map[string]interface{}) { + t.Helper() + req := httptest.NewRequest("GET", "/api/v1/risks", nil) + if bearer != "" { + req.Header.Set("Authorization", "Bearer "+bearer) + } + resp, err := app.Test(req, -1) + if err != nil { + t.Fatal(err) + } + defer resp.Body.Close() + body, _ := io.ReadAll(resp.Body) + var out map[string]interface{} + _ = json.Unmarshal(body, &out) + return resp.StatusCode, out +} + +// Criterion 1 of #782: the value Settings hands the user authenticates a real +// protected route, and the same call without it is refused. +func TestPAT_AuthenticatesProtectedRoute(t *testing.T) { + ctx := context.Background() + svc := auth.NewPersonalAccessTokenService(&patRepoFake{}) + user, tenant := uuid.New(), uuid.New() + app := patTestServer(t, svc, map[uuid.UUID]map[uuid.UUID][]string{ + user: {tenant: {"risks:read", "assets:read"}}, + }) + + _, raw, err := svc.CreateToken(ctx, user, tenant, "CI/CD", "", []string{"*"}, nil) + if err != nil { + t.Fatal(err) + } + + status, body := callRisks(t, app, raw) + if status != fiber.StatusOK { + t.Fatalf("with token: status %d, body %v", status, body) + } + if body["tenant_id"] != tenant.String() { + t.Errorf("tenant_id = %v, want %s", body["tenant_id"], tenant) + } + // "*" on a non-admin member means exactly the member's permissions. + perms, _ := body["permissions"].([]interface{}) + if len(perms) != 2 { + t.Errorf("permissions = %v, want the owner's two", body["permissions"]) + } + + if status, _ := callRisks(t, app, ""); status != fiber.StatusUnauthorized { + t.Errorf("without token: status %d, want 401", status) + } + if status, _ := callRisks(t, app, raw+"0"); status != fiber.StatusUnauthorized { + t.Errorf("with a tampered token: status %d, want 401", status) + } +} + +// Criterion 4 of #782: a token minted in tenant A acts in A only. Its owner +// being a member of B too does not let it reach B, and once the owner leaves A +// the token stops working instead of falling back to another organization. +func TestPAT_CrossTenant(t *testing.T) { + ctx := context.Background() + svc := auth.NewPersonalAccessTokenService(&patRepoFake{}) + user, tenantA, tenantB := uuid.New(), uuid.New(), uuid.New() + members := map[uuid.UUID]map[uuid.UUID][]string{ + user: {tenantA: {"*"}, tenantB: {"*"}}, + } + app := patTestServer(t, svc, members) + + _, raw, err := svc.CreateToken(ctx, user, tenantA, "A", "", []string{"*"}, nil) + if err != nil { + t.Fatal(err) + } + + status, body := callRisks(t, app, raw) + if status != fiber.StatusOK || body["tenant_id"] != tenantA.String() { + t.Fatalf("got %d tenant %v, want 200 in tenant A", status, body["tenant_id"]) + } + + delete(members[user], tenantA) + if status, body := callRisks(t, app, raw); status != fiber.StatusUnauthorized { + t.Fatalf("owner left A: got %d tenant %v, want 401", status, body["tenant_id"]) + } +} diff --git a/backend/internal/middleware/pat_middleware.go b/backend/internal/middleware/pat_middleware.go index ac395e7a..7274897a 100644 --- a/backend/internal/middleware/pat_middleware.go +++ b/backend/internal/middleware/pat_middleware.go @@ -17,7 +17,8 @@ import ( // PATMiddleware authenticates Personal Access Tokens (L5) and is designed to run // BEFORE the RS256 JWT middleware, alongside it: // -// - If the bearer credential is a PAT ("_", no dots) and valid, +// - If the bearer credential is a PAT ("orsk__", or the older +// "_"; never dots) and valid, // it populates the SAME request context a JWT login would (user_id, tenant_id, // org_roles, permissions, RequestContext) — but with permissions narrowed to // the PAT's scopes — then continues. @@ -27,7 +28,12 @@ import ( // // The JWT middleware, in turn, skips when a PAT has already authenticated the // request (c.Locals("is_pat") == true), so the two coexist cleanly. -func PATMiddleware(patService *auth.PersonalAccessTokenService, resolve auth.SessionResolver) fiber.Handler { +// +// The session is resolved for the token's OWN tenant (#782) through the org +// resolver, which refuses a user who is no longer an active member there. A +// token minted in organization A therefore never acts in B, and stops working +// the moment its owner leaves A. +func PATMiddleware(patService *auth.PersonalAccessTokenService, resolve auth.OrgSessionResolver) fiber.Handler { return func(c *fiber.Ctx) error { // Already authenticated (defensive) — don't double-process. if c.Locals("user_id") != nil { @@ -44,7 +50,7 @@ func PATMiddleware(patService *auth.PersonalAccessTokenService, resolve auth.Ses } raw := parts[1] - // A JWT is dot-delimited base64url; a PAT is "<8-hex>_" with no dots. + // A JWT is dot-delimited base64url; a PAT is "[orsk_]<8-hex>_" with no dots. // Only attempt PAT validation on the PAT shape so we never eat a JWT. if !looksLikePAT(raw) { return c.Next() @@ -57,15 +63,13 @@ func PATMiddleware(patService *auth.PersonalAccessTokenService, resolve auth.Ses return c.Next() } - // PAT is valid — resolve the owning user's tenant + permissions so the token - // carries a real tenant context (without this, every tenant-scoped handler - // would fall back to uuid.Nil). + // PAT is valid — resolve the owner's permissions in the token's tenant. if resolve == nil { return c.Status(fiber.StatusInternalServerError).JSON(fiber.Map{"code": "PAT_MISCONFIGURED", "message": "PAT resolver not configured"}) } - sc, err := resolve(c.UserContext(), pat.UserID) - if err != nil || sc == nil || sc.TenantID == uuid.Nil { - return c.Status(fiber.StatusUnauthorized).JSON(fiber.Map{"code": "UNAUTHORIZED", "message": "token owner has no organization"}) + sc, err := resolve(c.UserContext(), pat.UserID, pat.TenantID) + if err != nil || sc == nil || sc.TenantID != pat.TenantID { + return c.Status(fiber.StatusUnauthorized).JSON(fiber.Map{"code": "UNAUTHORIZED", "message": "token owner is not an active member of the token's organization"}) } // Respect PAT scopes: the effective permission set is the INTERSECTION of the @@ -74,12 +78,7 @@ func PATMiddleware(patService *auth.PersonalAccessTokenService, resolve auth.Ses // entitled to (so an owner with "*" keeps exactly the scoped permissions, // while a limited owner cannot widen their PAT beyond what they hold). scopes := patService.GetScopes(pat) - effective := make([]string, 0, len(scopes)) - for _, scope := range scopes { - if permsGrant(sc.Permissions, scope) { - effective = append(effective, scope) - } - } + effective := effectivePATPermissions(sc.Permissions, scopes) c.Locals("user_id", pat.UserID) c.Locals("userID", pat.UserID) @@ -97,6 +96,25 @@ func PATMiddleware(patService *auth.PersonalAccessTokenService, resolve auth.Ses } } +// effectivePATPermissions narrows the owner's permissions to the token's scopes. +// The "*" scope means "everything the owner holds": without that rule a full +// access token minted by a non-admin intersected to nothing, because a member's +// permission list never contains "*" itself. +func effectivePATPermissions(owner, scopes []string) []string { + for _, scope := range scopes { + if scope == "*" { + return append([]string(nil), owner...) + } + } + effective := make([]string, 0, len(scopes)) + for _, scope := range scopes { + if permsGrant(owner, scope) { + effective = append(effective, scope) + } + } + return effective +} + // permsGrant reports whether a permission list grants `required`, honoring the // "*" admin wildcard and "resource:*" scoped wildcards (same semantics as the // hasPermission helper used by the RBAC middleware). @@ -114,14 +132,14 @@ func permsGrant(perms []string, required string) bool { return false } -// looksLikePAT reports whether a bearer value has the PAT shape "<8-hex>_" -// and is therefore not a JWT (which is always dot-delimited). +// looksLikePAT reports whether a bearer value has the PAT shape +// "[orsk_]<8-hex>_" and is therefore not a JWT (always dot-delimited). func looksLikePAT(raw string) bool { if strings.Contains(raw, ".") { return false } - parts := strings.Split(raw, "_") - return len(parts) == 2 && len(parts[0]) == 8 + _, _, ok := auth.SplitPATValue(raw) + return ok } // RequireTokenScope checks if PAT has required scope diff --git a/backend/internal/middleware/pat_middleware_test.go b/backend/internal/middleware/pat_middleware_test.go index 1b1bde2f..eacf535e 100644 --- a/backend/internal/middleware/pat_middleware_test.go +++ b/backend/internal/middleware/pat_middleware_test.go @@ -7,14 +7,16 @@ package middleware import "testing" -// looksLikePAT must accept the "<8-hex>_" PAT shape and reject JWTs +// looksLikePAT must accept the "[orsk_]<8-hex>_" PAT shape and reject JWTs // (which are dot-delimited) so the PAT middleware never swallows a JWT. func TestLooksLikePAT(t *testing.T) { cases := []struct { in string want bool }{ - {"b3762387_deadbeefcafebabe", true}, + {"orsk_b3762387_deadbeefcafebabe", true}, // #782 format + {"b3762387_deadbeefcafebabe", true}, // pre-#782 format + {"orsk_short_secret", false}, {"12345678_x", true}, {"eyJhbGciOi.J9.eyJzdWIi.sig", false}, // JWT: has dots {"short_secret", false}, // prefix not 8 chars diff --git a/backend/internal/security/isolation/registry.go b/backend/internal/security/isolation/registry.go index 82edc370..cd04cccd 100644 --- a/backend/internal/security/isolation/registry.go +++ b/backend/internal/security/isolation/registry.go @@ -90,7 +90,7 @@ var decisions = []Decision{ // --- Caller's own identity ------------------------------------------- {"/api/v1/auth/pat/{id}", SelfScoped, - "PAT CRUD is scoped to the authenticated owner via claims, not the path"}, + "revoke deletes WHERE tenant_id AND user_id AND id from the session, never the path alone; a foreign token answers 404. Pinned by TestPATRepo_TenantIsolation (#782)"}, {"/api/v1/auth/sessions/{id}", SelfScoped, "session revocation is scoped to the authenticated owner: GormSessionRepository.Revoke " + "filters id AND user_id, so a foreign session id affects 0 rows and reads back as " + diff --git a/migrations/0067_personal_access_tokens_tenant.down.sql b/migrations/0067_personal_access_tokens_tenant.down.sql new file mode 100644 index 00000000..3132f9c2 --- /dev/null +++ b/migrations/0067_personal_access_tokens_tenant.down.sql @@ -0,0 +1,12 @@ +-- Reverses 0067's constraint and index only. The column is NOT dropped: the +-- application of this release reads it on every PAT request, and dropping it +-- would make every token unattributable on a later re-upgrade. + +DO $$ +BEGIN + IF to_regclass('personal_access_tokens') IS NOT NULL THEN + ALTER TABLE personal_access_tokens ALTER COLUMN tenant_id DROP NOT NULL; + END IF; +END $$; + +DROP INDEX IF EXISTS idx_personal_access_tokens_tenant_id; diff --git a/migrations/0067_personal_access_tokens_tenant.up.sql b/migrations/0067_personal_access_tokens_tenant.up.sql new file mode 100644 index 00000000..bd4aefbd --- /dev/null +++ b/migrations/0067_personal_access_tokens_tenant.up.sql @@ -0,0 +1,42 @@ +-- #782 — a personal access token belongs to one organization. +-- +-- Before this, the PAT middleware resolved a token's tenant from its owner's +-- default organization on every request, so changing that default silently +-- moved every token the person held. The tenant is now stored on the token and +-- the middleware resolves the session for that tenant only. +-- +-- The backfill copies users.default_org_id: the tenant each token already acted +-- in, so no token changes behaviour. A token whose owner has no default +-- organization never authenticated a single request, and is deleted rather +-- than pinned to a guessed tenant. +-- +-- A server boot builds this table with AutoMigrate and runs the same backfill +-- first (database.backfillPersonalAccessTokenTenant). Every statement below is +-- therefore idempotent. + +-- No SQL migration creates personal_access_tokens: only AutoMigrate does. Run +-- before the first boot, this migration finds no table and does nothing; the +-- boot then creates the table with the column already in place. + +DO $$ +BEGIN + IF to_regclass('personal_access_tokens') IS NULL THEN + RETURN; + END IF; + + ALTER TABLE personal_access_tokens ADD COLUMN IF NOT EXISTS tenant_id uuid; + + UPDATE personal_access_tokens p + SET tenant_id = u.default_org_id + FROM users u + WHERE p.user_id = u.id + AND p.tenant_id IS NULL + AND u.default_org_id IS NOT NULL; + + DELETE FROM personal_access_tokens WHERE tenant_id IS NULL; + + ALTER TABLE personal_access_tokens ALTER COLUMN tenant_id SET NOT NULL; + + CREATE INDEX IF NOT EXISTS idx_personal_access_tokens_tenant_id + ON personal_access_tokens (tenant_id); +END $$; From 65b7f042c444de53fa39acebf4e352833f7f3ec2 Mon Sep 17 00:00:00 2001 From: alex-dembele Date: Thu, 1 Oct 2026 11:16:59 +0100 Subject: [PATCH 2/4] refactor(auth): remove the in-memory API token service (#782) TokenService kept tokens in a map that a restart emptied, and the middleware that checked them (tokenauth.go) was never mounted, so no request carrying one was ever accepted. It is removed with its handler, the /tokens routes, the APIToken model and their tests. Personal access tokens on /auth/pat are now the only API token implementation. The authz coverage list, the isolation registry and the schema test drop their /tokens and APIToken entries with it. Signed-off-by: alex-dembele --- backend/cmd/server/main.go | 17 - backend/cmd/server/schema_test.go | 3 - backend/internal/domain/api_token.go | 218 ------- backend/internal/domain/api_token_test.go | 459 -------------- .../handler/authz_route_coverage_test.go | 7 - backend/internal/handler/test_helpers.go | 2 - .../handler/token_flow_integration_test.go | 210 ------- backend/internal/handler/token_handler.go | 307 ---------- .../internal/handler/token_handler_test.go | 295 --------- backend/internal/middleware/tokenauth.go | 181 ------ backend/internal/middleware/tokenauth_test.go | 330 ---------- .../internal/security/isolation/registry.go | 6 +- backend/internal/service/token_service.go | 375 ------------ .../internal/service/token_service_test.go | 574 ------------------ 14 files changed, 1 insertion(+), 2983 deletions(-) delete mode 100644 backend/internal/domain/api_token.go delete mode 100644 backend/internal/domain/api_token_test.go delete mode 100644 backend/internal/handler/token_flow_integration_test.go delete mode 100644 backend/internal/handler/token_handler.go delete mode 100644 backend/internal/handler/token_handler_test.go delete mode 100644 backend/internal/middleware/tokenauth.go delete mode 100644 backend/internal/middleware/tokenauth_test.go delete mode 100644 backend/internal/service/token_service.go delete mode 100644 backend/internal/service/token_service_test.go diff --git a/backend/cmd/server/main.go b/backend/cmd/server/main.go index 696d288d..ab2ad608 100644 --- a/backend/cmd/server/main.go +++ b/backend/cmd/server/main.go @@ -274,9 +274,6 @@ func main() { log.Fatalf("Failed to initialize default permission roles: %v", err) } - // Initialize Token Service for API token management - tokenService := service.NewTokenService() - // Initialize Score Engine Service for automatic risk score calculation scoreEngineService := service.NewScoreEngineService(database.DB) log.Println("Score Engine: Service initialized with default configuration") @@ -2091,20 +2088,6 @@ func main() { protected.Get("/audit-logs/user/:user_id", adminRole, auditHandler.GetUserAuditLogs) protected.Get("/audit-logs/action/:action", adminRole, auditHandler.GetAuditLogsByAction) - // --- API Token Management (Protected routes) --- - // Tokens can be managed by any authenticated user for their own tokens - tokenHandler := handlers.NewTokenHandler(tokenService) - - // API tokens are personal: every verb below loads the token and refuses when - // token.UserID is not the caller. The session is the authorization (#529). - protected.Post("/tokens", tokenHandler.CreateToken) - protected.Get("/tokens", tokenHandler.ListTokens) - protected.Get("/tokens/:id", tokenHandler.GetToken) - protected.Put("/tokens/:id", tokenHandler.UpdateToken) - protected.Post("/tokens/:id/revoke", tokenHandler.RevokeToken) - protected.Post("/tokens/:id/rotate", tokenHandler.RotateToken) - protected.Delete("/tokens/:id", tokenHandler.DeleteToken) - // --- Custom Fields Management (Protected routes) --- customFieldHandler := handlers.NewCustomFieldHandler() // Reading the field definitions is what every risk and asset FORM does, so diff --git a/backend/cmd/server/schema_test.go b/backend/cmd/server/schema_test.go index 3783fc50..fc142a0a 100644 --- a/backend/cmd/server/schema_test.go +++ b/backend/cmd/server/schema_test.go @@ -37,9 +37,6 @@ import ( // the issue that owns it. Adding a name requires an issue number (enforced // below); an entry must go as soon as its table is built (also enforced). var knownMissingTables = map[string]string{ - // Stored in an in-process map by service.TokenService; the table is never - // read or written. The feature itself is broken, see the issue. - "APIToken": "#709", // The marketplace is not built (ROADMAP 14.18): its models carry no gorm // tags and cannot be migrated as they stand, and its routes are not // mounted (main.go), so no request reaches these tables. diff --git a/backend/internal/domain/api_token.go b/backend/internal/domain/api_token.go deleted file mode 100644 index 91612818..00000000 --- a/backend/internal/domain/api_token.go +++ /dev/null @@ -1,218 +0,0 @@ -// Copyright (c) 2026 OpenDefender Contributors -// SPDX-License-Identifier: AGPL-3.0-only -// This program is free software: you can redistribute it and/or modify it under -// the terms of the GNU Affero General Public License v3.0 (see LICENSE). - -package domain - -import ( - "time" - - "github.com/google/uuid" -) - -// TokenType defines the type of API token -type TokenType string - -const ( - TokenTypeBearer TokenType = "bearer" // Standard Bearer token for API calls - TokenTypeBasic TokenType = "basic" // Basic auth credentials - TokenTypeOAuth TokenType = "oauth" // OAuth 2.0 tokens -) - -// TokenStatus represents the current status of a token -type TokenStatus string - -const ( - TokenStatusActive TokenStatus = "active" - TokenStatusRevoked TokenStatus = "revoked" - TokenStatusExpired TokenStatus = "expired" - TokenStatusDisabled TokenStatus = "disabled" -) - -// APIToken represents an API token for service accounts or integrations -type APIToken struct { - ID uuid.UUID `gorm:"primaryKey" json:"id"` - UserID uuid.UUID `json:"user_id"` - Name string `json:"name"` // User-friendly name for the token - Description string `json:"description"` // What this token is for - TokenHash string `json:"-"` // SHA256 hash of actual token (for storage) - TokenPrefix string `json:"token_prefix"` // First 8 chars of token (for display) - Type TokenType `json:"type"` // Type of token - Status TokenStatus `json:"status"` // Current status - Permissions []string `gorm:"serializer:json" json:"permissions"` // Specific permissions (if different from user) - LastUsed *time.Time `json:"last_used_at"` // When token was last used - ExpiresAt *time.Time `json:"expires_at"` // When token expires (nil = never) - RevokedAt *time.Time `json:"revoked_at"` // When token was revoked - RevokeReason string `json:"revoke_reason"` // Why it was revoked - CreatedAt time.Time `json:"created_at"` - UpdatedAt time.Time `json:"updated_at"` - CreatedByID uuid.UUID `json:"created_by_id"` // Which user/admin created this - IPWhitelist []string `gorm:"serializer:json" json:"ip_whitelist"` // Restrict to IPs - Scopes []string `gorm:"serializer:json" json:"scopes"` // OAuth scopes or permission scopes - Metadata map[string]interface{} `gorm:"serializer:json" json:"metadata"` // Custom metadata -} - -// TableName specifies the table name for the APIToken model -func (APIToken) TableName() string { - return "api_tokens" -} - -// IsExpired checks if the token has expired -func (t *APIToken) IsExpired() bool { - if t.ExpiresAt == nil { - return false - } - return time.Now().After(*t.ExpiresAt) -} - -// IsRevoked checks if the token is revoked -func (t *APIToken) IsRevoked() bool { - return t.Status == TokenStatusRevoked || t.RevokedAt != nil -} - -// IsValid checks if the token is valid for use -func (t *APIToken) IsValid() bool { - return t.Status == TokenStatusActive && !t.IsExpired() && !t.IsRevoked() -} - -// UpdateLastUsed updates the last used timestamp -func (t *APIToken) UpdateLastUsed() { - now := time.Now() - t.LastUsed = &now -} - -// Revoke marks the token as revoked -func (t *APIToken) Revoke(reason string) { - now := time.Now() - t.Status = TokenStatusRevoked - t.RevokedAt = &now - t.RevokeReason = reason -} - -// Disable marks the token as disabled -func (t *APIToken) Disable() { - t.Status = TokenStatusDisabled -} - -// Enable marks the token as active again -func (t *APIToken) Enable() { - t.Status = TokenStatusActive -} - -// HasPermission checks if token has a specific permission -func (t *APIToken) HasPermission(permission string) bool { - // If no specific permissions set, user's role permissions apply - if len(t.Permissions) == 0 { - return true // Inherits from user role - } - - // Check exact permission match - for _, p := range t.Permissions { - if p == permission { - return true - } - // Support wildcards - if p == "*" { - return true - } - } - - return false -} - -// HasScope checks if token has a specific scope -func (t *APIToken) HasScope(scope string) bool { - if len(t.Scopes) == 0 { - return true // No scope restrictions - } - - for _, s := range t.Scopes { - if s == scope { - return true - } - if s == "*" { - return true - } - } - - return false -} - -// IsIPAllowed checks if the request IP is in the whitelist -func (t *APIToken) IsIPAllowed(clientIP string) bool { - if len(t.IPWhitelist) == 0 { - return true // No IP restrictions - } - - for _, ip := range t.IPWhitelist { - if ip == clientIP || ip == "*" { - return true - } - } - - return false -} - -// TokenCreateRequest represents the request to create a new token -type TokenCreateRequest struct { - Name string `json:"name" binding:"required,max=255"` - Description string `json:"description" binding:"max=1000"` - Type TokenType `json:"type" binding:"required"` - ExpiresAt *time.Time `json:"expires_at"` - Permissions []string `json:"permissions"` - Scopes []string `json:"scopes"` - IPWhitelist []string `json:"ip_whitelist"` - Metadata map[string]interface{} `json:"metadata"` -} - -// TokenUpdateRequest represents the request to update a token -type TokenUpdateRequest struct { - Name string `json:"name" binding:"max=255"` - Description string `json:"description" binding:"max=1000"` - Permissions []string `json:"permissions"` - Scopes []string `json:"scopes"` - IPWhitelist []string `json:"ip_whitelist"` - Metadata map[string]interface{} `json:"metadata"` -} - -// TokenRevokeRequest represents the request to revoke a token -type TokenRevokeRequest struct { - Reason string `json:"reason" binding:"max=500"` -} - -// TokenResponse represents the response with token details (without the actual token value) -type TokenResponse struct { - ID uuid.UUID `json:"id"` - Name string `json:"name"` - Description string `json:"description"` - TokenPrefix string `json:"token_prefix"` - Type TokenType `json:"type"` - Status TokenStatus `json:"status"` - Permissions []string `json:"permissions"` - LastUsed *time.Time `json:"last_used_at"` - ExpiresAt *time.Time `json:"expires_at"` - RevokedAt *time.Time `json:"revoked_at"` - CreatedAt time.Time `json:"created_at"` - UpdatedAt time.Time `json:"updated_at"` - Scopes []string `json:"scopes"` - Metadata map[string]interface{} `json:"metadata"` -} - -// TokenWithValue represents a newly created token with its actual value -type TokenWithValue struct { - *TokenResponse - Token string `json:"token"` // The actual token value (only shown once) -} - -// RotateTokenRequest represents the request to rotate a token -type RotateTokenRequest struct { - Reason string `json:"reason" binding:"max=500"` -} - -// RotateTokenResponse represents the response when rotating a token -type RotateTokenResponse struct { - OldToken *TokenResponse `json:"old_token"` - NewToken *TokenWithValue `json:"new_token"` - RotatedAt time.Time `json:"rotated_at"` -} diff --git a/backend/internal/domain/api_token_test.go b/backend/internal/domain/api_token_test.go deleted file mode 100644 index 8d1b16d5..00000000 --- a/backend/internal/domain/api_token_test.go +++ /dev/null @@ -1,459 +0,0 @@ -// Copyright (c) 2026 OpenDefender Contributors -// SPDX-License-Identifier: AGPL-3.0-only -// This program is free software: you can redistribute it and/or modify it under -// the terms of the GNU Affero General Public License v3.0 (see LICENSE). - -package domain - -import ( - "testing" - "time" - - "github.com/google/uuid" - "github.com/stretchr/testify/assert" -) - -// Test data constants -const ( - testIPLocal = "127.0.0.1" - testIPPrivate1 = "10.0.0.1" - testIPPrivate2 = "172.16.0.1" - testIPPrivate3 = "192.168.1.1" - testIPPublic = "8.8.8.8" - testIPWildcard = "*" - testTokenPrefix = "tok_test_" - testTokenValue = "tok_test_value_1234567890" -) - -func TestAPITokenIsExpired(t *testing.T) { - tests := []struct { - name string - expiresAt *time.Time - expected bool - }{ - { - name: "no expiration", - expiresAt: nil, - expected: false, - }, - { - name: "expired in the past", - expiresAt: ptrTime(time.Now().Add(-1 * time.Hour)), - expected: true, - }, - { - name: "expires in the future", - expiresAt: ptrTime(time.Now().Add(1 * time.Hour)), - expected: false, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - token := &APIToken{ExpiresAt: tt.expiresAt} - assert.Equal(t, tt.expected, token.IsExpired()) - }) - } -} - -func TestAPITokenIsRevoked(t *testing.T) { - tests := []struct { - name string - status TokenStatus - revokedAt *time.Time - expected bool - }{ - { - name: "active token", - status: TokenStatusActive, - revokedAt: nil, - expected: false, - }, - { - name: "revoked status", - status: TokenStatusRevoked, - revokedAt: ptrTime(time.Now()), - expected: true, - }, - { - name: "revoked with nil revokedAt", - status: TokenStatusRevoked, - revokedAt: nil, - expected: true, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - token := &APIToken{Status: tt.status, RevokedAt: tt.revokedAt} - assert.Equal(t, tt.expected, token.IsRevoked()) - }) - } -} - -func TestAPITokenIsValid(t *testing.T) { - tests := []struct { - name string - token *APIToken - expected bool - }{ - { - name: "active and not expired", - token: &APIToken{ - Status: TokenStatusActive, - ExpiresAt: ptrTime(time.Now().Add(1 * time.Hour)), - }, - expected: true, - }, - { - name: "expired", - token: &APIToken{ - Status: TokenStatusActive, - ExpiresAt: ptrTime(time.Now().Add(-1 * time.Hour)), - }, - expected: false, - }, - { - name: "revoked", - token: &APIToken{ - Status: TokenStatusRevoked, - ExpiresAt: ptrTime(time.Now().Add(1 * time.Hour)), - RevokedAt: ptrTime(time.Now()), - }, - expected: false, - }, - { - name: "disabled", - token: &APIToken{ - Status: TokenStatusDisabled, - ExpiresAt: ptrTime(time.Now().Add(1 * time.Hour)), - }, - expected: false, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - assert.Equal(t, tt.expected, tt.token.IsValid()) - }) - } -} - -func TestAPITokenUpdateLastUsed(t *testing.T) { - token := &APIToken{ - LastUsed: nil, - } - - token.UpdateLastUsed() - - assert.NotNil(t, token.LastUsed) - assert.WithinDuration(t, time.Now(), *token.LastUsed, 100*time.Millisecond) -} - -func TestAPITokenRevoke(t *testing.T) { - token := &APIToken{ - Status: TokenStatusActive, - RevokedAt: nil, - } - - token.Revoke("Test revocation") - - assert.Equal(t, TokenStatusRevoked, token.Status) - assert.NotNil(t, token.RevokedAt) - assert.Equal(t, "Test revocation", token.RevokeReason) -} - -func TestAPITokenDisable(t *testing.T) { - token := &APIToken{ - Status: TokenStatusActive, - } - - token.Disable() - - assert.Equal(t, TokenStatusDisabled, token.Status) -} - -func TestAPITokenEnable(t *testing.T) { - token := &APIToken{ - Status: TokenStatusDisabled, - } - - token.Enable() - - assert.Equal(t, TokenStatusActive, token.Status) -} - -func TestAPITokenHasPermission(t *testing.T) { - tests := []struct { - name string - token *APIToken - permission string - expected bool - }{ - { - name: "no specific permissions - inherits from role", - token: &APIToken{ - Permissions: []string{}, - }, - permission: "read", - expected: true, - }, - { - name: "has exact permission", - token: &APIToken{ - Permissions: []string{"read", "write", "delete"}, - }, - permission: "write", - expected: true, - }, - { - name: "missing permission", - token: &APIToken{ - Permissions: []string{"read"}, - }, - permission: "delete", - expected: false, - }, - { - name: "wildcard permission", - token: &APIToken{ - Permissions: []string{"*"}, - }, - permission: "anything", - expected: true, - }, - { - name: "nil permissions", - token: &APIToken{ - Permissions: nil, - }, - permission: "read", - expected: true, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - assert.Equal(t, tt.expected, tt.token.HasPermission(tt.permission)) - }) - } -} - -func TestAPITokenHasScope(t *testing.T) { - tests := []struct { - name string - token *APIToken - scope string - expected bool - }{ - { - name: "no scope restrictions", - token: &APIToken{ - Scopes: []string{}, - }, - scope: "risk", - expected: true, - }, - { - name: "has exact scope", - token: &APIToken{ - Scopes: []string{"risk", "mitigation", "asset"}, - }, - scope: "mitigation", - expected: true, - }, - { - name: "missing scope", - token: &APIToken{ - Scopes: []string{"risk"}, - }, - scope: "user", - expected: false, - }, - { - name: "wildcard scope", - token: &APIToken{ - Scopes: []string{"*"}, - }, - scope: "anything", - expected: true, - }, - { - name: "nil scopes", - token: &APIToken{ - Scopes: nil, - }, - scope: "any", - expected: true, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - assert.Equal(t, tt.expected, tt.token.HasScope(tt.scope)) - }) - } -} - -func TestAPITokenIsIPAllowed(t *testing.T) { - tests := []struct { - name string - token *APIToken - clientIP string - expected bool - }{ - { - name: "no IP restrictions", - token: &APIToken{ - IPWhitelist: []string{}, - }, - clientIP: testIPPublic, - expected: true, - }, - { - name: "IP in whitelist", - token: &APIToken{ - IPWhitelist: []string{testIPPrivate1, testIPPrivate2, testIPLocal}, - }, - clientIP: testIPPrivate2, - expected: true, - }, - { - name: "IP not in whitelist", - token: &APIToken{ - IPWhitelist: []string{testIPPrivate1, testIPPrivate2}, - }, - clientIP: testIPPrivate3, - expected: false, - }, - { - name: "wildcard IP", - token: &APIToken{ - IPWhitelist: []string{"*"}, - }, - clientIP: "any.ip.address", - expected: true, - }, - { - name: "nil IP whitelist", - token: &APIToken{ - IPWhitelist: nil, - }, - clientIP: "any.ip", - expected: true, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - assert.Equal(t, tt.expected, tt.token.IsIPAllowed(tt.clientIP)) - }) - } -} - -func TestTokenCreateRequest_Valid(t *testing.T) { - req := &TokenCreateRequest{ - Name: "Test Token", - Type: TokenTypeBearer, - } - - assert.NotNil(t, req) - assert.Equal(t, "Test Token", req.Name) - assert.Equal(t, TokenTypeBearer, req.Type) -} - -func TestTokenUpdateRequest_PartialUpdate(t *testing.T) { - req := &TokenUpdateRequest{ - Name: "Updated Name", - } - - assert.Equal(t, "Updated Name", req.Name) - assert.Empty(t, req.Description) - assert.Nil(t, req.Permissions) -} - -func TestTokenRevokeRequest(t *testing.T) { - req := &TokenRevokeRequest{ - Reason: "Security issue", - } - - assert.Equal(t, "Security issue", req.Reason) -} - -func TestTokenResponse_Conversion(t *testing.T) { - now := time.Now() - token := &APIToken{ - ID: uuid.New(), - Name: "Test Token", - Description: "Test Description", - TokenPrefix: testTokenPrefix, - Type: TokenTypeBearer, - Status: TokenStatusActive, - ExpiresAt: &now, - CreatedAt: now, - UpdatedAt: now, - } - - response := &TokenResponse{ - ID: token.ID, - Name: token.Name, - Description: token.Description, - TokenPrefix: token.TokenPrefix, - Type: token.Type, - Status: token.Status, - ExpiresAt: token.ExpiresAt, - CreatedAt: token.CreatedAt, - UpdatedAt: token.UpdatedAt, - } - - assert.Equal(t, token.ID, response.ID) - assert.Equal(t, token.Name, response.Name) - assert.Equal(t, token.Status, response.Status) -} - -func TestTokenWithValue_SecureTokenDisplay(t *testing.T) { - tokenValue := testTokenValue - response := &TokenWithValue{ - TokenResponse: &TokenResponse{ - TokenPrefix: "orsk_ve", - }, - Token: tokenValue, - } - - // Token should only be visible in the Token field - assert.Equal(t, tokenValue, response.Token) - assert.NotEqual(t, tokenValue, response.TokenPrefix) -} - -func TestRotateTokenResponse(t *testing.T) { - oldResponse := &TokenResponse{ - ID: uuid.New(), - Name: "Old Token", - Status: TokenStatusRevoked, - } - - newResponse := &TokenWithValue{ - TokenResponse: &TokenResponse{ - ID: uuid.New(), - Name: "Old Token", - Status: TokenStatusActive, - }, - Token: "orsk_newtoken123", - } - - rotateResp := &RotateTokenResponse{ - OldToken: oldResponse, - NewToken: newResponse, - RotatedAt: time.Now(), - } - - assert.NotNil(t, rotateResp.OldToken) - assert.NotNil(t, rotateResp.NewToken) - assert.Equal(t, TokenStatusRevoked, rotateResp.OldToken.Status) - assert.Equal(t, TokenStatusActive, rotateResp.NewToken.Status) -} - -// Helper function -func ptrTime(t time.Time) *time.Time { - return &t -} diff --git a/backend/internal/handler/authz_route_coverage_test.go b/backend/internal/handler/authz_route_coverage_test.go index 6d77a74e..1a96719d 100644 --- a/backend/internal/handler/authz_route_coverage_test.go +++ b/backend/internal/handler/authz_route_coverage_test.go @@ -80,7 +80,6 @@ var routesWithoutPermissionGuard = []string{ "DELETE /auth/pat/:id", "DELETE /auth/sessions/:id", "DELETE /auth/sessions/others", - "DELETE /tokens/:id", "GET /action-center", "GET /activation/state", // #438 — the Posture Reveal, the recognition screen and the starter @@ -151,8 +150,6 @@ var routesWithoutPermissionGuard = []string{ "GET /stats", "GET /telemetry", "GET /timeline", - "GET /tokens", - "GET /tokens/:id", // #719 — self-service on the session's own user; the avatar read is gated // by tenant membership in the use case. "DELETE /users/me/avatar", @@ -180,11 +177,7 @@ var routesWithoutPermissionGuard = []string{ "POST /governance/delegations/:id/revoke", "POST /onboarding/complete", "POST /score/preview", - "POST /tokens", - "POST /tokens/:id/revoke", - "POST /tokens/:id/rotate", "PUT /onboarding/steps/:step", - "PUT /tokens/:id", } // protectedRoute is one `protected.(...)` registration. diff --git a/backend/internal/handler/test_helpers.go b/backend/internal/handler/test_helpers.go index 1ddbfb53..069e95d4 100644 --- a/backend/internal/handler/test_helpers.go +++ b/backend/internal/handler/test_helpers.go @@ -41,7 +41,6 @@ func SetupTestDB(t *testing.T) *gorm.DB { &domain.Mitigation{}, &domain.Asset{}, &domain.RiskHistory{}, - &domain.APIToken{}, ); err != nil { t.Fatalf("Failed to run migrations: %v", err) } @@ -52,7 +51,6 @@ func SetupTestDB(t *testing.T) *gorm.DB { // CleanupTestDB truncates all test data func CleanupTestDB(t *testing.T, db *gorm.DB) { tables := []string{ - "api_tokens", "risk_assets", "mitigations", "mitigation_subactions", diff --git a/backend/internal/handler/token_flow_integration_test.go b/backend/internal/handler/token_flow_integration_test.go deleted file mode 100644 index 86febef5..00000000 --- a/backend/internal/handler/token_flow_integration_test.go +++ /dev/null @@ -1,210 +0,0 @@ -// Copyright (c) 2026 OpenDefender Contributors -// SPDX-License-Identifier: AGPL-3.0-only -// This program is free software: you can redistribute it and/or modify it under -// the terms of the GNU Affero General Public License v3.0 (see LICENSE). - -//go:build integration -// +build integration - -package handler - -import ( - "testing" - - "github.com/google/uuid" - "github.com/opendefender/openrisk/internal/domain" - "github.com/opendefender/openrisk/internal/infrastructure/database" - "github.com/opendefender/openrisk/internal/service" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -// TestTokenServiceFlow tests the complete token service operations -func TestTokenServiceFlow(t *testing.T) { - InitTestDB(t) - defer CleanupTestDB(t, database.DB) - - tokenService := service.NewTokenService() - userID := uuid.New() - - t.Run("create-verify-token-flow", func(t *testing.T) { - // Create a token - tokenWithValue, err := tokenService.CreateToken(userID, &domain.TokenCreateRequest{ - Name: "test-token", - Description: "A test token", - }, userID) - - require.NoError(t, err, "should create token without error") - require.NotNil(t, tokenWithValue, "token with value should not be nil") - assert.Equal(t, "test-token", tokenWithValue.Name, "token name should match") - assert.NotEmpty(t, tokenWithValue.Token, "token value should not be empty") - assert.NotEmpty(t, tokenWithValue.ID, "token ID should not be empty") - - // Verify the token works - verified, err := tokenService.VerifyToken(tokenWithValue.Token) - require.NoError(t, err, "should verify token without error") - require.NotNil(t, verified, "verified token should not be nil") - assert.Equal(t, userID, verified.UserID, "verified token user ID should match") - assert.Equal(t, "test-token", verified.Name, "verified token name should match") - assert.Equal(t, "active", verified.Status, "new token should be active") - }) - - t.Run("list-tokens", func(t *testing.T) { - // Create two tokens - _, err := tokenService.CreateToken(userID, &domain.TokenCreateRequest{ - Name: "token1", - }, userID) - require.NoError(t, err) - - _, err = tokenService.CreateToken(userID, &domain.TokenCreateRequest{ - Name: "token2", - }, userID) - require.NoError(t, err) - - // List tokens - tokens, err := tokenService.ListTokens(userID) - require.NoError(t, err, "should list tokens without error") - assert.GreaterOrEqual(t, len(tokens), 2, "should have at least 2 tokens") - }) - - t.Run("get-token", func(t *testing.T) { - // Create a token - tokenWithValue, err := tokenService.CreateToken(userID, &domain.TokenCreateRequest{ - Name: "get-test-token", - }, userID) - require.NoError(t, err) - - // Get the token - token, err := tokenService.GetToken(tokenWithValue.ID) - require.NoError(t, err, "should get token without error") - require.NotNil(t, token, "token should not be nil") - assert.Equal(t, "get-test-token", token.Name, "token name should match") - }) - - t.Run("update-token", func(t *testing.T) { - // Create a token - tokenWithValue, err := tokenService.CreateToken(userID, &domain.TokenCreateRequest{ - Name: "update-test", - Description: "Original description", - }, userID) - require.NoError(t, err) - - // Update the token - updated, err := tokenService.UpdateToken(tokenWithValue.ID, &domain.TokenUpdateRequest{ - Name: "updated-name", - Description: "Updated description", - }) - require.NoError(t, err, "should update token without error") - require.NotNil(t, updated, "updated token should not be nil") - assert.Equal(t, "updated-name", updated.Name, "token name should be updated") - assert.Equal(t, "Updated description", updated.Description, "token description should be updated") - }) - - t.Run("rotate-token", func(t *testing.T) { - // Create a token - tokenWithValue, err := tokenService.CreateToken(userID, &domain.TokenCreateRequest{ - Name: "rotate-test", - }, userID) - require.NoError(t, err) - - oldToken := tokenWithValue.Token - - // Rotate the token - rotateResp, err := tokenService.RotateToken(tokenWithValue.ID, userID, "rotation test") - require.NoError(t, err, "should rotate token without error") - require.NotNil(t, rotateResp, "rotate response should not be nil") - require.NotNil(t, rotateResp.OldToken, "old token should be in response") - require.NotNil(t, rotateResp.NewToken, "new token should be in response") - assert.NotEqual(t, oldToken, rotateResp.NewToken.Token, "new token should be different from old token") - - // Verify old token no longer works - verified, err := tokenService.VerifyToken(oldToken) - require.Error(t, err, "old token should fail verification after rotation") - assert.Nil(t, verified, "verified old token should be nil") - - // Verify new token works - verified, err = tokenService.VerifyToken(rotateResp.NewToken.Token) - require.NoError(t, err, "new token should verify successfully") - require.NotNil(t, verified, "verified new token should not be nil") - }) - - t.Run("revoke-token", func(t *testing.T) { - // Create a token - tokenWithValue, err := tokenService.CreateToken(userID, &domain.TokenCreateRequest{ - Name: "revoke-test", - }, userID) - require.NoError(t, err) - - tokenID := tokenWithValue.ID - - // Revoke the token - revoked, err := tokenService.RevokeToken(tokenID, "testing revocation") - require.NoError(t, err, "should revoke token without error") - require.NotNil(t, revoked, "revoked token should not be nil") - assert.Equal(t, "revoked", revoked.Status, "revoked token status should be 'revoked'") - - // Verify revoked token cannot be used - verified, err := tokenService.VerifyToken(tokenWithValue.Token) - require.Error(t, err, "revoked token should fail verification") - assert.Nil(t, verified, "verified revoked token should be nil") - }) - - t.Run("delete-token", func(t *testing.T) { - // Create a token - tokenWithValue, err := tokenService.CreateToken(userID, &domain.TokenCreateRequest{ - Name: "delete-test", - }, userID) - require.NoError(t, err) - - tokenID := tokenWithValue.ID - - // Delete the token - err = tokenService.DeleteToken(tokenID) - require.NoError(t, err, "should delete token without error") - - // Verify token is deleted (should return error) - token, err := tokenService.GetToken(tokenID) - require.Error(t, err, "deleted token should return error on retrieval") - assert.Nil(t, token, "deleted token should be nil") - }) - - t.Run("invalid-token-verification", func(t *testing.T) { - // Try to verify an invalid token - verified, err := tokenService.VerifyToken("invalid-token-value") - require.Error(t, err, "should fail to verify invalid token") - assert.Nil(t, verified, "verified invalid token should be nil") - }) - - t.Run("token-ownership-enforcement", func(t *testing.T) { - // Create a token for user 1 - user1ID := uuid.New() - - token1, err := tokenService.CreateToken(user1ID, &domain.TokenCreateRequest{ - Name: "user1-token", - }, user1ID) - require.NoError(t, err) - - // Verify token belongs to user 1 - token, err := tokenService.GetToken(token1.ID) - require.NoError(t, err, "should be able to get any token by ID") - require.NotNil(t, token, "token should exist") - assert.Equal(t, user1ID, token.UserID, "token should belong to user 1") - }) - - t.Run("token-with-permissions", func(t *testing.T) { - // Create a token with specific permissions - tokenWithValue, err := tokenService.CreateToken(userID, &domain.TokenCreateRequest{ - Name: "restricted-token", - Permissions: []string{"risk:read:any", "mitigation:create:any"}, - Scopes: []string{"risk", "mitigation"}, - }, userID) - require.NoError(t, err) - - // Verify token has correct permissions - verified, err := tokenService.VerifyToken(tokenWithValue.Token) - require.NoError(t, err) - assert.NotNil(t, verified) - assert.Len(t, verified.Permissions, 2, "token should have 2 permissions") - assert.Len(t, verified.Scopes, 2, "token should have 2 scopes") - }) -} diff --git a/backend/internal/handler/token_handler.go b/backend/internal/handler/token_handler.go deleted file mode 100644 index 70ad6445..00000000 --- a/backend/internal/handler/token_handler.go +++ /dev/null @@ -1,307 +0,0 @@ -// Copyright (c) 2026 OpenDefender Contributors -// SPDX-License-Identifier: AGPL-3.0-only -// This program is free software: you can redistribute it and/or modify it under -// the terms of the GNU Affero General Public License v3.0 (see LICENSE). - -package handler - -import ( - "errors" - "net/http" - - "github.com/gofiber/fiber/v2" - "github.com/google/uuid" - - "github.com/opendefender/openrisk/internal/domain" - "github.com/opendefender/openrisk/internal/service" -) - -// TokenHandler handles API token management endpoints -type TokenHandler struct { - tokenService *service.TokenService -} - -// NewTokenHandler creates a new token handler -func NewTokenHandler(tokenService *service.TokenService) *TokenHandler { - return &TokenHandler{ - tokenService: tokenService, - } -} - -// CreateToken creates a new API token (POST /api/tokens) -func (h *TokenHandler) CreateToken(c *fiber.Ctx) error { - userID, err := GetUserIDFromContext(c) - if err != nil { - return c.Status(http.StatusUnauthorized).JSON(fiber.Map{"error": "unauthorized"}) - } - - var req domain.TokenCreateRequest - if err := c.BodyParser(&req); err != nil { - return c.Status(http.StatusBadRequest).JSON(fiber.Map{"error": "invalid request body"}) - } - - if req.Name == "" { - return c.Status(http.StatusBadRequest).JSON(fiber.Map{"error": "token name is required"}) - } - - if req.Type == "" { - req.Type = domain.TokenTypeBearer - } - - tokenWithValue, err := h.tokenService.CreateToken(userID, &req, userID) - if err != nil { - return c.Status(http.StatusInternalServerError).JSON(fiber.Map{"error": "failed to create token"}) - } - - return c.Status(http.StatusCreated).JSON(fiber.Map{ - "token": fiber.Map{ - "id": tokenWithValue.ID, - "name": tokenWithValue.Name, - "type": tokenWithValue.Type, - "token_prefix": tokenWithValue.TokenPrefix, - "value": tokenWithValue.Token, - "expires_at": tokenWithValue.ExpiresAt, - "created_at": tokenWithValue.CreatedAt, - }, - }) -} - -// ListTokens lists all API tokens for the user (GET /api/tokens) -func (h *TokenHandler) ListTokens(c *fiber.Ctx) error { - userID, err := GetUserIDFromContext(c) - if err != nil { - return c.Status(http.StatusUnauthorized).JSON(fiber.Map{"error": "unauthorized"}) - } - - tokens, err := h.tokenService.ListTokens(userID) - if err != nil { - return c.Status(http.StatusInternalServerError).JSON(fiber.Map{"error": "failed to list tokens"}) - } - - responses := make([]fiber.Map, len(tokens)) - for i, token := range tokens { - responses[i] = fiber.Map{ - "id": token.ID, - "name": token.Name, - "description": token.Description, - "type": token.Type, - "status": token.Status, - "token_prefix": token.TokenPrefix, - "last_used_at": token.LastUsed, - "expires_at": token.ExpiresAt, - "created_at": token.CreatedAt, - "permissions": token.Permissions, - "scopes": token.Scopes, - } - } - - return c.Status(http.StatusOK).JSON(fiber.Map{ - "tokens": responses, - "count": len(tokens), - }) -} - -// GetToken retrieves a single token (GET /api/tokens/:id) -func (h *TokenHandler) GetToken(c *fiber.Ctx) error { - userID, err := GetUserIDFromContext(c) - if err != nil { - return c.Status(http.StatusUnauthorized).JSON(fiber.Map{"error": "unauthorized"}) - } - - tokenID, err := uuid.Parse(c.Params("id")) - if err != nil { - return c.Status(http.StatusBadRequest).JSON(fiber.Map{"error": "invalid token id"}) - } - - token, err := h.tokenService.GetToken(tokenID) - if err != nil || token == nil { - return c.Status(http.StatusNotFound).JSON(fiber.Map{"error": "token not found"}) - } - - if token.UserID != userID { - return c.Status(http.StatusForbidden).JSON(fiber.Map{"error": "cannot access another user's token"}) - } - - return c.Status(http.StatusOK).JSON(fiber.Map{ - "token": fiber.Map{ - "id": token.ID, - "name": token.Name, - "description": token.Description, - "type": token.Type, - "status": token.Status, - "token_prefix": token.TokenPrefix, - "last_used_at": token.LastUsed, - "expires_at": token.ExpiresAt, - "created_at": token.CreatedAt, - "updated_at": token.UpdatedAt, - "permissions": token.Permissions, - "scopes": token.Scopes, - "ip_whitelist": token.IPWhitelist, - }, - }) -} - -// UpdateToken updates a token (PUT /api/tokens/:id) -func (h *TokenHandler) UpdateToken(c *fiber.Ctx) error { - userID, err := GetUserIDFromContext(c) - if err != nil { - return c.Status(http.StatusUnauthorized).JSON(fiber.Map{"error": "unauthorized"}) - } - - tokenID, err := uuid.Parse(c.Params("id")) - if err != nil { - return c.Status(http.StatusBadRequest).JSON(fiber.Map{"error": "invalid token id"}) - } - - token, err := h.tokenService.GetToken(tokenID) - if err != nil || token == nil { - return c.Status(http.StatusNotFound).JSON(fiber.Map{"error": "token not found"}) - } - - if token.UserID != userID { - return c.Status(http.StatusForbidden).JSON(fiber.Map{"error": "cannot update another user's token"}) - } - - var req domain.TokenUpdateRequest - if err := c.BodyParser(&req); err != nil { - return c.Status(http.StatusBadRequest).JSON(fiber.Map{"error": "invalid request body"}) - } - - updatedToken, err := h.tokenService.UpdateToken(tokenID, &req) - if err != nil || updatedToken == nil { - return c.Status(http.StatusInternalServerError).JSON(fiber.Map{"error": "failed to update token"}) - } - - return c.Status(http.StatusOK).JSON(fiber.Map{ - "token": fiber.Map{ - "id": updatedToken.ID, - "name": updatedToken.Name, - "description": updatedToken.Description, - "type": updatedToken.Type, - "status": updatedToken.Status, - "token_prefix": updatedToken.TokenPrefix, - "expires_at": updatedToken.ExpiresAt, - "created_at": updatedToken.CreatedAt, - "updated_at": updatedToken.UpdatedAt, - }, - }) -} - -// RevokeToken revokes a token (POST /api/tokens/:id/revoke) -func (h *TokenHandler) RevokeToken(c *fiber.Ctx) error { - userID, err := GetUserIDFromContext(c) - if err != nil { - return c.Status(http.StatusUnauthorized).JSON(fiber.Map{"error": "unauthorized"}) - } - - tokenID, err := uuid.Parse(c.Params("id")) - if err != nil { - return c.Status(http.StatusBadRequest).JSON(fiber.Map{"error": "invalid token id"}) - } - - token, err := h.tokenService.GetToken(tokenID) - if err != nil || token == nil { - return c.Status(http.StatusNotFound).JSON(fiber.Map{"error": "token not found"}) - } - - if token.UserID != userID { - return c.Status(http.StatusForbidden).JSON(fiber.Map{"error": "cannot revoke another user's token"}) - } - - var req domain.TokenRevokeRequest - _ = c.BodyParser(&req) - if req.Reason == "" { - req.Reason = "revoked by user" - } - - revokedToken, err := h.tokenService.RevokeToken(tokenID, req.Reason) - if err != nil || revokedToken == nil { - return c.Status(http.StatusInternalServerError).JSON(fiber.Map{"error": "failed to revoke token"}) - } - - return c.Status(http.StatusOK).JSON(fiber.Map{ - "token": fiber.Map{ - "id": revokedToken.ID, - "status": revokedToken.Status, - "revoked_at": revokedToken.RevokedAt, - "revoke_reason": revokedToken.RevokeReason, - }, - }) -} - -// RotateToken rotates a token (POST /api/tokens/:id/rotate) -func (h *TokenHandler) RotateToken(c *fiber.Ctx) error { - userID, err := GetUserIDFromContext(c) - if err != nil { - return c.Status(http.StatusUnauthorized).JSON(fiber.Map{"error": "unauthorized"}) - } - - tokenID, err := uuid.Parse(c.Params("id")) - if err != nil { - return c.Status(http.StatusBadRequest).JSON(fiber.Map{"error": "invalid token id"}) - } - - token, err := h.tokenService.GetToken(tokenID) - if err != nil || token == nil { - return c.Status(http.StatusNotFound).JSON(fiber.Map{"error": "token not found"}) - } - - if token.UserID != userID { - return c.Status(http.StatusForbidden).JSON(fiber.Map{"error": "cannot rotate another user's token"}) - } - - rotateResp, err := h.tokenService.RotateToken(tokenID, userID, "rotated by user") - if err != nil || rotateResp == nil { - return c.Status(http.StatusInternalServerError).JSON(fiber.Map{"error": "failed to rotate token"}) - } - - return c.Status(http.StatusCreated).JSON(fiber.Map{ - "old_token": rotateResp.OldToken, - "new_token": rotateResp.NewToken, - "rotated_at": rotateResp.RotatedAt, - }) -} - -// DeleteToken deletes a token (DELETE /api/tokens/:id) -func (h *TokenHandler) DeleteToken(c *fiber.Ctx) error { - userID, err := GetUserIDFromContext(c) - if err != nil { - return c.Status(http.StatusUnauthorized).JSON(fiber.Map{"error": "unauthorized"}) - } - - tokenID, err := uuid.Parse(c.Params("id")) - if err != nil { - return c.Status(http.StatusBadRequest).JSON(fiber.Map{"error": "invalid token id"}) - } - - token, err := h.tokenService.GetToken(tokenID) - if err != nil || token == nil { - return c.Status(http.StatusNotFound).JSON(fiber.Map{"error": "token not found"}) - } - - if token.UserID != userID { - return c.Status(http.StatusForbidden).JSON(fiber.Map{"error": "cannot delete another user's token"}) - } - - err = h.tokenService.DeleteToken(tokenID) - if err != nil { - return c.Status(http.StatusInternalServerError).JSON(fiber.Map{"error": "failed to delete token"}) - } - - return c.SendStatus(http.StatusNoContent) -} - -// GetUserIDFromContext extracts user ID from fiber context -func GetUserIDFromContext(c *fiber.Ctx) (uuid.UUID, error) { - userIDVal := c.Locals("userID") - if userIDVal == nil { - return uuid.UUID{}, errors.New("user id not found in context") - } - - userID, ok := userIDVal.(uuid.UUID) - if !ok { - return uuid.UUID{}, errors.New("invalid user id type in context") - } - - return userID, nil -} diff --git a/backend/internal/handler/token_handler_test.go b/backend/internal/handler/token_handler_test.go deleted file mode 100644 index 22737cab..00000000 --- a/backend/internal/handler/token_handler_test.go +++ /dev/null @@ -1,295 +0,0 @@ -// Copyright (c) 2026 OpenDefender Contributors -// SPDX-License-Identifier: AGPL-3.0-only -// This program is free software: you can redistribute it and/or modify it under -// the terms of the GNU Affero General Public License v3.0 (see LICENSE). - -package handler - -import ( - "bytes" - "encoding/json" - "net/http" - "net/http/httptest" - "testing" - - "github.com/gofiber/fiber/v2" - "github.com/google/uuid" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - - "github.com/opendefender/openrisk/internal/domain" - "github.com/opendefender/openrisk/internal/service" -) - -func setupTokenHandlerTest(t *testing.T) (*TokenHandler, *fiber.App, uuid.UUID) { - tokenService := service.NewTokenService() - handler := NewTokenHandler(tokenService) - - app := fiber.New() - userID := uuid.New() - - // Middleware to inject userID for tests - app.Use(func(c *fiber.Ctx) error { - c.Locals("userID", userID) - return c.Next() - }) - - // Register routes - app.Post("/tokens", handler.CreateToken) - app.Get("/tokens", handler.ListTokens) - app.Get("/tokens/:id", handler.GetToken) - app.Put("/tokens/:id", handler.UpdateToken) - app.Post("/tokens/:id/revoke", handler.RevokeToken) - app.Post("/tokens/:id/rotate", handler.RotateToken) - app.Delete("/tokens/:id", handler.DeleteToken) - - return handler, app, userID -} - -func TestTokenHandler_CreateToken_Success(t *testing.T) { - _, app, _ := setupTokenHandlerTest(t) - - body := domain.TokenCreateRequest{ - Name: "Test Token", - Description: "Token for testing", - Type: domain.TokenTypeBearer, - } - - jsonBody, _ := json.Marshal(body) - req := httptest.NewRequest(http.MethodPost, "/tokens", bytes.NewReader(jsonBody)) - req.Header.Set("Content-Type", "application/json") - - resp, err := app.Test(req) - require.NoError(t, err) - defer resp.Body.Close() - require.Equal(t, http.StatusCreated, resp.StatusCode) - - var result map[string]interface{} - err = json.NewDecoder(resp.Body).Decode(&result) - require.NoError(t, err) - - token := result["token"].(map[string]interface{}) - assert.NotEmpty(t, token["id"]) - assert.Equal(t, "Test Token", token["name"]) - assert.NotEmpty(t, token["value"]) - assert.NotEmpty(t, token["token_prefix"]) -} - -func TestTokenHandler_CreateToken_NoName(t *testing.T) { - _, app, _ := setupTokenHandlerTest(t) - - body := domain.TokenCreateRequest{ - Description: "Token without name", - } - - jsonBody, _ := json.Marshal(body) - req := httptest.NewRequest(http.MethodPost, "/tokens", bytes.NewReader(jsonBody)) - req.Header.Set("Content-Type", "application/json") - - resp, err := app.Test(req) - require.NoError(t, err) - defer resp.Body.Close() - require.Equal(t, http.StatusBadRequest, resp.StatusCode) -} - -func TestTokenHandler_ListTokens(t *testing.T) { - handler, _, userID := setupTokenHandlerTest(t) - - // Create some tokens for this user - for i := 0; i < 2; i++ { - handler.tokenService.CreateToken(userID, &domain.TokenCreateRequest{ - Name: "Token " + string(rune(48+i)), - }, userID) - } - - // Create app with this userID - app := fiber.New() - app.Use(func(c *fiber.Ctx) error { - c.Locals("userID", userID) - return c.Next() - }) - app.Get("/tokens", handler.ListTokens) - - req := httptest.NewRequest(http.MethodGet, "/tokens", nil) - resp, err := app.Test(req) - require.NoError(t, err) - defer resp.Body.Close() - require.Equal(t, http.StatusOK, resp.StatusCode) - - var result map[string]interface{} - err = json.NewDecoder(resp.Body).Decode(&result) - require.NoError(t, err) - - assert.Equal(t, float64(2), result["count"]) - assert.Len(t, result["tokens"], 2) -} - -func TestTokenHandler_GetToken_Success(t *testing.T) { - handler, app, userID := setupTokenHandlerTest(t) - - // Create a token - tokenWithValue, err := handler.tokenService.CreateToken(userID, &domain.TokenCreateRequest{ - Name: "Get Test Token", - Description: "Token to get", - }, userID) - require.NoError(t, err) - - req := httptest.NewRequest(http.MethodGet, "/tokens/"+tokenWithValue.ID.String(), nil) - resp, err := app.Test(req) - require.NoError(t, err) - defer resp.Body.Close() - require.Equal(t, http.StatusOK, resp.StatusCode) - - var result map[string]interface{} - err = json.NewDecoder(resp.Body).Decode(&result) - require.NoError(t, err) - - tokenData := result["token"].(map[string]interface{}) - assert.Equal(t, tokenWithValue.ID.String(), tokenData["id"]) - assert.Equal(t, "Get Test Token", tokenData["name"]) -} - -func TestTokenHandler_GetToken_NotFound(t *testing.T) { - _, app, _ := setupTokenHandlerTest(t) - - fakeID := uuid.New() - req := httptest.NewRequest(http.MethodGet, "/tokens/"+fakeID.String(), nil) - resp, err := app.Test(req) - require.NoError(t, err) - defer resp.Body.Close() - require.Equal(t, http.StatusNotFound, resp.StatusCode) -} - -func TestTokenHandler_RevokeToken_Success(t *testing.T) { - handler, app, userID := setupTokenHandlerTest(t) - - // Create a token - tokenWithValue, err := handler.tokenService.CreateToken(userID, &domain.TokenCreateRequest{ - Name: "Token to Revoke", - }, userID) - require.NoError(t, err) - - revokeReq := domain.TokenRevokeRequest{ - Reason: "Testing revoke", - } - - jsonBody, _ := json.Marshal(revokeReq) - req := httptest.NewRequest(http.MethodPost, "/tokens/"+tokenWithValue.ID.String()+"/revoke", bytes.NewReader(jsonBody)) - req.Header.Set("Content-Type", "application/json") - - resp, err := app.Test(req) - require.NoError(t, err) - defer resp.Body.Close() - require.Equal(t, http.StatusOK, resp.StatusCode) - - var result map[string]interface{} - err = json.NewDecoder(resp.Body).Decode(&result) - require.NoError(t, err) - - tokenData := result["token"].(map[string]interface{}) - assert.Equal(t, "revoked", tokenData["status"]) - assert.Equal(t, "Testing revoke", tokenData["revoke_reason"]) -} - -func TestTokenHandler_DeleteToken_Success(t *testing.T) { - handler, app, userID := setupTokenHandlerTest(t) - - // Create a token - tokenWithValue, err := handler.tokenService.CreateToken(userID, &domain.TokenCreateRequest{ - Name: "Token to Delete", - }, userID) - require.NoError(t, err) - - req := httptest.NewRequest(http.MethodDelete, "/tokens/"+tokenWithValue.ID.String(), nil) - resp, err := app.Test(req) - require.NoError(t, err) - defer resp.Body.Close() - require.Equal(t, http.StatusNoContent, resp.StatusCode) - - // Verify token is deleted - deleted, _ := handler.tokenService.GetToken(tokenWithValue.ID) - assert.Nil(t, deleted) -} - -func TestTokenHandler_RotateToken_Success(t *testing.T) { - handler, app, userID := setupTokenHandlerTest(t) - - // Create a token - oldTokenWithValue, err := handler.tokenService.CreateToken(userID, &domain.TokenCreateRequest{ - Name: "Token to Rotate", - }, userID) - require.NoError(t, err) - - req := httptest.NewRequest(http.MethodPost, "/tokens/"+oldTokenWithValue.ID.String()+"/rotate", nil) - resp, err := app.Test(req) - require.NoError(t, err) - defer resp.Body.Close() - require.Equal(t, http.StatusCreated, resp.StatusCode) - - var result map[string]interface{} - err = json.NewDecoder(resp.Body).Decode(&result) - require.NoError(t, err) - - assert.NotNil(t, result["old_token"]) - assert.NotNil(t, result["new_token"]) - - oldToken := result["old_token"].(map[string]interface{}) - assert.Equal(t, "revoked", oldToken["status"]) -} - -func TestTokenHandler_UpdateToken_Success(t *testing.T) { - handler, app, userID := setupTokenHandlerTest(t) - - // Create a token - tokenWithValue, err := handler.tokenService.CreateToken(userID, &domain.TokenCreateRequest{ - Name: "Token to Update", - }, userID) - require.NoError(t, err) - - newDesc := "Updated description" - updateReq := domain.TokenUpdateRequest{ - Description: newDesc, - } - - jsonBody, _ := json.Marshal(updateReq) - req := httptest.NewRequest(http.MethodPut, "/tokens/"+tokenWithValue.ID.String(), bytes.NewReader(jsonBody)) - req.Header.Set("Content-Type", "application/json") - - resp, err := app.Test(req) - require.NoError(t, err) - defer resp.Body.Close() - require.Equal(t, http.StatusOK, resp.StatusCode) - - var result map[string]interface{} - err = json.NewDecoder(resp.Body).Decode(&result) - require.NoError(t, err) - - token := result["token"].(map[string]interface{}) - assert.Equal(t, newDesc, token["description"]) -} - -func TestTokenHandler_OwnershipEnforcement(t *testing.T) { - handler, _, userID1 := setupTokenHandlerTest(t) - - // Create token for userID1 - tokenWithValue, err := handler.tokenService.CreateToken(userID1, &domain.TokenCreateRequest{ - Name: "User1 Token", - }, userID1) - require.NoError(t, err) - - // Simulate different user trying to access - userID2 := uuid.New() - app2 := fiber.New() - app2.Use(func(c *fiber.Ctx) error { - c.Locals("userID", userID2) - return c.Next() - }) - handler2 := NewTokenHandler(handler.tokenService) - app2.Get("/tokens/:id", handler2.GetToken) - - req := httptest.NewRequest(http.MethodGet, "/tokens/"+tokenWithValue.ID.String(), nil) - resp, err := app2.Test(req) - require.NoError(t, err) - defer resp.Body.Close() - require.Equal(t, http.StatusForbidden, resp.StatusCode) -} diff --git a/backend/internal/middleware/tokenauth.go b/backend/internal/middleware/tokenauth.go deleted file mode 100644 index e616a6b5..00000000 --- a/backend/internal/middleware/tokenauth.go +++ /dev/null @@ -1,181 +0,0 @@ -// Copyright (c) 2026 OpenDefender Contributors -// SPDX-License-Identifier: AGPL-3.0-only -// This program is free software: you can redistribute it and/or modify it under -// the terms of the GNU Affero General Public License v3.0 (see LICENSE). - -package middleware - -import ( - "errors" - "net/http" - "strings" - - "github.com/gofiber/fiber/v2" - "github.com/google/uuid" - - "github.com/opendefender/openrisk/internal/service" -) - -// TokenAuth provides token-based authentication middleware -type TokenAuth struct { - tokenService *service.TokenService -} - -// NewTokenAuth creates a new token authentication middleware -func NewTokenAuth(tokenService *service.TokenService) *TokenAuth { - return &TokenAuth{ - tokenService: tokenService, - } -} - -// ExtractTokenFromRequest extracts the token from the request -// Supports Bearer token in Authorization header: "Bearer " -func (ta *TokenAuth) ExtractTokenFromRequest(c *fiber.Ctx) (string, error) { - authHeader := c.Get("Authorization") - if authHeader == "" { - return "", errors.New("authorization header is missing") - } - - parts := strings.SplitN(authHeader, " ", 2) - if len(parts) != 2 { - return "", errors.New("authorization header format is invalid") - } - - if parts[0] != "Bearer" { - return "", errors.New("authorization scheme must be Bearer") - } - - token := parts[1] - if token == "" { - return "", errors.New("authorization token is empty") - } - - return token, nil -} - -// Verify is a middleware that verifies API tokens -// Usage: app.Use(tokenAuth.Verify) -func (ta *TokenAuth) Verify(c *fiber.Ctx) error { - tokenValue, err := ta.ExtractTokenFromRequest(c) - if err != nil { - return c.Status(http.StatusUnauthorized).JSON(fiber.Map{ - "error": "invalid authorization header", - }) - } - - // Verify token - token, err := ta.tokenService.VerifyToken(tokenValue) - if err != nil || token == nil { - return c.Status(http.StatusUnauthorized).JSON(fiber.Map{ - "error": "invalid or expired token", - }) - } - - // Check if token is valid - if !token.IsValid() { - return c.Status(http.StatusUnauthorized).JSON(fiber.Map{ - "error": "token is not valid for use", - }) - } - - // Check IP whitelist if configured - if len(token.IPWhitelist) > 0 && !token.IsIPAllowed(c.IP()) { - return c.Status(http.StatusForbidden).JSON(fiber.Map{ - "error": "client IP is not whitelisted for this token", - }) - } - - // Note: Last used timestamp is already updated by VerifyToken - - // Store in context for handlers - c.Locals("userID", token.UserID) - c.Locals("tokenID", token.ID) - c.Locals("tokenPermissions", token.Permissions) - c.Locals("tokenType", token.Type) - - return c.Next() -} - -// RequireTokenPermission checks if the token has a specific permission -// Usage: app.Get("/resource", tokenAuth.RequireTokenPermission("resource:action:scope")) -func (ta *TokenAuth) RequireTokenPermission(requiredPermission string) fiber.Handler { - return func(c *fiber.Ctx) error { - tokenID, ok := c.Locals("tokenID").(uuid.UUID) - if !ok { - return c.Status(http.StatusUnauthorized).JSON(fiber.Map{ - "error": "token not authenticated", - }) - } - - token, err := ta.tokenService.GetToken(tokenID) - if err != nil || token == nil { - return c.Status(http.StatusUnauthorized).JSON(fiber.Map{ - "error": "token not found", - }) - } - - if !token.HasPermission(requiredPermission) { - return c.Status(http.StatusForbidden).JSON(fiber.Map{ - "error": "token does not have required permission", - }) - } - - return c.Next() - } -} - -// RequireTokenScope checks if the token has a specific scope -// Usage: app.Get("/risks", tokenAuth.RequireTokenScope("risk")) -func (ta *TokenAuth) RequireTokenScope(requiredScope string) fiber.Handler { - return func(c *fiber.Ctx) error { - tokenID, ok := c.Locals("tokenID").(uuid.UUID) - if !ok { - return c.Status(http.StatusUnauthorized).JSON(fiber.Map{ - "error": "token not authenticated", - }) - } - - token, err := ta.tokenService.GetToken(tokenID) - if err != nil || token == nil { - return c.Status(http.StatusUnauthorized).JSON(fiber.Map{ - "error": "token not found", - }) - } - - if !token.HasScope(requiredScope) { - return c.Status(http.StatusForbidden).JSON(fiber.Map{ - "error": "token does not have required scope", - }) - } - - return c.Next() - } -} - -// Combined middleware: Verify + RequireTokenPermission -// Usage: app.Get("/resource", tokenAuth.VerifyAndRequirePermission("resource:action:scope")) -func (ta *TokenAuth) VerifyAndRequirePermission(requiredPermission string) fiber.Handler { - return func(c *fiber.Ctx) error { - // First verify token - if err := ta.Verify(c); err != nil { - return err - } - - // Then check permission - return ta.RequireTokenPermission(requiredPermission)(c) - } -} - -// Combined middleware: Verify + RequireTokenScope -// Usage: app.Get("/risks", tokenAuth.VerifyAndRequireScope("risk")) -func (ta *TokenAuth) VerifyAndRequireScope(requiredScope string) fiber.Handler { - return func(c *fiber.Ctx) error { - // First verify token - if err := ta.Verify(c); err != nil { - return err - } - - // Then check scope - return ta.RequireTokenScope(requiredScope)(c) - } -} diff --git a/backend/internal/middleware/tokenauth_test.go b/backend/internal/middleware/tokenauth_test.go deleted file mode 100644 index b53f6280..00000000 --- a/backend/internal/middleware/tokenauth_test.go +++ /dev/null @@ -1,330 +0,0 @@ -// Copyright (c) 2026 OpenDefender Contributors -// SPDX-License-Identifier: AGPL-3.0-only -// This program is free software: you can redistribute it and/or modify it under -// the terms of the GNU Affero General Public License v3.0 (see LICENSE). - -package middleware - -import ( - "net/http" - "net/http/httptest" - "testing" - - "github.com/gofiber/fiber/v2" - "github.com/google/uuid" - "github.com/stretchr/testify/require" - - "github.com/opendefender/openrisk/internal/domain" - "github.com/opendefender/openrisk/internal/service" -) - -func TestTokenAuth_ExtractTokenFromRequest_Success(t *testing.T) { - tokenAuth := &TokenAuth{} - - app := fiber.New() - app.Get("/", func(c *fiber.Ctx) error { - token, err := tokenAuth.ExtractTokenFromRequest(c) - if err != nil { - return c.Status(http.StatusBadRequest).JSON(fiber.Map{"error": err.Error()}) - } - return c.JSON(fiber.Map{"token": token}) - }) - - req := httptest.NewRequest(http.MethodGet, "/", nil) - req.Header.Set("Authorization", "Bearer test-token-value") - - resp, err := app.Test(req) - require.NoError(t, err) - defer resp.Body.Close() - require.Equal(t, http.StatusOK, resp.StatusCode) -} - -func TestTokenAuth_ExtractTokenFromRequest_MissingHeader(t *testing.T) { - tokenAuth := &TokenAuth{} - - app := fiber.New() - app.Get("/", func(c *fiber.Ctx) error { - _, err := tokenAuth.ExtractTokenFromRequest(c) - if err != nil { - return c.Status(http.StatusUnauthorized).JSON(fiber.Map{"error": err.Error()}) - } - return c.SendStatus(http.StatusOK) - }) - - req := httptest.NewRequest(http.MethodGet, "/", nil) - resp, err := app.Test(req) - require.NoError(t, err) - defer resp.Body.Close() - require.Equal(t, http.StatusUnauthorized, resp.StatusCode) -} - -func TestTokenAuth_ExtractTokenFromRequest_InvalidFormat(t *testing.T) { - tokenAuth := &TokenAuth{} - - app := fiber.New() - app.Get("/", func(c *fiber.Ctx) error { - _, err := tokenAuth.ExtractTokenFromRequest(c) - if err != nil { - return c.Status(http.StatusUnauthorized).JSON(fiber.Map{"error": err.Error()}) - } - return c.SendStatus(http.StatusOK) - }) - - req := httptest.NewRequest(http.MethodGet, "/", nil) - req.Header.Set("Authorization", "InvalidFormat") - resp, err := app.Test(req) - require.NoError(t, err) - defer resp.Body.Close() - require.Equal(t, http.StatusUnauthorized, resp.StatusCode) -} - -func TestTokenAuth_ExtractTokenFromRequest_WrongScheme(t *testing.T) { - tokenAuth := &TokenAuth{} - - app := fiber.New() - app.Get("/", func(c *fiber.Ctx) error { - _, err := tokenAuth.ExtractTokenFromRequest(c) - if err != nil { - return c.Status(http.StatusUnauthorized).JSON(fiber.Map{"error": err.Error()}) - } - return c.SendStatus(http.StatusOK) - }) - - req := httptest.NewRequest(http.MethodGet, "/", nil) - req.Header.Set("Authorization", "Basic dXNlcjpwYXNz") - resp, err := app.Test(req) - require.NoError(t, err) - defer resp.Body.Close() - require.Equal(t, http.StatusUnauthorized, resp.StatusCode) -} - -func TestTokenAuth_Verify_Success(t *testing.T) { - tokenService := service.NewTokenService() - tokenAuth := NewTokenAuth(tokenService) - - app := fiber.New() - app.Use(tokenAuth.Verify) - app.Get("/protected", func(c *fiber.Ctx) error { - return c.JSON(fiber.Map{"message": "protected resource"}) - }) - - userID := uuid.New() - tokenWithValue, err := tokenService.CreateToken(userID, &domain.TokenCreateRequest{ - Name: "Test Token", - }, userID) - require.NoError(t, err) - - req := httptest.NewRequest(http.MethodGet, "/protected", nil) - req.Header.Set("Authorization", "Bearer "+tokenWithValue.Token) - - resp, err := app.Test(req) - require.NoError(t, err) - defer resp.Body.Close() - require.Equal(t, http.StatusOK, resp.StatusCode) -} - -func TestTokenAuth_Verify_NoHeader(t *testing.T) { - tokenService := service.NewTokenService() - tokenAuth := NewTokenAuth(tokenService) - - app := fiber.New() - app.Use(tokenAuth.Verify) - app.Get("/protected", func(c *fiber.Ctx) error { - return c.JSON(fiber.Map{"message": "protected"}) - }) - - req := httptest.NewRequest(http.MethodGet, "/protected", nil) - resp, err := app.Test(req) - require.NoError(t, err) - defer resp.Body.Close() - require.Equal(t, http.StatusUnauthorized, resp.StatusCode) -} - -func TestTokenAuth_Verify_InvalidToken(t *testing.T) { - tokenService := service.NewTokenService() - tokenAuth := NewTokenAuth(tokenService) - - app := fiber.New() - app.Use(tokenAuth.Verify) - app.Get("/protected", func(c *fiber.Ctx) error { - return c.JSON(fiber.Map{"message": "protected"}) - }) - - req := httptest.NewRequest(http.MethodGet, "/protected", nil) - req.Header.Set("Authorization", "Bearer invalid-token") - - resp, err := app.Test(req) - require.NoError(t, err) - defer resp.Body.Close() - require.Equal(t, http.StatusUnauthorized, resp.StatusCode) -} - -func TestTokenAuth_Verify_RevokedToken(t *testing.T) { - tokenService := service.NewTokenService() - tokenAuth := NewTokenAuth(tokenService) - - app := fiber.New() - app.Use(tokenAuth.Verify) - app.Get("/protected", func(c *fiber.Ctx) error { - return c.JSON(fiber.Map{"message": "protected"}) - }) - - userID := uuid.New() - tokenWithValue, err := tokenService.CreateToken(userID, &domain.TokenCreateRequest{ - Name: "Revokable Token", - }, userID) - require.NoError(t, err) - - // Revoke token - tokenService.RevokeToken(tokenWithValue.ID, "testing") - - req := httptest.NewRequest(http.MethodGet, "/protected", nil) - req.Header.Set("Authorization", "Bearer "+tokenWithValue.Token) - - resp, err := app.Test(req) - require.NoError(t, err) - defer resp.Body.Close() - require.Equal(t, http.StatusUnauthorized, resp.StatusCode) -} - -func TestTokenAuth_RequireTokenPermission_Success(t *testing.T) { - tokenService := service.NewTokenService() - tokenAuth := NewTokenAuth(tokenService) - - app := fiber.New() - // Register both Verify and RequireTokenPermission - app.Use("/risks", tokenAuth.Verify) - app.Get("/risks", tokenAuth.RequireTokenPermission("risk:read:any"), func(c *fiber.Ctx) error { - return c.JSON(fiber.Map{"risks": []string{}}) - }) - - userID := uuid.New() - tokenWithValue, err := tokenService.CreateToken(userID, &domain.TokenCreateRequest{ - Name: "Read Risks Token", - Permissions: []string{"risk:read:any"}, - }, userID) - require.NoError(t, err) - - req := httptest.NewRequest(http.MethodGet, "/risks", nil) - req.Header.Set("Authorization", "Bearer "+tokenWithValue.Token) - - resp, err := app.Test(req) - require.NoError(t, err) - defer resp.Body.Close() - require.Equal(t, http.StatusOK, resp.StatusCode) -} - -func TestTokenAuth_RequireTokenPermission_Denied(t *testing.T) { - tokenService := service.NewTokenService() - tokenAuth := NewTokenAuth(tokenService) - - app := fiber.New() - app.Use("/risks", tokenAuth.Verify) - app.Get("/risks", tokenAuth.RequireTokenPermission("risk:delete:any"), func(c *fiber.Ctx) error { - return c.JSON(fiber.Map{"message": "deleted"}) - }) - - userID := uuid.New() - tokenWithValue, err := tokenService.CreateToken(userID, &domain.TokenCreateRequest{ - Name: "Read Only Token", - Permissions: []string{"risk:read:any"}, - }, userID) - require.NoError(t, err) - - req := httptest.NewRequest(http.MethodGet, "/risks", nil) - req.Header.Set("Authorization", "Bearer "+tokenWithValue.Token) - - resp, err := app.Test(req) - require.NoError(t, err) - defer resp.Body.Close() - require.Equal(t, http.StatusForbidden, resp.StatusCode) -} - -func TestTokenAuth_RequireTokenScope_Success(t *testing.T) { - tokenService := service.NewTokenService() - tokenAuth := NewTokenAuth(tokenService) - - app := fiber.New() - app.Use("/risks", tokenAuth.Verify) - app.Get("/risks", tokenAuth.RequireTokenScope("risk"), func(c *fiber.Ctx) error { - return c.JSON(fiber.Map{"risks": []string{}}) - }) - - userID := uuid.New() - tokenWithValue, err := tokenService.CreateToken(userID, &domain.TokenCreateRequest{ - Name: "Risk Scoped Token", - Scopes: []string{"risk", "mitigation"}, - }, userID) - require.NoError(t, err) - - req := httptest.NewRequest(http.MethodGet, "/risks", nil) - req.Header.Set("Authorization", "Bearer "+tokenWithValue.Token) - - resp, err := app.Test(req) - require.NoError(t, err) - defer resp.Body.Close() - require.Equal(t, http.StatusOK, resp.StatusCode) -} - -func TestTokenAuth_RequireTokenScope_Denied(t *testing.T) { - tokenService := service.NewTokenService() - tokenAuth := NewTokenAuth(tokenService) - - app := fiber.New() - app.Use("/assets", tokenAuth.Verify) - app.Get("/assets", tokenAuth.RequireTokenScope("asset"), func(c *fiber.Ctx) error { - return c.JSON(fiber.Map{"assets": []string{}}) - }) - - userID := uuid.New() - tokenWithValue, err := tokenService.CreateToken(userID, &domain.TokenCreateRequest{ - Name: "Risk Only Token", - Scopes: []string{"risk"}, - }, userID) - require.NoError(t, err) - - req := httptest.NewRequest(http.MethodGet, "/assets", nil) - req.Header.Set("Authorization", "Bearer "+tokenWithValue.Token) - - resp, err := app.Test(req) - require.NoError(t, err) - defer resp.Body.Close() - require.Equal(t, http.StatusForbidden, resp.StatusCode) -} - -func TestTokenAuth_ContextPopulation(t *testing.T) { - tokenService := service.NewTokenService() - tokenAuth := NewTokenAuth(tokenService) - - userID := uuid.New() - tokenWithValue, err := tokenService.CreateToken(userID, &domain.TokenCreateRequest{ - Name: "Context Test Token", - }, userID) - require.NoError(t, err) - - app := fiber.New() - app.Use(tokenAuth.Verify) - app.Get("/test", func(c *fiber.Ctx) error { - retrievedUserID := c.Locals("userID") - retrievedTokenID := c.Locals("tokenID") - - if retrievedUserID == nil || retrievedTokenID == nil { - return c.Status(http.StatusInternalServerError).JSON(fiber.Map{ - "error": "context not populated", - }) - } - - return c.JSON(fiber.Map{ - "userID": retrievedUserID, - "tokenID": retrievedTokenID, - }) - }) - - req := httptest.NewRequest(http.MethodGet, "/test", nil) - req.Header.Set("Authorization", "Bearer "+tokenWithValue.Token) - - resp, err := app.Test(req) - require.NoError(t, err) - defer resp.Body.Close() - require.Equal(t, http.StatusOK, resp.StatusCode) -} diff --git a/backend/internal/security/isolation/registry.go b/backend/internal/security/isolation/registry.go index cd04cccd..a302fc8c 100644 --- a/backend/internal/security/isolation/registry.go +++ b/backend/internal/security/isolation/registry.go @@ -251,8 +251,6 @@ var decisions = []Decision{ "sabotage — the predicate was removed from each query and every one of these failed"}, {"/api/v1/rbac/*", Pending, "admin-only tenant/user/role admin paths; GetTenantUsers gates on level, not pinned"}, - {"/api/v1/tokens/*", Pending, - "API token management; ownership checks not pinned by a test"}, {"/api/v1/bulk-operations/{id}", Pending, "service filters by tenant (see bulk_operation_service); not pinned by a test"}, {"/api/v1/integrations/{id}/test", Pending, @@ -597,9 +595,7 @@ var decisions = []Decision{ {"/api/v1/auth/sessions", SelfScoped, "the caller's own devices, keyed on mwCtx.UserID from the token; 401 when absent. The revoke sibling is already recorded SelfScoped with its cross-user test (TestRevoke_CrossUser_IsNotFound)"}, {"/api/v1/auth/pat", SelfScoped, - "ListUserTokens(callerUserID) — the caller's own personal access tokens, keyed on the token's user id; 401 when absent"}, - {"/api/v1/tokens", SelfScoped, - "TokenService.ListTokens(callerUserID) — the caller's own API tokens. Note this listing is scoped by USER and not by tenant, so a person who belongs to two organisations sees all their own tokens in one list regardless of the session's organisation. That is a deliberate consequence of tokens belonging to a person rather than to a membership; it discloses nothing across the tenant boundary, because the person owns every row either way"}, + "ListUserTokens(callerTenantID, callerUserID) — the tokens the caller minted in the session's organisation, filtered on tenant_id AND user_id; 401 when either is absent. Pinned by TestPATRepo_TenantIsolation (#782)"}, {"/api/v1/ownership/assignable", Pending, "assessed, not pinned: ListAssignableUseCase refuses uuid.Nil with ErrValidation and reads members.ListMembers(ctx, tenantID); the search, permission and only_capable filters narrow that set in memory afterwards. Unresolved: this is a DIFFERENT ListMembers from the paginated one covered by TestMembershipRepo_ListMembers_TenantIsolationAndFilters, so that test does not reach it. Settled by a two-tenant assertion on the unpaginated ListMembers"}, diff --git a/backend/internal/service/token_service.go b/backend/internal/service/token_service.go deleted file mode 100644 index 9a8c44c0..00000000 --- a/backend/internal/service/token_service.go +++ /dev/null @@ -1,375 +0,0 @@ -// Copyright (c) 2026 OpenDefender Contributors -// SPDX-License-Identifier: AGPL-3.0-only -// This program is free software: you can redistribute it and/or modify it under -// the terms of the GNU Affero General Public License v3.0 (see LICENSE). - -package service - -import ( - "crypto/rand" - "crypto/sha256" - "encoding/hex" - "fmt" - "strings" - "sync" - "time" - - "github.com/google/uuid" - "github.com/opendefender/openrisk/internal/domain" -) - -const ( - TokenLength = 32 // Length of random token bytes - TokenDisplayLength = 8 // Length of token prefix to display - TokenPrefix = "orsk_" // Prefix for OpenRisk tokens - DefaultTokenExpiry = 90 * 24 * time.Hour // 90 days default -) - -// TokenService handles API token management -type TokenService struct { - tokens map[string]*domain.APIToken // tokenHash -> token - mu sync.RWMutex -} - -// NewTokenService creates a new token service -func NewTokenService() *TokenService { - return &TokenService{ - tokens: make(map[string]*domain.APIToken), - } -} - -// GenerateToken creates a new random token string -func (ts *TokenService) GenerateToken() (string, error) { - randomBytes := make([]byte, TokenLength) - _, err := rand.Read(randomBytes) - if err != nil { - return "", fmt.Errorf("failed to generate random token: %w", err) - } - - token := TokenPrefix + hex.EncodeToString(randomBytes) - return token, nil -} - -// HashToken creates a SHA256 hash of a token -func (ts *TokenService) HashToken(token string) string { - hash := sha256.Sum256([]byte(token)) - return hex.EncodeToString(hash[:]) -} - -// GetTokenPrefix extracts the display prefix from a token -func (ts *TokenService) GetTokenPrefix(token string) string { - if len(token) > TokenDisplayLength { - return token[:TokenDisplayLength] - } - return token -} - -// CreateToken creates a new API token for a user -func (ts *TokenService) CreateToken(userID uuid.UUID, req *domain.TokenCreateRequest, createdByID uuid.UUID) (*domain.TokenWithValue, error) { - // Generate the token - tokenValue, err := ts.GenerateToken() - if err != nil { - return nil, err - } - - // Hash the token for storage - tokenHash := ts.HashToken(tokenValue) - tokenPrefix := ts.GetTokenPrefix(tokenValue) - - // Set expiration if not provided - expiresAt := req.ExpiresAt - if expiresAt == nil && req.Type == domain.TokenTypeBearer { - exp := time.Now().Add(DefaultTokenExpiry) - expiresAt = &exp - } - - // Create the token entity - token := &domain.APIToken{ - ID: uuid.New(), - UserID: userID, - Name: req.Name, - Description: req.Description, - TokenHash: tokenHash, - TokenPrefix: tokenPrefix, - Type: req.Type, - Status: domain.TokenStatusActive, - Permissions: req.Permissions, - Scopes: req.Scopes, - IPWhitelist: req.IPWhitelist, - Metadata: req.Metadata, - ExpiresAt: expiresAt, - CreatedAt: time.Now(), - UpdatedAt: time.Now(), - CreatedByID: createdByID, - } - - // Store token - ts.mu.Lock() - ts.tokens[tokenHash] = token - ts.mu.Unlock() - - // Return token with value (only shown once) - return &domain.TokenWithValue{ - TokenResponse: ts.tokenToResponse(token), - Token: tokenValue, - }, nil -} - -// VerifyToken verifies a token string and returns the token entity if valid -func (ts *TokenService) VerifyToken(tokenValue string) (*domain.APIToken, error) { - if !strings.HasPrefix(tokenValue, TokenPrefix) { - return nil, fmt.Errorf("invalid token format") - } - - tokenHash := ts.HashToken(tokenValue) - - ts.mu.RLock() - token, exists := ts.tokens[tokenHash] - ts.mu.RUnlock() - - if !exists { - return nil, fmt.Errorf("token not found") - } - - // Check if token is valid - if !token.IsValid() { - status := "revoked" - if token.IsExpired() { - status = "expired" - } else if token.Status == domain.TokenStatusDisabled { - status = "disabled" - } - return nil, fmt.Errorf("token is %s", status) - } - - // Update last used - token.UpdateLastUsed() - - return token, nil -} - -// GetToken retrieves a token by ID -func (ts *TokenService) GetToken(tokenID uuid.UUID) (*domain.APIToken, error) { - ts.mu.RLock() - defer ts.mu.RUnlock() - - for _, token := range ts.tokens { - if token.ID == tokenID { - return token, nil - } - } - - return nil, fmt.Errorf("token not found") -} - -// ListTokens retrieves all tokens for a user -func (ts *TokenService) ListTokens(userID uuid.UUID) ([]*domain.APIToken, error) { - ts.mu.RLock() - defer ts.mu.RUnlock() - - var userTokens []*domain.APIToken - for _, token := range ts.tokens { - if token.UserID == userID { - userTokens = append(userTokens, token) - } - } - - return userTokens, nil -} - -// UpdateToken updates a token's properties -func (ts *TokenService) UpdateToken(tokenID uuid.UUID, req *domain.TokenUpdateRequest) (*domain.APIToken, error) { - token, err := ts.GetToken(tokenID) - if err != nil { - return nil, err - } - - ts.mu.Lock() - defer ts.mu.Unlock() - - // Update only provided fields - if req.Name != "" { - token.Name = req.Name - } - if req.Description != "" { - token.Description = req.Description - } - if req.Permissions != nil { - token.Permissions = req.Permissions - } - if req.Scopes != nil { - token.Scopes = req.Scopes - } - if req.IPWhitelist != nil { - token.IPWhitelist = req.IPWhitelist - } - if req.Metadata != nil { - token.Metadata = req.Metadata - } - - token.UpdatedAt = time.Now() - - return token, nil -} - -// RevokeToken revokes a token -func (ts *TokenService) RevokeToken(tokenID uuid.UUID, reason string) (*domain.APIToken, error) { - token, err := ts.GetToken(tokenID) - if err != nil { - return nil, err - } - - ts.mu.Lock() - defer ts.mu.Unlock() - - token.Revoke(reason) - token.UpdatedAt = time.Now() - - return token, nil -} - -// RotateToken revokes the old token and creates a new one -func (ts *TokenService) RotateToken(tokenID uuid.UUID, userID uuid.UUID, reason string) (*domain.RotateTokenResponse, error) { - // Get old token - oldToken, err := ts.GetToken(tokenID) - if err != nil { - return nil, err - } - - if oldToken.UserID != userID { - return nil, fmt.Errorf("unauthorized: token does not belong to user") - } - - // Revoke old token - ts.mu.Lock() - oldToken.Revoke(fmt.Sprintf("Rotated: %s", reason)) - oldToken.UpdatedAt = time.Now() - ts.mu.Unlock() - - // Create new token with same properties - createReq := &domain.TokenCreateRequest{ - Name: oldToken.Name, - Description: oldToken.Description, - Type: oldToken.Type, - Permissions: oldToken.Permissions, - Scopes: oldToken.Scopes, - IPWhitelist: oldToken.IPWhitelist, - Metadata: oldToken.Metadata, - } - - // Set expiration if old token had one - if oldToken.ExpiresAt != nil { - // New token expires in same time period as old one - duration := oldToken.ExpiresAt.Sub(oldToken.CreatedAt) - newExpiry := time.Now().Add(duration) - createReq.ExpiresAt = &newExpiry - } - - newToken, err := ts.CreateToken(userID, createReq, oldToken.CreatedByID) - if err != nil { - return nil, err - } - - return &domain.RotateTokenResponse{ - OldToken: ts.tokenToResponse(oldToken), - NewToken: newToken, - RotatedAt: time.Now(), - }, nil -} - -// DeleteToken permanently deletes a token (hard delete) -func (ts *TokenService) DeleteToken(tokenID uuid.UUID) error { - token, err := ts.GetToken(tokenID) - if err != nil { - return err - } - - ts.mu.Lock() - defer ts.mu.Unlock() - - delete(ts.tokens, token.TokenHash) - return nil -} - -// DisableToken disables a token without revoking it -func (ts *TokenService) DisableToken(tokenID uuid.UUID) (*domain.APIToken, error) { - token, err := ts.GetToken(tokenID) - if err != nil { - return nil, err - } - - ts.mu.Lock() - defer ts.mu.Unlock() - - token.Disable() - token.UpdatedAt = time.Now() - - return token, nil -} - -// EnableToken re-enables a disabled token -func (ts *TokenService) EnableToken(tokenID uuid.UUID) (*domain.APIToken, error) { - token, err := ts.GetToken(tokenID) - if err != nil { - return nil, err - } - - ts.mu.Lock() - defer ts.mu.Unlock() - - token.Enable() - token.UpdatedAt = time.Now() - - return token, nil -} - -// CheckTokenExpiry marks expired tokens as such -func (ts *TokenService) CheckTokenExpiry(tokenID uuid.UUID) (*domain.APIToken, error) { - token, err := ts.GetToken(tokenID) - if err != nil { - return nil, err - } - - ts.mu.Lock() - defer ts.mu.Unlock() - - if token.IsExpired() && token.Status == domain.TokenStatusActive { - token.Status = domain.TokenStatusExpired - } - - return token, nil -} - -// CleanupExpiredTokens removes expired tokens (maintenance task) -func (ts *TokenService) CleanupExpiredTokens() error { - ts.mu.Lock() - defer ts.mu.Unlock() - - for hash, token := range ts.tokens { - if token.IsExpired() { - delete(ts.tokens, hash) - } - } - - return nil -} - -// Helper function to convert token to response -func (ts *TokenService) tokenToResponse(token *domain.APIToken) *domain.TokenResponse { - return &domain.TokenResponse{ - ID: token.ID, - Name: token.Name, - Description: token.Description, - TokenPrefix: token.TokenPrefix, - Type: token.Type, - Status: token.Status, - Permissions: token.Permissions, - LastUsed: token.LastUsed, - ExpiresAt: token.ExpiresAt, - RevokedAt: token.RevokedAt, - CreatedAt: token.CreatedAt, - UpdatedAt: token.UpdatedAt, - Scopes: token.Scopes, - Metadata: token.Metadata, - } -} diff --git a/backend/internal/service/token_service_test.go b/backend/internal/service/token_service_test.go deleted file mode 100644 index e22429f6..00000000 --- a/backend/internal/service/token_service_test.go +++ /dev/null @@ -1,574 +0,0 @@ -// Copyright (c) 2026 OpenDefender Contributors -// SPDX-License-Identifier: AGPL-3.0-only -// This program is free software: you can redistribute it and/or modify it under -// the terms of the GNU Affero General Public License v3.0 (see LICENSE). - -package service - -import ( - "testing" - "time" - - "github.com/google/uuid" - "github.com/opendefender/openrisk/internal/domain" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestGenerateToken(t *testing.T) { - ts := NewTokenService() - - token, err := ts.GenerateToken() - require.NoError(t, err) - assert.NotEmpty(t, token) - assert.True(t, len(token) > 0) - assert.Equal(t, TokenPrefix, token[:len(TokenPrefix)]) -} - -func TestHashToken(t *testing.T) { - ts := NewTokenService() - - token := "test_token_value" - hash1 := ts.HashToken(token) - hash2 := ts.HashToken(token) - - assert.Equal(t, hash1, hash2, "same token should produce same hash") - assert.NotEmpty(t, hash1) -} - -func TestGetTokenPrefix(t *testing.T) { - ts := NewTokenService() - - token := TokenPrefix + "abcdefghijklmnop" - prefix := ts.GetTokenPrefix(token) - - assert.Equal(t, TokenPrefix+"ab", prefix[:len(TokenPrefix)+2]) -} - -func TestCreateToken_Success(t *testing.T) { - ts := NewTokenService() - userID := uuid.New() - createdByID := uuid.New() - - req := &domain.TokenCreateRequest{ - Name: "Test Token", - Description: "For testing", - Type: domain.TokenTypeBearer, - Permissions: []string{"read", "write"}, - Scopes: []string{"risk"}, - } - - tokenWithValue, err := ts.CreateToken(userID, req, createdByID) - require.NoError(t, err) - assert.NotNil(t, tokenWithValue) - assert.Equal(t, "Test Token", tokenWithValue.Name) - assert.Equal(t, domain.TokenStatusActive, tokenWithValue.Status) - assert.NotEmpty(t, tokenWithValue.Token) - assert.Equal(t, TokenPrefix, tokenWithValue.TokenPrefix[:len(TokenPrefix)]) -} - -func TestCreateToken_WithExpiry(t *testing.T) { - ts := NewTokenService() - userID := uuid.New() - createdByID := uuid.New() - - expiryTime := time.Now().Add(24 * time.Hour) - req := &domain.TokenCreateRequest{ - Name: "Test Token", - Type: domain.TokenTypeBearer, - ExpiresAt: &expiryTime, - } - - tokenWithValue, err := ts.CreateToken(userID, req, createdByID) - require.NoError(t, err) - assert.NotNil(t, tokenWithValue.ExpiresAt) -} - -func TestVerifyToken_Valid(t *testing.T) { - ts := NewTokenService() - userID := uuid.New() - createdByID := uuid.New() - - req := &domain.TokenCreateRequest{ - Name: "Test Token", - Type: domain.TokenTypeBearer, - } - - tokenWithValue, err := ts.CreateToken(userID, req, createdByID) - require.NoError(t, err) - - // Verify the token - token, err := ts.VerifyToken(tokenWithValue.Token) - require.NoError(t, err) - assert.NotNil(t, token) - assert.Equal(t, userID, token.UserID) - assert.Equal(t, domain.TokenStatusActive, token.Status) -} - -func TestVerifyToken_InvalidFormat(t *testing.T) { - ts := NewTokenService() - - _, err := ts.VerifyToken("invalid_token") - assert.Error(t, err) - assert.Contains(t, err.Error(), "invalid token format") -} - -func TestVerifyToken_NotFound(t *testing.T) { - ts := NewTokenService() - - _, err := ts.VerifyToken(TokenPrefix + "nonexistenttoken") - assert.Error(t, err) - assert.Contains(t, err.Error(), "token not found") -} - -func TestVerifyToken_UpdatesLastUsed(t *testing.T) { - ts := NewTokenService() - userID := uuid.New() - createdByID := uuid.New() - - req := &domain.TokenCreateRequest{ - Name: "Test Token", - Type: domain.TokenTypeBearer, - } - - tokenWithValue, err := ts.CreateToken(userID, req, createdByID) - require.NoError(t, err) - - time.Sleep(10 * time.Millisecond) - - token, err := ts.VerifyToken(tokenWithValue.Token) - require.NoError(t, err) - assert.NotNil(t, token.LastUsed) -} - -func TestGetToken(t *testing.T) { - ts := NewTokenService() - userID := uuid.New() - createdByID := uuid.New() - - req := &domain.TokenCreateRequest{ - Name: "Test Token", - Type: domain.TokenTypeBearer, - } - - tokenWithValue, err := ts.CreateToken(userID, req, createdByID) - require.NoError(t, err) - - // Retrieve token by ID - token, err := ts.GetToken(tokenWithValue.ID) - require.NoError(t, err) - assert.Equal(t, "Test Token", token.Name) -} - -func TestListTokens(t *testing.T) { - ts := NewTokenService() - userID := uuid.New() - otherUserID := uuid.New() - createdByID := uuid.New() - - req := &domain.TokenCreateRequest{ - Name: "User Token", - Type: domain.TokenTypeBearer, - } - - // Create tokens for different users - _, err := ts.CreateToken(userID, req, createdByID) - require.NoError(t, err) - - _, err = ts.CreateToken(userID, req, createdByID) - require.NoError(t, err) - - _, err = ts.CreateToken(otherUserID, req, createdByID) - require.NoError(t, err) - - // List tokens for first user - tokens, err := ts.ListTokens(userID) - require.NoError(t, err) - assert.Equal(t, 2, len(tokens)) - - // List tokens for second user - tokens, err = ts.ListTokens(otherUserID) - require.NoError(t, err) - assert.Equal(t, 1, len(tokens)) -} - -func TestUpdateToken(t *testing.T) { - ts := NewTokenService() - userID := uuid.New() - createdByID := uuid.New() - - req := &domain.TokenCreateRequest{ - Name: "Original Name", - Type: domain.TokenTypeBearer, - } - - tokenWithValue, err := ts.CreateToken(userID, req, createdByID) - require.NoError(t, err) - - // Update token - updateReq := &domain.TokenUpdateRequest{ - Name: "Updated Name", - Description: "New description", - } - - updated, err := ts.UpdateToken(tokenWithValue.ID, updateReq) - require.NoError(t, err) - assert.Equal(t, "Updated Name", updated.Name) - assert.Equal(t, "New description", updated.Description) -} - -func TestRevokeToken(t *testing.T) { - ts := NewTokenService() - userID := uuid.New() - createdByID := uuid.New() - - req := &domain.TokenCreateRequest{ - Name: "Test Token", - Type: domain.TokenTypeBearer, - } - - tokenWithValue, err := ts.CreateToken(userID, req, createdByID) - require.NoError(t, err) - - // Revoke token - revoked, err := ts.RevokeToken(tokenWithValue.ID, "Security concerns") - require.NoError(t, err) - assert.Equal(t, domain.TokenStatusRevoked, revoked.Status) - assert.NotNil(t, revoked.RevokedAt) - assert.Equal(t, "Security concerns", revoked.RevokeReason) - - // Verification should fail - _, err = ts.VerifyToken(tokenWithValue.Token) - assert.Error(t, err) - assert.Contains(t, err.Error(), "revoked") -} - -func TestRotateToken(t *testing.T) { - ts := NewTokenService() - userID := uuid.New() - createdByID := uuid.New() - - req := &domain.TokenCreateRequest{ - Name: "Original Token", - Type: domain.TokenTypeBearer, - } - - tokenWithValue, err := ts.CreateToken(userID, req, createdByID) - require.NoError(t, err) - - // Rotate token - rotateResp, err := ts.RotateToken(tokenWithValue.ID, userID, "Regular rotation") - require.NoError(t, err) - assert.NotNil(t, rotateResp.OldToken) - assert.NotNil(t, rotateResp.NewToken) - assert.Equal(t, domain.TokenStatusRevoked, rotateResp.OldToken.Status) - assert.Equal(t, domain.TokenStatusActive, rotateResp.NewToken.Status) -} - -func TestDeleteToken(t *testing.T) { - ts := NewTokenService() - userID := uuid.New() - createdByID := uuid.New() - - req := &domain.TokenCreateRequest{ - Name: "Test Token", - Type: domain.TokenTypeBearer, - } - - tokenWithValue, err := ts.CreateToken(userID, req, createdByID) - require.NoError(t, err) - - // Delete token - err = ts.DeleteToken(tokenWithValue.ID) - require.NoError(t, err) - - // Token should not exist - _, err = ts.GetToken(tokenWithValue.ID) - assert.Error(t, err) -} - -func TestDisableToken(t *testing.T) { - ts := NewTokenService() - userID := uuid.New() - createdByID := uuid.New() - - req := &domain.TokenCreateRequest{ - Name: "Test Token", - Type: domain.TokenTypeBearer, - } - - tokenWithValue, err := ts.CreateToken(userID, req, createdByID) - require.NoError(t, err) - - // Disable token - disabled, err := ts.DisableToken(tokenWithValue.ID) - require.NoError(t, err) - assert.Equal(t, domain.TokenStatusDisabled, disabled.Status) - - // Verification should fail - _, err = ts.VerifyToken(tokenWithValue.Token) - assert.Error(t, err) - assert.Contains(t, err.Error(), "disabled") -} - -func TestEnableToken(t *testing.T) { - ts := NewTokenService() - userID := uuid.New() - createdByID := uuid.New() - - req := &domain.TokenCreateRequest{ - Name: "Test Token", - Type: domain.TokenTypeBearer, - } - - tokenWithValue, err := ts.CreateToken(userID, req, createdByID) - require.NoError(t, err) - - // Disable then enable - ts.DisableToken(tokenWithValue.ID) - enabled, err := ts.EnableToken(tokenWithValue.ID) - require.NoError(t, err) - assert.Equal(t, domain.TokenStatusActive, enabled.Status) - - // Verification should succeed - _, err = ts.VerifyToken(tokenWithValue.Token) - require.NoError(t, err) -} - -func TestCheckTokenExpiry(t *testing.T) { - ts := NewTokenService() - userID := uuid.New() - createdByID := uuid.New() - - // Create token that expires in the past - pastTime := time.Now().Add(-1 * time.Hour) - req := &domain.TokenCreateRequest{ - Name: "Expired Token", - Type: domain.TokenTypeBearer, - ExpiresAt: &pastTime, - } - - tokenWithValue, err := ts.CreateToken(userID, req, createdByID) - require.NoError(t, err) - - // Check expiry - checked, err := ts.CheckTokenExpiry(tokenWithValue.ID) - require.NoError(t, err) - assert.Equal(t, domain.TokenStatusExpired, checked.Status) -} - -func TestCleanupExpiredTokens(t *testing.T) { - ts := NewTokenService() - userID := uuid.New() - createdByID := uuid.New() - - // Create an expired token - pastTime := time.Now().Add(-1 * time.Hour) - expiredReq := &domain.TokenCreateRequest{ - Name: "Expired Token", - Type: domain.TokenTypeBearer, - ExpiresAt: &pastTime, - } - - _, err := ts.CreateToken(userID, expiredReq, createdByID) - require.NoError(t, err) - - // Create a valid token - validReq := &domain.TokenCreateRequest{ - Name: "Valid Token", - Type: domain.TokenTypeBearer, - } - - validToken, err := ts.CreateToken(userID, validReq, createdByID) - require.NoError(t, err) - - // Cleanup - err = ts.CleanupExpiredTokens() - require.NoError(t, err) - - // Expired token should be deleted - _, err = ts.GetToken(validToken.ID) - require.NoError(t, err) // Valid token should still exist -} - -func TestAPITokenIsExpired(t *testing.T) { - pastTime := time.Now().Add(-1 * time.Hour) - futureTime := time.Now().Add(1 * time.Hour) - - expiredToken := &domain.APIToken{ - ExpiresAt: &pastTime, - } - - validToken := &domain.APIToken{ - ExpiresAt: &futureTime, - } - - noExpiryToken := &domain.APIToken{ - ExpiresAt: nil, - } - - assert.True(t, expiredToken.IsExpired()) - assert.False(t, validToken.IsExpired()) - assert.False(t, noExpiryToken.IsExpired()) -} - -func TestAPITokenIsValid(t *testing.T) { - futureTime := time.Now().Add(1 * time.Hour) - - tests := []struct { - name string - token *domain.APIToken - valid bool - }{ - { - name: "active and not expired", - token: &domain.APIToken{ - Status: domain.TokenStatusActive, - ExpiresAt: &futureTime, - }, - valid: true, - }, - { - name: "revoked token", - token: &domain.APIToken{ - Status: domain.TokenStatusRevoked, - ExpiresAt: &futureTime, - }, - valid: false, - }, - { - name: "disabled token", - token: &domain.APIToken{ - Status: domain.TokenStatusDisabled, - ExpiresAt: &futureTime, - }, - valid: false, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - assert.Equal(t, tt.valid, tt.token.IsValid()) - }) - } -} - -func TestAPITokenHasPermission(t *testing.T) { - tests := []struct { - name string - token *domain.APIToken - permission string - has bool - }{ - { - name: "no specific permissions - inherits role", - token: &domain.APIToken{Permissions: []string{}}, - permission: "read", - has: true, - }, - { - name: "has specific permission", - token: &domain.APIToken{Permissions: []string{"read", "write"}}, - permission: "read", - has: true, - }, - { - name: "missing permission", - token: &domain.APIToken{Permissions: []string{"read"}}, - permission: "delete", - has: false, - }, - { - name: "wildcard permission", - token: &domain.APIToken{Permissions: []string{"*"}}, - permission: "anything", - has: true, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - assert.Equal(t, tt.has, tt.token.HasPermission(tt.permission)) - }) - } -} - -func TestAPITokenHasScope(t *testing.T) { - tests := []struct { - name string - token *domain.APIToken - scope string - has bool - }{ - { - name: "no scope restrictions", - token: &domain.APIToken{Scopes: []string{}}, - scope: "any", - has: true, - }, - { - name: "has specific scope", - token: &domain.APIToken{Scopes: []string{"risk", "mitigation"}}, - scope: "risk", - has: true, - }, - { - name: "missing scope", - token: &domain.APIToken{Scopes: []string{"risk"}}, - scope: "user", - has: false, - }, - { - name: "wildcard scope", - token: &domain.APIToken{Scopes: []string{"*"}}, - scope: "anything", - has: true, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - assert.Equal(t, tt.has, tt.token.HasScope(tt.scope)) - }) - } -} - -func TestAPITokenIsIPAllowed(t *testing.T) { - tests := []struct { - name string - token *domain.APIToken - clientIP string - allowed bool - }{ - { - name: "no IP restrictions", - token: &domain.APIToken{IPWhitelist: []string{}}, - clientIP: "192.168.1.1", - allowed: true, - }, - { - name: "IP in whitelist", - token: &domain.APIToken{IPWhitelist: []string{"192.168.1.1", "10.0.0.1"}}, - clientIP: "192.168.1.1", - allowed: true, - }, - { - name: "IP not in whitelist", - token: &domain.APIToken{IPWhitelist: []string{"192.168.1.1"}}, - clientIP: "10.0.0.1", - allowed: false, - }, - { - name: "wildcard IP", - token: &domain.APIToken{IPWhitelist: []string{"*"}}, - clientIP: "any.ip", - allowed: true, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - assert.Equal(t, tt.allowed, tt.token.IsIPAllowed(tt.clientIP)) - }) - } -} From ba2f9d60f193566705d50b097b3e456451ff3671 Mon Sep 17 00:00:00 2001 From: alex-dembele Date: Thu, 1 Oct 2026 11:17:16 +0100 Subject: [PATCH 3/4] fix(settings): create API tokens on /auth/pat and show the secret once (#782) The tab posted to /tokens, typed the answer as { token?: string } while the server sent an object, and copied res.data.token. The clipboard got "[object Object]" and the toast still said the token was created and copied. The tab moves to its own component on /auth/pat. The secret is shown once in a panel with a copy button and stays until the user dismisses it. The toast says "copied" only when the clipboard write resolved, and an answer with no secret string is reported as a failure. Revoke is a DELETE, removed from the table optimistically and restored if the server refuses; the dead "revoked" facet goes with it, since a revoked PAT no longer exists. The name is checked with zod before the request. Signed-off-by: alex-dembele --- .../src/features/settings/ApiTokensPanel.tsx | 340 ++++++++++++++++++ .../src/features/settings/SettingsScreen.tsx | 234 +----------- .../__tests__/apiTokensPanel.test.tsx | 141 ++++++++ frontend/src/features/settings/adminData.ts | 48 ++- 4 files changed, 522 insertions(+), 241 deletions(-) create mode 100644 frontend/src/features/settings/ApiTokensPanel.tsx create mode 100644 frontend/src/features/settings/__tests__/apiTokensPanel.test.tsx diff --git a/frontend/src/features/settings/ApiTokensPanel.tsx b/frontend/src/features/settings/ApiTokensPanel.tsx new file mode 100644 index 00000000..369db228 --- /dev/null +++ b/frontend/src/features/settings/ApiTokensPanel.tsx @@ -0,0 +1,340 @@ +// Copyright (c) 2026 OpenDefender Contributors +// SPDX-License-Identifier: AGPL-3.0-only +// +// Settings › API tokens (#782). A token created here is a personal access token: +// stored hashed in PostgreSQL, bound to the current organization, and accepted +// on every /api/v1 route as `Authorization: Bearer orsk_…`. +// +// The secret exists in the browser exactly once, in the reveal panel below. The +// screen says "copied" only when the clipboard write actually resolved, and the +// panel stays until the user dismisses it, so a refused clipboard never leaves +// them with a token they cannot see. + +import { useMemo, useState } from 'react'; +import { toast } from 'sonner'; +import { z } from 'zod'; +import { Check, Copy, KeyRound, Plus, Trash2 } from 'lucide-react'; + +import { Btn, EmptyState } from '../../shared/ui'; +import { DataTable, useTableState, type Column, type RowAction } from '../../shared/datatable'; +import { DangerConfirm } from '../../shared/DangerConfirm'; +import { relTime } from '../risks/riskMap'; +import type { LocaleCode } from '../../i18n/locales'; +import { useTokens, type ApiToken } from './adminData'; + +type Tr = (fr: string, en: string) => string; + +const TOKEN_NAME_MAX = 100; +const tokenNameSchema = z.string().trim().max(TOKEN_NAME_MAX); + +async function copyText(value: string): Promise { + if (!navigator.clipboard) return false; + try { + await navigator.clipboard.writeText(value); + return true; + } catch { + return false; + } +} + +// The token prefix cell used to render a Copy glyph that copied nothing. Module +// scope keeps the handler stable so the columns memo survives. +function copyPrefix(prefix: string, tr: Tr) { + void copyText(prefix).then((ok) => + ok + ? toast.success(tr('Préfixe copié', 'Prefix copied')) + : toast.error(tr('Copie impossible', 'Could not copy')), + ); +} + +export function ApiTokensPanel({ tr, lang }: { tr: Tr; lang: LocaleCode }) { + const { tokens, isLoading, isError, refetch, create, revoke } = useTokens(); + const [name, setName] = useState(''); + const [nameError, setNameError] = useState(null); + const [revealed, setRevealed] = useState(null); + // Revoking a token breaks any integration using it → impact-radiography confirm. + const [revokingToken, setRevokingToken] = useState(null); + const table = useTableState({ + defaultSort: { key: 'created', dir: 'desc' }, + defaultPageSize: 25, + urlPrefix: 'tok_', + }); + + const doCreate = () => { + const parsed = tokenNameSchema.safeParse(name); + if (!parsed.success) { + setNameError( + tr( + `Le nom ne doit pas dépasser ${TOKEN_NAME_MAX} caractères.`, + `The name must be ${TOKEN_NAME_MAX} characters or fewer.`, + ), + ); + return; + } + setNameError(null); + const n = parsed.data || tr('Nouveau jeton', 'New token'); + create.mutate(n, { + onSuccess: async (created) => { + setName(''); + const secret = typeof created?.token === 'string' ? created.token : ''; + if (!secret) { + // A 2xx without a secret is not a usable token; never call it a success. + toast.error( + tr( + 'Le serveur n’a renvoyé aucun jeton. Réessayez.', + 'The server returned no token. Try again.', + ), + ); + return; + } + setRevealed({ name: created.name, value: secret }); + const copied = await copyText(secret); + if (copied) + toast.success( + tr( + 'Jeton créé et copié dans le presse-papiers', + 'Token created and copied to clipboard', + ), + ); + else + toast.success(tr('Jeton créé — copiez-le ci-dessous', 'Token created — copy it below')); + }, + onError: () => toast.error(tr('Création échouée', 'Creation failed')), + }); + }; + + const copyRevealed = () => { + if (!revealed) return; + void copyText(revealed.value).then((ok) => + ok + ? toast.success(tr('Jeton copié', 'Token copied')) + : toast.error( + tr( + 'Copie impossible — sélectionnez le jeton et copiez-le à la main.', + 'Could not copy — select the token and copy it by hand.', + ), + ), + ); + }; + + const columns: Column[] = useMemo( + () => [ + { + key: 'name', + header: tr('Nom', 'Name'), + frozen: true, + hideable: false, + sortValue: (t) => t.name.toLowerCase(), + exportValue: (t) => t.name, + render: (t) => {t.name}, + }, + { + key: 'prefix', + header: tr('Préfixe', 'Prefix'), + exportValue: (t) => t.token_prefix ?? '', + render: (t) => + t.token_prefix ? ( + + ) : ( + — + ), + }, + { + key: 'created', + header: tr('Créé', 'Created'), + sortValue: (t) => new Date(t.created_at ?? 0).getTime(), + exportValue: (t) => t.created_at ?? '', + render: (t) => ( + {relTime(t.created_at, lang)} + ), + }, + { + key: 'used', + header: tr('Dernière util.', 'Last used'), + sortValue: (t) => new Date(t.last_used_at ?? 0).getTime(), + exportValue: (t) => t.last_used_at ?? '', + render: (t) => ( + + {t.last_used_at ? relTime(t.last_used_at, lang) : tr('jamais', 'never')} + + ), + }, + ], + [tr, lang], + ); + + const rowActions: RowAction[] = useMemo( + () => [ + { + key: 'revoke', + label: tr('Révoquer', 'Revoke'), + icon: Trash2, + danger: true, + onSelect: (t) => setRevokingToken({ id: t.id, name: t.name, lastUsed: t.last_used_at }), + }, + ], + [tr], + ); + + return ( + <> +
+
+ { + setName(e.target.value); + if (nameError) setNameError(null); + }} + onKeyDown={(e) => { + if (e.key === 'Enter' && !create.isPending) doCreate(); + }} + placeholder={tr('Nom du jeton (ex. CI/CD)', 'Token name (e.g. CI/CD)')} + aria-label={tr('Nom du jeton', 'Token name')} + aria-invalid={nameError ? true : undefined} + aria-describedby={nameError ? 'api-token-name-error' : undefined} + className="flex-1 min-w-[200px] h-9 px-3.5 rounded-[10px] text-[13px] text-ink outline-none" + style={{ + border: `1px solid ${nameError ? 'var(--critical)' : 'var(--border-strong)'}`, + background: 'var(--bg-elevated)', + }} + /> + +
+ {nameError && ( + + )} +
+ + {revealed && ( +
+

+ {tr( + `Copiez le jeton « ${revealed.name} » maintenant`, + `Copy the “${revealed.name}” token now`, + )} +

+

+ {tr( + 'Il ne sera plus jamais affiché. Envoyez-le dans l’en-tête Authorization: Bearer .', + 'It will never be shown again. Send it in the Authorization: Bearer header.', + )} +

+
+ e.currentTarget.select()} + className="mono flex-1 min-w-[240px] h-9 px-3 rounded-[10px] text-[12px] text-ink outline-none" + style={{ border: '1px solid var(--border-strong)', background: 'var(--bg)' }} + /> + + setRevealed(null)} + /> +
+
+ )} + + t.id} + api={table} + mode="client" + loading={isLoading} + error={isError} + onRetry={() => void refetch()} + clientSearch={(t, q) => `${t.name} ${t.token_prefix ?? ''}`.toLowerCase().includes(q)} + searchPlaceholder={tr('Nom ou préfixe…', 'Name or prefix…')} + rowActions={rowActions} + exportFilename="jetons-api" + minWidth={620} + pageSizeOptions={[10, 25, 50]} + empty={ + + } + /> + + setRevokingToken(null)} + title={tr('Révoquer le jeton API', 'Revoke API token')} + subject={revokingToken?.name} + intro={tr( + 'Toute intégration ou script utilisant ce jeton cessera immédiatement de fonctionner. Cette action est irréversible.', + 'Any integration or script using this token stops working immediately. This action is irreversible.', + )} + impact={ + revokingToken + ? [ + { + label: tr('Dernière utilisation', 'Last used'), + value: revokingToken.lastUsed + ? relTime(revokingToken.lastUsed, lang) + : tr('jamais', 'never'), + }, + ] + : [] + } + confirmLabel={tr('Révoquer le jeton', 'Revoke token')} + onConfirm={() => { + if (!revokingToken) return; + const target = revokingToken; + // Optimistic: the row leaves the table now; useTokens restores it if + // the server refuses. + setRevokingToken(null); + revoke.mutate(target.id, { + onSuccess: () => toast.success(tr('Jeton révoqué', 'Token revoked')), + onError: () => + toast.error(tr('Révocation échouée — réessayez.', 'Revocation failed — retry.')), + }); + }} + busy={revoke.isPending} + /> + + ); +} diff --git a/frontend/src/features/settings/SettingsScreen.tsx b/frontend/src/features/settings/SettingsScreen.tsx index 68681d27..e4a14c4c 100644 --- a/frontend/src/features/settings/SettingsScreen.tsx +++ b/frontend/src/features/settings/SettingsScreen.tsx @@ -3,7 +3,7 @@ // // Settings (OpenRisk.dc.html §6.16) — the consolidation point for every admin // feature. Internal nav + tabs: General, Members (real /users), RBAC, API Tokens -// (real /tokens), Organizations, Audit log, Custom Fields (real /custom-fields), +// (real /auth/pat), Organizations, Audit log, Custom Fields (real /custom-fields), // Integrations, Notifications, Security, Billing, Danger. Endpoints whose tables // aren't migrated yet (roles/tenants/audit) degrade to an honest unavailable state. @@ -48,7 +48,8 @@ import { ChangePasswordCard } from '../auth/ChangePasswordCard'; import { MembersView } from '../organization/MembersView'; import { relTime } from '../risks/riskMap'; import { api } from '../../lib/api'; -import { useTokens, useCustomFields, useTenants, type ApiToken } from './adminData'; +import { useCustomFields, useTenants } from './adminData'; +import { ApiTokensPanel } from './ApiTokensPanel'; import { useNotificationPreferences, useUpdateNotificationPreferences, @@ -366,7 +367,7 @@ export function SettingsScreen() { {tab === 'profile' && } {tab === 'general' && } {tab === 'members' && } - {tab === 'tokens' && } + {tab === 'tokens' && } {tab === 'orgs' && } {tab === 'fields' && } {tab === 'integrations' && } @@ -382,233 +383,6 @@ export function SettingsScreen() { /* ==================== real tabs ==================== */ -// The token prefix cell used to render a Copy glyph that copied nothing. Module -// scope keeps the handler stable so the columns memo survives. -function copyPrefix(prefix: string, tr: Tr) { - navigator.clipboard - ?.writeText(prefix) - .then(() => toast.success(tr('Préfixe copié', 'Prefix copied'))) - .catch(() => toast.error(tr('Copie impossible', 'Could not copy'))); -} - -function TokensTab({ tr, lang }: { tr: Tr; lang: LocaleCode }) { - const { tokens, isLoading, isError, refetch, create, revoke } = useTokens(); - const [name, setName] = useState(''); - // Revoking a token breaks any integration using it → impact-radiography confirm. - const [revokingToken, setRevokingToken] = useState(null); - const table = useTableState({ - defaultSort: { key: 'created', dir: 'desc' }, - defaultPageSize: 25, - urlPrefix: 'tok_', - }); - - const doCreate = () => { - const n = name.trim() || tr('Nouveau jeton', 'New token'); - create.mutate(n, { - onSuccess: (res) => { - setName(''); - const secret = res.data?.token; - if (secret) { - navigator.clipboard?.writeText(secret).catch(() => {}); - toast.success( - tr( - 'Jeton créé et copié dans le presse-papiers', - 'Token created and copied to clipboard', - ), - ); - } else toast.success(tr('Jeton créé', 'Token created')); - }, - onError: () => toast.error(tr('Création échouée', 'Creation failed')), - }); - }; - - const facets: Facet[] = useMemo( - () => [ - { - key: 'state', - label: tr('État', 'State'), - single: true, - options: [ - { value: 'active', label: tr('Actifs', 'Active'), color: 'var(--low)' }, - { value: 'revoked', label: tr('Révoqués', 'Revoked'), color: 'var(--critical)' }, - ], - matches: (t, selected) => (selected.includes('revoked') ? !!t.revoked : !t.revoked), - }, - ], - [tr], - ); - - const columns: Column[] = useMemo( - () => [ - { - key: 'name', - header: tr('Nom', 'Name'), - frozen: true, - hideable: false, - sortValue: (t) => t.name.toLowerCase(), - exportValue: (t) => t.name, - render: (t) => ( - - {t.name} - {t.revoked ? ` · ${tr('révoqué', 'revoked')}` : ''} - - ), - }, - { - key: 'prefix', - header: tr('Préfixe', 'Prefix'), - exportValue: (t) => t.token_prefix ?? '', - render: (t) => - t.token_prefix ? ( - - ) : ( - — - ), - }, - { - key: 'created', - header: tr('Créé', 'Created'), - sortValue: (t) => new Date(t.created_at ?? 0).getTime(), - exportValue: (t) => t.created_at ?? '', - render: (t) => ( - {relTime(t.created_at, lang)} - ), - }, - { - key: 'used', - header: tr('Dernière util.', 'Last used'), - sortValue: (t) => new Date(t.last_used_at ?? 0).getTime(), - exportValue: (t) => t.last_used_at ?? '', - render: (t) => ( - - {t.last_used_at ? relTime(t.last_used_at, lang) : tr('jamais', 'never')} - - ), - }, - ], - [tr, lang], - ); - - const rowActions: RowAction[] = useMemo( - () => [ - { - key: 'revoke', - label: tr('Révoquer', 'Revoke'), - icon: Trash2, - danger: true, - hidden: (t) => !!t.revoked, - onSelect: (t) => setRevokingToken({ id: t.id, name: t.name, lastUsed: t.last_used_at }), - }, - ], - [tr], - ); - - return ( - <> -
- setName(e.target.value)} - placeholder={tr('Nom du jeton (ex. CI/CD)', 'Token name (e.g. CI/CD)')} - aria-label={tr('Nom du jeton', 'Token name')} - className="flex-1 min-w-[200px] h-9 px-3.5 rounded-[10px] text-[13px] text-ink outline-none" - style={{ border: '1px solid var(--border-strong)', background: 'var(--bg-elevated)' }} - /> - -
- - t.id} - api={table} - mode="client" - loading={isLoading} - error={isError} - onRetry={() => void refetch()} - facets={facets} - clientSearch={(t, q) => `${t.name} ${t.token_prefix ?? ''}`.toLowerCase().includes(q)} - searchPlaceholder={tr('Nom ou préfixe…', 'Name or prefix…')} - rowActions={rowActions} - exportFilename="jetons-api" - minWidth={620} - pageSizeOptions={[10, 25, 50]} - empty={ - - } - /> - - setRevokingToken(null)} - title={tr('Révoquer le jeton API', 'Revoke API token')} - subject={revokingToken?.name} - intro={tr( - 'Toute intégration ou script utilisant ce jeton cessera immédiatement de fonctionner. Cette action est irréversible.', - 'Any integration or script using this token stops working immediately. This action is irreversible.', - )} - impact={ - revokingToken - ? [ - { - label: tr('Dernière utilisation', 'Last used'), - value: revokingToken.lastUsed - ? relTime(revokingToken.lastUsed, lang) - : tr('jamais', 'never'), - }, - ] - : [] - } - confirmLabel={tr('Révoquer le jeton', 'Revoke token')} - onConfirm={() => { - if (revokingToken) - revoke.mutate(revokingToken.id, { - onSuccess: () => { - toast.success(tr('Jeton révoqué', 'Token revoked')); - setRevokingToken(null); - }, - onError: () => - toast.error(tr('Révocation échouée — réessayez.', 'Revocation failed — retry.')), - }); - }} - busy={revoke.isPending} - /> - - ); -} - function CustomFieldsTab({ tr }: { tr: Tr }) { const { fields, isLoading, isError } = useCustomFields(); if (isError) return ; diff --git a/frontend/src/features/settings/__tests__/apiTokensPanel.test.tsx b/frontend/src/features/settings/__tests__/apiTokensPanel.test.tsx new file mode 100644 index 00000000..da5b1941 --- /dev/null +++ b/frontend/src/features/settings/__tests__/apiTokensPanel.test.tsx @@ -0,0 +1,141 @@ +// #782 — Settings › API tokens. +// +// The screen used to type the create answer as `{ token?: string }` while the +// server sent an object, so the clipboard received "[object Object]" and the +// toast still announced success. These tests pin what reaches the clipboard and +// that nothing claims a success that did not happen. + +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { render, screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; +import { MemoryRouter } from 'react-router'; + +import { ApiTokensPanel } from '../ApiTokensPanel'; + +const get = vi.fn(); +const post = vi.fn(); +const del = vi.fn(); +vi.mock('../../../lib/api', () => ({ + api: { + get: (...a: unknown[]) => get(...a), + post: (...a: unknown[]) => post(...a), + delete: (...a: unknown[]) => del(...a), + defaults: { baseURL: '' }, + }, +})); + +const toastSuccess = vi.fn(); +const toastError = vi.fn(); +vi.mock('sonner', () => ({ + toast: { + success: (...a: unknown[]) => toastSuccess(...a), + error: (...a: unknown[]) => toastError(...a), + }, +})); + +const writeText = vi.fn(); + +const SECRET = 'orsk_a1b2c3d4_' + 'f'.repeat(64); +const en = (_fr: string, english: string) => english; + +function renderPanel() { + const qc = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + return render( + + + + + , + ); +} + +async function generate(name = 'CI/CD') { + const user = userEvent.setup(); + // userEvent installs its own clipboard; ours must be the one the panel sees. + Object.defineProperty(navigator, 'clipboard', { + value: { writeText }, + configurable: true, + }); + await user.type(screen.getByLabelText('Token name'), name); + await user.click(screen.getByRole('button', { name: 'Generate token' })); +} + +beforeEach(() => { + vi.clearAllMocks(); + get.mockResolvedValue({ data: { tokens: [] } }); + writeText.mockResolvedValue(undefined); +}); + +describe('ApiTokensPanel', () => { + it('creates a personal access token and puts the orsk_ secret on the clipboard', async () => { + post.mockResolvedValue({ + data: { + id: 't1', + name: 'CI/CD', + token_prefix: 'a1b2c3d4', + created_at: '2026-10-01T00:00:00Z', + token: SECRET, + }, + }); + renderPanel(); + await generate(); + + await waitFor(() => expect(writeText).toHaveBeenCalledTimes(1)); + expect(post).toHaveBeenCalledWith('/auth/pat', { name: 'CI/CD', scopes: ['*'] }); + + const copied = writeText.mock.calls[0][0]; + expect(typeof copied).toBe('string'); + expect(copied).toMatch(/^orsk_/); + expect(copied).not.toContain('[object Object]'); + expect(copied).toBe(SECRET); + + expect(toastSuccess).toHaveBeenCalledWith('Token created and copied to clipboard'); + // Shown once, so the user can still copy it if they missed the clipboard. + expect(screen.getByLabelText('Token value')).toHaveValue(SECRET); + }); + + it('never copies an object nor announces success when no secret string comes back', async () => { + // The pre-#782 answer shape: the secret nested in an object. + post.mockResolvedValue({ data: { token: { id: 't1', value: SECRET } } }); + renderPanel(); + await generate(); + + await waitFor(() => expect(toastError).toHaveBeenCalled()); + expect(writeText).not.toHaveBeenCalled(); + expect(toastSuccess).not.toHaveBeenCalled(); + expect(screen.queryByLabelText('Token value')).not.toBeInTheDocument(); + }); + + it('does not claim the token was copied when the clipboard refuses', async () => { + post.mockResolvedValue({ + data: { id: 't1', name: 'CI/CD', token_prefix: 'a1b2c3d4', created_at: '', token: SECRET }, + }); + writeText.mockRejectedValue(new Error('denied')); + renderPanel(); + await generate(); + + await waitFor(() => expect(toastSuccess).toHaveBeenCalled()); + expect(toastSuccess).not.toHaveBeenCalledWith('Token created and copied to clipboard'); + expect(toastSuccess).toHaveBeenCalledWith('Token created — copy it below'); + expect(screen.getByLabelText('Token value')).toHaveValue(SECRET); + }); + + it('announces nothing but the failure when the server refuses', async () => { + post.mockRejectedValue(new Error('500')); + renderPanel(); + await generate(); + + await waitFor(() => expect(toastError).toHaveBeenCalledWith('Creation failed')); + expect(toastSuccess).not.toHaveBeenCalled(); + expect(writeText).not.toHaveBeenCalled(); + }); + + it('rejects an over-long name client-side without calling the server', async () => { + renderPanel(); + await generate('x'.repeat(101)); + + expect(await screen.findByRole('alert')).toHaveTextContent('100 characters or fewer'); + expect(post).not.toHaveBeenCalled(); + }); +}); diff --git a/frontend/src/features/settings/adminData.ts b/frontend/src/features/settings/adminData.ts index 1d4daa24..7f8f0c16 100644 --- a/frontend/src/features/settings/adminData.ts +++ b/frontend/src/features/settings/adminData.ts @@ -52,31 +52,57 @@ export function useUsers() { }; } -/* ---------------- API Tokens (/tokens) ---------------- */ +/* ---------------- API Tokens (/auth/pat) ---------------- */ +// Personal access tokens (#782). Stored hashed in PostgreSQL, bound to the +// organization they were created in, and accepted by the PAT middleware on +// every /api/v1 route as `Authorization: Bearer orsk_…`. export interface ApiToken { id: string; name: string; - token_prefix?: string; - expires_at?: string; + token_prefix: string; + expires_at?: string | null; created_at: string; - last_used_at?: string; - revoked?: boolean; + last_used_at?: string | null; } +/** POST /auth/pat answer. `token` is the secret, returned this once only. */ +export interface CreatedApiToken { + id: string; + name: string; + token_prefix: string; + created_at: string; + token: string; +} + +const TOKENS_KEY = ['admin', 'tokens'] as const; + export function useTokens() { const qc = useQueryClient(); const query = useQuery({ - queryKey: ['admin', 'tokens'], - queryFn: async () => (await api.get<{ tokens: ApiToken[] }>('/tokens')).data?.tokens ?? [], + queryKey: TOKENS_KEY, + queryFn: async () => + (await api.get<{ tokens: ApiToken[] | null }>('/auth/pat')).data?.tokens ?? [], }); - const invalidate = () => qc.invalidateQueries({ queryKey: ['admin', 'tokens'] }); + const invalidate = () => qc.invalidateQueries({ queryKey: TOKENS_KEY }); const create = useMutation({ - mutationFn: (name: string) => api.post<{ token?: string }>('/tokens', { name }), + // "*" = everything the creator holds in this organization, never more: the + // server intersects it with the creator's own permissions on every request. + mutationFn: async (name: string) => + (await api.post('/auth/pat', { name, scopes: ['*'] })).data, onSuccess: invalidate, }); const revoke = useMutation({ - mutationFn: (id: string) => api.post(`/tokens/${id}/revoke`), - onSuccess: invalidate, + mutationFn: (id: string) => api.delete(`/auth/pat/${id}`), + onMutate: async (id: string) => { + await qc.cancelQueries({ queryKey: TOKENS_KEY }); + const previous = qc.getQueryData(TOKENS_KEY); + qc.setQueryData(TOKENS_KEY, (rows) => (rows ?? []).filter((t) => t.id !== id)); + return { previous }; + }, + onError: (_err, _id, ctx) => { + if (ctx?.previous) qc.setQueryData(TOKENS_KEY, ctx.previous); + }, + onSettled: invalidate, }); return { tokens: query.data ?? [], From 8d3199bf34d1aec1eb9d767ae922caf7931bdef1 Mon Sep 17 00:00:00 2001 From: alex-dembele Date: Thu, 1 Oct 2026 11:17:53 +0100 Subject: [PATCH 4/4] docs(api): document personal access tokens on /auth/pat (#782) Both API references described /tokens, an in-memory service whose middleware was never mounted, plus rotate, update and per-token rate limit calls that never worked. They now describe /auth/pat as it runs: create, list and revoke, the orsk_ format, the one-time secret and the tenant binding. Signed-off-by: alex-dembele --- docs/API_COMPLETE_ENDPOINTS.md | 44 ++++++++----------------- docs/API_SECURITY_GUIDE.md | 60 ++++++++++++++-------------------- 2 files changed, 37 insertions(+), 67 deletions(-) diff --git a/docs/API_COMPLETE_ENDPOINTS.md b/docs/API_COMPLETE_ENDPOINTS.md index 06a53a6f..889d7fa4 100644 --- a/docs/API_COMPLETE_ENDPOINTS.md +++ b/docs/API_COMPLETE_ENDPOINTS.md @@ -454,44 +454,26 @@ --- -## API Tokens (7 endpoints) +## API Tokens (3 endpoints) + +Personal access tokens, bound to the organization they were created in. Send one +as `Authorization: Bearer orsk_…` on any `/api/v1` route. ### 1. Create Token -- **Endpoint**: `POST /tokens` -- **Auth**: ✅ Yes -- **Body**: `{ "name": "CI/CD", "description": "...", "expires_at": "2026-12-31T..." }` -- **Response**: `{ "id": "uuid", "token": "opnrsk_...", ... }` +- **Endpoint**: `POST /auth/pat` +- **Auth**: ✅ Yes (session only; a token cannot create tokens) +- **Body**: `{ "name": "CI/CD", "scopes": ["*"], "expires_in_days": 90 }` +- **Response**: `201 { "id": "uuid", "name": "...", "token_prefix": "a1b2c3d4", "token": "orsk_a1b2c3d4_...", ... }` (the secret is returned once) ### 2. List Tokens -- **Endpoint**: `GET /tokens` -- **Auth**: ✅ Yes -- **Response**: `[ { "id": "uuid", "name": "...", "created_at": "..." } ]` - -### 3. Get Token -- **Endpoint**: `GET /tokens/:id` -- **Auth**: ✅ Yes -- **Response**: `{ "id": "uuid", ... }` - -### 4. Update Token -- **Endpoint**: `PUT /tokens/:id` +- **Endpoint**: `GET /auth/pat` - **Auth**: ✅ Yes -- **Body**: `{ "name": "...", "expires_at": "..." }` -- **Response**: `{ "id": "uuid", ... }` - -### 5. Revoke Token -- **Endpoint**: `POST /tokens/:id/revoke` -- **Auth**: ✅ Yes -- **Response**: `204 No Content` +- **Response**: `{ "tokens": [ { "id": "uuid", "name": "...", "token_prefix": "...", "last_used_at": "...", "created_at": "..." } ] }` (the caller's tokens in the current organization) -### 6. Rotate Token -- **Endpoint**: `POST /tokens/:id/rotate` +### 3. Revoke Token +- **Endpoint**: `DELETE /auth/pat/:id` - **Auth**: ✅ Yes -- **Response**: `{ "token": "opnrsk_new_token..." }` - -### 7. Delete Token -- **Endpoint**: `DELETE /tokens/:id` -- **Auth**: ✅ Yes -- **Response**: `204 No Content` +- **Response**: `204 No Content`; `404` for a token that is not the caller's in this organization --- diff --git a/docs/API_SECURITY_GUIDE.md b/docs/API_SECURITY_GUIDE.md index d23389c0..02ba7e86 100644 --- a/docs/API_SECURITY_GUIDE.md +++ b/docs/API_SECURITY_GUIDE.md @@ -101,57 +101,58 @@ curl -X POST https://api.openrisk.io/api/v1/auth/refresh \ #### API Token Generation -Users can generate API tokens for programmatic access: +Users create personal access tokens for scripts and CI pipelines, from +Settings › API tokens or with a session token: ```bash -curl -X POST https://api.openrisk.io/api/v1/tokens \ +curl -X POST https://api.openrisk.io/api/v1/auth/pat \ -H "Authorization: Bearer " \ -H "Content-Type: application/json" \ -d '{ "name": "CI/CD Pipeline", - "description": "Token for automated deployments", - "expires_at": "2026-12-31T23:59:59Z" + "scopes": ["*"], + "expires_in_days": 90 }' ``` -**Response**: +**Response** (`201`): ```json { "id": "token_uuid", - "token": "opnrsk_abcd1234efgh5678ijkl9012", "name": "CI/CD Pipeline", - "created_at": "2026-03-10T10:00:00Z", - "expires_at": "2026-12-31T23:59:59Z" + "token_prefix": "a1b2c3d4", + "scopes": ["*"], + "expires_at": "2026-12-30T10:00:00Z", + "created_at": "2026-10-01T10:00:00Z", + "token": "orsk_a1b2c3d4_<64 hex characters>" } ``` +`token` is returned this once. Only its SHA-256 hash is stored. + #### API Token Usage -Use the token as a Bearer token: +Use the token as a Bearer token on any `/api/v1` route: ```bash -curl -H "Authorization: Bearer opnrsk_abcd1234efgh5678ijkl9012" \ +curl -H "Authorization: Bearer orsk_a1b2c3d4_" \ https://api.openrisk.io/api/v1/risks ``` #### API Token Management -- Create: `POST /tokens` -- List: `GET /tokens` -- Get: `GET /tokens/:id` -- Update: `PUT /tokens/:id` -- Revoke: `POST /tokens/:id/revoke` -- Rotate: `POST /tokens/:id/rotate` -- Delete: `DELETE /tokens/:id` +- Create: `POST /auth/pat` (requires a session; a token cannot create tokens) +- List: `GET /auth/pat` (metadata only, never the secret) +- Revoke: `DELETE /auth/pat/:id` (the token stops working immediately) #### API Token Security -- ✅ Scoped to user account -- ✅ Can be revoked immediately -- ✅ Should be rotated regularly -- ✅ Never commit to version control -- ✅ Use `.gitignore` for token files -- ✅ Rotate if compromised: `POST /tokens/:id/rotate` +- Bound to the organization it was created in. It never acts in another + organization, and it stops working when its owner leaves that one. +- Never exceeds its owner: the effective permissions are the owner's current + permissions narrowed to the token's scopes. `"*"` means all of the owner's. +- Stored as a SHA-256 hash; the `orsk_` prefix makes a leaked token easy to spot. +- Never commit a token to version control. Revoke and recreate one that leaked. ### OAuth2 & SAML2 @@ -489,19 +490,6 @@ GET /api/v1/risks -H "Authorization: Bearer token_b" # 100 remaining ``` -### Custom Rate Limits - -Admins can set custom limits per user or API token: - -```bash -curl -X PUT https://api.openrisk.io/api/v1/tokens/token_id \ - -H "Authorization: Bearer " \ - -H "Content-Type: application/json" \ - -d '{ - "rate_limit": "1000/hour" - }' -``` - --- ## Best Practices