From 1ea91abed5f5225e1d444c8293048089618291cd Mon Sep 17 00:00:00 2001 From: Hsukqi Lee Date: Sat, 14 Nov 2026 19:24:55 +0800 Subject: [PATCH] fix: deliver reviewed audit repairs --- internal/httpapi/logout_test.go | 88 ++++++++++++++++++ internal/platform/auth/auth.go | 12 ++- internal/platform/auth/logout_test.go | 129 ++++++++++++++++++++++++++ 3 files changed, 228 insertions(+), 1 deletion(-) create mode 100644 internal/httpapi/logout_test.go create mode 100644 internal/platform/auth/logout_test.go diff --git a/internal/httpapi/logout_test.go b/internal/httpapi/logout_test.go new file mode 100644 index 0000000..8ab3747 --- /dev/null +++ b/internal/httpapi/logout_test.go @@ -0,0 +1,88 @@ +package httpapi + +import ( + "context" + "encoding/json" + "net/http/httptest" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/FastR-D/FastTask/internal/application" + "github.com/FastR-D/FastTask/internal/config" + "github.com/FastR-D/FastTask/internal/persistence" + platformauth "github.com/FastR-D/FastTask/internal/platform/auth" +) + +func TestLogoutHTTPRevokesOnlyCurrentLogin(t *testing.T) { + store, err := persistence.Open(filepath.Join(t.TempDir(), "logout.db")) + if err != nil { + t.Fatal(err) + } + defer store.Close() + ctx := context.Background() + if err := store.Migrate(ctx); err != nil { + t.Fatal(err) + } + cfg := config.Config{PublicURL: "http://localhost", JWTSecret: strings.Repeat("j", 40), AccessTTL: time.Hour, RefreshTTL: 24 * time.Hour, AdminIdentifier: "admin", AdminPassword: "test-password", AdminName: "Admin", WebDist: t.TempDir()} + auth := platformauth.New(store, cfg) + if err := auth.EnsureAdmin(ctx); err != nil { + t.Fatal(err) + } + server := New(application.New(store), auth, cfg, nil) + request := func(method, path, body, access string) *httptest.ResponseRecorder { + req := httptest.NewRequest(method, "/api/v1"+path, strings.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + req.Header.Set("Idempotency-Key", persistence.NewID("request")) + if access != "" { + req.Header.Set("Authorization", "Bearer "+access) + } + res := httptest.NewRecorder() + server.Engine.ServeHTTP(res, req) + return res + } + type tokens struct { + Access string `json:"access_token"` + Refresh string `json:"refresh_token"` + } + login := func() tokens { + res := request("POST", "/auth/login", `{"identifier":"admin","password":"test-password"}`, "") + if res.Code != 200 { + t.Fatalf("login status: %d", res.Code) + } + var token tokens + if err := json.Unmarshal(res.Body.Bytes(), &token); err != nil { + t.Fatal(err) + } + if token.Access == "" || token.Refresh == "" { + t.Fatal("login missing credentials") + } + return token + } + current, independent := login(), login() + res := request("POST", "/auth/refresh", `{"refresh_token":"`+current.Refresh+`"}`, "") + if res.Code != 200 { + t.Fatalf("refresh status: %d", res.Code) + } + var successor tokens + if err := json.Unmarshal(res.Body.Bytes(), &successor); err != nil { + t.Fatal(err) + } + if res := request("POST", "/auth/logout", "", successor.Access); res.Code < 200 || res.Code >= 300 { + t.Fatalf("logout status: %d", res.Code) + } + for _, token := range []string{current.Access, successor.Access} { + if res := request("GET", "/auth/session", "", token); res.Code != 401 { + t.Errorf("session status after logout: %d", res.Code) + } + } + for _, token := range []string{successor.Refresh, current.Refresh} { + if res := request("POST", "/auth/refresh", `{"refresh_token":"`+token+`"}`, ""); res.Code != 401 { + t.Errorf("refresh status after logout: %d", res.Code) + } + } + if res := request("GET", "/auth/session", "", independent.Access); res.Code != 200 { + t.Errorf("independent login status: %d", res.Code) + } +} diff --git a/internal/platform/auth/auth.go b/internal/platform/auth/auth.go index 09f8f15..3231be3 100644 --- a/internal/platform/auth/auth.go +++ b/internal/platform/auth/auth.go @@ -232,7 +232,17 @@ func (s *Service) refreshGraceSuccessor(tx *gorm.DB, session persistence.Session } func (s *Service) Logout(ctx context.Context, sessionID string) error { - return s.store.DB.WithContext(ctx).Model(&persistence.Session{}).Where("id = ?", sessionID).Updates(map[string]any{"status": "revoked", "updated_at": persistence.Now()}).Error + return s.store.Transaction(ctx, func(tx *gorm.DB) error { + // An already-admitted logout may carry a SID rotated by another request. + // Resolve its stable family inside the same write transaction as revocation. + var session persistence.Session + if err := tx.First(&session, "id = ?", sessionID).Error; err != nil { + return err + } + return tx.Model(&persistence.Session{}). + Where("family_id = ? AND user_id = ?", session.FamilyID, session.UserID). + Updates(map[string]any{"status": "revoked", "updated_at": persistence.Now()}).Error + }) } func (s *Service) Authenticate(token string) (Principal, error) { diff --git a/internal/platform/auth/logout_test.go b/internal/platform/auth/logout_test.go new file mode 100644 index 0000000..9e711f5 --- /dev/null +++ b/internal/platform/auth/logout_test.go @@ -0,0 +1,129 @@ +package auth_test + +import ( + "context" + "path/filepath" + "strings" + "sync" + "testing" + "time" + + "github.com/FastR-D/FastTask/internal/config" + "github.com/FastR-D/FastTask/internal/persistence" + "github.com/FastR-D/FastTask/internal/platform/auth" +) + +func TestLogoutRotationFamily(t *testing.T) { + for _, order := range []string{"refresh-first", "logout-first", "concurrent"} { + t.Run(order, func(t *testing.T) { + ctx := context.Background() + path := filepath.Join(t.TempDir(), "auth.db") + store, err := persistence.Open(path) + if err != nil { + t.Fatal(err) + } + defer store.Close() + if err := store.Migrate(ctx); err != nil { + t.Fatal(err) + } + otherStore, err := persistence.Open(path) + if err != nil { + t.Fatal(err) + } + defer otherStore.Close() + cfg := config.Config{JWTSecret: strings.Repeat("j", 40), AccessTTL: time.Hour, RefreshTTL: 24 * time.Hour, AdminIdentifier: "admin", AdminPassword: "test-password", AdminName: "Admin"} + service, other := auth.New(store, cfg), auth.New(otherStore, cfg) + if err := service.EnsureAdmin(ctx); err != nil { + t.Fatal(err) + } + _, access, refresh, err := service.Login(ctx, cfg.AdminIdentifier, cfg.AdminPassword) + if err != nil { + t.Fatal(err) + } + principal, err := service.Authenticate(access) + if err != nil { + t.Fatal(err) + } + _, independentAccess, independentRefresh, err := other.Login(ctx, cfg.AdminIdentifier, cfg.AdminPassword) + if err != nil { + t.Fatal(err) + } + accesses, refreshes := []string{access}, []string{refresh} + rotate := func() { + _, a, r, err := other.Refresh(ctx, refresh) + if err != nil { + if order == "refresh-first" { + t.Error(err) + } + return + } + accesses, refreshes = append(accesses, a), append(refreshes, r) + refresh = r + } + logout := func() { + if err := service.Logout(ctx, principal.SessionID); err != nil { + t.Error(err) + } + } + switch order { + case "refresh-first": + rotate() + rotate() + rotate() + // A real SQLite failure must roll back and propagate, leaving the + // family usable until a later successful logout. + if err := store.DB.Exec(`CREATE TRIGGER reject_logout BEFORE UPDATE ON sessions + WHEN NEW.status = 'revoked' BEGIN SELECT RAISE(ABORT, 'storage failure'); END`).Error; err != nil { + t.Fatal(err) + } + if err := service.Logout(ctx, principal.SessionID); err == nil { + t.Fatal("failed database write acknowledged") + } + if _, err := other.Authenticate(accesses[len(accesses)-1]); err != nil { + t.Fatalf("failed logout changed family: %v", err) + } + if err := store.DB.Exec("DROP TRIGGER reject_logout").Error; err != nil { + t.Fatal(err) + } + logout() + case "logout-first": + logout() + rotate() + case "concurrent": + var wg sync.WaitGroup + wg.Add(2) + go func() { defer wg.Done(); rotate() }() + go func() { defer wg.Done(); logout() }() + wg.Wait() + } + // Check access before refresh: reuse detection must not hide a failed logout. + for i, token := range accesses { + if _, err := other.Authenticate(token); err == nil { + t.Errorf("generation %d access accepted after logout", i) + } + } + for i := len(refreshes) - 1; i >= 0; i-- { + if _, _, _, err := other.Refresh(ctx, refreshes[i]); err == nil { + t.Errorf("generation %d refresh/grace accepted after logout", i) + } + } + if err := other.Logout(ctx, principal.SessionID); err != nil { + t.Errorf("repeat logout: %v", err) + } + if _, err := service.Authenticate(independentAccess); err != nil { + t.Errorf("other login revoked: %v", err) + } + if _, _, _, err := service.Refresh(ctx, independentRefresh); err != nil { + t.Errorf("other login refresh revoked: %v", err) + } + if err := service.Logout(ctx, "missing"); err == nil { + t.Error("unknown SID must return an error") + } + cancelled, cancel := context.WithCancel(ctx) + cancel() + if err := service.Logout(cancelled, principal.SessionID); err == nil { + t.Error("database cancellation swallowed") + } + }) + } +}