fix(auth): limit MFA challenge attempts and revoke the access token on logout (#689)

backend/cmd/server/main.go

@@ -687,7 +687,9 @@ func main() {
 		// Organization + owner + root membership in one transaction (#687).
 		WithAccounts(repository.NewGormRegistrationRepository(database.DB))
 	refreshUseCase := auth.NewRefreshTokenUseCase(tokenManager)
-	logoutUseCase := auth.NewLogoutUseCase(tokenManager)
+	// #689 — logout also blacklists the access token it was called with, so the
+	// session ends now instead of at the end of the access TTL.
+	logoutUseCase := auth.NewLogoutUseCase(tokenManager).WithAccessTokenRevocation(tokenBlacklistManager)
 
 	// MFA use cases + handler.
 	setupMFAUseCase := auth.NewSetupMFAUseCase(mfaRepo, mfaKey[:])
@@ -699,7 +701,12 @@ func main() {
 		WithTOTPKey(mfaKey[:]).
 		RequireMFAForRoles(mfaRequiredRoles, mfaRequiredBusinessRoles).
 		WithMailer(securityMailer)
-	challengeMFAUseCase := auth.NewChallengeMFAUseCase(mfaRepo, mfaKey[:])
+	// #689 — five codes per challenge token, ten failures per account in
+	// fifteen minutes before a fixed fifteen-minute lock, counted in Redis so
+	// the budget holds across instances. The owner is mailed when a lock is set.
+	challengeMFAUseCase := auth.NewChallengeMFAUseCase(mfaRepo, mfaKey[:]).
+		WithAttemptLimits(authmfa.NewAttemptStore(redisClientInstance), tokenBlacklistManager).
+		WithLockNotice(userRepo, securityMailer)
 	// OR26-03 — one resolver answers "must this member enrol now?" for /auth/me
 	// and for the request-time guard, so the banner and the enforcement can never
 	// disagree. Cached per (user, tenant) for a minute; enrolment, disabling and
@@ -759,7 +766,8 @@ func main() {
 		// The same repository resolveSessionForOrg reads at token mint time, so
 		// /auth/me's business role and the token's permissions come from one
 		// membership row and cannot disagree (#338).
-		WithMemberLookup(userRepo)
+		WithMemberLookup(userRepo).
+		WithAccessTokenRevocation(rsaKeys)
 
 	// OAuth identity resolution: known link → verified-email link → provision.
 	// No provisioner is wired, so an identity with no OpenRisk account is refused
@@ -895,7 +903,19 @@ func main() {
 	// on `api` BEFORE the Protected group: MFATokenMiddleware validates the special
 	// token itself and rejects full/absent tokens. On a valid code the handler
 	// mints the real access+refresh pair.
-	api.Post("/auth/mfa/challenge", middleware.MFATokenMiddleware(rsaKeys, jtiBlacklistChecker), mfaHandler.Challenge)
+	//
+	// #689 — the per-IP limit runs first, in its own bucket: a flood is refused
+	// before any token is parsed, and it never spends the login budget. The
+	// per-token and per-account limits live in the use case.
+	mfaChallengeRateLimit := middleware.RateLimit(middleware.RateLimitConfig{
+		MaxRequests: 20,
+		WindowSize:  5 * time.Minute,
+		Store: handlers.PrefixedRateLimitBackend{
+			Prefix: "mfa-challenge-ip:",
+			Inner:  authLimiterStore,
+		},
+	})
+	api.Post("/auth/mfa/challenge", mfaChallengeRateLimit, middleware.MFATokenMiddleware(rsaKeys, jtiBlacklistChecker), mfaHandler.Challenge)
 
 	// Scanner AGENT endpoints (register/stream/push) are mounted on `app` HERE —
 	// deliberately BEFORE the /api/v1 user-token middleware below — so they are

backend/internal/application/auth/logout.go

@@ -7,39 +7,70 @@ package auth
 
 import (
 	"context"
+	"errors"
 	"fmt"
-
-	"github.com/opendefender/openrisk/internal/auth"
+	"time"
 )
 
 // LogoutInput represents the input for user logout
 type LogoutInput struct {
 	RefreshToken string
+	// AccessJTI and AccessExpiresAt identify the access token the request came
+	// with, already verified by the caller. Revoking it is what makes logout
+	// end the session now rather than when the token expires (#689).
+	AccessJTI       string
+	AccessExpiresAt time.Time
+}
+
+// RefreshTokenRevoker deletes a refresh token. Satisfied by *auth.TokenManager.
+type RefreshTokenRevoker interface {
+	RevokeRefreshToken(ctx context.Context, refreshTokenValue string) error
 }
 
 // LogoutUseCase handles user logout
 type LogoutUseCase struct {
-	tokenManager *auth.TokenManager
+	tokenManager RefreshTokenRevoker
+	accessTokens TokenRevoker
 }
 
 // NewLogoutUseCase creates a new logout use case
-func NewLogoutUseCase(tokenManager *auth.TokenManager) *LogoutUseCase {
+func NewLogoutUseCase(tokenManager RefreshTokenRevoker) *LogoutUseCase {
 	return &LogoutUseCase{
 		tokenManager: tokenManager,
 	}
 }
 
-// Execute performs user logout
+// WithAccessTokenRevocation blacklists the access token on logout. Without it
+// the access token stays valid until it expires, up to AccessTokenTTL.
+func (uc *LogoutUseCase) WithAccessTokenRevocation(r TokenRevoker) *LogoutUseCase {
+	uc.accessTokens = r
+	return uc
+}
+
+// Execute performs user logout.
+//
+// The access token is revoked first: a refresh token that is already gone (a
+// second logout, a rotated cookie) must not leave the access token working.
+// Both revocations are always attempted, so a Redis outage on the first never
+// spares the refresh token.
 func (uc *LogoutUseCase) Execute(ctx context.Context, input LogoutInput) error {
-	// Validate input
-	if input.RefreshToken == "" {
+	if input.RefreshToken == "" && input.AccessJTI == "" {
 		return fmt.Errorf("refresh token is required")
 	}
 
-	// Revoke the refresh token
-	if err := uc.tokenManager.RevokeRefreshToken(ctx, input.RefreshToken); err != nil {
-		return fmt.Errorf("failed to revoke refresh token: %w", err)
+	var accessErr error
+	if input.AccessJTI != "" && uc.accessTokens != nil {
+		if ttl := time.Until(input.AccessExpiresAt); ttl > 0 {
+			if err := uc.accessTokens.BlacklistJTI(ctx, input.AccessJTI, ttl); err != nil {
+				accessErr = fmt.Errorf("failed to revoke access token: %w", err)
+			}
+		}
 	}
 
-	return nil
+	if input.RefreshToken != "" {
+		if err := uc.tokenManager.RevokeRefreshToken(ctx, input.RefreshToken); err != nil {
+			return errors.Join(fmt.Errorf("failed to revoke refresh token: %w", err), accessErr)
+		}
+	}
+	return accessErr
 }

backend/internal/application/auth/logout_test.go

@@ -0,0 +1,107 @@
+// 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"
+	"testing"
+	"time"
+)
+
+type fakeRefreshRevoker struct {
+	known   map[string]bool
+	revoked []string
+}
+
+func (f *fakeRefreshRevoker) RevokeRefreshToken(_ context.Context, v string) error {
+	if !f.known[v] {
+		return errors.New("refresh token not found")
+	}
+	f.revoked = append(f.revoked, v)
+	return nil
+}
+
+type failingJTIRevoker struct{ calls int }
+
+func (f *failingJTIRevoker) BlacklistJTI(context.Context, string, time.Duration) error {
+	f.calls++
+	return errors.New("redis down")
+}
+
+func newLogoutFixture() (*LogoutUseCase, *fakeRefreshRevoker, *fakeJTIRevoker) {
+	refresh := &fakeRefreshRevoker{known: map[string]bool{"rt-1": true}}
+	access := &fakeJTIRevoker{jtis: map[string]time.Duration{}}
+	return NewLogoutUseCase(refresh).WithAccessTokenRevocation(access), refresh, access
+}
+
+func TestLogout_Success(t *testing.T) {
+	uc, refresh, access := newLogoutFixture()
+	err := uc.Execute(context.Background(), LogoutInput{
+		RefreshToken: "rt-1", AccessJTI: "jti-1", AccessExpiresAt: time.Now().Add(10 * time.Minute),
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(refresh.revoked) != 1 {
+		t.Fatal("the refresh token must be revoked")
+	}
+	ttl, ok := access.jtis["jti-1"]
+	if !ok || ttl <= 9*time.Minute || ttl > 10*time.Minute {
+		t.Fatalf("the access token must be blacklisted for what it has left, got %v %v", ok, ttl)
+	}
+}
+
+// A refresh token that is already gone still lets the access token die.
+func TestLogout_NotFound(t *testing.T) {
+	uc, _, access := newLogoutFixture()
+	err := uc.Execute(context.Background(), LogoutInput{
+		RefreshToken: "rt-unknown", AccessJTI: "jti-1", AccessExpiresAt: time.Now().Add(time.Minute),
+	})
+	if err == nil {
+		t.Fatal("an unknown refresh token must be reported")
+	}
+	if !access.revoked("jti-1") {
+		t.Fatal("the access token must be revoked even when the refresh token is unknown")
+	}
+}
+
+func TestLogout_Unauthorized(t *testing.T) {
+	uc, refresh, access := newLogoutFixture()
+	if err := uc.Execute(context.Background(), LogoutInput{}); err == nil {
+		t.Fatal("a logout with no credential must fail")
+	}
+	if len(refresh.revoked) != 0 || len(access.jtis) != 0 {
+		t.Fatal("nothing may be revoked without a credential")
+	}
+}
+
+func TestLogout_ExpiredAccessTokenNeedsNoBlacklist(t *testing.T) {
+	uc, _, access := newLogoutFixture()
+	_ = uc.Execute(context.Background(), LogoutInput{
+		RefreshToken: "rt-1", AccessJTI: "jti-1", AccessExpiresAt: time.Now().Add(-time.Second),
+	})
+	if access.revoked("jti-1") {
+		t.Fatal("an expired token is already dead; blacklisting it wastes a key")
+	}
+}
+
+// A Redis outage on the blacklist must not spare the refresh token.
+func TestLogout_BlacklistOutageStillRevokesRefreshToken(t *testing.T) {
+	refresh := &fakeRefreshRevoker{known: map[string]bool{"rt-1": true}}
+	broken := &failingJTIRevoker{}
+	uc := NewLogoutUseCase(refresh).WithAccessTokenRevocation(broken)
+
+	err := uc.Execute(context.Background(), LogoutInput{
+		RefreshToken: "rt-1", AccessJTI: "jti-1", AccessExpiresAt: time.Now().Add(time.Minute),
+	})
+	if err == nil {
+		t.Fatal("the outage must be reported")
+	}
+	if broken.calls != 1 || len(refresh.revoked) != 1 {
+		t.Fatalf("both revocations must be attempted: blacklist=%d refresh=%d", broken.calls, len(refresh.revoked))
+	}
+}