diff --git a/internal/auth/interfaces.go b/internal/auth/interfaces.go index 610874183..b2171df15 100644 --- a/internal/auth/interfaces.go +++ b/internal/auth/interfaces.go @@ -65,7 +65,6 @@ type StoreInterface interface { GetAPIKeyByHash(ctx context.Context, keyHash string) (*UserAPIKey, error) ListAPIKeysByUser(ctx context.Context, userID string) ([]*UserAPIKey, error) UpdateAPIKey(ctx context.Context, key *UserAPIKey) error - UpdateAPIKeyLastUsed(ctx context.Context, keyID string) error // RecordAPIKeyUsage atomically updates last_used_at and adds delta to // request_count_total + request_count_window. delta is the number of // requests in this flush (greater than 1 when concurrent requests were @@ -85,6 +84,5 @@ type StoreInterface interface { // EmailSenderInterface defines the methods required for sending emails. type EmailSenderInterface interface { SendPasswordResetEmail(ctx context.Context, email, resetURL string) error - SendWelcomeEmail(ctx context.Context, email, dashboardURL, role string) error SendUserInviteEmail(ctx context.Context, email, setupURL string) error } diff --git a/internal/auth/service.go b/internal/auth/service.go index edaa3795d..02b82d13c 100644 --- a/internal/auth/service.go +++ b/internal/auth/service.go @@ -325,9 +325,12 @@ func (s *Service) ValidateSession(ctx context.Context, token string) (*Session, return nil, fmt.Errorf("session not found") } - // Constant-time comparison after fetch to close the SQL-equality timing oracle. - // GetSession does `WHERE token = $1` which is not constant-time in PostgreSQL; - // align with ValidateUserAPIKey / validateResetToken (issue #392 PR #837). + // Store-integrity check, not a timing defense (issue #313). GetSession + // selected this row by `WHERE token = $1` on hashedToken, so the two are + // equal unless the store returned a row for a different token; refuse it + // if it ever does. The equality timing of that lookup is harmless: the + // value compared is the SHA-256 of a 256-bit random token, which an + // attacker can neither guess nor steer byte by byte. if subtle.ConstantTimeCompare([]byte(session.Token), []byte(hashedToken)) != 1 { return nil, fmt.Errorf("session not found") } diff --git a/internal/auth/service_apikeys.go b/internal/auth/service_apikeys.go index bd5585dc6..d402bac46 100644 --- a/internal/auth/service_apikeys.go +++ b/internal/auth/service_apikeys.go @@ -45,27 +45,6 @@ const ( MaxAPIKeyLifetime = 365 * 24 * time.Hour ) -// loadActiveUserForAPIKey resolves userID and ensures the account exists and -// is active. Pulled out of CreateAPIKey to keep its cyclomatic complexity -// under the project's gocyclo gate as new checks (e.g. issue #61's empty- -// permissions rejection, issue #102's password/expiry validation) are added. -func (s *Service) loadActiveUserForAPIKey(ctx context.Context, userID string) (*User, error) { - user, err := s.store.GetUserByID(ctx, userID) - if err != nil { - if errors.Is(err, pgx.ErrNoRows) { - return nil, fmt.Errorf("user not found") - } - return nil, fmt.Errorf("failed to get user: %w", err) - } - if user == nil { - return nil, fmt.Errorf("user not found") - } - if !user.Active { - return nil, fmt.Errorf("user account is not active") - } - return user, nil -} - // CreateAPIKey creates a new user API key with scoped permissions. The // caller's current password must be supplied and is re-verified against the // stored hash before minting a key: a stolen session token alone must not be @@ -73,7 +52,7 @@ func (s *Service) loadActiveUserForAPIKey(ctx context.Context, userID string) (* // same defense-in-depth MFASetup/MFADisable already apply (issue #102). // Returns the full API key (shown only once), key info, and error. func (s *Service) CreateAPIKey(ctx context.Context, userID, name, password string, permissions []Permission, expiresAt *time.Time) (string, *UserAPIKey, error) { - user, err := s.loadActiveUserForAPIKey(ctx, userID) + user, err := s.loadActiveUser(ctx, userID) if err != nil { return "", nil, err } @@ -213,16 +192,9 @@ func (s *Service) validateAPIKeyPermissions(ctx context.Context, user *User, per // already rejects it, and the row only stays active when the best-effort // revocation scan after the password change failed (issue #534). func (s *Service) ListUserAPIKeys(ctx context.Context, userID string) ([]*UserAPIKey, error) { - // Validate user exists - user, err := s.store.GetUserByID(ctx, userID) + user, err := s.loadUser(ctx, userID) if err != nil { - if errors.Is(err, pgx.ErrNoRows) { - return nil, fmt.Errorf("user not found") - } - return nil, fmt.Errorf("failed to get user: %w", err) - } - if user == nil { - return nil, fmt.Errorf("user not found") + return nil, err } keys, err := s.store.ListAPIKeysByUser(ctx, userID) @@ -286,15 +258,8 @@ func (s *Service) authorizeAPIKeyAccess(ctx context.Context, userID, keyID, acti return nil, fmt.Errorf("API key not found") } - user, err := s.store.GetUserByID(ctx, userID) - if err != nil { - if errors.Is(err, pgx.ErrNoRows) { - return nil, fmt.Errorf("user not found") - } - return nil, fmt.Errorf("failed to get user: %w", err) - } - if user == nil { - return nil, fmt.Errorf("user not found") + if _, err := s.loadUser(ctx, userID); err != nil { + return nil, err } if key.UserID != userID { @@ -382,24 +347,6 @@ func validateAPIKeyStatus(key *UserAPIKey) error { return nil } -// lookupAPIKeyUser retrieves and validates the user associated with an API key. -func (s *Service) lookupAPIKeyUser(ctx context.Context, userID string) (*User, error) { - user, err := s.store.GetUserByID(ctx, userID) - if err != nil { - if errors.Is(err, pgx.ErrNoRows) { - return nil, fmt.Errorf("user account not found") - } - return nil, fmt.Errorf("failed to get user: %w", err) - } - if user == nil { - return nil, fmt.Errorf("user account not found") - } - if !user.Active { - return nil, fmt.Errorf("user account is not active") - } - return user, nil -} - // ValidateUserAPIKey validates an API key and returns the key info and associated user. // // Validation is deliberately side-effect free: it books no usage. A single @@ -422,9 +369,12 @@ func (s *Service) ValidateUserAPIKey(ctx context.Context, apiKey string) (*UserA return nil, nil, fmt.Errorf("invalid API key") } - // Constant-time comparison after fetch to close the SQL-equality timing oracle. - // The DB lookup already found the row by hash; this redundant check ensures the - // hot path does not leak timing information through early return differences. + // Store-integrity check, not a timing defense (issue #313). The store + // selected this row by `WHERE key_hash = $1` on keyHash, so the two are + // equal unless it returned a row for a different key; refuse it if it ever + // does. The equality timing of that lookup is harmless: the value compared + // is the SHA-256 of a 256-bit random key, which an attacker can neither + // guess nor steer byte by byte. if subtle.ConstantTimeCompare([]byte(key.KeyHash), []byte(keyHash)) != 1 { return nil, nil, fmt.Errorf("invalid API key") } @@ -434,7 +384,7 @@ func (s *Service) ValidateUserAPIKey(ctx context.Context, apiKey string) (*UserA return nil, nil, err } - user, err := s.lookupAPIKeyUser(ctx, key.UserID) + user, err := s.loadActiveUser(ctx, key.UserID) if err != nil { return nil, nil, err } @@ -564,13 +514,6 @@ func (s *Service) peekPendingUsage(keyID string) int64 { return counter.Load() } -// UpdateLastUsed updates only the last used timestamp for an API key. -// Retained for backwards compatibility; new code paths should use -// RecordUsage so the request_count_* counters stay current. -func (s *Service) UpdateLastUsed(ctx context.Context, keyID string) error { - return s.store.UpdateAPIKeyLastUsed(ctx, keyID) -} - // RecordUsage updates last_used_at and adds delta to both the lifetime // counter and the fixed-window request counter for the key. delta is the // number of requests being recorded, which is greater than 1 whenever diff --git a/internal/auth/service_apikeys_api.go b/internal/auth/service_apikeys_api.go index 20fa21312..456a8ad8d 100644 --- a/internal/auth/service_apikeys_api.go +++ b/internal/auth/service_apikeys_api.go @@ -61,9 +61,9 @@ func effectiveWindowUsage(key *UserAPIKey, now time.Time) (int64, *time.Time) { // predates the counter. Reporting that as 0 would assert a request volume we // do not have; nil says "unknown" and lets the UI show it as such. // -// This relies on last_used_at and request_count_total moving together. Any new -// caller of the legacy UpdateAPIKeyLastUsed (which bumps only the timestamp) -// would break the invariant and make a genuinely-unused key look unknown. +// This relies on last_used_at and request_count_total moving together. A new +// writer that bumps only the timestamp (as the removed UpdateAPIKeyLastUsed +// did) would break the invariant and make a genuinely-unused key look unknown. func effectiveLifetimeUsage(key *UserAPIKey) *int64 { if key.RequestCountTotal == 0 && key.LastUsedAt != nil { return nil @@ -424,8 +424,8 @@ func (s *Service) HasAPIKeyPermissionForConstraintsAPI(ctx context.Context, keyI if key == nil { return false, fmt.Errorf("API key not found") } - // lookupAPIKeyUser also verifies that the user is still active. - user, err := s.lookupAPIKeyUser(ctx, userID) + // loadActiveUser also verifies that the user is still active. + user, err := s.loadActiveUser(ctx, userID) if err != nil { return false, err } @@ -477,7 +477,7 @@ func (s *Service) GetAPIKeyEffectivePermissionsAPI(ctx context.Context, keyID, u if err != nil { return nil, err } - user, err := s.lookupAPIKeyUser(ctx, userID) + user, err := s.loadActiveUser(ctx, userID) if err != nil { return nil, err } diff --git a/internal/auth/service_apikeys_test.go b/internal/auth/service_apikeys_test.go index 9a6c16db2..be3db233e 100644 --- a/internal/auth/service_apikeys_test.go +++ b/internal/auth/service_apikeys_test.go @@ -873,8 +873,8 @@ func TestService_ValidateUserAPIKey(t *testing.T) { }) t.Run("reject when stored hash does not match derived hash", func(t *testing.T) { - // Simulates a DB row whose key_hash field was tampered or - // belongs to a different key -- the constant-time check must reject it. + // Simulates a store returning a row whose key_hash belongs to a + // different key -- the post-fetch store-integrity check must reject it. mockStore := new(MockStore) service := &Service{store: mockStore} @@ -901,34 +901,6 @@ func TestService_ValidateUserAPIKey(t *testing.T) { }) } -func TestService_UpdateLastUsed(t *testing.T) { - ctx := context.Background() - - t.Run("successfully update last used", func(t *testing.T) { - mockStore := new(MockStore) - service := &Service{store: mockStore} - - mockStore.On("UpdateAPIKeyLastUsed", ctx, "key-1").Return(nil) - - err := service.UpdateLastUsed(ctx, "key-1") - - require.NoError(t, err) - mockStore.AssertExpectations(t) - }) - - t.Run("return error when update fails", func(t *testing.T) { - mockStore := new(MockStore) - service := &Service{store: mockStore} - - mockStore.On("UpdateAPIKeyLastUsed", ctx, "key-1").Return(assert.AnError) - - err := service.UpdateLastUsed(ctx, "key-1") - - assert.Error(t, err) - mockStore.AssertExpectations(t) - }) -} - func TestService_RecordUsage(t *testing.T) { ctx := context.Background() @@ -1308,7 +1280,7 @@ func TestService_HasAPIKeyPermissionForConstraintsAPI_OwnerCapEnforced(t *testin mockStore := new(MockStore) t.Cleanup(func() { mockStore.AssertExpectations(t) }) mockStore.On("GetAPIKeyByID", ctx, keyID).Return(key, nil) - // lookupAPIKeyUser + GetAuthContext both call GetUserByID. + // loadActiveUser + GetAuthContext both call GetUserByID. mockStore.On("GetUserByID", ctx, userID).Return(owner, nil) mockStore.On("GetGroup", ctx, grpID).Return(ownerGroup, nil) return mockStore diff --git a/internal/auth/service_credentials_db_test.go b/internal/auth/service_credentials_db_test.go index 8eae7d460..fd9987c39 100644 --- a/internal/auth/service_credentials_db_test.go +++ b/internal/auth/service_credentials_db_test.go @@ -65,9 +65,6 @@ func (m *recordingMailSink) SendPasswordResetEmail(_ context.Context, email, _ s m.resets = append(m.resets, email) return m.err } -func (m *recordingMailSink) SendWelcomeEmail(context.Context, string, string, string) error { - return nil -} func (m *recordingMailSink) SendUserInviteEmail(context.Context, string, string) error { return nil } type credentialRaceFixture struct { diff --git a/internal/auth/service_group.go b/internal/auth/service_group.go index 39ca52ad0..e1ad905d8 100644 --- a/internal/auth/service_group.go +++ b/internal/auth/service_group.go @@ -73,16 +73,10 @@ func (s *Service) ListGroups(ctx context.Context) ([]Group, error) { // permission set. A nil group (the store returns nil, nil for a deleted/ // missing group) is skipped without error. func (s *Service) GetUserPermissions(ctx context.Context, userID string) ([]Permission, error) { - user, err := s.store.GetUserByID(ctx, userID) + user, err := s.loadUser(ctx, userID) if err != nil { - if errors.Is(err, pgx.ErrNoRows) { - return nil, fmt.Errorf("user not found") - } return nil, err } - if user == nil { - return nil, fmt.Errorf("user not found") - } // Permissions come exclusively from group memberships. return s.permissionsForGroups(ctx, user.GroupIDs) @@ -171,16 +165,10 @@ func (s *Service) accountsForGroups(ctx context.Context, groupIDs []string) ([]s // user's group memberships; a user with no groups gets an empty context and // is denied everything (fail closed). func (s *Service) BuildAuthContext(ctx context.Context, userID string) (*AuthContext, error) { - user, err := s.store.GetUserByID(ctx, userID) + user, err := s.loadUser(ctx, userID) if err != nil { - if errors.Is(err, pgx.ErrNoRows) { - return nil, fmt.Errorf("user not found") - } return nil, err } - if user == nil { - return nil, fmt.Errorf("user not found") - } authCtx := &AuthContext{ User: user, diff --git a/internal/auth/service_password.go b/internal/auth/service_password.go index 3be2d3027..567d11297 100644 --- a/internal/auth/service_password.go +++ b/internal/auth/service_password.go @@ -229,16 +229,10 @@ func (s *Service) invalidateUserCredentialsBestEffort(ctx context.Context, userI // ChangePassword allows a user to change their password. func (s *Service) ChangePassword(ctx context.Context, userID string, req ChangePasswordRequest) error { - user, err := s.store.GetUserByID(ctx, userID) + user, err := s.loadUser(ctx, userID) if err != nil { - if errors.Is(err, pgx.ErrNoRows) { - return fmt.Errorf("user not found") - } return err } - if user == nil { - return fmt.Errorf("user not found") - } // Verify current password if !s.verifyPassword(req.CurrentPassword, user.PasswordHash) { @@ -491,7 +485,10 @@ func (s *Service) ResetTokenStatus(ctx context.Context, token string) (ResetToke return ResetTokenStateUsed, ResetTokenFlowReset, nil } - // Constant-time comparison after fetch to close the SQL-equality timing oracle. + // Store-integrity check, not a timing defense (issue #313): the row was + // selected by `WHERE password_reset_token = $1` on tokenHash, so this only + // fails if the store returned a row for a different token. See the note + // in ValidateSession on why the lookup's equality timing is harmless. if subtle.ConstantTimeCompare([]byte(user.PasswordResetToken), []byte(tokenHash)) != 1 { return ResetTokenStateUsed, ResetTokenFlowReset, nil } @@ -524,7 +521,10 @@ func (s *Service) validateResetToken(ctx context.Context, token string) (*User, return nil, ErrInvalidResetToken } - // Constant-time comparison after fetch to close the SQL-equality timing oracle. + // Store-integrity check, not a timing defense (issue #313): the row was + // selected by `WHERE password_reset_token = $1` on tokenHash, so this only + // fails if the store returned a row for a different token. See the note + // in ValidateSession on why the lookup's equality timing is harmless. if subtle.ConstantTimeCompare([]byte(user.PasswordResetToken), []byte(tokenHash)) != 1 { return nil, ErrInvalidResetToken } diff --git a/internal/auth/service_password_test.go b/internal/auth/service_password_test.go index 6c85eddfc..b10c1b445 100644 --- a/internal/auth/service_password_test.go +++ b/internal/auth/service_password_test.go @@ -629,10 +629,11 @@ func TestService_ConfirmPasswordReset(t *testing.T) { mockStore.AssertExpectations(t) }) - t.Run("constant-time mismatch rejects token even when DB row exists", func(t *testing.T) { - // The DB returned a user but the stored reset-token hash does not match - // the hash we derived from the supplied token string. The constant-time - // guard must reject the request to avoid timing-based disclosure. + t.Run("stored hash mismatch rejects token even when DB row exists", func(t *testing.T) { + // The store returned a user whose reset-token hash does not match the + // hash we derived from the supplied token string, which a correct store + // never does (it selects by that hash). The post-fetch store-integrity + // check must still reject it (issue #313). mockStore := new(MockStore) mockEmail := new(MockEmailSender) service := createTestService(mockStore, mockEmail) @@ -894,11 +895,11 @@ func TestService_ResetTokenStatus(t *testing.T) { mockStore.AssertExpectations(t) }) - t.Run("constant-time mismatch returns used even if DB row exists", func(t *testing.T) { - // The DB returned a row but the stored token hash does not match the - // hash derived from the supplied token. The constant-time check must - // treat this as a missing/consumed token rather than branching on - // expiry state, to avoid leaking information through timing. + t.Run("stored hash mismatch returns used even if DB row exists", func(t *testing.T) { + // The store returned a row whose token hash does not match the hash + // derived from the supplied token, which a correct store never does. + // The post-fetch store-integrity check must treat this as a + // missing/consumed token rather than reporting the row's expiry state. mockStore := new(MockStore) mockEmail := new(MockEmailSender) service := createTestService(mockStore, mockEmail) diff --git a/internal/auth/service_test.go b/internal/auth/service_test.go index 2dfc78ed8..b399dd4d9 100644 --- a/internal/auth/service_test.go +++ b/internal/auth/service_test.go @@ -226,12 +226,11 @@ func TestService_ValidateSession(t *testing.T) { mockStore.AssertExpectations(t) }) - t.Run("constant-time mismatch rejects session even when DB row exists", func(t *testing.T) { - // The DB returned a session row but its stored token hash does not - // match the hash we derived from the supplied token string. The - // constant-time guard (added alongside the API-key and reset-token - // equivalents in PR #837) must reject the request to avoid leaking - // timing information through the SQL equality oracle. + t.Run("stored hash mismatch rejects session even when DB row exists", func(t *testing.T) { + // The store returned a session row whose token hash does not match + // the hash we derived from the supplied token string, which a correct + // store never does (it selects by that hash). The post-fetch + // store-integrity check must still reject it (issue #313). mockStore := new(MockStore) mockEmail := new(MockEmailSender) service := createTestService(mockStore, mockEmail) diff --git a/internal/auth/service_user.go b/internal/auth/service_user.go index 61e865d21..7972bfa7f 100644 --- a/internal/auth/service_user.go +++ b/internal/auth/service_user.go @@ -295,16 +295,17 @@ func (s *Service) sendInviteEmail(ctx context.Context, user *User, inviteToken s } // loadUser fetches a user by ID, normalising a missing row (either pgx.ErrNoRows -// or a nil user) into a uniform "user not found" error. Extracted so the mutating -// service methods share one lookup-or-not-found path and stay under gocyclo's -// complexity threshold. +// or a nil user) into a uniform "user not found" error and wrapping any other +// store error as "failed to get user: %w". Every service method that looks a +// user up by ID goes through here (or loadActiveUser) so the not-found and +// error paths cannot drift between copies (issue #313). func (s *Service) loadUser(ctx context.Context, userID string) (*User, error) { user, err := s.store.GetUserByID(ctx, userID) if err != nil { if errors.Is(err, pgx.ErrNoRows) { return nil, fmt.Errorf("user not found") } - return nil, err + return nil, fmt.Errorf("failed to get user: %w", err) } if user == nil { return nil, fmt.Errorf("user not found") @@ -312,6 +313,19 @@ func (s *Service) loadUser(ctx context.Context, userID string) (*User, error) { return user, nil } +// loadActiveUser is loadUser for paths that must refuse a deactivated +// account: minting an API key and authenticating with one. +func (s *Service) loadActiveUser(ctx context.Context, userID string) (*User, error) { + user, err := s.loadUser(ctx, userID) + if err != nil { + return nil, err + } + if !user.Active { + return nil, fmt.Errorf("user account is not active") + } + return user, nil +} + // UpdateUser updates user details (requires manage-users permission). // // actorUserID is the authenticated user performing the change (from the @@ -718,12 +732,8 @@ func (s *Service) DeleteUser(ctx context.Context, userID string) error { } if containsGroup(user.GroupIDs, DefaultAdminGroupID) { - count, err := s.store.CountGroupMembers(ctx, DefaultAdminGroupID) - if err != nil { - return fmt.Errorf("failed to count administrators: %w", err) - } - if count <= 1 { - return ErrLastAdmin + if err := s.checkLastAdminConstraint(ctx); err != nil { + return err } } @@ -752,15 +762,9 @@ func (s *Service) GetUser(ctx context.Context, userID string) (*User, error) { // UpdateUserProfile allows a user to update their own email and password. func (s *Service) UpdateUserProfile(ctx context.Context, userID, email, currentPassword, newPassword string) error { - user, err := s.store.GetUserByID(ctx, userID) + user, err := s.loadUser(ctx, userID) if err != nil { - if errors.Is(err, pgx.ErrNoRows) { - return fmt.Errorf("user not found") - } - return fmt.Errorf("failed to get user: %w", err) - } - if user == nil { - return fmt.Errorf("user not found") + return err } if !s.verifyPassword(currentPassword, user.PasswordHash) { diff --git a/internal/auth/service_user_lookup_test.go b/internal/auth/service_user_lookup_test.go new file mode 100644 index 000000000..1c8fac3b0 --- /dev/null +++ b/internal/auth/service_user_lookup_test.go @@ -0,0 +1,135 @@ +package auth + +import ( + "context" + "crypto/sha256" + "encoding/base64" + "errors" + "testing" + "time" + + "github.com/jackc/pgx/v5" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// lookupCaller is one service entry point that resolves a user by ID before +// doing anything else. setup stubs whatever the caller reads before that +// lookup; call runs it against lookupUserID. +type lookupCaller struct { + name string + activeOnly bool + setup func(ctx context.Context, m *MockStore) + call func(ctx context.Context, s *Service) error +} + +const ( + lookupUserID = "lookup-user" + lookupAPIKey = "lookup-api-key" +) + +// lookupCallers lists every caller routed through loadUser / loadActiveUser +// (issue #313). Before that, each one carried its own copy of the lookup and +// they disagreed on the not-found message ("user account not found" on the +// API-key path) and on whether a store error was wrapped. +func lookupCallers() []lookupCaller { + noSetup := func(context.Context, *MockStore) {} + ownKey := &UserAPIKey{ID: "key-1", UserID: lookupUserID, IsActive: true} + return []lookupCaller{ + {name: "UpdateUser", setup: noSetup, call: func(ctx context.Context, s *Service) error { + _, err := s.UpdateUser(ctx, "", lookupUserID, UpdateUserRequest{}) + return err + }}, + {name: "DeleteUser", setup: noSetup, call: func(ctx context.Context, s *Service) error { + return s.DeleteUser(ctx, lookupUserID) + }}, + {name: "UpdateUserProfile", setup: noSetup, call: func(ctx context.Context, s *Service) error { + return s.UpdateUserProfile(ctx, lookupUserID, "", "current", "") + }}, + {name: "ChangePassword", setup: noSetup, call: func(ctx context.Context, s *Service) error { + return s.ChangePassword(ctx, lookupUserID, ChangePasswordRequest{CurrentPassword: "current"}) + }}, + {name: "GetUserPermissions", setup: noSetup, call: func(ctx context.Context, s *Service) error { + _, err := s.GetUserPermissions(ctx, lookupUserID) + return err + }}, + {name: "BuildAuthContext", setup: noSetup, call: func(ctx context.Context, s *Service) error { + _, err := s.BuildAuthContext(ctx, lookupUserID) + return err + }}, + {name: "ListUserAPIKeys", setup: noSetup, call: func(ctx context.Context, s *Service) error { + _, err := s.ListUserAPIKeys(ctx, lookupUserID) + return err + }}, + {name: "RevokeAPIKey", setup: func(ctx context.Context, m *MockStore) { + m.On("GetAPIKeyByID", ctx, "key-1").Return(ownKey, nil) + }, call: func(ctx context.Context, s *Service) error { + return s.RevokeAPIKey(ctx, lookupUserID, "key-1") + }}, + {name: "CreateAPIKey", activeOnly: true, setup: noSetup, call: func(ctx context.Context, s *Service) error { + _, _, err := s.CreateAPIKey(ctx, lookupUserID, "k", "current", nil, nil) + return err + }}, + {name: "ValidateUserAPIKey", activeOnly: true, setup: func(ctx context.Context, m *MockStore) { + // A live, unexpired key, so validation reaches the owner lookup. + sum := sha256.Sum256([]byte(lookupAPIKey)) + hash := base64.RawURLEncoding.EncodeToString(sum[:]) + expires := time.Now().Add(time.Hour) + m.On("GetAPIKeyByHash", ctx, hash).Return(&UserAPIKey{ + ID: "key-1", UserID: lookupUserID, KeyHash: hash, IsActive: true, ExpiresAt: &expires, + }, nil) + }, call: func(ctx context.Context, s *Service) error { + _, _, err := s.ValidateUserAPIKey(ctx, lookupAPIKey) + return err + }}, + } +} + +func newLookupService(ctx context.Context, t *testing.T, c lookupCaller, user *User, err error) *Service { + t.Helper() + m := new(MockStore) + c.setup(ctx, m) + m.On("GetUserByID", ctx, lookupUserID).Return(user, err) + return createTestService(m, new(MockEmailSender)) +} + +// TestUserLookup_UniformNotFoundAndStoreError pins one lookup contract on every +// caller: a missing row (pgx.ErrNoRows or a nil user) reads "user not found", +// and any other store error is wrapped as "failed to get user: %w". +func TestUserLookup_UniformNotFoundAndStoreError(t *testing.T) { + ctx := context.Background() + storeErr := errors.New("connection reset") + + for _, c := range lookupCallers() { + t.Run(c.name+"/ErrNoRows", func(t *testing.T) { + err := c.call(ctx, newLookupService(ctx, t, c, nil, pgx.ErrNoRows)) + require.EqualError(t, err, "user not found") + }) + t.Run(c.name+"/nil user", func(t *testing.T) { + err := c.call(ctx, newLookupService(ctx, t, c, nil, nil)) + require.EqualError(t, err, "user not found") + }) + t.Run(c.name+"/store error", func(t *testing.T) { + err := c.call(ctx, newLookupService(ctx, t, c, nil, storeErr)) + require.ErrorIs(t, err, storeErr) + assert.EqualError(t, err, "failed to get user: connection reset") + }) + } +} + +// TestUserLookup_ActiveOnlyCallersRefuseInactiveUser: the API-key mint and +// authenticate paths go through loadActiveUser and refuse a deactivated +// owner with the same message. +func TestUserLookup_ActiveOnlyCallersRefuseInactiveUser(t *testing.T) { + ctx := context.Background() + for _, c := range lookupCallers() { + if !c.activeOnly { + continue + } + t.Run(c.name, func(t *testing.T) { + inactive := &User{ID: lookupUserID, Email: "inactive@example.com", Active: false} + err := c.call(ctx, newLookupService(ctx, t, c, inactive, nil)) + require.EqualError(t, err, "user account is not active") + }) + } +} diff --git a/internal/auth/store_postgres_apikeys.go b/internal/auth/store_postgres_apikeys.go index 0ee96903b..4a2355521 100644 --- a/internal/auth/store_postgres_apikeys.go +++ b/internal/auth/store_postgres_apikeys.go @@ -164,22 +164,6 @@ func (s *PostgresStore) UpdateAPIKey(ctx context.Context, key *UserAPIKey) error return nil } -// UpdateAPIKeyLastUsed atomically updates the last_used_at timestamp for an -// API key. Retained for backwards compatibility with callers that only need -// the timestamp update -- production code routes through RecordAPIKeyUsage, -// which additionally maintains the usage counters from migration 000094. -func (s *PostgresStore) UpdateAPIKeyLastUsed(ctx context.Context, keyID string) error { - query := `UPDATE api_keys SET last_used_at = NOW() WHERE id = $1` - result, err := s.db.Exec(ctx, query, keyID) - if err != nil { - return fmt.Errorf("failed to update API key last used: %w", err) - } - if result.RowsAffected() == 0 { - return fmt.Errorf("API key not found: %s", keyID) - } - return nil -} - // RecordAPIKeyUsage atomically updates last_used_at, adds delta to // request_count_total, and adds-or-resets request_count_window based on // whether the current window is still within apiKeyUsageWindow of its start. diff --git a/internal/auth/store_postgres_pgxmock_test.go b/internal/auth/store_postgres_pgxmock_test.go index d7b7aff8d..35b2d463d 100644 --- a/internal/auth/store_postgres_pgxmock_test.go +++ b/internal/auth/store_postgres_pgxmock_test.go @@ -3,9 +3,8 @@ package auth // store_postgres_pgxmock_test.go - pgxmock tests for the PostgresStore SQL // paths that the MockDBConnection suite never executes (TEST-01, issue #1153): // CreateAdminIfNone (bootstrap-race guard), CountGroupMembers, ListUsers, -// ListAPIKeysByUser, UpdateAPIKeyLastUsed and Ping. These tests pin the actual -// SQL text (argument count and order, the NOT EXISTS predicate) so that -// column drift or predicate divergence between AdminExists and +// ListAPIKeysByUser and Ping. These tests pin the actual SQL text (argument +// count and order, the NOT EXISTS predicate) so that column drift or predicate divergence between AdminExists and // CreateAdminIfNone fails a test instead of shipping green. import ( @@ -499,46 +498,6 @@ func TestPGXMock_ListAPIKeysByUser_QueryError(t *testing.T) { assert.Nil(t, keys) } -// ---- UpdateAPIKeyLastUsed ------------------------------------------------------ - -func TestPGXMock_UpdateAPIKeyLastUsed_Success(t *testing.T) { - mock := newAuthPgxMock(t) - store := NewPostgresStore(mock) - - mock.ExpectExec(`UPDATE api_keys SET last_used_at = NOW\(\) WHERE id = \$1`). - WithArgs("key-1"). - WillReturnResult(pgxmock.NewResult("UPDATE", 1)) - - err := store.UpdateAPIKeyLastUsed(context.Background(), "key-1") - assert.NoError(t, err) -} - -func TestPGXMock_UpdateAPIKeyLastUsed_NotFound(t *testing.T) { - mock := newAuthPgxMock(t) - store := NewPostgresStore(mock) - - mock.ExpectExec(`UPDATE api_keys SET last_used_at = NOW\(\) WHERE id = \$1`). - WithArgs("missing-key"). - WillReturnResult(pgxmock.NewResult("UPDATE", 0)) - - err := store.UpdateAPIKeyLastUsed(context.Background(), "missing-key") - require.Error(t, err) - assert.Contains(t, err.Error(), "API key not found") -} - -func TestPGXMock_UpdateAPIKeyLastUsed_ExecError(t *testing.T) { - mock := newAuthPgxMock(t) - store := NewPostgresStore(mock) - - mock.ExpectExec(`UPDATE api_keys SET last_used_at = NOW\(\) WHERE id = \$1`). - WithArgs("key-1"). - WillReturnError(errors.New("db down")) - - err := store.UpdateAPIKeyLastUsed(context.Background(), "key-1") - require.Error(t, err) - assert.Contains(t, err.Error(), "failed to update API key last used") -} - // ---- RecordAPIKeyUsage ---------------------------------------------------------- func TestPGXMock_RecordAPIKeyUsage_Success(t *testing.T) { diff --git a/internal/auth/test_helpers.go b/internal/auth/test_helpers.go index c09ce3d55..dcd7b6a0a 100644 --- a/internal/auth/test_helpers.go +++ b/internal/auth/test_helpers.go @@ -269,11 +269,6 @@ func (m *MockStore) UpdateAPIKey(ctx context.Context, key *UserAPIKey) error { return args.Error(0) } -func (m *MockStore) UpdateAPIKeyLastUsed(ctx context.Context, keyID string) error { - args := m.Called(ctx, keyID) - return args.Error(0) -} - func (m *MockStore) RecordAPIKeyUsage(ctx context.Context, keyID string, delta int64) error { args := m.Called(ctx, keyID, delta) return args.Error(0) @@ -299,11 +294,6 @@ func (m *MockEmailSender) SendPasswordResetEmail(ctx context.Context, email, res return args.Error(0) } -func (m *MockEmailSender) SendWelcomeEmail(ctx context.Context, email, dashboardURL, role string) error { - args := m.Called(ctx, email, dashboardURL, role) - return args.Error(0) -} - func (m *MockEmailSender) SendUserInviteEmail(ctx context.Context, email, setupURL string) error { args := m.Called(ctx, email, setupURL) return args.Error(0) diff --git a/internal/auth/types.go b/internal/auth/types.go index 898465075..974673902 100644 --- a/internal/auth/types.go +++ b/internal/auth/types.go @@ -345,13 +345,6 @@ type CreateAPIKeyResponse struct { KeyID string `json:"key_id"` } -// Predefined roles. -const ( - RoleAdmin = "admin" - RoleUser = "user" - RoleReadOnly = "readonly" -) - // DefaultAdminGroupID is the fixed UUID of the Administrators group seeded // by migration 000024. SetupAdmin auto-assigns new admin users to this // group so the group card shows members on a fresh install. @@ -389,14 +382,14 @@ const ( // button on pending Purchase History rows (issue #46). // // Default grants: - // * RoleAdmin — implicit via {ActionAdmin, ResourceAll}; covers + // * Administrators group — implicit via {ActionAdmin, ResourceAll}; covers // both verbs. - // * RoleUser — DefaultUserPermissions() adds cancel-own:purchases. + // * Standard Users group — DefaultUserPermissions() adds cancel-own:purchases. // Allows canceling pending executions whose created_by_user_id // matches the session user. Legacy rows with NULL creator are // out of reach for non-admins via this verb; admins still cancel // them via cancel-any. - // * RoleReadOnly — neither verb. Read-only users cannot cancel. + // * Read-Only Users group — neither verb. Read-only users cannot cancel. // // cancel-any has no default non-admin grant; the constant exists so // future operator roles can be granted broad cancel rights without @@ -411,14 +404,14 @@ const ( // button on failed Purchase History rows (issue #47). Mirror image // of the cancel verbs above: // - // * RoleAdmin — implicit via {ActionAdmin, ResourceAll}; covers + // * Administrators group — implicit via {ActionAdmin, ResourceAll}; covers // both verbs. - // * RoleUser — DefaultUserPermissions() adds retry-own:purchases. + // * Standard Users group — DefaultUserPermissions() adds retry-own:purchases. // Allows retrying failed executions whose created_by_user_id // matches the session user. Legacy rows with NULL creator are // out of reach for non-admins via this verb; admins still // retry them via retry-any. - // * RoleReadOnly — neither verb. Read-only users cannot retry. + // * Read-Only Users group — neither verb. Read-only users cannot retry. // // retry-any has no default non-admin grant; the constant exists so // future operator roles can be granted broad retry rights without @@ -439,13 +432,13 @@ const ( // button on pending Purchase History rows (issue #286). // // Default grants (four-eyes policy, issue #1407): - // * RoleAdmin — implicit via {ActionAdmin, ResourceAll}; covers + // * Administrators group — implicit via {ActionAdmin, ResourceAll}; covers // both verbs. - // * RoleUser — NO default grant (issue #1407). Four-eyes principle: + // * Standard Users group — NO default grant (issue #1407). Four-eyes principle: // the submitter of a purchase must not approve it by default. // approve-own must be explicitly added to a custom group for // organizations that deliberately permit self-approval. - // * RoleReadOnly — neither verb. Read-only users cannot approve. + // * Read-Only Users group — neither verb. Read-only users cannot approve. // // approve-any: seeded by the Purchaser group (migration 000059); // not a default non-admin grant. Add it to a custom group's @@ -466,13 +459,13 @@ const ( // (executed_by_user_id, executed_at, pre_approval_skip_reason) stamped // on the execution row. // - // * RoleAdmin — implicit via {ActionAdmin, ResourceAll}; covers + // * Administrators group — implicit via {ActionAdmin, ResourceAll}; covers // both verbs. - // * RoleUser — NO default grant. This is a finance-impacting permission + // * Standard Users group — NO default grant. This is a finance-impacting permission // that must be explicitly granted per-user/per-role. Even trusted // users submit via the approval flow by default; only deliberately // privileged accounts should hold this verb. - // * RoleReadOnly — neither verb. + // * Read-Only Users group — neither verb. // // execute-own: allows direct-execute only for executions where // created_by_user_id == session user (the user drafted the purchase @@ -490,9 +483,9 @@ const ( // (created_by_user_id == session.UserID), while update-any drops the // per-record ownership check. // - // * RoleAdmin — implicit via {ActionAdmin, ResourceAll}; update-any is + // * Administrators group — implicit via {ActionAdmin, ResourceAll}; update-any is // NOT in adminCarvedOuts, so admins manage every scheduled purchase. - // * RoleUser — NO default grant. A standard user manages only the + // * Standard Users group — NO default grant. A standard user manages only the // scheduled purchases they created (base update:purchases + creator // match). Legacy rows with NULL created_by_user_id are out of reach // for non-admins (they hold neither update-any nor a creator match). @@ -509,8 +502,8 @@ const ( // free-cancel window (issue #290). // // Default grants: - // * RoleAdmin -- implicit via {ActionAdmin, ResourceAll}. - // * RoleUser -- DefaultUserPermissions() adds revoke-own:purchases. + // * Administrators group -- implicit via {ActionAdmin, ResourceAll}. + // * Standard Users group -- DefaultUserPermissions() adds revoke-own:purchases. // "Own" is currently enforced at ACCOUNT scope, not creator scope: // a user may revoke a completed purchase in any cloud account they // are allowed to access (the check in @@ -522,7 +515,7 @@ const ( // NOTE: whether revoke-own should instead be creator-scoped is a // product decision tracked in issue #950; do not tighten this to // created_by_user_id without resolving that issue first. - // * RoleReadOnly -- neither verb. + // * Read-Only Users group -- neither verb. // // revoke-any has no default non-admin grant; the constant exists so // future operator roles can be granted broad revoke rights without @@ -533,11 +526,11 @@ const ( // Standard RI purchase history rows (issue #292). Mirror image of the // cancel-{own,any} / retry-{own,any} / approve-{own,any} family: // - // * RoleAdmin — implicit via {ActionAdmin, ResourceAll}; covers + // * Administrators group — implicit via {ActionAdmin, ResourceAll}; covers // both verbs. - // * RoleUser — DefaultUserPermissions() adds sell-own:purchases so + // * Standard Users group — DefaultUserPermissions() adds sell-own:purchases so // every user can list RIs they purchased themselves. - // * RoleReadOnly — neither verb. + // * Read-Only Users group — neither verb. // // sell-any has no default non-admin grant; the constant exists so a // custom operator group can list any Standard RI without admin escalation. diff --git a/internal/credentials/resolver.go b/internal/credentials/resolver.go index d2863c3b9..1bd2eab05 100644 --- a/internal/credentials/resolver.go +++ b/internal/credentials/resolver.go @@ -206,13 +206,8 @@ func resolveRoleARNProvider( return ambient, nil } - sessionSuffix := account.ID - if len(sessionSuffix) > 8 { - sessionSuffix = sessionSuffix[:8] - } - provider := stscreds.NewAssumeRoleProvider(stsClient, account.AWSRoleARN, func(o *stscreds.AssumeRoleOptions) { - o.RoleSessionName = "cudly-" + sessionSuffix + o.RoleSessionName = roleSessionName(account.ID) if account.AWSExternalID != "" { o.ExternalID = aws.String(account.AWSExternalID) } @@ -220,6 +215,19 @@ func resolveRoleARNProvider( return provider, nil } +// roleSessionName is the STS RoleSessionName CUDly uses when it assumes a role +// for accountID: "cudly-" plus at most the first 8 bytes of the ID, so the +// session is attributable in CloudTrail while staying well inside STS's +// 64-character limit. AssumeRole and AssumeRoleWithWebIdentity share it so the +// two paths cannot drift (issue #313). +func roleSessionName(accountID string) string { + suffix := accountID + if len(suffix) > 8 { + suffix = suffix[:8] + } + return "cudly-" + suffix +} + // resolveBastionProvider implements the bastion auth mode: target account is // reached via STS AssumeRole using credentials from a separate bastion account. // @@ -290,14 +298,10 @@ func resolveWebIdentityProvider(account *config.CloudAccount, stsClient STSClien if strings.Contains(tokenFile, "..") || !filepath.IsAbs(tokenFile) { return nil, awsConfigErrorf("aws_web_identity_token_file", "must be an absolute path without '..'", "credentials: aws_web_identity_token_file must be an absolute path without '..' (account %s)", account.ID) } - sessionSuffix := account.ID - if len(sessionSuffix) > 8 { - sessionSuffix = sessionSuffix[:8] - } return stscreds.NewWebIdentityRoleProvider(stsClient, account.AWSRoleARN, stscreds.IdentityTokenFile(tokenFile), func(o *stscreds.WebIdentityRoleOptions) { - o.RoleSessionName = "cudly-" + sessionSuffix + o.RoleSessionName = roleSessionName(account.ID) // Note: WebIdentityRoleOptions does not have ExternalID; // OIDC identity is verified by the token subject claim instead. }), nil @@ -325,18 +329,6 @@ func ResolveAzureCredentials(ctx context.Context, account *config.CloudAccount, return &AzureCredentials{ClientSecret: payload.ClientSecret}, nil } -// ResolveGCPCredentials decrypts and returns the GCP service account JSON for the account. -func ResolveGCPCredentials(ctx context.Context, account *config.CloudAccount, store CredentialStore) ([]byte, error) { - raw, err := store.LoadRaw(ctx, account.ID, CredTypeGCPServiceAccount) - if err != nil { - return nil, fmt.Errorf("credentials: load gcp service account for account %s: %w", account.ID, err) - } - if raw == nil { - return nil, fmt.Errorf("credentials: no service account stored for account %s", account.ID) - } - return raw, nil -} - // ResolveAzureTokenCredential is a convenience wrapper that passes // zero-valued options to ResolveAzureTokenCredentialWithOpts. Since // the legacy cert-based WIF path has been removed, this entry point diff --git a/internal/credentials/resolver_extra_test.go b/internal/credentials/resolver_extra_test.go index 931860d38..f0cff7958 100644 --- a/internal/credentials/resolver_extra_test.go +++ b/internal/credentials/resolver_extra_test.go @@ -324,24 +324,6 @@ func TestResolveGCPTokenSource_InvalidJSON(t *testing.T) { // Additional coverage for already-partially-covered functions // --------------------------------------------------------------------------- -func TestResolveGCPCredentials_LoadError(t *testing.T) { - store := newMockStore() - store.err = assert.AnError - - account := &config.CloudAccount{ID: "acct1"} - _, err := ResolveGCPCredentials(context.Background(), account, store) - assert.Error(t, err) - assert.Contains(t, err.Error(), "load gcp service account") -} - -func TestResolveGCPCredentials_NotStored(t *testing.T) { - store := newMockStore() - account := &config.CloudAccount{ID: "acct1"} - _, err := ResolveGCPCredentials(context.Background(), account, store) - assert.Error(t, err) - assert.Contains(t, err.Error(), "no service account stored") -} - func TestResolveAzureCredentials_LoadError(t *testing.T) { store := newMockStore() store.err = assert.AnError @@ -383,7 +365,7 @@ func TestResolveAccessKeyProvider_InvalidJSON(t *testing.T) { } func TestResolveRoleARNProvider_LongID_WithExternalID(t *testing.T) { - // Exercise the branch where account.ID > 8 chars (sessionSuffix truncated) + // Exercise the branch where account.ID > 8 chars (roleSessionName truncates) // and AWSExternalID is set. account := &config.CloudAccount{ ID: "a-very-long-account-id", @@ -397,7 +379,7 @@ func TestResolveRoleARNProvider_LongID_WithExternalID(t *testing.T) { } func TestResolveWebIdentityProvider_LongID_WithTokenFile(t *testing.T) { - // Exercises resolveWebIdentityProvider with a long account ID (truncation branch) + // Exercises resolveWebIdentityProvider with a long account ID (roleSessionName truncates) // and with the token file path set in the account struct. f, err := os.CreateTemp("", "wif-token-*.txt") require.NoError(t, err) diff --git a/internal/credentials/resolver_test.go b/internal/credentials/resolver_test.go index a9c98c874..e1462f665 100644 --- a/internal/credentials/resolver_test.go +++ b/internal/credentials/resolver_test.go @@ -4,6 +4,8 @@ import ( "context" "encoding/json" "errors" + "os" + "path/filepath" "testing" "github.com/LeanerCloud/cloud-commitments-platform/internal/config" @@ -425,16 +427,6 @@ func TestResolveAzureCredentials_NotStored(t *testing.T) { assert.Contains(t, err.Error(), "no client secret stored") } -func TestResolveGCPCredentials_Success(t *testing.T) { - store := newMockStore() - saJSON := []byte(`{"type":"service_account","project_id":"my-project"}`) - store.data["acct1/gcp_service_account"] = saJSON - - result, err := ResolveGCPCredentials(context.Background(), &config.CloudAccount{ID: "acct1"}, store) - require.NoError(t, err) - assert.Equal(t, saJSON, result) -} - func TestAWSCredentials_String_IsRedacted(t *testing.T) { c := &AWSCredentials{AccessKeyID: "AKIAIOSFODNN7EXAMPLE", SecretAccessKey: "secret"} assert.NotContains(t, c.String(), "AKIAIOSFODNN7EXAMPLE") @@ -445,3 +437,51 @@ func TestAzureCredentials_String_IsRedacted(t *testing.T) { c := &AzureCredentials{ClientSecret: "supersecret"} assert.NotContains(t, c.String(), "supersecret") } + +// sessionNameRecorder is an STSClient that records the RoleSessionName of +// each call and then fails it, so a provider's Retrieve stops right after. +type sessionNameRecorder struct{ names []string } + +func (r *sessionNameRecorder) AssumeRole(_ context.Context, in *sts.AssumeRoleInput, _ ...func(*sts.Options)) (*sts.AssumeRoleOutput, error) { + r.names = append(r.names, aws.ToString(in.RoleSessionName)) + return nil, errors.New("recorded") +} + +func (r *sessionNameRecorder) AssumeRoleWithWebIdentity(_ context.Context, in *sts.AssumeRoleWithWebIdentityInput, _ ...func(*sts.Options)) (*sts.AssumeRoleWithWebIdentityOutput, error) { + r.names = append(r.names, aws.ToString(in.RoleSessionName)) + return nil, errors.New("recorded") +} + +// TestRoleSessionName_SharedByAssumeRoleAndWebIdentity pins the session name +// both STS paths send: "cudly-" plus the first 8 bytes of the account ID +// (issue #313 folded the two inline copies into roleSessionName). +func TestRoleSessionName_SharedByAssumeRoleAndWebIdentity(t *testing.T) { + tokenFile := filepath.Join(t.TempDir(), "token") + require.NoError(t, os.WriteFile(tokenFile, []byte("dummy-oidc-token"), 0o600)) + + for _, tc := range []struct{ id, want string }{ + {id: "a-very-long-account-id", want: "cudly-a-very-l"}, + {id: "12345678", want: "cudly-12345678"}, + {id: "short", want: "cudly-short"}, + } { + t.Run(tc.id, func(t *testing.T) { + assert.Equal(t, tc.want, roleSessionName(tc.id)) + + rec := &sessionNameRecorder{} + account := &config.CloudAccount{ID: tc.id, AWSRoleARN: "arn:aws:iam::123456789012:role/CUDly"} + + roleProvider, err := resolveRoleARNProvider(context.Background(), account, rec, nil, nil) + require.NoError(t, err) + _, err = roleProvider.Retrieve(context.Background()) + require.Error(t, err) + + account.AWSWebIdentityTokenFile = tokenFile + wifProvider, err := resolveWebIdentityProvider(account, rec) + require.NoError(t, err) + _, err = wifProvider.Retrieve(context.Background()) + require.Error(t, err) + + assert.Equal(t, []string{tc.want, tc.want}, rec.names) + }) + } +} diff --git a/internal/mocks/stores.go b/internal/mocks/stores.go index e946ed5f8..83dbf5b6a 100644 --- a/internal/mocks/stores.go +++ b/internal/mocks/stores.go @@ -1349,12 +1349,6 @@ func (m *MockAuthStore) UpdateAPIKey(ctx context.Context, key *auth.UserAPIKey) return args.Error(0) } -// UpdateAPIKeyLastUsed mocks the UpdateAPIKeyLastUsed operation. -func (m *MockAuthStore) UpdateAPIKeyLastUsed(ctx context.Context, keyID string) error { - args := m.Called(ctx, keyID) - return args.Error(0) -} - // RecordAPIKeyUsage mocks the RecordAPIKeyUsage operation. func (m *MockAuthStore) RecordAPIKeyUsage(ctx context.Context, keyID string, delta int64) error { args := m.Called(ctx, keyID, delta) diff --git a/internal/server/health_test.go b/internal/server/health_test.go index 53727f78c..a2f7df910 100644 --- a/internal/server/health_test.go +++ b/internal/server/health_test.go @@ -172,10 +172,6 @@ func (m *mockAuthStoreForHealth) UpdateAPIKey(ctx context.Context, key *auth.Use return nil } -func (m *mockAuthStoreForHealth) UpdateAPIKeyLastUsed(ctx context.Context, keyID string) error { - return nil -} - func (m *mockAuthStoreForHealth) RecordAPIKeyUsage(ctx context.Context, keyID string, delta int64) error { return nil }