Compare commits

...
Author SHA1 Message Date
Sonarly Claude Code 4a40997d3f N+1 UPDATE queries in refresh token suspicious activity detection
https://sonarly.com/issue/3877?type=bug

The suspicious activity detection in RefreshTokenService.verifyRefreshToken() fires N individual UPDATE queries (one per user token) instead of a single bulk UPDATE when revoking all tokens, causing ~600 DB round-trips and ~2.9 seconds of cumulative DB time for users with many accumulated tokens.

Fix: ## Root cause

`verifyRefreshToken()` in `refresh-token.service.ts` was triggering the suspicious-activity path by iterating over every `AppTokenEntity` for a user and firing one `UPDATE` per row, producing ~600 individual SQL statements for a user who had accumulated many tokens:

```typescript
// BEFORE (N+1)
await Promise.all(
  user.appTokens.map(async ({ id, type }) => {
    if (type === AppTokenType.RefreshToken) {
      await this.appTokenRepository.update({ id }, { revokedAt: new Date() });
    }
  }),
);
```

## Fix

Replace the N+1 loop with a single bulk `UPDATE` — exactly the same pattern already used in `auth.service.ts` — and remove the now-unnecessary `relations: ['appTokens']` eager-load from the `userRepository.findOne()` call:

```typescript file=packages/twenty-server/src/engine/core-modules/auth/token/services/refresh-token.service.ts lines=70-95
// User fetch no longer needs to load the full token list
const user = await this.userRepository.findOne({
  where: { id: jwtPayload.sub },
});
// …
// AFTER: single bulk UPDATE regardless of token count
await this.appTokenRepository.update(
  {
    userId: user.id,
    type: AppTokenType.RefreshToken,
    revokedAt: IsNull(),
  },
  { revokedAt: new Date() },
);
```

`IsNull()` is imported from `typeorm` (added to the existing import line) and ensures only currently-active (non-revoked) tokens are touched, matching the semantics of the original loop's `if (type === AppTokenType.RefreshToken)` filter while collapsing all work into one SQL statement:

```sql
UPDATE core."appToken"
SET "revokedAt" = $1, "updatedAt" = CURRENT_TIMESTAMP
WHERE "userId" = $2
  AND "type" = 'REFRESH_TOKEN'
  AND "revokedAt" IS NULL
```

This reduces ~600 round-trips (~2.9 s of cumulative DB time) to a single query for any user regardless of how many accumulated tokens they have.
2026-03-05 09:45:13 +00:00
@@ -3,7 +3,7 @@ import { InjectRepository } from '@nestjs/typeorm';
import { addMilliseconds } from 'date-fns';
import ms from 'ms';
import { Repository } from 'typeorm';
import { IsNull, Repository } from 'typeorm';
import {
AppTokenEntity,
@@ -69,7 +69,6 @@ export class RefreshTokenService {
const user = await this.userRepository.findOne({
where: { id: jwtPayload.sub },
relations: ['appTokens'],
});
if (!user) {
@@ -86,17 +85,13 @@ export class RefreshTokenService {
if (wasRevokedBeforeGracePeriod) {
// Token was revoked long ago and is being reused -- suspicious.
// Revoke all user refresh tokens as a safety measure.
await Promise.all(
user.appTokens.map(async ({ id, type }) => {
if (type === AppTokenType.RefreshToken) {
await this.appTokenRepository.update(
{ id },
{
revokedAt: new Date(),
},
);
}
}),
await this.appTokenRepository.update(
{
userId: user.id,
type: AppTokenType.RefreshToken,
revokedAt: IsNull(),
},
{ revokedAt: new Date() },
);
throw new AuthException(