Merge branch 'develop' into merge/2024-02-03

# Conflicts:
#	packages/backend/src/server/ActivityPubServerService.ts
#	pnpm-lock.yaml

12 files changed, 101 insertions, 70 deletions

diff --git a/packages/backend/src/core/CoreModule.ts b/packages/backend/src/core/CoreModule.ts
index 141f905d7f..8c9f419c44 100644
--- a/packages/backend/src/core/CoreModule.ts
+++ b/packages/backend/src/core/CoreModule.ts
@@ -232,6 +232,8 @@ const $FanoutTimelineEndpointService: Provider = { provide: 'FanoutTimelineEndpo
 const $ChannelFollowingService: Provider = { provide: 'ChannelFollowingService', useExisting: ChannelFollowingService };
 const $RegistryApiService: Provider = { provide: 'RegistryApiService', useExisting: RegistryApiService };
 const $ReversiService: Provider = { provide: 'ReversiService', useExisting: ReversiService };
+const $TimeService: Provider = { provide: 'TimeService', useExisting: TimeService };
+const $EnvService: Provider = { provide: 'EnvService', useExisting: EnvService };
 
 const $ChartLoggerService: Provider = { provide: 'ChartLoggerService', useExisting: ChartLoggerService };
 const $FederationChart: Provider = { provide: 'FederationChart', useExisting: FederationChart };
@@ -538,6 +540,8 @@ const $SponsorsService: Provider = { provide: 'SponsorsService', useExisting: Sp
 		$ChannelFollowingService,
 		$RegistryApiService,
 		$ReversiService,
+		$TimeService,
+		$EnvService,
 
 		$ChartLoggerService,
 		$FederationChart,
@@ -839,6 +843,8 @@ const $SponsorsService: Provider = { provide: 'SponsorsService', useExisting: Sp
 		$ChannelFollowingService,
 		$RegistryApiService,
 		$ReversiService,
+		$TimeService,
+		$EnvService,
 
 		$FederationChart,
 		$NotesChart,
diff --git a/packages/backend/src/core/UserListService.ts b/packages/backend/src/core/UserListService.ts
index 6333356fe9..4f4d59a02c 100644
--- a/packages/backend/src/core/UserListService.ts
+++ b/packages/backend/src/core/UserListService.ts
@@ -58,7 +58,7 @@ export class UserListService implements OnApplicationShutdown, OnModuleInit {
 	}
 
 	async onModuleInit() {
-		this.roleService = this.moduleRef.get(RoleService.name);
+		this.roleService = this.moduleRef.get('RoleService');
 	}
 
 	@bindThis
diff --git a/packages/backend/src/misc/gen-identicon.ts b/packages/backend/src/misc/gen-identicon.ts
index 342e0f8602..f3c08cc76e 100644
--- a/packages/backend/src/misc/gen-identicon.ts
+++ b/packages/backend/src/misc/gen-identicon.ts
@@ -8,7 +8,7 @@
  * https://en.wikipedia.org/wiki/Identicon
  */
 
-import { createCanvas } from '@napi-rs/canvas';
+import { createCanvas } from 'canvas';
 import gen from 'random-seed';
 
 const size = 128; // px
@@ -100,5 +100,5 @@ export async function genIdenticon(seed: string): Promise<Buffer> {
 		}
 	}
 
-	return await canvas.encode('png');
+	return await canvas.toBuffer('image/png');
 }
diff --git a/packages/backend/src/server/ActivityPubServerService.ts b/packages/backend/src/server/ActivityPubServerService.ts
index 1f838ffdbe..72faa3318c 100644
--- a/packages/backend/src/server/ActivityPubServerService.ts
+++ b/packages/backend/src/server/ActivityPubServerService.ts
@@ -30,12 +30,12 @@ import type { MiNote } from '@/models/Note.js';
 import { QueryService } from '@/core/QueryService.js';
 import { UtilityService } from '@/core/UtilityService.js';
 import { UserEntityService } from '@/core/entities/UserEntityService.js';
+import type Logger from '@/logger.js';
+import { LoggerService } from '@/core/LoggerService.js';
 import { bindThis } from '@/decorators.js';
 import { IActivity } from '@/core/activitypub/type.js';
 import { isQuote, isRenote } from '@/misc/is-renote.js';
 import * as Acct from '@/misc/acct.js';
-import type Logger from '@/logger.js';
-import { LoggerService } from '@/core/LoggerService.js';
 import type { FastifyInstance, FastifyRequest, FastifyReply, FastifyPluginOptions, FastifyBodyParser } from 'fastify';
 import type { FindOptionsWhere } from 'typeorm';
 
diff --git a/packages/backend/src/server/FileServerService.ts b/packages/backend/src/server/FileServerService.ts
index 5293d529ad..a7e13a1b78 100644
--- a/packages/backend/src/server/FileServerService.ts
+++ b/packages/backend/src/server/FileServerService.ts
@@ -11,7 +11,7 @@ import rename from 'rename';
 import sharp from 'sharp';
 import { sharpBmp } from '@misskey-dev/sharp-read-bmp';
 import type { Config } from '@/config.js';
-import type { MiDriveFile, DriveFilesRepository } from '@/models/_.js';
+import type { MiDriveFile, DriveFilesRepository, MiUser } from '@/models/_.js';
 import { DI } from '@/di-symbols.js';
 import { createTemp } from '@/misc/create-temp.js';
 import { FILE_TYPE_BROWSERSAFE } from '@/const.js';
@@ -30,8 +30,7 @@ import { correctFilename } from '@/misc/correct-filename.js';
 import { handleRequestRedirectToOmitSearch } from '@/misc/fastify-hook-handlers.js';
 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 { SkRateLimiterService } from '@/server/SkRateLimiterService.js';
 import { Keyed, RateLimit, sendRateLimitHeaders } from '@/misc/rate-limit-utils.js';
 import type { FastifyInstance, FastifyRequest, FastifyReply, FastifyPluginOptions } from 'fastify';
 
@@ -59,7 +58,6 @@ export class FileServerService {
 		private loggerService: LoggerService,
 		private authenticateService: AuthenticateService,
 		private rateLimiterService: SkRateLimiterService,
-		private roleService: RoleService,
 	) {
 		this.logger = this.loggerService.getLogger('server', 'gray');
 
@@ -625,14 +623,13 @@ export class FileServerService {
 
 		// koa will automatically load the `X-Forwarded-For` header if `proxy: true` is configured in the app.
 		const [user] = await this.authenticateService.authenticate(token);
-		const actor = user?.id ?? getIpHash(request.ip);
-		const factor = user ? (await this.roleService.getUserPolicies(user.id)).rateLimitFactor : 1;
+		const actor = user ?? getIpHash(request.ip);
 
 		// Call both limits: the per-resource limit and the shared cross-resource limit
-		return await this.checkResourceLimit(reply, actor, group, resource, factor) && await this.checkSharedLimit(reply, actor, group, factor);
+		return await this.checkResourceLimit(reply, actor, group, resource) && await this.checkSharedLimit(reply, actor, group);
 	}
 
-	private async checkResourceLimit(reply: FastifyReply, actor: string, group: string, resource: string, factor = 1): Promise<boolean> {
+	private async checkResourceLimit(reply: FastifyReply, actor: string | MiUser, group: string, resource: string): Promise<boolean> {
 		const limit: Keyed<RateLimit> = {
 			// Group by resource
 			key: `${group}${resource}`,
@@ -643,10 +640,10 @@ export class FileServerService {
 			dripRate: 1000 * 60,
 		};
 
-		return await this.checkLimit(reply, actor, limit, factor);
+		return await this.checkLimit(reply, actor, limit);
 	}
 
-	private async checkSharedLimit(reply: FastifyReply, actor: string, group: string, factor = 1): Promise<boolean> {
+	private async checkSharedLimit(reply: FastifyReply, actor: string | MiUser, group: string): Promise<boolean> {
 		const limit: Keyed<RateLimit> = {
 			key: group,
 			type: 'bucket',
@@ -655,11 +652,11 @@ export class FileServerService {
 			size: 3600,
 		};
 
-		return await this.checkLimit(reply, actor, limit, factor);
+		return await this.checkLimit(reply, actor, limit);
 	}
 
-	private async checkLimit(reply: FastifyReply, actor: string, limit: Keyed<RateLimit>, factor = 1): Promise<boolean> {
-		const info = await this.rateLimiterService.limit(limit, actor, factor);
+	private async checkLimit(reply: FastifyReply, actor: string | MiUser, limit: Keyed<RateLimit>): Promise<boolean> {
+		const info = await this.rateLimiterService.limit(limit, actor);
 
 		sendRateLimitHeaders(reply, info);
 
diff --git a/packages/backend/src/server/ServerModule.ts b/packages/backend/src/server/ServerModule.ts
index c1d7c088f1..6b5156106a 100644
--- a/packages/backend/src/server/ServerModule.ts
+++ b/packages/backend/src/server/ServerModule.ts
@@ -6,7 +6,7 @@
 import { Module } from '@nestjs/common';
 import { EndpointsModule } from '@/server/api/EndpointsModule.js';
 import { CoreModule } from '@/core/CoreModule.js';
-import { SkRateLimiterService } from '@/server/api/SkRateLimiterService.js';
+import { SkRateLimiterService } from '@/server/SkRateLimiterService.js';
 import { ApiCallService } from './api/ApiCallService.js';
 import { FileServerService } from './FileServerService.js';
 import { HealthServerService } from './HealthServerService.js';
diff --git a/packages/backend/src/server/SkRateLimiterService.md b/packages/backend/src/server/SkRateLimiterService.md
index 762f8dfe14..fb007538fa 100644
--- a/packages/backend/src/server/SkRateLimiterService.md
+++ b/packages/backend/src/server/SkRateLimiterService.md
@@ -12,6 +12,11 @@ SkRateLimiterService is not quite plug-and-play compatible with existing call si
 Instead, the returned LimitInfo object will have `blocked` set to true.
 Callers are responsible for checking this property and taking any desired action, such as rejecting a request or returning limit details.
 
+Rate limit factors are also handled differently.
+Instead of providing an optional parameter for callers, SkRateLimiterServer accepts an `MiUser` parameter that is used to compute the factor directly.
+If a user is not available (such as for unauthenticated callers), then the Role Template factor is used instead.
+To avoid confusion, the `factor` parameter has been removed entirely and is now an implementation detail.
+
 ## Headers
 
 LimitInfo objects (returned by `SkRateLimitService.limit()`) can be passed to `rate-limit-utils.sendRateLimitHeaders()` to send standard rate limit headers with an HTTP response.
diff --git a/packages/backend/src/server/api/SkRateLimiterService.ts b/packages/backend/src/server/SkRateLimiterService.ts
index 38c97b63df..038f12cb25 100644
--- a/packages/backend/src/server/api/SkRateLimiterService.ts
+++ b/packages/backend/src/server/SkRateLimiterService.ts
@@ -5,36 +5,67 @@
 
 import { Inject, Injectable } from '@nestjs/common';
 import Redis from 'ioredis';
-import { TimeService } from '@/core/TimeService.js';
-import { EnvService } from '@/core/EnvService.js';
+import type { TimeService } from '@/core/TimeService.js';
+import type { EnvService } from '@/core/EnvService.js';
 import { BucketRateLimit, LegacyRateLimit, LimitInfo, RateLimit, hasMinLimit, isLegacyRateLimit, Keyed, hasMaxLimit, disabledLimitInfo, MaxLegacyLimit, MinLegacyLimit } from '@/misc/rate-limit-utils.js';
 import { DI } from '@/di-symbols.js';
+import { MemoryKVCache } from '@/misc/cache.js';
+import type { MiUser } from '@/models/_.js';
+import type { RoleService } from '@/core/RoleService.js';
+
+// Sentinel value used for caching the default role template.
+// Required because MemoryKVCache doesn't support null keys.
+const defaultUserKey = '';
 
 @Injectable()
 export class SkRateLimiterService {
+	// 1-minute cache interval
+	private readonly factorCache = new MemoryKVCache<number>(1000 * 60);
 	private readonly disabled: boolean;
 
 	constructor(
-		@Inject(TimeService)
+		@Inject('TimeService')
 		private readonly timeService: TimeService,
 
 		@Inject(DI.redis)
 		private readonly redisClient: Redis.Redis,
 
-		@Inject(EnvService)
+		@Inject('RoleService')
+		private readonly roleService: RoleService,
+
+		@Inject('EnvService')
 		envService: EnvService,
 	) {
 		this.disabled = envService.env.NODE_ENV === 'test';
 	}
 
 	/**
-	 * Check & increment a rate limit
+	 * Check & increment a rate limit for a client.
+	 *
+	 * If the client (actorOrUser) is passed as a string, then it uses the default rate limit factor from the role template.
+	 * If the client (actorOrUser) is passed as an MiUser, then it queries the user's actual rate limit factor from their assigned roles.
+	 *
+	 * A factor of zero (0) will disable the limit, while any negative number will produce an error.
+	 * A factor between zero (0) and one (1) will increase the limit from its default values (allowing more actions per time interval).
+	 * A factor greater than one (1) will decrease the limit from its default values (allowing fewer actions per time interval).
+	 *
 	 * @param limit The limit definition
-	 * @param actor Client who is calling this limit
-	 * @param factor Scaling factor - smaller = larger limit (less restrictive)
+	 * @param actorOrUser authenticated client user or IP hash
 	 */
-	public async limit(limit: Keyed<RateLimit>, actor: string, factor = 1): Promise<LimitInfo> {
-		if (this.disabled || factor === 0) {
+	public async limit(limit: Keyed<RateLimit>, actorOrUser: string | MiUser): Promise<LimitInfo> {
+		if (this.disabled) {
+			return disabledLimitInfo;
+		}
+
+		const actor = typeof(actorOrUser) === 'object' ? actorOrUser.id : actorOrUser;
+		const userCacheKey = typeof(actorOrUser) === 'object' ? actorOrUser.id : defaultUserKey;
+		const userRoleKey = typeof(actorOrUser) === 'object' ? actorOrUser.id : null;
+		const factor = this.factorCache.get(userCacheKey) ?? await this.factorCache.fetch(userCacheKey, async () => {
+			const role = await this.roleService.getUserPolicies(userRoleKey);
+			return role.rateLimitFactor;
+		});
+
+		if (factor === 0) {
 			return disabledLimitInfo;
 		}
 
@@ -42,10 +73,6 @@ export class SkRateLimiterService {
 			throw new Error(`Rate limit factor is zero or negative: ${factor}`);
 		}
 
-		return await this.tryLimit(limit, actor, factor);
-	}
-
-	private async tryLimit(limit: Keyed<RateLimit>, actor: string, factor: number): Promise<LimitInfo> {
 		if (isLegacyRateLimit(limit)) {
 			return await this.limitLegacy(limit, actor, factor);
 		} else {
diff --git a/packages/backend/src/server/api/ApiCallService.ts b/packages/backend/src/server/api/ApiCallService.ts
index 03f25a51fe..5ce358d68f 100644
--- a/packages/backend/src/server/api/ApiCallService.ts
+++ b/packages/backend/src/server/api/ApiCallService.ts
@@ -19,7 +19,7 @@ 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 { SkRateLimiterService } from '@/server/api/SkRateLimiterService.js';
+import { SkRateLimiterService } from '@/server/SkRateLimiterService.js';
 import { ApiError } from './error.js';
 import { ApiLoggerService } from './ApiLoggerService.js';
 import { AuthenticateService, AuthenticationError } from './AuthenticateService.js';
@@ -313,35 +313,30 @@ export class ApiCallService implements OnApplicationShutdown {
 		// eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
 		if (endpointLimit) {
 			// koa will automatically load the `X-Forwarded-For` header if `proxy: true` is configured in the app.
-			let limitActor: string;
+			let limitActor: string | MiLocalUser;
 			if (user) {
-				limitActor = user.id;
+				limitActor = user;
 			} else {
 				limitActor = getIpHash(request.ip);
 			}
 
-			// TODO: 毎リクエスト計算するのもあれだしキャッシュしたい
-			const factor = user ? (await this.roleService.getUserPolicies(user.id)).rateLimitFactor : 1;
+			const limit = {
+				key: ep.name,
+				...endpointLimit,
+			};
 
-			if (factor > 0) {
-				const limit = {
-					key: ep.name,
-					...endpointLimit,
-				};
+			// Rate limit
+			const info = await this.rateLimiterService.limit(limit, limitActor);
 
-				// Rate limit
-				const info = await this.rateLimiterService.limit(limit, limitActor, factor);
+			sendRateLimitHeaders(reply, 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);
-				}
+			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/SigninApiService.ts b/packages/backend/src/server/api/SigninApiService.ts
index fa9155d82d..72712bce60 100644
--- a/packages/backend/src/server/api/SigninApiService.ts
+++ b/packages/backend/src/server/api/SigninApiService.ts
@@ -26,12 +26,19 @@ import { UserAuthService } from '@/core/UserAuthService.js';
 import { CaptchaService } from '@/core/CaptchaService.js';
 import { FastifyReplyError } from '@/misc/fastify-reply-error.js';
 import { isSystemAccount } from '@/misc/is-system-account.js';
-import { SkRateLimiterService } from '@/server/api/SkRateLimiterService.js';
-import { sendRateLimitHeaders } from '@/misc/rate-limit-utils.js';
+import { SkRateLimiterService } from '@/server/SkRateLimiterService.js';
+import { Keyed, RateLimit, sendRateLimitHeaders } from '@/misc/rate-limit-utils.js';
 import { SigninService } from './SigninService.js';
 import type { AuthenticationResponseJSON } from '@simplewebauthn/types';
 import type { FastifyReply, FastifyRequest } from 'fastify';
 
+// Up to 10 attempts, then 1 per minute
+const signinRateLimit: Keyed<RateLimit> = {
+	key: 'signin',
+	max: 10,
+	dripRate: 1000 * 60,
+};
+
 @Injectable()
 export class SigninApiService {
 	constructor(
@@ -94,7 +101,7 @@ export class SigninApiService {
 		}
 
 		// not more than 1 attempt per second and not more than 10 attempts per hour
-		const rateLimit = await this.rateLimiterService.limit({ key: 'signin', duration: 60 * 60 * 1000, max: 10, minInterval: 1000 }, getIpHash(request.ip));
+		const rateLimit = await this.rateLimiterService.limit(signinRateLimit, getIpHash(request.ip));
 
 		sendRateLimitHeaders(reply, rateLimit);
 
diff --git a/packages/backend/src/server/api/SigninWithPasskeyApiService.ts b/packages/backend/src/server/api/SigninWithPasskeyApiService.ts
index e94d2b6b68..f84f50523b 100644
--- a/packages/backend/src/server/api/SigninWithPasskeyApiService.ts
+++ b/packages/backend/src/server/api/SigninWithPasskeyApiService.ts
@@ -21,7 +21,7 @@ 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 { SkRateLimiterService } from '@/server/api/SkRateLimiterService.js';
+import { SkRateLimiterService } from '@/server/SkRateLimiterService.js';
 import { sendRateLimitHeaders } from '@/misc/rate-limit-utils.js';
 import { SigninService } from './SigninService.js';
 import type { AuthenticationResponseJSON } from '@simplewebauthn/types';
diff --git a/packages/backend/src/server/api/StreamingApiServerService.ts b/packages/backend/src/server/api/StreamingApiServerService.ts
index e3fd1312ae..6e7abcfae6 100644
--- a/packages/backend/src/server/api/StreamingApiServerService.ts
+++ b/packages/backend/src/server/api/StreamingApiServerService.ts
@@ -18,10 +18,9 @@ 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 { SkRateLimiterService } from '@/server/SkRateLimiterService.js';
 import { AuthenticateService, AuthenticationError } from './AuthenticateService.js';
 import MainStreamConnection from './stream/Connection.js';
 import { ChannelsService } from './stream/ChannelsService.js';
@@ -49,7 +48,6 @@ export class StreamingApiServerService {
 		private usersService: UserService,
 		private channelFollowingService: ChannelFollowingService,
 		private rateLimiterService: SkRateLimiterService,
-		private roleService: RoleService,
 		private loggerService: LoggerService,
 	) {
 	}
@@ -57,22 +55,18 @@ export class StreamingApiServerService {
 	@bindThis
 	private async rateLimitThis(
 		user: MiLocalUser | null | undefined,
-		requestIp: string | undefined,
+		requestIp: string,
 		limit: IEndpointMeta['limit'] & { key: NonNullable<string> },
 	) : Promise<boolean> {
-		let limitActor: string;
+		let limitActor: string | MiLocalUser;
 		if (user) {
-			limitActor = user.id;
+			limitActor = user;
 		} else {
-			limitActor = getIpHash(requestIp || 'wtf');
+			limitActor = getIpHash(requestIp);
 		}
 
-		const factor = user ? (await this.roleService.getUserPolicies(user.id)).rateLimitFactor : 1;
-
-		if (factor <= 0) return false;
-
 		// Rate limit
-		const rateLimit = await this.rateLimiterService.limit(limit, limitActor, factor);
+		const rateLimit = await this.rateLimiterService.limit(limit, limitActor);
 		return rateLimit.blocked;
 	}