From ffc2737478c6f9efd5de9fbaf526b13164727f87 Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Sat, 7 Dec 2024 10:22:45 -0500 Subject: implement SkRateLimiterService with Leaky Bucket rate limiting --- packages/backend/src/server/api/ApiCallService.ts | 64 +++++++++++++++++------ 1 file changed, 47 insertions(+), 17 deletions(-) (limited to 'packages/backend/src/server/api/ApiCallService.ts') diff --git a/packages/backend/src/server/api/ApiCallService.ts b/packages/backend/src/server/api/ApiCallService.ts index 6f51825494..14367e02bb 100644 --- a/packages/backend/src/server/api/ApiCallService.ts +++ b/packages/backend/src/server/api/ApiCallService.ts @@ -8,6 +8,7 @@ import * as fs from 'node:fs'; import * as stream from 'node:stream/promises'; import { Inject, Injectable } from '@nestjs/common'; import * as Sentry from '@sentry/node'; +import { LimiterInfo } from 'ratelimiter'; import { DI } from '@/di-symbols.js'; import { getIpHash } from '@/misc/get-ip-hash.js'; import type { MiLocalUser, MiUser } from '@/models/User.js'; @@ -18,6 +19,7 @@ import { createTemp } from '@/misc/create-temp.js'; import { bindThis } from '@/decorators.js'; import { RoleService } from '@/core/RoleService.js'; import type { Config } from '@/config.js'; +import { isLimitInfo } from '@/server/api/SkRateLimiterService.js'; import { ApiError } from './error.js'; import { RateLimiterService } from './RateLimiterService.js'; import { ApiLoggerService } from './ApiLoggerService.js'; @@ -68,12 +70,17 @@ export class ApiCallService implements OnApplicationShutdown { } else if (err.code === 'RATE_LIMIT_EXCEEDED') { const info: unknown = err.info; const unixEpochInSeconds = Date.now(); - if (typeof(info) === 'object' && info && 'resetMs' in info && typeof(info.resetMs) === 'number') { + if (isLimitInfo(info)) { + // Number of seconds to wait before trying again. Left for backwards compatibility. + reply.header('Retry-After', info.resetSec.toString()); + // Number of milliseconds to wait before trying again. + reply.header('X-RateLimit-Reset', info.resetMs.toString()); + } else if (typeof(info) === 'object' && info && 'resetMs' in info && typeof(info.resetMs) === 'number') { const cooldownInSeconds = Math.ceil((info.resetMs - unixEpochInSeconds) / 1000); // もしかするとマイナスになる可能性がなくはないのでマイナスだったら0にしておく reply.header('Retry-After', Math.max(cooldownInSeconds, 0).toString(10)); } else { - this.logger.warn(`rate limit information has unexpected type ${typeof(err.info?.reset)}`); + this.logger.warn(`rate limit information has unexpected type: ${JSON.stringify(info)}`); } } else if (err.kind === 'client') { reply.header('WWW-Authenticate', `Bearer realm="Misskey", error="invalid_request", error_description="${err.message}"`); @@ -168,7 +175,7 @@ export class ApiCallService implements OnApplicationShutdown { return; } this.authenticateService.authenticate(token).then(([user, app]) => { - this.call(endpoint, user, app, body, null, request).then((res) => { + this.call(endpoint, user, app, body, null, request, reply).then((res) => { if (request.method === 'GET' && endpoint.meta.cacheSec && !token && !user) { reply.header('Cache-Control', `public, max-age=${endpoint.meta.cacheSec}`); } @@ -229,7 +236,7 @@ export class ApiCallService implements OnApplicationShutdown { this.call(endpoint, user, app, fields, { name: multipartData.filename, path: path, - }, request).then((res) => { + }, request, reply).then((res) => { this.send(reply, res); }).catch((err: ApiError) => { this.#sendApiError(reply, err); @@ -304,6 +311,7 @@ export class ApiCallService implements OnApplicationShutdown { path: string; } | null, request: FastifyRequest<{ Body: Record | undefined, Querystring: Record }>, + reply: FastifyReply, ) { const isSecure = user != null && token == null; @@ -339,19 +347,41 @@ export class ApiCallService implements OnApplicationShutdown { if (factor > 0) { // Rate limit - await this.rateLimiterService.limit(limit as IEndpointMeta['limit'] & { key: NonNullable }, limitActor, factor).catch(err => { - if ('info' in err) { - // errはLimiter.LimiterInfoであることが期待される - throw new ApiError({ - message: 'Rate limit exceeded. Please try again later.', - code: 'RATE_LIMIT_EXCEEDED', - id: 'd5826d14-3982-4d2e-8011-b9e9f02499ef', - httpStatusCode: 429, - }, err.info); - } else { - throw new TypeError('information must be a rate-limiter information.'); - } - }); + const info = await this.rateLimiterService.limit(limit as IEndpointMeta['limit'] & { key: NonNullable }, limitActor, factor) + .then(info => { + // We always want these headers, because clients need them for pacing. + // Conditional check in case we somehow revert to the old limiter, which does not return info. + if (info) { + // Number of seconds until the limit has fully reset. + reply.header('X-RateLimit-Clear', info.fullResetSec.toString()); + // Number of calls that can be made before being limited. + reply.header('X-RateLimit-Remaining', info.remaining.toString()); + + // Only forward the info object if it's blocked, otherwise we'll reject *all* requests + if (info.blocked) { + return info; + } + } + + return undefined; + }) + .catch(err => { + // The old limiter throws info instead of returning it. + if ('info' in err) { + return err.info as LimiterInfo; + } else { + throw err; + } + }); + + if (info) { + throw new ApiError({ + message: 'Rate limit exceeded. Please try again later.', + code: 'RATE_LIMIT_EXCEEDED', + id: 'd5826d14-3982-4d2e-8011-b9e9f02499ef', + httpStatusCode: 429, + }, info); + } } } -- cgit v1.2.3-freya From f6b256620b9637ffe4bd29a07cfba1a7880c9bb1 Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Sat, 7 Dec 2024 13:13:19 -0500 Subject: separate SkRateLimiterService from RateLimiterService and update all usages --- packages/backend/src/misc/rate-limit-utils.ts | 21 +++++++++ packages/backend/src/server/FileServerService.ts | 45 ++++++++---------- packages/backend/src/server/ServerModule.ts | 7 ++- packages/backend/src/server/api/ApiCallService.ts | 55 ++++------------------ .../backend/src/server/api/RateLimiterService.ts | 1 + .../backend/src/server/api/SigninApiService.ts | 17 ++++--- .../src/server/api/SigninWithPasskeyApiService.ts | 17 ++++--- .../backend/src/server/api/SkRateLimiterService.ts | 28 ++++------- .../src/server/api/StreamingApiServerService.ts | 20 ++++---- .../test/unit/SigninWithPasskeyApiService.ts | 15 ++++-- 10 files changed, 102 insertions(+), 124 deletions(-) create mode 100644 packages/backend/src/misc/rate-limit-utils.ts (limited to 'packages/backend/src/server/api/ApiCallService.ts') diff --git a/packages/backend/src/misc/rate-limit-utils.ts b/packages/backend/src/misc/rate-limit-utils.ts new file mode 100644 index 0000000000..0f38755502 --- /dev/null +++ b/packages/backend/src/misc/rate-limit-utils.ts @@ -0,0 +1,21 @@ +/* + * SPDX-FileCopyrightText: hazelnoot and other Sharkey contributors + * SPDX-License-Identifier: AGPL-3.0-only + */ + +import { FastifyReply } from 'fastify'; +import { LimitInfo } from '@/server/api/SkRateLimiterService.js'; + +export function sendRateLimitHeaders(reply: FastifyReply, info: LimitInfo): void { + // Number of seconds until the limit has fully reset. + reply.header('X-RateLimit-Clear', info.fullResetSec.toString()); + // Number of calls that can be made before being limited. + reply.header('X-RateLimit-Remaining', info.remaining.toString()); + + if (info.blocked) { + // Number of seconds to wait before trying again. Left for backwards compatibility. + reply.header('Retry-After', info.resetSec.toString()); + // Number of milliseconds to wait before trying again. + reply.header('X-RateLimit-Reset', info.resetMs.toString()); + } +} diff --git a/packages/backend/src/server/FileServerService.ts b/packages/backend/src/server/FileServerService.ts index ca1a6ebe2e..9903113f43 100644 --- a/packages/backend/src/server/FileServerService.ts +++ b/packages/backend/src/server/FileServerService.ts @@ -28,13 +28,12 @@ import { bindThis } from '@/decorators.js'; import { isMimeImage } from '@/misc/is-mime-image.js'; import { correctFilename } from '@/misc/correct-filename.js'; import { handleRequestRedirectToOmitSearch } from '@/misc/fastify-hook-handlers.js'; -import { RateLimiterService } from '@/server/api/RateLimiterService.js'; import { getIpHash } from '@/misc/get-ip-hash.js'; import { AuthenticateService } from '@/server/api/AuthenticateService.js'; -import type { IEndpointMeta } from '@/server/api/endpoints.js'; import { RoleService } from '@/core/RoleService.js'; +import { RateLimit, SkRateLimiterService } from '@/server/api/SkRateLimiterService.js'; +import { sendRateLimitHeaders } from '@/misc/rate-limit-utils.js'; import type { FastifyInstance, FastifyRequest, FastifyReply, FastifyPluginOptions } from 'fastify'; -import type Limiter from 'ratelimiter'; const _filename = fileURLToPath(import.meta.url); const _dirname = dirname(_filename); @@ -59,7 +58,7 @@ export class FileServerService { private internalStorageService: InternalStorageService, private loggerService: LoggerService, private authenticateService: AuthenticateService, - private rateLimiterService: RateLimiterService, + private rateLimiterService: SkRateLimiterService, private roleService: RoleService, ) { this.logger = this.loggerService.getLogger('server', 'gray'); @@ -634,42 +633,37 @@ export class FileServerService { } private async checkResourceLimit(reply: FastifyReply, actor: string, group: string, resource: string, factor = 1): Promise { - const limit = { + const limit: RateLimit = { // Group by resource key: `${group}${resource}`, + type: 'bucket', - // Maximum of 10 requests / 10 minutes - max: 10, - duration: 1000 * 60 * 10, + // Maximum of 10 requests, average rate of 1 per minute + size: 10, + dripRate: 1000 * 60, }; return await this.checkLimit(reply, actor, limit, factor); } private async checkSharedLimit(reply: FastifyReply, actor: string, group: string, factor = 1): Promise { - const limit = { + const limit: RateLimit = { key: group, + type: 'bucket', - // Maximum of 3600 requests per hour, which is an average of 1 per second. - max: 3600, - duration: 1000 * 60 * 60, + // Maximum of 3600 requests, average rate of 1 per second. + size: 3600, }; return await this.checkLimit(reply, actor, limit, factor); } - private async checkLimit(reply: FastifyReply, actor: string, limit: IEndpointMeta['limit'] & { key: NonNullable }, factor = 1): Promise { - try { - await this.rateLimiterService.limit(limit, actor, factor); - return true; - } catch (err) { - // errはLimiter.LimiterInfoであることが期待される - if (hasRateLimitInfo(err)) { - const cooldownInSeconds = Math.ceil((err.info.resetMs - Date.now()) / 1000); - // もしかするとマイナスになる可能性がなくはないのでマイナスだったら0にしておく - reply.header('Retry-After', Math.max(cooldownInSeconds, 0).toString(10)); - } + private async checkLimit(reply: FastifyReply, actor: string, limit: RateLimit, factor = 1): Promise { + const info = await this.rateLimiterService.limit(limit, actor, factor); + + sendRateLimitHeaders(reply, info); + if (info.blocked) { reply.code(429); reply.send({ message: 'Rate limit exceeded. Please try again later.', @@ -679,9 +673,8 @@ export class FileServerService { return false; } + + return true; } } -function hasRateLimitInfo(err: unknown): err is { info: Limiter.LimiterInfo } { - return err != null && typeof(err) === 'object' && 'info' in err; -} diff --git a/packages/backend/src/server/ServerModule.ts b/packages/backend/src/server/ServerModule.ts index 890447a47f..c1d7c088f1 100644 --- a/packages/backend/src/server/ServerModule.ts +++ b/packages/backend/src/server/ServerModule.ts @@ -74,10 +74,9 @@ import { SigninWithPasskeyApiService } from './api/SigninWithPasskeyApiService.j ApiLoggerService, ApiServerService, AuthenticateService, - { - provide: RateLimiterService, - useClass: SkRateLimiterService, - }, + SkRateLimiterService, + // No longer used, but kept for backwards compatibility + RateLimiterService, SigninApiService, SigninWithPasskeyApiService, SigninService, diff --git a/packages/backend/src/server/api/ApiCallService.ts b/packages/backend/src/server/api/ApiCallService.ts index 14367e02bb..38d33c761d 100644 --- a/packages/backend/src/server/api/ApiCallService.ts +++ b/packages/backend/src/server/api/ApiCallService.ts @@ -8,7 +8,6 @@ import * as fs from 'node:fs'; import * as stream from 'node:stream/promises'; import { Inject, Injectable } from '@nestjs/common'; import * as Sentry from '@sentry/node'; -import { LimiterInfo } from 'ratelimiter'; import { DI } from '@/di-symbols.js'; import { getIpHash } from '@/misc/get-ip-hash.js'; import type { MiLocalUser, MiUser } from '@/models/User.js'; @@ -19,9 +18,9 @@ import { createTemp } from '@/misc/create-temp.js'; import { bindThis } from '@/decorators.js'; import { RoleService } from '@/core/RoleService.js'; import type { Config } from '@/config.js'; -import { isLimitInfo } from '@/server/api/SkRateLimiterService.js'; +import { sendRateLimitHeaders } from '@/misc/rate-limit-utils.js'; +import { LegacyRateLimit, SkRateLimiterService } from '@/server/api/SkRateLimiterService.js'; import { ApiError } from './error.js'; -import { RateLimiterService } from './RateLimiterService.js'; import { ApiLoggerService } from './ApiLoggerService.js'; import { AuthenticateService, AuthenticationError } from './AuthenticateService.js'; import type { FastifyRequest, FastifyReply } from 'fastify'; @@ -51,7 +50,7 @@ export class ApiCallService implements OnApplicationShutdown { private userIpsRepository: UserIpsRepository, private authenticateService: AuthenticateService, - private rateLimiterService: RateLimiterService, + private rateLimiterService: SkRateLimiterService, private roleService: RoleService, private apiLoggerService: ApiLoggerService, ) { @@ -67,21 +66,6 @@ export class ApiCallService implements OnApplicationShutdown { let statusCode = err.httpStatusCode; if (err.httpStatusCode === 401) { reply.header('WWW-Authenticate', 'Bearer realm="Misskey"'); - } else if (err.code === 'RATE_LIMIT_EXCEEDED') { - const info: unknown = err.info; - const unixEpochInSeconds = Date.now(); - if (isLimitInfo(info)) { - // Number of seconds to wait before trying again. Left for backwards compatibility. - reply.header('Retry-After', info.resetSec.toString()); - // Number of milliseconds to wait before trying again. - reply.header('X-RateLimit-Reset', info.resetMs.toString()); - } else if (typeof(info) === 'object' && info && 'resetMs' in info && typeof(info.resetMs) === 'number') { - const cooldownInSeconds = Math.ceil((info.resetMs - unixEpochInSeconds) / 1000); - // もしかするとマイナスになる可能性がなくはないのでマイナスだったら0にしておく - reply.header('Retry-After', Math.max(cooldownInSeconds, 0).toString(10)); - } else { - this.logger.warn(`rate limit information has unexpected type: ${JSON.stringify(info)}`); - } } else if (err.kind === 'client') { reply.header('WWW-Authenticate', `Bearer realm="Misskey", error="invalid_request", error_description="${err.message}"`); statusCode = statusCode ?? 400; @@ -347,40 +331,17 @@ export class ApiCallService implements OnApplicationShutdown { if (factor > 0) { // Rate limit - const info = await this.rateLimiterService.limit(limit as IEndpointMeta['limit'] & { key: NonNullable }, limitActor, factor) - .then(info => { - // We always want these headers, because clients need them for pacing. - // Conditional check in case we somehow revert to the old limiter, which does not return info. - if (info) { - // Number of seconds until the limit has fully reset. - reply.header('X-RateLimit-Clear', info.fullResetSec.toString()); - // Number of calls that can be made before being limited. - reply.header('X-RateLimit-Remaining', info.remaining.toString()); - - // Only forward the info object if it's blocked, otherwise we'll reject *all* requests - if (info.blocked) { - return info; - } - } - - return undefined; - }) - .catch(err => { - // The old limiter throws info instead of returning it. - if ('info' in err) { - return err.info as LimiterInfo; - } else { - throw err; - } - }); + const info = await this.rateLimiterService.limit(limit as LegacyRateLimit, limitActor, factor); - if (info) { + sendRateLimitHeaders(reply, info); + + if (info.blocked) { throw new ApiError({ message: 'Rate limit exceeded. Please try again later.', code: 'RATE_LIMIT_EXCEEDED', id: 'd5826d14-3982-4d2e-8011-b9e9f02499ef', httpStatusCode: 429, - }, info); + }); } } } diff --git a/packages/backend/src/server/api/RateLimiterService.ts b/packages/backend/src/server/api/RateLimiterService.ts index 33db016a7c..6037f9bf92 100644 --- a/packages/backend/src/server/api/RateLimiterService.ts +++ b/packages/backend/src/server/api/RateLimiterService.ts @@ -14,6 +14,7 @@ import type { LimitInfo } from '@/server/api/SkRateLimiterService.js'; import { EnvService } from '@/core/EnvService.js'; import type { IEndpointMeta } from './endpoints.js'; +/** @deprecated Use SkRateLimiterService instead */ @Injectable() export class RateLimiterService { protected readonly logger: Logger; diff --git a/packages/backend/src/server/api/SigninApiService.ts b/packages/backend/src/server/api/SigninApiService.ts index 64af7da7a6..1a4ce0a54c 100644 --- a/packages/backend/src/server/api/SigninApiService.ts +++ b/packages/backend/src/server/api/SigninApiService.ts @@ -21,12 +21,13 @@ import { IdService } from '@/core/IdService.js'; import { bindThis } from '@/decorators.js'; import { WebAuthnService } from '@/core/WebAuthnService.js'; import { UserAuthService } from '@/core/UserAuthService.js'; -import { RateLimiterService } from './RateLimiterService.js'; +import { isSystemAccount } from '@/misc/is-system-account.js'; +import type { MiMeta } from '@/models/_.js'; +import { SkRateLimiterService } from '@/server/api/SkRateLimiterService.js'; +import { sendRateLimitHeaders } from '@/misc/rate-limit-utils.js'; import { SigninService } from './SigninService.js'; import type { AuthenticationResponseJSON } from '@simplewebauthn/types'; import type { FastifyReply, FastifyRequest } from 'fastify'; -import { isSystemAccount } from '@/misc/is-system-account.js'; -import type { MiMeta } from '@/models/_.js'; @Injectable() export class SigninApiService { @@ -47,7 +48,7 @@ export class SigninApiService { private signinsRepository: SigninsRepository, private idService: IdService, - private rateLimiterService: RateLimiterService, + private rateLimiterService: SkRateLimiterService, private signinService: SigninService, private userAuthService: UserAuthService, private webAuthnService: WebAuthnService, @@ -79,10 +80,12 @@ export class SigninApiService { return { error }; } - try { // not more than 1 attempt per second and not more than 10 attempts per hour - await this.rateLimiterService.limit({ key: 'signin', duration: 60 * 60 * 1000, max: 10, minInterval: 1000 }, getIpHash(request.ip)); - } catch (err) { + const rateLimit = await this.rateLimiterService.limit({ key: 'signin', duration: 60 * 60 * 1000, max: 10, minInterval: 1000 }, getIpHash(request.ip)); + + sendRateLimitHeaders(reply, rateLimit); + + if (rateLimit.blocked) { reply.code(429); return { error: { diff --git a/packages/backend/src/server/api/SigninWithPasskeyApiService.ts b/packages/backend/src/server/api/SigninWithPasskeyApiService.ts index 9ba23c54e2..ad08dad79c 100644 --- a/packages/backend/src/server/api/SigninWithPasskeyApiService.ts +++ b/packages/backend/src/server/api/SigninWithPasskeyApiService.ts @@ -21,10 +21,11 @@ import { WebAuthnService } from '@/core/WebAuthnService.js'; import Logger from '@/logger.js'; import { LoggerService } from '@/core/LoggerService.js'; import type { IdentifiableError } from '@/misc/identifiable-error.js'; -import { RateLimiterService } from './RateLimiterService.js'; +import { SkRateLimiterService } from '@/server/api/SkRateLimiterService.js'; import { SigninService } from './SigninService.js'; import type { AuthenticationResponseJSON } from '@simplewebauthn/types'; import type { FastifyReply, FastifyRequest } from 'fastify'; +import { sendRateLimitHeaders } from '@/misc/rate-limit-utils.js'; @Injectable() export class SigninWithPasskeyApiService { @@ -43,7 +44,7 @@ export class SigninWithPasskeyApiService { private signinsRepository: SigninsRepository, private idService: IdService, - private rateLimiterService: RateLimiterService, + private rateLimiterService: SkRateLimiterService, private signinService: SigninService, private webAuthnService: WebAuthnService, private loggerService: LoggerService, @@ -84,11 +85,13 @@ export class SigninWithPasskeyApiService { return error(status ?? 500, failure ?? { id: '4e30e80c-e338-45a0-8c8f-44455efa3b76' }); }; - try { - // Not more than 1 API call per 250ms and not more than 100 attempts per 30min - // NOTE: 1 Sign-in require 2 API calls - await this.rateLimiterService.limit({ key: 'signin-with-passkey', duration: 60 * 30 * 1000, max: 200, minInterval: 250 }, getIpHash(request.ip)); - } catch (err) { + // Not more than 1 API call per 250ms and not more than 100 attempts per 30min + // NOTE: 1 Sign-in require 2 API calls + const rateLimit = await this.rateLimiterService.limit({ key: 'signin-with-passkey', duration: 60 * 30 * 1000, max: 200, minInterval: 250 }, getIpHash(request.ip)); + + sendRateLimitHeaders(reply, rateLimit); + + if (rateLimit.blocked) { reply.code(429); return { error: { diff --git a/packages/backend/src/server/api/SkRateLimiterService.ts b/packages/backend/src/server/api/SkRateLimiterService.ts index 763de0029b..943348c41d 100644 --- a/packages/backend/src/server/api/SkRateLimiterService.ts +++ b/packages/backend/src/server/api/SkRateLimiterService.ts @@ -10,7 +10,7 @@ import { LoggerService } from '@/core/LoggerService.js'; import { TimeService } from '@/core/TimeService.js'; import { EnvService } from '@/core/EnvService.js'; import { DI } from '@/di-symbols.js'; -import { RateLimiterService } from './RateLimiterService.js'; +import type Logger from '@/logger.js'; /** * Metadata about the current status of a rate limiter @@ -51,18 +51,6 @@ export interface LimitInfo { fullResetMs: number; } -export function isLimitInfo(info: unknown): info is LimitInfo { - if (info == null) return false; - if (typeof(info) !== 'object') return false; - if (!('blocked' in info) || typeof(info.blocked) !== 'boolean') return false; - if (!('remaining' in info) || typeof(info.remaining) !== 'number') return false; - if (!('resetSec' in info) || typeof(info.resetSec) !== 'number') return false; - if (!('resetMs' in info) || typeof(info.resetMs) !== 'number') return false; - if (!('fullResetSec' in info) || typeof(info.fullResetSec) !== 'number') return false; - if (!('fullResetMs' in info) || typeof(info.fullResetMs) !== 'number') return false; - return true; -} - /** * Rate limit based on "leaky bucket" logic. * The bucket count increases with each call, and decreases gradually at a given rate. @@ -99,10 +87,10 @@ export interface RateLimit { } export type SupportedRateLimit = RateLimit | LegacyRateLimit; -export type LegacyRateLimit = IEndpointMeta['limit'] & { key: NonNullable, type: undefined | 'legacy' }; +export type LegacyRateLimit = IEndpointMeta['limit'] & { key: NonNullable, type?: undefined }; export function isLegacyRateLimit(limit: SupportedRateLimit): limit is LegacyRateLimit { - return limit.type === undefined || limit.type === 'legacy'; + return limit.type === undefined; } export function hasMinLimit(limit: LegacyRateLimit): limit is LegacyRateLimit & { minInterval: number } { @@ -110,13 +98,16 @@ export function hasMinLimit(limit: LegacyRateLimit): limit is LegacyRateLimit & } @Injectable() -export class SkRateLimiterService extends RateLimiterService { +export class SkRateLimiterService { + private readonly logger: Logger; + private readonly disabled: boolean; + constructor( @Inject(TimeService) private readonly timeService: TimeService, @Inject(DI.redis) - redisClient: Redis.Redis, + private readonly redisClient: Redis.Redis, @Inject(LoggerService) loggerService: LoggerService, @@ -124,7 +115,8 @@ export class SkRateLimiterService extends RateLimiterService { @Inject(EnvService) envService: EnvService, ) { - super(redisClient, loggerService, envService); + this.logger = loggerService.getLogger('limiter'); + this.disabled = envService.env.NODE_ENV !== 'production'; } public async limit(limit: SupportedRateLimit, actor: string, factor = 1): Promise { diff --git a/packages/backend/src/server/api/StreamingApiServerService.ts b/packages/backend/src/server/api/StreamingApiServerService.ts index 9b8464f705..e3fd1312ae 100644 --- a/packages/backend/src/server/api/StreamingApiServerService.ts +++ b/packages/backend/src/server/api/StreamingApiServerService.ts @@ -7,6 +7,8 @@ import { EventEmitter } from 'events'; import { Inject, Injectable } from '@nestjs/common'; import * as Redis from 'ioredis'; import * as WebSocket from 'ws'; +import proxyAddr from 'proxy-addr'; +import ms from 'ms'; import { DI } from '@/di-symbols.js'; import type { UsersRepository, MiAccessToken } from '@/models/_.js'; import { NoteReadService } from '@/core/NoteReadService.js'; @@ -16,18 +18,15 @@ import { CacheService } from '@/core/CacheService.js'; import { MiLocalUser } from '@/models/User.js'; import { UserService } from '@/core/UserService.js'; import { ChannelFollowingService } from '@/core/ChannelFollowingService.js'; +import { RoleService } from '@/core/RoleService.js'; +import { getIpHash } from '@/misc/get-ip-hash.js'; +import { LoggerService } from '@/core/LoggerService.js'; +import { SkRateLimiterService } from '@/server/api/SkRateLimiterService.js'; import { AuthenticateService, AuthenticationError } from './AuthenticateService.js'; import MainStreamConnection from './stream/Connection.js'; import { ChannelsService } from './stream/ChannelsService.js'; -import { RateLimiterService } from './RateLimiterService.js'; -import { RoleService } from '@/core/RoleService.js'; -import { getIpHash } from '@/misc/get-ip-hash.js'; -import proxyAddr from 'proxy-addr'; -import ms from 'ms'; import type * as http from 'node:http'; import type { IEndpointMeta } from './endpoints.js'; -import { LoggerService } from '@/core/LoggerService.js'; -import type Logger from '@/logger.js'; @Injectable() export class StreamingApiServerService { @@ -49,7 +48,7 @@ export class StreamingApiServerService { private notificationService: NotificationService, private usersService: UserService, private channelFollowingService: ChannelFollowingService, - private rateLimiterService: RateLimiterService, + private rateLimiterService: SkRateLimiterService, private roleService: RoleService, private loggerService: LoggerService, ) { @@ -73,9 +72,8 @@ export class StreamingApiServerService { if (factor <= 0) return false; // Rate limit - return await this.rateLimiterService.limit(limit, limitActor, factor) - .then(() => { return false; }) - .catch(err => { return true; }); + const rateLimit = await this.rateLimiterService.limit(limit, limitActor, factor); + return rateLimit.blocked; } @bindThis diff --git a/packages/backend/test/unit/SigninWithPasskeyApiService.ts b/packages/backend/test/unit/SigninWithPasskeyApiService.ts index 4d73ba5af1..f0630f2d2b 100644 --- a/packages/backend/test/unit/SigninWithPasskeyApiService.ts +++ b/packages/backend/test/unit/SigninWithPasskeyApiService.ts @@ -17,16 +17,23 @@ import { GlobalModule } from '@/GlobalModule.js'; import { DI } from '@/di-symbols.js'; import { CoreModule } from '@/core/CoreModule.js'; import { SigninWithPasskeyApiService } from '@/server/api/SigninWithPasskeyApiService.js'; -import { RateLimiterService } from '@/server/api/RateLimiterService.js'; import { WebAuthnService } from '@/core/WebAuthnService.js'; import { SigninService } from '@/server/api/SigninService.js'; import { IdentifiableError } from '@/misc/identifiable-error.js'; +import { LimitInfo, SkRateLimiterService } from '@/server/api/SkRateLimiterService.js'; const moduleMocker = new ModuleMocker(global); class FakeLimiter { - public async limit() { - return; + public async limit(): Promise { + return { + blocked: false, + remaining: Number.MAX_SAFE_INTEGER, + resetMs: 0, + resetSec: 0, + fullResetMs: 0, + fullResetSec: 0, + }; } } @@ -90,7 +97,7 @@ describe('SigninWithPasskeyApiService', () => { imports: [GlobalModule, CoreModule], providers: [ SigninWithPasskeyApiService, - { provide: RateLimiterService, useClass: FakeLimiter }, + { provide: SkRateLimiterService, useClass: FakeLimiter }, { provide: SigninService, useClass: FakeSigninService }, ], }).useMocker((token) => { -- cgit v1.2.3-freya From 7c002ce56ef86f8a375275a78c0bda38d540c131 Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Sun, 8 Dec 2024 11:33:57 -0500 Subject: move all Rate Limit type defs to rate-limit-utils.ts --- packages/backend/src/misc/rate-limit-utils.ts | 120 ++++++++++++++++++++- packages/backend/src/server/FileServerService.ts | 4 +- packages/backend/src/server/api/ApiCallService.ts | 19 ++-- .../backend/src/server/api/SkRateLimiterService.ts | 99 ++--------------- packages/backend/src/server/api/endpoints.ts | 26 +---- .../test/unit/SigninWithPasskeyApiService.ts | 3 +- .../unit/server/api/SkRateLimiterServiceTests.ts | 5 +- 7 files changed, 144 insertions(+), 132 deletions(-) (limited to 'packages/backend/src/server/api/ApiCallService.ts') diff --git a/packages/backend/src/misc/rate-limit-utils.ts b/packages/backend/src/misc/rate-limit-utils.ts index 0f38755502..00c0701ad6 100644 --- a/packages/backend/src/misc/rate-limit-utils.ts +++ b/packages/backend/src/misc/rate-limit-utils.ts @@ -4,7 +4,125 @@ */ import { FastifyReply } from 'fastify'; -import { LimitInfo } from '@/server/api/SkRateLimiterService.js'; + +export type RateLimit = BucketRateLimit | LegacyRateLimit; + +/** + * Rate limit based on "leaky bucket" logic. + * The bucket count increases with each call, and decreases gradually at a given rate. + * The subject is blocked until the bucket count drops below the limit. + */ +export interface BucketRateLimit { + /** + * Unique key identifying the particular resource (or resource group) being limited. + */ + key: string; + + /** + * Constant value identifying the type of rate limit. + */ + type: 'bucket'; + + /** + * Size of the bucket, in number of requests. + * The subject will be blocked when the number of calls exceeds this size. + */ + size: number; + + /** + * How often the bucket should "drip" and reduce the counter, measured in milliseconds. + * Defaults to 1000 (1 second). + */ + dripRate?: number; + + /** + * Amount to reduce the counter on each drip. + * Defaults to 1. + */ + dripSize?: number; +} + +/** + * Legacy rate limit based on a "request window" with a maximum number of requests within a given time box. + * These will be translated into a bucket with linear drip rate. + */ +export interface LegacyRateLimit { + /** + * Unique key identifying the particular resource (or resource group) being limited. + */ + key: string; + + /** + * Constant value identifying the type of rate limit. + * Must be excluded or explicitly set to undefined + */ + type?: undefined; + + /** + * Duration of the request window, in milliseconds. + * If present, then "max" must also be included. + */ + duration?: number; + + /** + * Maximum number of requests allowed in the request window. + * If present, then "duration" must also be included. + */ + max?: number; + + /** + * Optional minimum interval between consecutive requests. + * Will apply in addition to the primary rate limit. + */ + minInterval?: number; +} + +/** + * Metadata about the current status of a rate limiter + */ +export interface LimitInfo { + /** + * True if the limit has been reached, and the call should be blocked. + */ + blocked: boolean; + + /** + * Number of calls that can be made before the limit is triggered. + */ + remaining: number; + + /** + * Time in seconds until the next call can be made, or zero if the next call can be made immediately. + * Rounded up to the nearest second. + */ + resetSec: number; + + /** + * Time in milliseconds until the next call can be made, or zero if the next call can be made immediately. + * Rounded up to the nearest milliseconds. + */ + resetMs: number; + + /** + * Time in seconds until the limit has fully reset. + * Rounded up to the nearest second. + */ + fullResetSec: number; + + /** + * Time in milliseconds until the limit has fully reset. + * Rounded up to the nearest millisecond. + */ + fullResetMs: number; +} + +export function isLegacyRateLimit(limit: RateLimit): limit is LegacyRateLimit { + return limit.type === undefined; +} + +export function hasMinLimit(limit: LegacyRateLimit): limit is LegacyRateLimit & { minInterval: number } { + return !!limit.minInterval; +} export function sendRateLimitHeaders(reply: FastifyReply, info: LimitInfo): void { // Number of seconds until the limit has fully reset. diff --git a/packages/backend/src/server/FileServerService.ts b/packages/backend/src/server/FileServerService.ts index 9903113f43..3a03cd8c00 100644 --- a/packages/backend/src/server/FileServerService.ts +++ b/packages/backend/src/server/FileServerService.ts @@ -31,8 +31,8 @@ import { handleRequestRedirectToOmitSearch } from '@/misc/fastify-hook-handlers. import { getIpHash } from '@/misc/get-ip-hash.js'; import { AuthenticateService } from '@/server/api/AuthenticateService.js'; import { RoleService } from '@/core/RoleService.js'; -import { RateLimit, SkRateLimiterService } from '@/server/api/SkRateLimiterService.js'; -import { sendRateLimitHeaders } from '@/misc/rate-limit-utils.js'; +import { SkRateLimiterService } from '@/server/api/SkRateLimiterService.js'; +import { RateLimit, sendRateLimitHeaders } from '@/misc/rate-limit-utils.js'; import type { FastifyInstance, FastifyRequest, FastifyReply, FastifyPluginOptions } from 'fastify'; const _filename = fileURLToPath(import.meta.url); diff --git a/packages/backend/src/server/api/ApiCallService.ts b/packages/backend/src/server/api/ApiCallService.ts index 38d33c761d..6ad4bc8cb5 100644 --- a/packages/backend/src/server/api/ApiCallService.ts +++ b/packages/backend/src/server/api/ApiCallService.ts @@ -18,8 +18,8 @@ import { createTemp } from '@/misc/create-temp.js'; import { bindThis } from '@/decorators.js'; import { RoleService } from '@/core/RoleService.js'; import type { Config } from '@/config.js'; -import { sendRateLimitHeaders } from '@/misc/rate-limit-utils.js'; -import { LegacyRateLimit, SkRateLimiterService } from '@/server/api/SkRateLimiterService.js'; +import { RateLimit, sendRateLimitHeaders } from '@/misc/rate-limit-utils.js'; +import { SkRateLimiterService } from '@/server/api/SkRateLimiterService.js'; import { ApiError } from './error.js'; import { ApiLoggerService } from './ApiLoggerService.js'; import { AuthenticateService, AuthenticationError } from './AuthenticateService.js'; @@ -304,7 +304,7 @@ export class ApiCallService implements OnApplicationShutdown { } // For endpoints without a limit, the default is 10 calls per second - const endpointLimit: IEndpointMeta['limit'] = ep.meta.limit ?? { + const endpointLimit = ep.meta.limit ?? { duration: 1000, max: 10, }; @@ -320,18 +320,17 @@ export class ApiCallService implements OnApplicationShutdown { limitActor = getIpHash(request.ip); } - const limit = Object.assign({}, endpointLimit); - - if (limit.key == null) { - (limit as any).key = ep.name; - } - // TODO: 毎リクエスト計算するのもあれだしキャッシュしたい const factor = user ? (await this.roleService.getUserPolicies(user.id)).rateLimitFactor : 1; if (factor > 0) { + const limit = { + key: ep.name, + ...endpointLimit, + } as RateLimit; + // Rate limit - const info = await this.rateLimiterService.limit(limit as LegacyRateLimit, limitActor, factor); + const info = await this.rateLimiterService.limit(limit, limitActor, factor); sendRateLimitHeaders(reply, info); diff --git a/packages/backend/src/server/api/SkRateLimiterService.ts b/packages/backend/src/server/api/SkRateLimiterService.ts index b3c09d01c2..027d05310b 100644 --- a/packages/backend/src/server/api/SkRateLimiterService.ts +++ b/packages/backend/src/server/api/SkRateLimiterService.ts @@ -5,97 +5,12 @@ import { Inject, Injectable } from '@nestjs/common'; import Redis from 'ioredis'; -import type { IEndpointMeta } from '@/server/api/endpoints.js'; import { LoggerService } from '@/core/LoggerService.js'; import { TimeService } from '@/core/TimeService.js'; import { EnvService } from '@/core/EnvService.js'; import { DI } from '@/di-symbols.js'; import type Logger from '@/logger.js'; - -/** - * Metadata about the current status of a rate limiter - */ -export interface LimitInfo { - /** - * True if the limit has been reached, and the call should be blocked. - */ - blocked: boolean; - - /** - * Number of calls that can be made before the limit is triggered. - */ - remaining: number; - - /** - * Time in seconds until the next call can be made, or zero if the next call can be made immediately. - * Rounded up to the nearest second. - */ - resetSec: number; - - /** - * Time in milliseconds until the next call can be made, or zero if the next call can be made immediately. - * Rounded up to the nearest milliseconds. - */ - resetMs: number; - - /** - * Time in seconds until the limit has fully reset. - * Rounded up to the nearest second. - */ - fullResetSec: number; - - /** - * Time in milliseconds until the limit has fully reset. - * Rounded up to the nearest millisecond. - */ - fullResetMs: number; -} - -/** - * Rate limit based on "leaky bucket" logic. - * The bucket count increases with each call, and decreases gradually at a given rate. - * The subject is blocked until the bucket count drops below the limit. - */ -export interface RateLimit { - /** - * Unique key identifying the particular resource (or resource group) being limited. - */ - key: string; - - /** - * Constant value identifying the type of rate limit. - */ - type: 'bucket'; - - /** - * Size of the bucket, in number of requests. - * The subject will be blocked when the number of calls exceeds this size. - */ - size: number; - - /** - * How often the bucket should "drip" and reduce the counter, measured in milliseconds. - * Defaults to 1000 (1 second). - */ - dripRate?: number; - - /** - * Amount to reduce the counter on each drip. - * Defaults to 1. - */ - dripSize?: number; -} - -export type SupportedRateLimit = RateLimit | LegacyRateLimit; -export type LegacyRateLimit = IEndpointMeta['limit'] & { key: NonNullable, type?: undefined }; - -export function isLegacyRateLimit(limit: SupportedRateLimit): limit is LegacyRateLimit { - return limit.type === undefined; -} - -export function hasMinLimit(limit: LegacyRateLimit): limit is LegacyRateLimit & { minInterval: number } { - return !!limit.minInterval; -} +import { BucketRateLimit, LegacyRateLimit, LimitInfo, RateLimit, hasMinLimit, isLegacyRateLimit } from '@/misc/rate-limit-utils.js'; @Injectable() export class SkRateLimiterService { @@ -116,10 +31,10 @@ export class SkRateLimiterService { envService: EnvService, ) { this.logger = loggerService.getLogger('limiter'); - this.disabled = envService.env.NODE_ENV !== 'production'; + this.disabled = envService.env.NODE_ENV !== 'production'; // TODO disable in TEST *only* } - public async limit(limit: SupportedRateLimit, actor: string, factor = 1): Promise { + public async limit(limit: RateLimit, actor: string, factor = 1): Promise { if (this.disabled) { return { blocked: false, @@ -211,7 +126,7 @@ export class SkRateLimiterService { return limitInfo; } - private async limitBucket(limit: RateLimit, actor: string, factor: number): Promise { + private async limitBucket(limit: BucketRateLimit, actor: string, factor: number): Promise { if (limit.size < 1) throw new Error(`Invalid rate limit ${limit.key}: size is less than 1 (${limit.size})`); if (limit.dripRate != null && limit.dripRate < 1) throw new Error(`Invalid rate limit ${limit.key}: dripRate is less than 1 (${limit.dripRate})`); if (limit.dripSize != null && limit.dripSize < 1) throw new Error(`Invalid rate limit ${limit.key}: dripSize is less than 1 (${limit.dripSize})`); @@ -251,7 +166,7 @@ export class SkRateLimiterService { return limitInfo; } - private async getLimitCounter(limit: SupportedRateLimit, actor: string, subject: string): Promise { + private async getLimitCounter(limit: RateLimit, actor: string, subject: string): Promise { const key = createLimitKey(limit, actor, subject); const value = await this.redisClient.get(key); @@ -262,7 +177,7 @@ export class SkRateLimiterService { return JSON.parse(value); } - private async setLimitCounter(limit: SupportedRateLimit, actor: string, counter: LimitCounter, expiration: number, subject: string): Promise { + private async setLimitCounter(limit: RateLimit, actor: string, counter: LimitCounter, expiration: number, subject: string): Promise { const key = createLimitKey(limit, actor, subject); const value = JSON.stringify(counter); const expirationSec = Math.max(expiration, 1); @@ -270,7 +185,7 @@ export class SkRateLimiterService { } } -function createLimitKey(limit: SupportedRateLimit, actor: string, subject: string): string { +function createLimitKey(limit: RateLimit, actor: string, subject: string): string { return `rl_${actor}_${limit.key}_${subject}`; } diff --git a/packages/backend/src/server/api/endpoints.ts b/packages/backend/src/server/api/endpoints.ts index 14e002929a..a4193a64ec 100644 --- a/packages/backend/src/server/api/endpoints.ts +++ b/packages/backend/src/server/api/endpoints.ts @@ -5,6 +5,7 @@ import { permissions } from 'misskey-js'; import type { KeyOf, Schema } from '@/misc/json-schema.js'; +import type { RateLimit } from '@/misc/rate-limit-utils.js'; import * as ep___admin_abuseReport_notificationRecipient_list from '@/server/api/endpoints/admin/abuse-report/notification-recipient/list.js'; @@ -855,30 +856,7 @@ interface IEndpointMetaBase { * エンドポイントのリミテーションに関するやつ * 省略した場合はリミテーションは無いものとして解釈されます。 */ - readonly limit?: { - - /** - * 複数のエンドポイントでリミットを共有したい場合に指定するキー - */ - readonly key?: string; - - /** - * リミットを適用する期間(ms) - * このプロパティを設定する場合、max プロパティも設定する必要があります。 - */ - readonly duration?: number; - - /** - * durationで指定した期間内にいくつまでリクエストできるのか - * このプロパティを設定する場合、duration プロパティも設定する必要があります。 - */ - readonly max?: number; - - /** - * 最低でもどれくらいの間隔を開けてリクエストしなければならないか(ms) - */ - readonly minInterval?: number; - }; + readonly limit?: Readonly>; /** * ファイルの添付を必要とするか否か diff --git a/packages/backend/test/unit/SigninWithPasskeyApiService.ts b/packages/backend/test/unit/SigninWithPasskeyApiService.ts index f0630f2d2b..7df991c15c 100644 --- a/packages/backend/test/unit/SigninWithPasskeyApiService.ts +++ b/packages/backend/test/unit/SigninWithPasskeyApiService.ts @@ -20,7 +20,8 @@ import { SigninWithPasskeyApiService } from '@/server/api/SigninWithPasskeyApiSe import { WebAuthnService } from '@/core/WebAuthnService.js'; import { SigninService } from '@/server/api/SigninService.js'; import { IdentifiableError } from '@/misc/identifiable-error.js'; -import { LimitInfo, SkRateLimiterService } from '@/server/api/SkRateLimiterService.js'; +import { SkRateLimiterService } from '@/server/api/SkRateLimiterService.js'; +import { LimitInfo } from '@/misc/rate-limit-utils.js'; const moduleMocker = new ModuleMocker(global); diff --git a/packages/backend/test/unit/server/api/SkRateLimiterServiceTests.ts b/packages/backend/test/unit/server/api/SkRateLimiterServiceTests.ts index 2297c2bc03..215b83b53f 100644 --- a/packages/backend/test/unit/server/api/SkRateLimiterServiceTests.ts +++ b/packages/backend/test/unit/server/api/SkRateLimiterServiceTests.ts @@ -6,8 +6,9 @@ import { KEYWORD } from 'color-convert/conversions.js'; import { jest } from '@jest/globals'; import type Redis from 'ioredis'; -import { LegacyRateLimit, LimitCounter, RateLimit, SkRateLimiterService } from '@/server/api/SkRateLimiterService.js'; +import { LimitCounter, SkRateLimiterService } from '@/server/api/SkRateLimiterService.js'; import { LoggerService } from '@/core/LoggerService.js'; +import { BucketRateLimit, LegacyRateLimit } from '@/misc/rate-limit-utils.js'; /* eslint-disable @typescript-eslint/no-non-null-assertion */ /* eslint-disable @typescript-eslint/no-unnecessary-condition */ @@ -141,7 +142,7 @@ describe(SkRateLimiterService, () => { }); describe('with bucket limit', () => { - let limit: RateLimit = null!; + let limit: BucketRateLimit = null!; beforeEach(() => { limit = { -- cgit v1.2.3-freya From 2946f85592022f5aaa490d3f40d6e3068957d33f Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Sun, 8 Dec 2024 13:07:55 -0500 Subject: fix type errors from new rate limit definitions --- packages/backend/src/misc/rate-limit-utils.ts | 5 +++-- packages/backend/src/server/FileServerService.ts | 8 ++++---- packages/backend/src/server/api/ApiCallService.ts | 4 ++-- packages/backend/src/server/api/RateLimiterService.ts | 3 ++- .../backend/src/server/api/SkRateLimiterService.ts | 16 ++++++++-------- packages/backend/src/server/api/endpoints.ts | 2 +- .../test/unit/server/api/SkRateLimiterServiceTests.ts | 18 +++++------------- 7 files changed, 25 insertions(+), 31 deletions(-) (limited to 'packages/backend/src/server/api/ApiCallService.ts') diff --git a/packages/backend/src/misc/rate-limit-utils.ts b/packages/backend/src/misc/rate-limit-utils.ts index 6336f29cb7..9909bb97fa 100644 --- a/packages/backend/src/misc/rate-limit-utils.ts +++ b/packages/backend/src/misc/rate-limit-utils.ts @@ -6,6 +6,7 @@ import { FastifyReply } from 'fastify'; export type RateLimit = BucketRateLimit | LegacyRateLimit; +export type Keyed = T & { key: string }; /** * Rate limit based on "leaky bucket" logic. @@ -16,7 +17,7 @@ export interface BucketRateLimit { /** * Unique key identifying the particular resource (or resource group) being limited. */ - key: string; + key?: string; /** * Constant value identifying the type of rate limit. @@ -50,7 +51,7 @@ export interface LegacyRateLimit { /** * Unique key identifying the particular resource (or resource group) being limited. */ - key: string; + key?: string; /** * Constant value identifying the type of rate limit. diff --git a/packages/backend/src/server/FileServerService.ts b/packages/backend/src/server/FileServerService.ts index 3a03cd8c00..5293d529ad 100644 --- a/packages/backend/src/server/FileServerService.ts +++ b/packages/backend/src/server/FileServerService.ts @@ -32,7 +32,7 @@ import { getIpHash } from '@/misc/get-ip-hash.js'; import { AuthenticateService } from '@/server/api/AuthenticateService.js'; import { RoleService } from '@/core/RoleService.js'; import { SkRateLimiterService } from '@/server/api/SkRateLimiterService.js'; -import { RateLimit, sendRateLimitHeaders } from '@/misc/rate-limit-utils.js'; +import { Keyed, RateLimit, sendRateLimitHeaders } from '@/misc/rate-limit-utils.js'; import type { FastifyInstance, FastifyRequest, FastifyReply, FastifyPluginOptions } from 'fastify'; const _filename = fileURLToPath(import.meta.url); @@ -633,7 +633,7 @@ export class FileServerService { } private async checkResourceLimit(reply: FastifyReply, actor: string, group: string, resource: string, factor = 1): Promise { - const limit: RateLimit = { + const limit: Keyed = { // Group by resource key: `${group}${resource}`, type: 'bucket', @@ -647,7 +647,7 @@ export class FileServerService { } private async checkSharedLimit(reply: FastifyReply, actor: string, group: string, factor = 1): Promise { - const limit: RateLimit = { + const limit: Keyed = { key: group, type: 'bucket', @@ -658,7 +658,7 @@ export class FileServerService { return await this.checkLimit(reply, actor, limit, factor); } - private async checkLimit(reply: FastifyReply, actor: string, limit: RateLimit, factor = 1): Promise { + private async checkLimit(reply: FastifyReply, actor: string, limit: Keyed, factor = 1): Promise { const info = await this.rateLimiterService.limit(limit, actor, factor); sendRateLimitHeaders(reply, info); diff --git a/packages/backend/src/server/api/ApiCallService.ts b/packages/backend/src/server/api/ApiCallService.ts index 6ad4bc8cb5..c6c33f7303 100644 --- a/packages/backend/src/server/api/ApiCallService.ts +++ b/packages/backend/src/server/api/ApiCallService.ts @@ -18,7 +18,7 @@ import { createTemp } from '@/misc/create-temp.js'; import { bindThis } from '@/decorators.js'; import { RoleService } from '@/core/RoleService.js'; import type { Config } from '@/config.js'; -import { RateLimit, sendRateLimitHeaders } from '@/misc/rate-limit-utils.js'; +import { sendRateLimitHeaders } from '@/misc/rate-limit-utils.js'; import { SkRateLimiterService } from '@/server/api/SkRateLimiterService.js'; import { ApiError } from './error.js'; import { ApiLoggerService } from './ApiLoggerService.js'; @@ -327,7 +327,7 @@ export class ApiCallService implements OnApplicationShutdown { const limit = { key: ep.name, ...endpointLimit, - } as RateLimit; + }; // Rate limit const info = await this.rateLimiterService.limit(limit, limitActor, factor); diff --git a/packages/backend/src/server/api/RateLimiterService.ts b/packages/backend/src/server/api/RateLimiterService.ts index 305dcc1d6d..879529090f 100644 --- a/packages/backend/src/server/api/RateLimiterService.ts +++ b/packages/backend/src/server/api/RateLimiterService.ts @@ -10,6 +10,7 @@ import { DI } from '@/di-symbols.js'; import type Logger from '@/logger.js'; import { LoggerService } from '@/core/LoggerService.js'; import { bindThis } from '@/decorators.js'; +import { LegacyRateLimit } from '@/misc/rate-limit-utils.js'; import type { IEndpointMeta } from './endpoints.js'; /** @deprecated Use SkRateLimiterService instead */ @@ -32,7 +33,7 @@ export class RateLimiterService { } @bindThis - public limit(limitation: IEndpointMeta['limit'] & { key: NonNullable }, actor: string, factor = 1) { + public limit(limitation: LegacyRateLimit & { key: NonNullable }, actor: string, factor = 1) { return new Promise((ok, reject) => { if (this.disabled) ok(); diff --git a/packages/backend/src/server/api/SkRateLimiterService.ts b/packages/backend/src/server/api/SkRateLimiterService.ts index 1aee8aa799..6415ee905c 100644 --- a/packages/backend/src/server/api/SkRateLimiterService.ts +++ b/packages/backend/src/server/api/SkRateLimiterService.ts @@ -10,7 +10,7 @@ import { TimeService } from '@/core/TimeService.js'; import { EnvService } from '@/core/EnvService.js'; import { DI } from '@/di-symbols.js'; import type Logger from '@/logger.js'; -import { BucketRateLimit, LegacyRateLimit, LimitInfo, RateLimit, hasMinLimit, isLegacyRateLimit } from '@/misc/rate-limit-utils.js'; +import { BucketRateLimit, LegacyRateLimit, LimitInfo, RateLimit, hasMinLimit, isLegacyRateLimit, Keyed } from '@/misc/rate-limit-utils.js'; @Injectable() export class SkRateLimiterService { @@ -34,7 +34,7 @@ export class SkRateLimiterService { this.disabled = envService.env.NODE_ENV !== 'production'; // TODO disable in TEST *only* } - public async limit(limit: RateLimit, actor: string, factor = 1): Promise { + public async limit(limit: Keyed, actor: string, factor = 1): Promise { if (this.disabled || factor === 0) { return { blocked: false, @@ -57,7 +57,7 @@ export class SkRateLimiterService { } } - private async limitLegacy(limit: LegacyRateLimit, actor: string, factor: number): Promise { + private async limitLegacy(limit: Keyed, actor: string, factor: number): Promise { const promises: Promise[] = []; // The "min" limit - if present - is handled directly. @@ -90,7 +90,7 @@ export class SkRateLimiterService { }; } - private async limitMin(limit: LegacyRateLimit & { minInterval: number }, actor: string, factor: number): Promise { + private async limitMin(limit: Keyed & { minInterval: number }, actor: string, factor: number): Promise { if (limit.minInterval === 0) return null; if (limit.minInterval < 0) throw new Error(`Invalid rate limit ${limit.key}: minInterval is negative (${limit.minInterval})`); @@ -126,7 +126,7 @@ export class SkRateLimiterService { return limitInfo; } - private async limitBucket(limit: BucketRateLimit, actor: string, factor: number): Promise { + private async limitBucket(limit: Keyed, actor: string, factor: number): Promise { if (limit.size < 1) throw new Error(`Invalid rate limit ${limit.key}: size is less than 1 (${limit.size})`); if (limit.dripRate != null && limit.dripRate < 1) throw new Error(`Invalid rate limit ${limit.key}: dripRate is less than 1 (${limit.dripRate})`); if (limit.dripSize != null && limit.dripSize < 1) throw new Error(`Invalid rate limit ${limit.key}: dripSize is less than 1 (${limit.dripSize})`); @@ -166,7 +166,7 @@ export class SkRateLimiterService { return limitInfo; } - private async getLimitCounter(limit: RateLimit, actor: string, subject: string): Promise { + private async getLimitCounter(limit: Keyed, actor: string, subject: string): Promise { const key = createLimitKey(limit, actor, subject); const value = await this.redisClient.get(key); @@ -177,7 +177,7 @@ export class SkRateLimiterService { return JSON.parse(value); } - private async setLimitCounter(limit: RateLimit, actor: string, counter: LimitCounter, expiration: number, subject: string): Promise { + private async setLimitCounter(limit: Keyed, actor: string, counter: LimitCounter, expiration: number, subject: string): Promise { const key = createLimitKey(limit, actor, subject); const value = JSON.stringify(counter); const expirationSec = Math.max(expiration, 1); @@ -185,7 +185,7 @@ export class SkRateLimiterService { } } -function createLimitKey(limit: RateLimit, actor: string, subject: string): string { +function createLimitKey(limit: Keyed, actor: string, subject: string): string { return `rl_${actor}_${limit.key}_${subject}`; } diff --git a/packages/backend/src/server/api/endpoints.ts b/packages/backend/src/server/api/endpoints.ts index a4193a64ec..7eb18fbfe2 100644 --- a/packages/backend/src/server/api/endpoints.ts +++ b/packages/backend/src/server/api/endpoints.ts @@ -856,7 +856,7 @@ interface IEndpointMetaBase { * エンドポイントのリミテーションに関するやつ * 省略した場合はリミテーションは無いものとして解釈されます。 */ - readonly limit?: Readonly>; + readonly limit?: Readonly; /** * ファイルの添付を必要とするか否か diff --git a/packages/backend/test/unit/server/api/SkRateLimiterServiceTests.ts b/packages/backend/test/unit/server/api/SkRateLimiterServiceTests.ts index 910a3e5582..dbf7795fc6 100644 --- a/packages/backend/test/unit/server/api/SkRateLimiterServiceTests.ts +++ b/packages/backend/test/unit/server/api/SkRateLimiterServiceTests.ts @@ -8,7 +8,7 @@ import { jest } from '@jest/globals'; import type Redis from 'ioredis'; import { LimitCounter, SkRateLimiterService } from '@/server/api/SkRateLimiterService.js'; import { LoggerService } from '@/core/LoggerService.js'; -import { BucketRateLimit, LegacyRateLimit } from '@/misc/rate-limit-utils.js'; +import { BucketRateLimit, Keyed, LegacyRateLimit } from '@/misc/rate-limit-utils.js'; /* eslint-disable @typescript-eslint/no-non-null-assertion */ /* eslint-disable @typescript-eslint/no-unnecessary-condition */ @@ -142,7 +142,7 @@ describe(SkRateLimiterService, () => { }); describe('with bucket limit', () => { - let limit: BucketRateLimit = null!; + let limit: Keyed = null!; beforeEach(() => { limit = { @@ -387,7 +387,7 @@ describe(SkRateLimiterService, () => { }); describe('with min interval', () => { - let limit: MutableLegacyRateLimit = null!; + let limit: Keyed = null!; beforeEach(() => { limit = { @@ -570,7 +570,7 @@ describe(SkRateLimiterService, () => { }); describe('with legacy limit', () => { - let limit: MutableLegacyRateLimit = null!; + let limit: Keyed = null!; beforeEach(() => { limit = { @@ -726,7 +726,7 @@ describe(SkRateLimiterService, () => { }); describe('with legacy limit and min interval', () => { - let limit: MutableLegacyRateLimit = null!; + let limit: Keyed = null!; beforeEach(() => { limit = { @@ -855,11 +855,3 @@ describe(SkRateLimiterService, () => { }); }); }); - -// The same thing, but mutable -interface MutableLegacyRateLimit extends LegacyRateLimit { - key: string; - duration?: number; - max?: number; - minInterval?: number; -} -- cgit v1.2.3-freya From 0c1dd73341bdbdb05dfea0b6215fa4e0c3cd7a8b Mon Sep 17 00:00:00 2001 From: dakkar Date: Fri, 13 Dec 2024 15:56:07 +0000 Subject: on 429, retry `fetchAccount` instead of failing when switching between accounts, with many tabs open (10 seem to be enough), they all hit the endpoint at the same time, and some get rate limited. treating a 429 as a fatal error confuses the frontend, which ends up logging the user out of all their accounts. this code makes the frontend retry, after waiting the appropriate amount of time. seems to work fine in my testing. --- packages/backend/src/server/api/ApiCallService.ts | 1 + packages/frontend/src/account.ts | 7 +++++++ 2 files changed, 8 insertions(+) (limited to 'packages/backend/src/server/api/ApiCallService.ts') diff --git a/packages/backend/src/server/api/ApiCallService.ts b/packages/backend/src/server/api/ApiCallService.ts index c6c33f7303..974be7e4e1 100644 --- a/packages/backend/src/server/api/ApiCallService.ts +++ b/packages/backend/src/server/api/ApiCallService.ts @@ -340,6 +340,7 @@ export class ApiCallService implements OnApplicationShutdown { code: 'RATE_LIMIT_EXCEEDED', id: 'd5826d14-3982-4d2e-8011-b9e9f02499ef', httpStatusCode: 429, + info, }); } } diff --git a/packages/frontend/src/account.ts b/packages/frontend/src/account.ts index e3416f2c29..f0a464084f 100644 --- a/packages/frontend/src/account.ts +++ b/packages/frontend/src/account.ts @@ -147,6 +147,13 @@ function fetchAccount(token: string, id?: string, forceShowDialog?: boolean): Pr text: i18n.ts.tokenRevokedDescription, }); } + } else if (res.error.id === 'd5826d14-3982-4d2e-8011-b9e9f02499ef') { + // rate limited + const timeToWait = res.error.info?.resetMs ?? 1000; + window.setTimeout(timeToWait, () => { + fetchAccount(token, id, forceShowDialog).then(done, fail); + }); + return; } else { await alert({ type: 'error', -- cgit v1.2.3-freya From 9b1fc969086fb0c3ea88c979f254e964f9218dc5 Mon Sep 17 00:00:00 2001 From: dakkar Date: Fri, 13 Dec 2024 16:17:43 +0000 Subject: fix passing rate limiting info via ApiError --- packages/backend/src/server/api/ApiCallService.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'packages/backend/src/server/api/ApiCallService.ts') diff --git a/packages/backend/src/server/api/ApiCallService.ts b/packages/backend/src/server/api/ApiCallService.ts index 974be7e4e1..03f25a51fe 100644 --- a/packages/backend/src/server/api/ApiCallService.ts +++ b/packages/backend/src/server/api/ApiCallService.ts @@ -340,8 +340,7 @@ export class ApiCallService implements OnApplicationShutdown { code: 'RATE_LIMIT_EXCEEDED', id: 'd5826d14-3982-4d2e-8011-b9e9f02499ef', httpStatusCode: 429, - info, - }); + }, info); } } } -- cgit v1.2.3-freya