From fd3ee3db34943e199030f478d4c5f4575bff6678 Mon Sep 17 00:00:00 2001
From: fadymak <dev@fadymak.com>
Date: Mon, 7 Sep 2026 15:22:17 +0200
Subject: [PATCH] fix(mfa): regenerate duplicate recovery codes

---
 internal/api/recovery_codes.go      | 26 +++++++++++++++++------
 internal/api/recovery_codes_test.go | 32 +++++++++++++++++++++++++++++
 2 files changed, 52 insertions(+), 6 deletions(-)

diff --git a/internal/api/recovery_codes.go b/internal/api/recovery_codes.go
index bd39ee9d93..c8e3e18d4a 100644
--- a/internal/api/recovery_codes.go
+++ b/internal/api/recovery_codes.go
@@ -38,21 +38,35 @@ type RecoveryCodesResponse struct {
 	Codes        []string  `json:"codes,omitempty"`
 }
 
+func uniqueRecoveryCodes(count, length int, generate func(int) string) []string {
+	seen := make(map[string]struct{}, count)
+	codes := make([]string, 0, count)
+
+	for len(codes) < count {
+		code := generate(length)
+		if _, dup := seen[code]; dup {
+			continue
+		}
+
+		seen[code] = struct{}{}
+		codes = append(codes, code)
+	}
+
+	return codes
+}
+
 // generateRecoveryCodes generates the configured number of recovery codes,
 // returning the canonical plaintexts and their hashes.
 func generateRecoveryCodes(config *conf.GlobalConfiguration) ([]string, []string, error) {
-	count := config.MFA.RecoveryCodes.Count
-	codes := make([]string, 0, count)
-	hashes := make([]string, 0, count)
+	codes := uniqueRecoveryCodes(config.MFA.RecoveryCodes.Count, config.MFA.RecoveryCodes.CodeLength, crypto.GenerateRecoveryCode)
+	hashes := make([]string, 0, len(codes))
 
-	for range count {
-		code := crypto.GenerateRecoveryCode(config.MFA.RecoveryCodes.CodeLength)
+	for _, code := range codes {
 		hash, err := crypto.GenerateRecoveryCodeHash(code)
 		if err != nil {
 			return nil, nil, apierrors.NewInternalServerError("Error generating recovery codes").WithInternalError(err)
 		}
 
-		codes = append(codes, code)
 		hashes = append(hashes, hash)
 	}
 
diff --git a/internal/api/recovery_codes_test.go b/internal/api/recovery_codes_test.go
index c83f4d1725..f4881bd374 100644
--- a/internal/api/recovery_codes_test.go
+++ b/internal/api/recovery_codes_test.go
@@ -48,6 +48,38 @@ func TestRecoveryCodes(t *testing.T) {
 	suite.Run(t, ts)
 }
 
+func TestUniqueRecoveryCodes(t *testing.T) {
+	t.Run("regenerates a replacement for each duplicate", func(t *testing.T) {
+		scripted := []string{"a", "b", "a", "c", "b", "d"}
+		calls := 0
+		generate := func(length int) string {
+			require.Equal(t, 16, length)
+			require.Less(t, calls, len(scripted), "generator called more times than scripted")
+			code := scripted[calls]
+			calls++
+			return code
+		}
+
+		codes := uniqueRecoveryCodes(4, 16, generate)
+
+		require.Equal(t, []string{"a", "b", "c", "d"}, codes)
+		require.Equal(t, 6, calls, "4 for the batch plus one replacement per duplicate")
+	})
+
+	t.Run("no duplicates calls the generator exactly count times", func(t *testing.T) {
+		calls := 0
+		generate := func(int) string {
+			calls++
+			return fmt.Sprintf("code-%d", calls)
+		}
+
+		codes := uniqueRecoveryCodes(10, 16, generate)
+
+		require.Len(t, codes, 10)
+		require.Equal(t, 10, calls)
+	})
+}
+
 func (ts *RecoveryCodesTestSuite) SetupTest() {
 	models.TruncateAll(ts.API.db)