Skip to content

Commit fa78847

Browse files
AchoArnoldCopilot
andcommitted
fix: invalidate auth cache when rotating user API key (#881)
RotateAPIKey now loads the old API key before updating and calls cache.Del(oldKey) after the DB transaction succeeds. This ensures the old key immediately stops authenticating instead of remaining valid for up to 2 hours (the ristretto cache TTL). Follows the surgical approach (Option B) matching how gorm_phone_api_key_repository already handles cache invalidation. Adds an integration test that rotates the key and verifies the old key returns 401 while the new key returns 200. Closes #881 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent aa8bb70 commit fa78847

2 files changed

Lines changed: 91 additions & 0 deletions

File tree

api/pkg/repositories/gorm_user_repository.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,13 @@ func (repository *gormUserRepository) RotateAPIKey(ctx context.Context, userID e
6565
}
6666

6767
user := new(entities.User)
68+
var oldAPIKey string
6869
err = crdbgorm.ExecuteTx(ctx, repository.db, nil,
6970
func(tx *gorm.DB) error {
71+
if err := tx.WithContext(ctx).Where("id = ?", userID).First(user).Error; err != nil {
72+
return err
73+
}
74+
oldAPIKey = user.APIKey
7075
return tx.WithContext(ctx).Model(user).
7176
Clauses(clause.Returning{}).
7277
Where("id = ?", userID).
@@ -78,6 +83,10 @@ func (repository *gormUserRepository) RotateAPIKey(ctx context.Context, userID e
7883
return nil, repository.tracer.WrapErrorSpan(span, stacktrace.PropagateWithCode(err, ErrCodeNotFound, msg))
7984
}
8085

86+
if err == nil && oldAPIKey != "" {
87+
repository.cache.Del(oldAPIKey)
88+
}
89+
8190
return user, nil
8291
}
8392

tests/integration_test.go

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"bytes"
55
"context"
66
"encoding/json"
7+
"fmt"
78
"io"
89
"net/http"
910
"strings"
@@ -213,6 +214,87 @@ func TestSendSMS_RateLimit(t *testing.T) {
213214
}
214215
}
215216

217+
func TestRotateAPIKey_InvalidatesCache(t *testing.T) {
218+
ctx := context.Background()
219+
220+
// 1) Confirm the current user API key works
221+
url := apiBaseURL + "/v1/users/me"
222+
req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil)
223+
require.NoError(t, err)
224+
req.Header.Set("x-api-key", userAPIKey)
225+
226+
resp, err := http.DefaultClient.Do(req)
227+
require.NoError(t, err)
228+
defer resp.Body.Close()
229+
230+
body, err := io.ReadAll(resp.Body)
231+
require.NoError(t, err)
232+
require.Equal(t, http.StatusOK, resp.StatusCode, "initial auth failed: %s", string(body))
233+
234+
// Parse user ID from the response
235+
var meResp struct {
236+
Data struct {
237+
ID string `json:"id"`
238+
APIKey string `json:"api_key"`
239+
} `json:"data"`
240+
}
241+
require.NoError(t, json.Unmarshal(body, &meResp))
242+
userID := meResp.Data.ID
243+
oldAPIKey := meResp.Data.APIKey
244+
require.NotEmpty(t, userID)
245+
require.NotEmpty(t, oldAPIKey)
246+
t.Logf("user ID: %s, old API key prefix: %s...", userID, oldAPIKey[:10])
247+
248+
// 2) Rotate the API key
249+
rotateURL := fmt.Sprintf("%s/v1/users/%s/api-keys", apiBaseURL, userID)
250+
req, err = http.NewRequestWithContext(ctx, http.MethodDelete, rotateURL, nil)
251+
require.NoError(t, err)
252+
req.Header.Set("x-api-key", userAPIKey)
253+
254+
resp, err = http.DefaultClient.Do(req)
255+
require.NoError(t, err)
256+
defer resp.Body.Close()
257+
258+
body, err = io.ReadAll(resp.Body)
259+
require.NoError(t, err)
260+
require.Equal(t, http.StatusOK, resp.StatusCode, "rotate failed: %s", string(body))
261+
262+
// Parse new API key from rotate response
263+
var rotateResp struct {
264+
Data struct {
265+
APIKey string `json:"api_key"`
266+
} `json:"data"`
267+
}
268+
require.NoError(t, json.Unmarshal(body, &rotateResp))
269+
newAPIKey := rotateResp.Data.APIKey
270+
require.NotEmpty(t, newAPIKey)
271+
require.NotEqual(t, oldAPIKey, newAPIKey, "API key should have changed after rotation")
272+
t.Logf("new API key prefix: %s...", newAPIKey[:10])
273+
274+
// 3) Old API key should immediately fail (401) — this is the bug regression check
275+
req, err = http.NewRequestWithContext(ctx, http.MethodGet, url, nil)
276+
require.NoError(t, err)
277+
req.Header.Set("x-api-key", oldAPIKey)
278+
279+
resp, err = http.DefaultClient.Do(req)
280+
require.NoError(t, err)
281+
defer resp.Body.Close()
282+
assert.Equal(t, http.StatusUnauthorized, resp.StatusCode, "old API key should return 401 after rotation")
283+
284+
// 4) New API key should work
285+
req, err = http.NewRequestWithContext(ctx, http.MethodGet, url, nil)
286+
require.NoError(t, err)
287+
req.Header.Set("x-api-key", newAPIKey)
288+
289+
resp, err = http.DefaultClient.Do(req)
290+
require.NoError(t, err)
291+
defer resp.Body.Close()
292+
293+
body, err = io.ReadAll(resp.Body)
294+
require.NoError(t, err)
295+
assert.Equal(t, http.StatusOK, resp.StatusCode, "new API key should work: %s", string(body))
296+
}
297+
216298
func TestSendSMS_OutstandingFlow(t *testing.T) {
217299
ctx := context.Background()
218300
phone := setupPhone(ctx, t, 60)

0 commit comments

Comments
 (0)