From bff29d8f07d7bf0bb0719a8be9b2193e7d3bd506 Mon Sep 17 00:00:00 2001 From: Hampus Date: Sun, 6 Sep 2026 15:07:00 +0200 Subject: [PATCH] fix(api)!: always send Retry-After and reclassify two limits (#2489) --- .../src/api/auth/tests/IPAuthResend.test.ts | 21 +++-- .../services/message/MessageEditService.ts | 2 +- .../errors/ConnectionLimitReachedError.ts | 4 +- .../connection/tests/ConnectionLimit.test.ts | 91 +++++++++++++++++++ .../src/__tests__/ThrottledError.test.ts | 78 ++++++++++++++++ .../IpAuthorizationResendCooldownError.ts | 5 +- ...IpAuthorizationResendLimitExceededError.ts | 4 +- .../errors/src/domains/core/RateLimitError.ts | 3 +- .../errors/src/domains/core/ThrottledError.ts | 12 ++- 9 files changed, 202 insertions(+), 18 deletions(-) create mode 100644 fluxer_api/src/api/connection/tests/ConnectionLimit.test.ts create mode 100644 packages/errors/src/__tests__/ThrottledError.test.ts diff --git a/fluxer_api/src/api/auth/tests/IPAuthResend.test.ts b/fluxer_api/src/api/auth/tests/IPAuthResend.test.ts index 76b411a62..9be62c92d 100644 --- a/fluxer_api/src/api/auth/tests/IPAuthResend.test.ts +++ b/fluxer_api/src/api/auth/tests/IPAuthResend.test.ts @@ -77,7 +77,7 @@ describe('Auth IP Authorization Resend', () => { .execute(); expect(errorResp.code).toBe(APIErrorCodes.IP_AUTHORIZATION_RESEND_COOLDOWN); }); - it('returns resend_available_in when rate limited', async () => { + it('returns resend_available_in and a matching Retry-After header when rate limited', async () => { const email = createUniqueEmail('ip-resend-cooldown'); const password = 'a-strong-password'; await registerUser(harness, { @@ -90,15 +90,17 @@ describe('Auth IP Authorization Resend', () => { }); const newIP = '10.230.240.250'; const ipAuthResp = await triggerIpAuthorization(email, password, newIP); - const errorResp = await createBuilderWithoutAuth(harness) + const {response, json: errorResp} = await createBuilderWithoutAuth(harness) .post('/auth/ip-authorization/resend') .body({ticket: ipAuthResp.ticket}) .header('x-forwarded-for', newIP) .expect(HTTP_TOO_MANY_REQUESTS) - .execute(); + .executeWithResponse(); expect(errorResp.code).toBe(APIErrorCodes.IP_AUTHORIZATION_RESEND_COOLDOWN); expect(typeof errorResp.resend_available_in).toBe('number'); expect(errorResp.resend_available_in).toBeGreaterThan(0); + expect(response.headers.get('Retry-After')).toBe(String(errorResp.resend_available_in)); + expect(errorResp.retry_after).toBe(errorResp.resend_available_in); }); }); describe('Multiple resend attempts handling', () => { @@ -126,7 +128,7 @@ describe('Auth IP Authorization Resend', () => { }); }); describe('Resend with already-used ticket', () => { - it('returns 429 when resend flag is already set on the ticket', async () => { + it('returns 400 when resend flag is already set on the ticket', async () => { const email = createUniqueEmail('ip-resend-used'); const password = 'a-strong-password'; const reg = await registerUser(harness, { @@ -155,14 +157,15 @@ describe('Auth IP Authorization Resend', () => { ttl_seconds: 900, }) .execute(); - const errorResp = await createBuilderWithoutAuth(harness) + const {response, json: errorResp} = await createBuilderWithoutAuth(harness) .post('/auth/ip-authorization/resend') .body({ticket}) - .expect(HTTP_TOO_MANY_REQUESTS) - .execute(); + .expect(HTTP_STATUS.BAD_REQUEST) + .executeWithResponse(); expect(errorResp.code).toBe(APIErrorCodes.IP_AUTHORIZATION_RESEND_LIMIT_EXCEEDED); + expect(response.headers.get('Retry-After')).toBeNull(); }); - it('returns 429 after IP has already been authorized', async () => { + it('returns 400 after IP has already been authorized', async () => { const email = createUniqueEmail('ip-resend-after-auth'); const password = 'a-strong-password'; await registerUser(harness, { @@ -304,7 +307,7 @@ describe('Auth IP Authorization Resend', () => { const errorResp = await createBuilderWithoutAuth(harness) .post('/auth/ip-authorization/resend') .body({ticket}) - .expect(HTTP_TOO_MANY_REQUESTS) + .expect(HTTP_STATUS.BAD_REQUEST) .execute(); expect(errorResp.code).toBe(APIErrorCodes.IP_AUTHORIZATION_RESEND_LIMIT_EXCEEDED); expect(typeof errorResp.message).toBe('string'); diff --git a/fluxer_api/src/api/channel/services/message/MessageEditService.ts b/fluxer_api/src/api/channel/services/message/MessageEditService.ts index 990678e22..79836d028 100644 --- a/fluxer_api/src/api/channel/services/message/MessageEditService.ts +++ b/fluxer_api/src/api/channel/services/message/MessageEditService.ts @@ -230,7 +230,7 @@ export class MessageEditService { if (!lockToken) { throw new ThrottledError({ code: APIErrorCodes.RESOURCE_LOCKED, - headers: {'Retry-After': '1'}, + retryAfterSeconds: 1, data: {retry_after: 1}, }); } diff --git a/fluxer_api/src/api/connection/errors/ConnectionLimitReachedError.ts b/fluxer_api/src/api/connection/errors/ConnectionLimitReachedError.ts index 411e6d156..e435d6c98 100644 --- a/fluxer_api/src/api/connection/errors/ConnectionLimitReachedError.ts +++ b/fluxer_api/src/api/connection/errors/ConnectionLimitReachedError.ts @@ -2,9 +2,9 @@ import {APIErrorCodes} from '@fluxer/constants/src/ApiErrorCodes'; import {MAX_CONNECTIONS_PER_USER} from '@fluxer/constants/src/ConnectionConstants'; -import {ThrottledError} from '@fluxer/errors/src/domains/core/ThrottledError'; +import {BadRequestError} from '@fluxer/errors/src/domains/core/BadRequestError'; -export class ConnectionLimitReachedError extends ThrottledError { +export class ConnectionLimitReachedError extends BadRequestError { constructor(limit: number = MAX_CONNECTIONS_PER_USER) { super({ code: APIErrorCodes.CONNECTION_LIMIT_REACHED, diff --git a/fluxer_api/src/api/connection/tests/ConnectionLimit.test.ts b/fluxer_api/src/api/connection/tests/ConnectionLimit.test.ts new file mode 100644 index 000000000..5ae667b64 --- /dev/null +++ b/fluxer_api/src/api/connection/tests/ConnectionLimit.test.ts @@ -0,0 +1,91 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later + +import {APIErrorCodes} from '@fluxer/constants/src/ApiErrorCodes'; +import { + ConnectionTypes, + ConnectionVisibilityFlags, + MAX_CONNECTIONS_PER_USER, +} from '@fluxer/constants/src/ConnectionConstants'; +import {afterEach, beforeEach, describe, expect, it} from 'vitest'; +import {createUserID, type UserID} from '../../BrandedTypes'; +import type {IBlueskyOAuthService} from '../../bluesky/IBlueskyOAuthService'; +import {setCassandraQueryExecutorForTesting} from '../../database/CassandraQueryExecution'; +import type {UserConnectionRow} from '../../database/types/ConnectionTypes'; +import type {IGatewayService} from '../../infrastructure/IGatewayService'; +import {InMemoryCassandraQueryExecutor} from '../../test/InMemoryCassandraQueryExecutor'; +import {ConnectionRepository} from '../ConnectionRepository'; +import {ConnectionService} from '../ConnectionService'; +import {ConnectionLimitReachedError} from '../errors/ConnectionLimitReachedError'; + +function connectionRow(userId: UserID, connectionId: string, sortOrder: number): UserConnectionRow { + return { + user_id: userId, + connection_id: connectionId, + connection_type: ConnectionTypes.DOMAIN, + identifier: `${connectionId}.example`, + name: connectionId, + verified: true, + visibility_flags: ConnectionVisibilityFlags.EVERYONE, + sort_order: sortOrder, + verification_token: 'token', + verified_at: null, + last_verified_at: null, + created_at: new Date(0), + version: 1, + }; +} + +describe('ConnectionService connection limit', () => { + let executor: InMemoryCassandraQueryExecutor; + + beforeEach(() => { + executor = new InMemoryCassandraQueryExecutor(); + setCassandraQueryExecutorForTesting(executor); + }); + + afterEach(() => { + executor.reset(); + setCassandraQueryExecutorForTesting(null); + }); + + it('rejects the connection past the ceiling with 400 and no retry guidance', async () => { + const userId = createUserID(1n); + const repository = new ConnectionRepository(); + for (let index = 0; index < MAX_CONNECTIONS_PER_USER; index++) { + await repository.create(connectionRow(userId, `connection-${index}`, index)); + } + const service = new ConnectionService( + repository, + {} as unknown as IGatewayService, + {} as unknown as IBlueskyOAuthService, + ); + + const error = await service.initiateConnection(userId, ConnectionTypes.DOMAIN, 'over-the-limit.example').then( + () => null, + (caught: unknown) => caught, + ); + + expect(error).toBeInstanceOf(ConnectionLimitReachedError); + const response = (error as ConnectionLimitReachedError).getResponse(); + expect(response.status).toBe(400); + expect(response.headers.get('Retry-After')).toBeNull(); + expect(await response.json()).toMatchObject({code: APIErrorCodes.CONNECTION_LIMIT_REACHED}); + }); + + it('allows a connection while a slot is free', async () => { + const userId = createUserID(2n); + const repository = new ConnectionRepository(); + for (let index = 0; index < MAX_CONNECTIONS_PER_USER - 1; index++) { + await repository.create(connectionRow(userId, `connection-${index}`, index)); + } + const service = new ConnectionService( + repository, + {} as unknown as IGatewayService, + {} as unknown as IBlueskyOAuthService, + ); + + await expect( + service.initiateConnection(userId, ConnectionTypes.DOMAIN, 'under-the-limit.example'), + ).resolves.toEqual({}); + }); +}); diff --git a/packages/errors/src/__tests__/ThrottledError.test.ts b/packages/errors/src/__tests__/ThrottledError.test.ts new file mode 100644 index 000000000..786061949 --- /dev/null +++ b/packages/errors/src/__tests__/ThrottledError.test.ts @@ -0,0 +1,78 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later + +import {APIErrorCodes} from '@fluxer/constants/src/ApiErrorCodes'; +import {IpAuthorizationResendCooldownError} from '@fluxer/errors/src/domains/auth/IpAuthorizationResendCooldownError'; +import {IpAuthorizationResendLimitExceededError} from '@fluxer/errors/src/domains/auth/IpAuthorizationResendLimitExceededError'; +import {ThrottledError} from '@fluxer/errors/src/domains/core/ThrottledError'; +import type {FluxerError} from '@fluxer/errors/src/FluxerError'; +import {describe, expect, it} from 'vitest'; + +interface ThrottledResponseBody { + code: string; + [key: string]: unknown; +} + +async function readResponse(error: FluxerError): Promise<{ + status: number; + retryAfter: string | null; + body: ThrottledResponseBody; +}> { + const response = error.getResponse(); + const body = (await response.json()) as ThrottledResponseBody; + return {status: response.status, retryAfter: response.headers.get('Retry-After'), body}; +} + +describe('ThrottledError', () => { + it('answers 429 with a Retry-After header taken from the caller', async () => { + const {status, retryAfter} = await readResponse( + new ThrottledError({code: APIErrorCodes.RESOURCE_LOCKED, retryAfterSeconds: 1}), + ); + expect(status).toBe(429); + expect(retryAfter).toBe('1'); + }); + + it('rounds a fractional delay up to whole seconds', async () => { + const {retryAfter} = await readResponse( + new ThrottledError({code: APIErrorCodes.RESOURCE_LOCKED, retryAfterSeconds: 2.1}), + ); + expect(retryAfter).toBe('3'); + }); + + it('falls back to one second when the delay is not a usable number', async () => { + const {retryAfter} = await readResponse( + new ThrottledError({code: APIErrorCodes.RESOURCE_LOCKED, retryAfterSeconds: Number.NaN}), + ); + expect(retryAfter).toBe('1'); + }); + + it('keeps the computed Retry-After when the caller passes other headers', async () => { + const error = new ThrottledError({ + code: APIErrorCodes.RESOURCE_LOCKED, + retryAfterSeconds: 4, + headers: {'X-RateLimit-Scope': 'user'}, + }); + const response = error.getResponse(); + expect(response.headers.get('Retry-After')).toBe('4'); + expect(response.headers.get('X-RateLimit-Scope')).toBe('user'); + }); +}); + +describe('IpAuthorizationResendCooldownError', () => { + it('reports the cooldown as a header and in the body', async () => { + const {status, retryAfter, body} = await readResponse(new IpAuthorizationResendCooldownError(17)); + expect(status).toBe(429); + expect(body.code).toBe(APIErrorCodes.IP_AUTHORIZATION_RESEND_COOLDOWN); + expect(body.resend_available_in).toBe(17); + expect(body.retry_after).toBe(17); + expect(retryAfter).toBe('17'); + }); +}); + +describe('IpAuthorizationResendLimitExceededError', () => { + it('answers 400 with no retry guidance because the allowance never refills', async () => { + const {status, retryAfter, body} = await readResponse(new IpAuthorizationResendLimitExceededError()); + expect(status).toBe(400); + expect(body.code).toBe(APIErrorCodes.IP_AUTHORIZATION_RESEND_LIMIT_EXCEEDED); + expect(retryAfter).toBeNull(); + }); +}); diff --git a/packages/errors/src/domains/auth/IpAuthorizationResendCooldownError.ts b/packages/errors/src/domains/auth/IpAuthorizationResendCooldownError.ts index cce332235..eb01cd3bd 100644 --- a/packages/errors/src/domains/auth/IpAuthorizationResendCooldownError.ts +++ b/packages/errors/src/domains/auth/IpAuthorizationResendCooldownError.ts @@ -1,13 +1,16 @@ // SPDX-License-Identifier: AGPL-3.0-or-later import {APIErrorCodes} from '@fluxer/constants/src/ApiErrorCodes'; +import {sanitizeRetryAfterSeconds} from '@fluxer/errors/src/domains/core/RetryAfterSeconds'; import {ThrottledError} from '@fluxer/errors/src/domains/core/ThrottledError'; export class IpAuthorizationResendCooldownError extends ThrottledError { constructor(resendAvailableIn: number) { + const retryAfter = sanitizeRetryAfterSeconds(resendAvailableIn); super({ code: APIErrorCodes.IP_AUTHORIZATION_RESEND_COOLDOWN, - data: {resend_available_in: resendAvailableIn}, + retryAfterSeconds: retryAfter, + data: {resend_available_in: resendAvailableIn, retry_after: retryAfter}, }); } } diff --git a/packages/errors/src/domains/auth/IpAuthorizationResendLimitExceededError.ts b/packages/errors/src/domains/auth/IpAuthorizationResendLimitExceededError.ts index 923734092..8a96a2ab8 100644 --- a/packages/errors/src/domains/auth/IpAuthorizationResendLimitExceededError.ts +++ b/packages/errors/src/domains/auth/IpAuthorizationResendLimitExceededError.ts @@ -1,9 +1,9 @@ // SPDX-License-Identifier: AGPL-3.0-or-later import {APIErrorCodes} from '@fluxer/constants/src/ApiErrorCodes'; -import {ThrottledError} from '@fluxer/errors/src/domains/core/ThrottledError'; +import {BadRequestError} from '@fluxer/errors/src/domains/core/BadRequestError'; -export class IpAuthorizationResendLimitExceededError extends ThrottledError { +export class IpAuthorizationResendLimitExceededError extends BadRequestError { constructor() { super({code: APIErrorCodes.IP_AUTHORIZATION_RESEND_LIMIT_EXCEEDED}); } diff --git a/packages/errors/src/domains/core/RateLimitError.ts b/packages/errors/src/domains/core/RateLimitError.ts index 0b4df94b1..dced8f8d0 100644 --- a/packages/errors/src/domains/core/RateLimitError.ts +++ b/packages/errors/src/domains/core/RateLimitError.ts @@ -68,7 +68,6 @@ export class RateLimitError extends ThrottledError { retry_after: safeRetryAfterDecimal, }; const headers: Record = { - 'Retry-After': safeRetryAfter.toString(), 'X-RateLimit-Scope': safeScope, }; if (global) { @@ -82,6 +81,6 @@ export class RateLimitError extends ThrottledError { headers['X-RateLimit-Bucket'] = bucketHash; } } - super({code, message, data, headers}); + super({code, message, retryAfterSeconds: safeRetryAfter, data, headers}); } } diff --git a/packages/errors/src/domains/core/ThrottledError.ts b/packages/errors/src/domains/core/ThrottledError.ts index 0d0f462c7..69424e5de 100644 --- a/packages/errors/src/domains/core/ThrottledError.ts +++ b/packages/errors/src/domains/core/ThrottledError.ts @@ -1,21 +1,31 @@ // SPDX-License-Identifier: AGPL-3.0-or-later +import {sanitizeRetryAfterSeconds} from '@fluxer/errors/src/domains/core/RetryAfterSeconds'; import {FluxerError, type FluxerErrorData} from '@fluxer/errors/src/FluxerError'; export class ThrottledError extends FluxerError { constructor({ code, message, + retryAfterSeconds, data, headers, messageVariables, }: { code: string; message?: string; + retryAfterSeconds: number; data?: FluxerErrorData; headers?: Record; messageVariables?: Record; }) { - super({code, message, status: 429, data, headers, messageVariables}); + super({ + code, + message, + status: 429, + data, + headers: {...headers, 'Retry-After': sanitizeRetryAfterSeconds(retryAfterSeconds).toString()}, + messageVariables, + }); } }