fix(auth): answer a deactivated user's reset token like an unknown token

internal/api/handler_auth.go

@@ -372,9 +372,6 @@ func (h *Handler) resetPassword(ctx context.Context, req *events.LambdaFunctionU
 	pwdResetReq.NewPassword = decoded
 
 	if err := h.auth.ConfirmPasswordReset(ctx, pwdResetReq); err != nil {
-		if errors.Is(err, auth.ErrAccountDeactivated) {
-			return nil, NewClientError(403, err.Error())
-		}
 		if errors.Is(err, auth.ErrUserChanged) {
 			return nil, NewClientError(409, err.Error())
 		}

internal/api/handler_auth_test.go

@@ -732,18 +732,16 @@ func TestHandler_resetPassword_Error(t *testing.T) {
 	assert.Contains(t, ce.Error(), "invalid or expired reset token")
 }
 
-// TestHandler_resetPassword_AccountDeactivated is the regression test for
-// issue #89 / audit finding A03-006: ConfirmPasswordReset returning
-// auth.ErrAccountDeactivated must map to 403, ahead of the generic
-// isResetPasswordClientError(400) path -- deleting that branch in the
-// handler would leave every OTHER resetPassword test green while this one
-// alone catches the regression.
-func TestHandler_resetPassword_AccountDeactivated(t *testing.T) {
+// TestHandler_resetPassword_InvalidResetToken: the service answers a
+// deactivated user's live token with auth.ErrInvalidResetToken (A03-006,
+// issue #595), which must map to the same 400 and message as an unknown
+// token rather than a 500 or a status that reveals account state.
+func TestHandler_resetPassword_InvalidResetToken(t *testing.T) {
 	ctx := context.Background()
 	mockAuth := new(MockAuthService)
 	t.Cleanup(func() { mockAuth.AssertExpectations(t) })
 
-	mockAuth.On("ConfirmPasswordReset", ctx, mock.Anything).Return(auth.ErrAccountDeactivated)
+	mockAuth.On("ConfirmPasswordReset", ctx, mock.Anything).Return(auth.ErrInvalidResetToken)
 
 	handler := &Handler{auth: mockAuth}
 
@@ -754,9 +752,9 @@ func TestHandler_resetPassword_AccountDeactivated(t *testing.T) {
 	assert.Nil(t, result)
 
 	ce, ok := IsClientError(err)
-	require.True(t, ok, "ErrAccountDeactivated must be wrapped as a client error, not a 500")
-	assert.Equal(t, 403, ce.code)
-	assert.Contains(t, ce.Error(), "deactivated")
+	require.True(t, ok, "ErrInvalidResetToken must be wrapped as a client error, not a 500")
+	assert.Equal(t, 400, ce.code)
+	assert.Equal(t, "invalid or expired reset token", ce.Error())
 }
 
 // A reset that lost a race to a concurrent account change is a 409, not a 500 (issue #493).

internal/auth/errors.go

@@ -86,10 +86,11 @@ var (
 	// than the most-permissive default. Mapped to 400 (issue #61).
 	ErrEmptyAPIKeyPermissions = errors.New("at least one permission is required")
 
-	// ErrAccountDeactivated: ConfirmPasswordReset refused an admin-deactivated
-	// account (A03-006). Safe to name, since the caller already holds a valid
-	// reset token mailed to the account.
-	ErrAccountDeactivated = errors.New("account is deactivated")
+	// ErrInvalidResetToken: the reset token is unknown, already used, or
+	// belongs to an admin-deactivated account (A03-006). The deactivated case
+	// shares this error so the reset endpoint never reveals account state
+	// (issue #595). Mapped to 400.
+	ErrInvalidResetToken = errors.New("invalid or expired reset token")
 
 	// ErrUserChanged: a field-scoped user write found the row changed since the
 	// caller read it (issue #474). Mapped to 409.

internal/auth/service_credentials_db_test.go

@@ -413,7 +413,7 @@ func TestIntegration_ResetConfirmRejectsStaleRead(t *testing.T) {
 		err := f.svc.ConfirmPasswordReset(ctx, PasswordResetConfirm{Token: token, NewPassword: credentialRaceNew})
 		require.ErrorIs(t, err, ErrUserChanged)
 		assert.Equal(t, winner(), f.stored())
-		require.ErrorIs(t, f.svc.ConfirmPasswordReset(ctx, PasswordResetConfirm{Token: token, NewPassword: credentialRaceNew}), ErrAccountDeactivated)
+		require.ErrorIs(t, f.svc.ConfirmPasswordReset(ctx, PasswordResetConfirm{Token: token, NewPassword: credentialRaceNew}), ErrInvalidResetToken)
 		assert.Empty(t, f.stored().PasswordResetToken)
 	})
 }

internal/auth/service_password.go

@@ -391,12 +391,14 @@ func (s *Service) ConfirmPasswordReset(ctx context.Context, req PasswordResetCon
 	readResetToken, readPasswordHash := user.PasswordResetToken, user.PasswordHash
 
 	// An admin-deactivated account must not reactivate itself through a reset
-	// (A03-006). The token is still consumed so the link cannot be replayed.
+	// (A03-006). The token is still consumed so the link cannot be replayed,
+	// and the caller gets the unknown-token error so the response does not
+	// reveal that the account is deactivated (issue #595).
 	if user.DeactivatedAt != nil {
 		if updateErr := s.store.ConsumePasswordResetToken(ctx, user.ID, readResetToken); updateErr != nil {
 			logging.Warnf("Failed to invalidate reset token for deactivated user %s: %v", user.ID, updateErr)
 		}
-		return ErrAccountDeactivated
+		return ErrInvalidResetToken
 	}
 
 	if err := s.processPasswordReset(user, req.NewPassword); err != nil {
@@ -514,17 +516,17 @@ func (s *Service) validateResetToken(ctx context.Context, token string) (*User,
 	user, err := s.store.GetUserByResetToken(ctx, tokenHash)
 	if err != nil {
 		if errors.Is(err, pgx.ErrNoRows) {
-			return nil, fmt.Errorf("invalid or expired reset token")
+			return nil, ErrInvalidResetToken
 		}
 		return nil, err
 	}
 	if user == nil {
-		return nil, fmt.Errorf("invalid or expired reset token")
+		return nil, ErrInvalidResetToken
 	}
 
 	// Constant-time comparison after fetch to close the SQL-equality timing oracle.
 	if subtle.ConstantTimeCompare([]byte(user.PasswordResetToken), []byte(tokenHash)) != 1 {
-		return nil, fmt.Errorf("invalid or expired reset token")
+		return nil, ErrInvalidResetToken
 	}
 
 	if user.PasswordResetExpiry == nil || time.Now().After(*user.PasswordResetExpiry) {

internal/auth/service_password_test.go

@@ -697,7 +697,9 @@ func TestService_ConfirmPasswordReset(t *testing.T) {
 
 		err := service.ConfirmPasswordReset(ctx, req)
 		require.Error(t, err)
-		assert.ErrorIs(t, err, ErrAccountDeactivated)
+		// Same error as an unknown token so account state is not revealed (issue #595).
+		assert.ErrorIs(t, err, ErrInvalidResetToken)
+		assert.Equal(t, "invalid or expired reset token", err.Error())
 		// Active must stay false: this is the exact regression this test
 		// guards against.
 		assert.False(t, deactivatedUser.Active)

internal/server/reset_token_status_deactivated_test.go

@@ -87,22 +87,14 @@ func TestHandleRequest_ResetPasswordStatus_DeactivatedUserLooksLikeUnknownToken(
 		"deactivated token must be indistinguishable from an unknown token")
 }
 
-func TestHandleRequest_ConfirmPasswordReset_DeactivatedUserRefused(t *testing.T) {
-	ctx := context.Background()
-	handler, mockStore := newResetTokenHarness(t)
-
-	user := deactivatedResetUser()
-	mockStore.On("GetUserByResetToken", mock.Anything, hashSessionTokenForTest(deactivatedResetRawToken)).
-		Return(user, nil).Once()
-	mockStore.On("ConsumePasswordResetToken", mock.Anything, user.ID, user.PasswordResetToken).
-		Return(nil).Once()
-
+func resetConfirmRequest(t *testing.T, token string) *events.LambdaFunctionURLRequest {
+	t.Helper()
 	body, err := json.Marshal(map[string]string{
-		"token":        deactivatedResetRawToken,
+		"token":        token,
 		"new_password": base64.StdEncoding.EncodeToString([]byte("Str0ng-Passw0rd-For-Test!")),
 	})
 	require.NoError(t, err)
-	resp, err := handler.HandleRequest(ctx, &events.LambdaFunctionURLRequest{
+	return &events.LambdaFunctionURLRequest{
 		Headers: map[string]string{"Content-Type": "application/json"},
 		Body:    string(body),
 		RequestContext: events.LambdaFunctionURLRequestContext{
@@ -111,7 +103,36 @@ func TestHandleRequest_ConfirmPasswordReset_DeactivatedUserRefused(t *testing.T)
 				Path:   "/api/auth/reset-password",
 			},
 		},
-	})
+	}
+}
+
+// Issue #595: the confirm endpoint refuses a deactivated user's live token
+// (A03-006) with the same response as an unknown token, so it does not
+// reveal that the account is deactivated. The token is still consumed (the
+// ConsumePasswordResetToken expectation) and no password write happens (any
+// other store call would fail the mock).
+func TestHandleRequest_ConfirmPasswordReset_DeactivatedUserLooksLikeUnknownToken(t *testing.T) {
+	ctx := context.Background()
+	handler, mockStore := newResetTokenHarness(t)
+
+	user := deactivatedResetUser()
+	mockStore.On("GetUserByResetToken", mock.Anything, hashSessionTokenForTest(deactivatedResetRawToken)).
+		Return(user, nil).Once()
+	mockStore.On("ConsumePasswordResetToken", mock.Anything, user.ID, user.PasswordResetToken).
+		Return(nil).Once()
+	mockStore.On("GetUserByResetToken", mock.Anything, hashSessionTokenForTest("unknown-token")).
+		Return((*auth.User)(nil), pgx.ErrNoRows).Once()
+
+	deactivated, err := handler.HandleRequest(ctx, resetConfirmRequest(t, deactivatedResetRawToken))
 	require.NoError(t, err)
-	assert.Equal(t, 403, resp.StatusCode, "body: %s", resp.Body)
+	unknown, err := handler.HandleRequest(ctx, resetConfirmRequest(t, "unknown-token"))
+	require.NoError(t, err)
+
+	require.Equal(t, 400, unknown.StatusCode, "body: %s", unknown.Body)
+	assert.Equal(t, unknown.StatusCode, deactivated.StatusCode, "body: %s", deactivated.Body)
+	assert.Equal(t, unknown.Headers, deactivated.Headers,
+		"deactivated token must be indistinguishable from an unknown token")
+	assert.JSONEq(t, unknown.Body, deactivated.Body,
+		"deactivated token must be indistinguishable from an unknown token")
+	assert.NotContains(t, deactivated.Body, "deactivated")
 }