diff --git a/Server/api/auth_characterization_test.go b/Server/api/auth_characterization_test.go index 5d15b185..859b6a62 100644 --- a/Server/api/auth_characterization_test.go +++ b/Server/api/auth_characterization_test.go @@ -824,18 +824,31 @@ func TestAuthCharacterization_VerifyTOTPFailurePaths(t *testing.T) { } wantErr(t, verify(t, router, pt, totpCode(t, secret), ""), http.StatusInternalServerError, "INTERNAL_ERROR", "failed to verify two-factor code") }) - t.Run("session insert fails -> 500 and the challenge is consumed", func(t *testing.T) { + t.Run("session insert fails -> 500, the challenge and the code survive", func(t *testing.T) { database, router, _, secret := setup(t) pt := loginPartial(t, router, "two", "correctPass1", "", "") failWrite(t, database, "INSERT", "sessions") - wantErr(t, verify(t, router, pt, totpCode(t, secret), ""), http.StatusInternalServerError, "INTERNAL_ERROR", "failed to create session") - // known: the partial token was consumed before the session insert, so - // the user must repeat the password step after a transient store - // failure (ledger OC-0378). Pinned as-is; the code is also marked used. + code := totpCode(t, secret) + wantErr(t, verify(t, router, pt, code, ""), http.StatusInternalServerError, "INTERNAL_ERROR", "failed to create session") + // OC-0378 (fixed in B3-9): the verified second factor is not discarded + // by a store fault — the challenge is restored under the same partial + // token and the accepted code is released, so once the store is back + // the same token and the same code complete the login. (Restore alone + // would refuse the retry as a replay.) if _, err := database.ExecContext(context.Background(), `DROP TRIGGER fault_insert_sessions`); err != nil { t.Fatalf("drop trigger: %v", err) } - wantErr(t, verify(t, router, pt, totpCode(t, secret), "203.0.113.2"), http.StatusUnauthorized, "UNAUTHORIZED", challengeGone) + rr := verify(t, router, pt, code, "203.0.113.2") + if rr.Code != http.StatusOK { + t.Fatalf("retry status = %d, want 200; body = %s", rr.Code, rr.Body.String()) + } + var res struct { + Token string `json:"token"` + Requires2FA bool `json:"requires_2fa"` + } + if err := json.Unmarshal(rr.Body.Bytes(), &res); err != nil || res.Token == "" || res.Requires2FA { + t.Fatalf("retry body = %s (err %v), want a session token", rr.Body.String(), err) + } }) t.Run("per-user failure cap spans challenges -> 429 on the 11th attempt", func(t *testing.T) { _, router, _, secret := setup(t) diff --git a/Server/auth/totp.go b/Server/auth/totp.go index dd9ad9ff..e87b7929 100644 --- a/Server/auth/totp.go +++ b/Server/auth/totp.go @@ -108,6 +108,16 @@ func (s *PartialAuthStore) Consume(token string) (PartialAuthChallenge, bool) { return entry, true } +// Restore puts a challenge Consume returned back under its original token — +// the recovery path for a caller that claimed the challenge and then could +// not finish the login (OC-0378). The entry keeps its expiry and failure +// count, so a challenge that expired meanwhile is dropped by the next Lookup. +func (s *PartialAuthStore) Restore(token string, challenge PartialAuthChallenge) { + s.mu.Lock() + defer s.mu.Unlock() + s.entries[token] = challenge +} + func (s *PartialAuthStore) RegisterFailure(token string, maxFailures int) bool { s.mu.Lock() defer s.mu.Unlock() @@ -198,6 +208,15 @@ func (s *UsedTOTPCodeStore) MarkUsed(userID int64, code string) bool { return true } +// Unmark forgets a code MarkUsed recorded so it can be accepted once more — +// the companion of PartialAuthStore.Restore: a verification the caller could +// not complete is released together with its challenge. +func (s *UsedTOTPCodeStore) Unmark(userID int64, code string) { + s.mu.Lock() + defer s.mu.Unlock() + delete(s.entries, fmt.Sprintf("%d:%s", userID, code)) +} + func (s *UsedTOTPCodeStore) cleanupExpiredLocked() { now := time.Now() for key, expiry := range s.entries { diff --git a/Server/auth/totp_test.go b/Server/auth/totp_test.go index e7a9aef6..87b9c792 100644 --- a/Server/auth/totp_test.go +++ b/Server/auth/totp_test.go @@ -376,3 +376,73 @@ func TestBuildTOTPURI_ContainsIssuerAndSecret(t *testing.T) { t.Fatalf("issuer = %q, want OwnCord", query.Get("issuer")) } } + +// ─── OC-0378: Restore / Unmark ────────────────────────────────────────────── + +func TestPartialAuthStore_RestoreAfterConsume(t *testing.T) { + store := auth.NewPartialAuthStore(time.Minute) + token, err := store.Issue(7, "device", "203.0.113.7") + if err != nil { + t.Fatalf("Issue: %v", err) + } + store.RegisterFailure(token, 5) // Failures=1 must survive the round trip + + claimed, ok := store.Consume(token) + if !ok { + t.Fatal("Consume: challenge not found") + } + if _, ok := store.Lookup(token); ok { + t.Fatal("consumed token still resolves") + } + + store.Restore(token, claimed) + got, ok := store.Lookup(token) + if !ok { + t.Fatal("restored token does not resolve") + } + if got != claimed { + t.Fatalf("restored challenge = %+v, want %+v", got, claimed) + } + if got.Failures != 1 { + t.Fatalf("Failures = %d, want 1 (restore keeps the count)", got.Failures) + } +} + +func TestPartialAuthStore_RestoreExpiredStaysGone(t *testing.T) { + store := auth.NewPartialAuthStore(20 * time.Millisecond) + token, err := store.Issue(7, "device", "203.0.113.7") + if err != nil { + t.Fatalf("Issue: %v", err) + } + claimed, ok := store.Consume(token) + if !ok { + t.Fatal("Consume: challenge not found") + } + time.Sleep(40 * time.Millisecond) + + store.Restore(token, claimed) + if _, ok := store.Lookup(token); ok { + t.Fatal("an expired challenge came back to life") + } +} + +func TestUsedTOTPCodeStore_UnmarkAllowsReuse(t *testing.T) { + store := auth.NewUsedTOTPCodeStore() + if !store.MarkUsed(1, "123456") { + t.Fatal("first MarkUsed refused") + } + if store.MarkUsed(1, "123456") { + t.Fatal("replay accepted before Unmark") + } + if !store.MarkUsed(2, "123456") { + t.Fatal("another user's identical code refused") + } + + store.Unmark(1, "123456") + if !store.MarkUsed(1, "123456") { + t.Fatal("code still marked after Unmark") + } + if store.MarkUsed(2, "123456") { + t.Fatal("Unmark for user 1 released user 2's code") + } +} diff --git a/Server/service/auth.go b/Server/service/auth.go index 2de715ee..c64a275b 100644 --- a/Server/service/auth.go +++ b/Server/service/auth.go @@ -470,6 +470,7 @@ func (s *AuthService) authenticate(ctx context.Context, in LoginInput) (*db.User // VerifyTOTP completes a challenge Login started and issues the session, // bound to the login request's device and IP rather than this one's. func (s *AuthService) VerifyTOTP(ctx context.Context, partialToken, code string) (*AuthResult, error) { + code = strings.TrimSpace(code) challenge, ok := s.partial.Lookup(partialToken) if !ok { return nil, ErrTOTPChallengeInvalid @@ -500,7 +501,7 @@ func (s *AuthService) VerifyTOTP(ctx context.Context, partialToken, code string) return nil, ErrTooManyAttempts } - if !auth.VerifyTOTPCodeOnce(secret, strings.TrimSpace(code), time.Now().UTC(), user.ID, s.usedCodes) { + if !auth.VerifyTOTPCodeOnce(secret, code, time.Now().UTC(), user.ID, s.usedCodes) { // The attempt was already recorded atomically up-front via // limiter.Allow; only the per-partial-token counter is advanced here. s.partial.RegisterFailure(partialToken, partialAuthMaxFailures) @@ -509,12 +510,22 @@ func (s *AuthService) VerifyTOTP(ctx context.Context, partialToken, code string) s.limiter.Reset(ctx, totpRateLimitKey) - if _, ok := s.partial.Consume(partialToken); !ok { + claimed, ok := s.partial.Consume(partialToken) + if !ok { return nil, ErrTOTPChallengeInvalid } token, err := issueSession(ctx, s.st, user.ID, challenge.Device, challenge.IP) if err != nil { + // The second factor was verified; a store fault must not discard it + // (OC-0378). The claim stays atomic and first — two concurrent + // verifies can never both reach issueSession — so on failure put the + // challenge back under the same token (the client still holds it) and + // release the accepted code: the retry completes the login without + // another password step. Code first, then token, so a concurrent + // retry never finds a live token with a dead code. + s.usedCodes.Unmark(user.ID, code) + s.partial.Restore(partialToken, claimed) return nil, ErrSessionIssue }