From ef354e94f20ace67b94faa2859c458a436cdd3f7 Mon Sep 17 00:00:00 2001 From: Kagami Sascha Rosylight Date: Sun, 25 Jun 2023 04:04:33 +0200 Subject: [PATCH] refactor(backend): replace rndstr with secureRndstr (#11044) * refactor(backend): replace rndstr with secureRndstr * Update pnpm-lock.yaml * .js --- packages/backend/package.json | 1 - .../src/misc/generate-native-user-token.ts | 2 +- packages/backend/src/misc/secure-rndstr.ts | 5 ++-- .../src/server/api/SignupApiService.ts | 28 ++++++++--------- .../server/api/endpoints/admin/emoji/add.ts | 1 - .../api/endpoints/admin/reset-password.ts | 4 +-- .../src/server/api/endpoints/app/create.ts | 2 +- .../src/server/api/endpoints/auth/accept.ts | 2 +- .../server/api/endpoints/i/update-email.ts | 4 +-- .../src/server/api/endpoints/invite.ts | 7 ++--- .../server/api/endpoints/miauth/gen-token.ts | 2 +- .../api/endpoints/request-reset-password.ts | 6 ++-- packages/backend/test/e2e/move.ts | 14 ++++----- packages/backend/test/unit/RoleService.ts | 30 +++++++++---------- packages/backend/test/unit/activitypub.ts | 10 +++---- pnpm-lock.yaml | 3 -- 16 files changed, 57 insertions(+), 64 deletions(-) diff --git a/packages/backend/package.json b/packages/backend/package.json index b9995d8117..c13f292c76 100644 --- a/packages/backend/package.json +++ b/packages/backend/package.json @@ -133,7 +133,6 @@ "redis-lock": "0.1.4", "reflect-metadata": "0.1.13", "rename": "1.0.4", - "rndstr": "1.0.0", "rss-parser": "3.13.0", "rxjs": "7.8.1", "s-age": "1.1.2", diff --git a/packages/backend/src/misc/generate-native-user-token.ts b/packages/backend/src/misc/generate-native-user-token.ts index 5d8a4c5378..7292d765a8 100644 --- a/packages/backend/src/misc/generate-native-user-token.ts +++ b/packages/backend/src/misc/generate-native-user-token.ts @@ -1,3 +1,3 @@ import { secureRndstr } from '@/misc/secure-rndstr.js'; -export default () => secureRndstr(16, true); +export default () => secureRndstr(16); diff --git a/packages/backend/src/misc/secure-rndstr.ts b/packages/backend/src/misc/secure-rndstr.ts index 8d4fcb1ba9..cde64c8142 100644 --- a/packages/backend/src/misc/secure-rndstr.ts +++ b/packages/backend/src/misc/secure-rndstr.ts @@ -1,10 +1,9 @@ import * as crypto from 'node:crypto'; -const L_CHARS = '0123456789abcdefghijklmnopqrstuvwxyz'; +export const L_CHARS = '0123456789abcdefghijklmnopqrstuvwxyz'; const LU_CHARS = '0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ'; -export function secureRndstr(length = 32, useLU = true): string { - const chars = useLU ? LU_CHARS : L_CHARS; +export function secureRndstr(length = 32, { chars = LU_CHARS } = {}): string { const chars_len = chars.length; let str = ''; diff --git a/packages/backend/src/server/api/SignupApiService.ts b/packages/backend/src/server/api/SignupApiService.ts index b2bd7d82e7..fc5f3811eb 100644 --- a/packages/backend/src/server/api/SignupApiService.ts +++ b/packages/backend/src/server/api/SignupApiService.ts @@ -1,5 +1,4 @@ import { Inject, Injectable } from '@nestjs/common'; -import rndstr from 'rndstr'; import bcrypt from 'bcryptjs'; import { IsNull } from 'typeorm'; import { DI } from '@/di-symbols.js'; @@ -16,6 +15,7 @@ import { FastifyReplyError } from '@/misc/fastify-reply-error.js'; import { bindThis } from '@/decorators.js'; import { SigninService } from './SigninService.js'; import type { FastifyRequest, FastifyReply } from 'fastify'; +import { L_CHARS, secureRndstr } from '@/misc/secure-rndstr.js'; @Injectable() export class SignupApiService { @@ -67,7 +67,7 @@ export class SignupApiService { const body = request.body; const instance = await this.metaService.fetch(true); - + // Verify *Captcha // ただしテスト時はこの機構は障害となるため無効にする if (process.env.NODE_ENV !== 'test') { @@ -76,7 +76,7 @@ export class SignupApiService { throw new FastifyReplyError(400, err); }); } - + if (instance.enableRecaptcha && instance.recaptchaSecretKey) { await this.captchaService.verifyRecaptcha(instance.recaptchaSecretKey, body['g-recaptcha-response']).catch(err => { throw new FastifyReplyError(400, err); @@ -89,44 +89,44 @@ export class SignupApiService { }); } } - + const username = body['username']; const password = body['password']; const host: string | null = process.env.NODE_ENV === 'test' ? (body['host'] ?? null) : null; const invitationCode = body['invitationCode']; const emailAddress = body['emailAddress']; - + if (instance.emailRequiredForSignup) { if (emailAddress == null || typeof emailAddress !== 'string') { reply.code(400); return; } - + const res = await this.emailService.validateEmailForAccount(emailAddress); if (!res.available) { reply.code(400); return; } } - + if (instance.disableRegistration) { if (invitationCode == null || typeof invitationCode !== 'string') { reply.code(400); return; } - + const ticket = await this.registrationTicketsRepository.findOneBy({ code: invitationCode, }); - + if (ticket == null) { reply.code(400); return; } - + this.registrationTicketsRepository.delete(ticket.id); } - + if (instance.emailRequiredForSignup) { if (await this.usersRepository.findOneBy({ usernameLower: username.toLowerCase(), host: IsNull() })) { throw new FastifyReplyError(400, 'DUPLICATED_USERNAME'); @@ -142,7 +142,7 @@ export class SignupApiService { throw new FastifyReplyError(400, 'DENIED_USERNAME'); } - const code = rndstr('a-z0-9', 16); + const code = secureRndstr(16, { chars: L_CHARS }); // Generate hash of password const salt = await bcrypt.genSalt(8); @@ -170,12 +170,12 @@ export class SignupApiService { const { account, secret } = await this.signupService.signup({ username, password, host, }); - + const res = await this.userEntityService.pack(account, account, { detail: true, includeSecrets: true, }); - + return { ...res, token: secret, diff --git a/packages/backend/src/server/api/endpoints/admin/emoji/add.ts b/packages/backend/src/server/api/endpoints/admin/emoji/add.ts index 509224e9c3..2fcf0da3f0 100644 --- a/packages/backend/src/server/api/endpoints/admin/emoji/add.ts +++ b/packages/backend/src/server/api/endpoints/admin/emoji/add.ts @@ -1,5 +1,4 @@ import { Inject, Injectable } from '@nestjs/common'; -import rndstr from 'rndstr'; import { Endpoint } from '@/server/api/endpoint-base.js'; import type { DriveFilesRepository } from '@/models/index.js'; import { DI } from '@/di-symbols.js'; diff --git a/packages/backend/src/server/api/endpoints/admin/reset-password.ts b/packages/backend/src/server/api/endpoints/admin/reset-password.ts index d263f99f6e..e9c3b0e69f 100644 --- a/packages/backend/src/server/api/endpoints/admin/reset-password.ts +++ b/packages/backend/src/server/api/endpoints/admin/reset-password.ts @@ -1,9 +1,9 @@ import { Inject, Injectable } from '@nestjs/common'; import bcrypt from 'bcryptjs'; -import rndstr from 'rndstr'; import { Endpoint } from '@/server/api/endpoint-base.js'; import type { UsersRepository, UserProfilesRepository } from '@/models/index.js'; import { DI } from '@/di-symbols.js'; +import { secureRndstr } from '@/misc/secure-rndstr.js'; export const meta = { tags: ['admin'], @@ -54,7 +54,7 @@ export default class extends Endpoint { throw new Error('cannot reset password of root'); } - const passwd = rndstr('a-zA-Z0-9', 8); + const passwd = secureRndstr(8); // Generate hash of password const hash = bcrypt.hashSync(passwd); diff --git a/packages/backend/src/server/api/endpoints/app/create.ts b/packages/backend/src/server/api/endpoints/app/create.ts index c1d0a9dd74..aaef02d03f 100644 --- a/packages/backend/src/server/api/endpoints/app/create.ts +++ b/packages/backend/src/server/api/endpoints/app/create.ts @@ -44,7 +44,7 @@ export default class extends Endpoint { ) { super(meta, paramDef, async (ps, me) => { // Generate secret - const secret = secureRndstr(32, true); + const secret = secureRndstr(32); // for backward compatibility const permission = unique(ps.permission.map(v => v.replace(/^(.+)(\/|-)(read|write)$/, '$3:$1'))); diff --git a/packages/backend/src/server/api/endpoints/auth/accept.ts b/packages/backend/src/server/api/endpoints/auth/accept.ts index 05842460cf..e69f9c12e2 100644 --- a/packages/backend/src/server/api/endpoints/auth/accept.ts +++ b/packages/backend/src/server/api/endpoints/auth/accept.ts @@ -55,7 +55,7 @@ export default class extends Endpoint { throw new ApiError(meta.errors.noSuchSession); } - const accessToken = secureRndstr(32, true); + const accessToken = secureRndstr(32); // Fetch exist access token const exist = await this.accessTokensRepository.findOneBy({ diff --git a/packages/backend/src/server/api/endpoints/i/update-email.ts b/packages/backend/src/server/api/endpoints/i/update-email.ts index 4f543a6472..58e056bd37 100644 --- a/packages/backend/src/server/api/endpoints/i/update-email.ts +++ b/packages/backend/src/server/api/endpoints/i/update-email.ts @@ -1,5 +1,4 @@ import { Inject, Injectable } from '@nestjs/common'; -import rndstr from 'rndstr'; import ms from 'ms'; import bcrypt from 'bcryptjs'; import { Endpoint } from '@/server/api/endpoint-base.js'; @@ -9,6 +8,7 @@ import { EmailService } from '@/core/EmailService.js'; import type { Config } from '@/config.js'; import { DI } from '@/di-symbols.js'; import { GlobalEventService } from '@/core/GlobalEventService.js'; +import { L_CHARS, secureRndstr } from '@/misc/secure-rndstr.js'; import { ApiError } from '../../error.js'; export const meta = { @@ -94,7 +94,7 @@ export default class extends Endpoint { this.globalEventService.publishMainStream(me.id, 'meUpdated', iObj); if (ps.email != null) { - const code = rndstr('a-z0-9', 16); + const code = secureRndstr(16, { chars: L_CHARS }); await this.userProfilesRepository.update(me.id, { emailVerifyCode: code, diff --git a/packages/backend/src/server/api/endpoints/invite.ts b/packages/backend/src/server/api/endpoints/invite.ts index 5d2c479e79..276adcb07f 100644 --- a/packages/backend/src/server/api/endpoints/invite.ts +++ b/packages/backend/src/server/api/endpoints/invite.ts @@ -1,9 +1,9 @@ -import rndstr from 'rndstr'; import { Inject, Injectable } from '@nestjs/common'; import { Endpoint } from '@/server/api/endpoint-base.js'; import type { RegistrationTicketsRepository } from '@/models/index.js'; import { IdService } from '@/core/IdService.js'; import { DI } from '@/di-symbols.js'; +import { secureRndstr } from '@/misc/secure-rndstr.js'; export const meta = { tags: ['meta'], @@ -42,9 +42,8 @@ export default class extends Endpoint { private idService: IdService, ) { super(meta, paramDef, async (ps, me) => { - const code = rndstr({ - length: 8, - chars: '2-9A-HJ-NP-Z', // [0-9A-Z] w/o [01IO] (32 patterns) + const code = secureRndstr(8, { + chars: '23456789ABCDEFGHJKLMNPQRSTUVWXYZ', // [0-9A-Z] w/o [01IO] (32 patterns) }); await this.registrationTicketsRepository.insert({ diff --git a/packages/backend/src/server/api/endpoints/miauth/gen-token.ts b/packages/backend/src/server/api/endpoints/miauth/gen-token.ts index 97def86262..0ea29f04dc 100644 --- a/packages/backend/src/server/api/endpoints/miauth/gen-token.ts +++ b/packages/backend/src/server/api/endpoints/miauth/gen-token.ts @@ -49,7 +49,7 @@ export default class extends Endpoint { ) { super(meta, paramDef, async (ps, me) => { // Generate access token - const accessToken = secureRndstr(32, true); + const accessToken = secureRndstr(32); const now = new Date(); diff --git a/packages/backend/src/server/api/endpoints/request-reset-password.ts b/packages/backend/src/server/api/endpoints/request-reset-password.ts index 3b6ebfe281..284ed8410d 100644 --- a/packages/backend/src/server/api/endpoints/request-reset-password.ts +++ b/packages/backend/src/server/api/endpoints/request-reset-password.ts @@ -1,4 +1,3 @@ -import rndstr from 'rndstr'; import ms from 'ms'; import { IsNull } from 'typeorm'; import { Inject, Injectable } from '@nestjs/common'; @@ -8,6 +7,7 @@ import { IdService } from '@/core/IdService.js'; import type { Config } from '@/config.js'; import { DI } from '@/di-symbols.js'; import { EmailService } from '@/core/EmailService.js'; +import { L_CHARS, secureRndstr } from '@/misc/secure-rndstr.js'; export const meta = { tags: ['reset password'], @@ -41,7 +41,7 @@ export default class extends Endpoint { constructor( @Inject(DI.config) private config: Config, - + @Inject(DI.usersRepository) private usersRepository: UsersRepository, @@ -77,7 +77,7 @@ export default class extends Endpoint { return; } - const token = rndstr('a-z0-9', 64); + const token = secureRndstr(64, { chars: L_CHARS }); await this.passwordResetRequestsRepository.insert({ id: this.idService.genId(), diff --git a/packages/backend/test/e2e/move.ts b/packages/backend/test/e2e/move.ts index cd9459fa52..2fefcd0f0e 100644 --- a/packages/backend/test/e2e/move.ts +++ b/packages/backend/test/e2e/move.ts @@ -1,10 +1,10 @@ process.env.NODE_ENV = 'test'; import * as assert from 'assert'; -import rndstr from 'rndstr'; import { loadConfig } from '@/config.js'; import { User, UsersRepository } from '@/models/index.js'; import { jobQueue } from '@/boot/common.js'; +import { secureRndstr } from '@/misc/secure-rndstr.js'; import { uploadFile, signup, startServer, initTestDb, api, sleep, successfulApiCall } from '../utils.js'; import type { INestApplicationContext } from '@nestjs/common'; import type * as misskey from 'misskey-js'; @@ -163,7 +163,7 @@ describe('Account Move', () => { alsoKnownAs: [`@alice@${url.hostname}`], }, root); const listRoot = await api('/users/lists/create', { - name: rndstr('0-9a-z', 8), + name: secureRndstr(8), }, root); await api('/users/lists/push', { listId: listRoot.body.id, @@ -177,9 +177,9 @@ describe('Account Move', () => { userId: eve.id, }, alice); const antenna = await api('/antennas/create', { - name: rndstr('0-9a-z', 8), + name: secureRndstr(8), src: 'home', - keywords: [rndstr('0-9a-z', 8)], + keywords: [secureRndstr(8)], excludeKeywords: [], users: [], caseSensitive: false, @@ -211,7 +211,7 @@ describe('Account Move', () => { userId: dave.id, }, eve); const listEve = await api('/users/lists/create', { - name: rndstr('0-9a-z', 8), + name: secureRndstr(8), }, eve); await api('/users/lists/push', { listId: listEve.body.id, @@ -420,9 +420,9 @@ describe('Account Move', () => { test('Prohibit access after moving: /antennas/update', async () => { const res = await api('/antennas/update', { antennaId, - name: rndstr('0-9a-z', 8), + name: secureRndstr(8), src: 'users', - keywords: [rndstr('0-9a-z', 8)], + keywords: [secureRndstr(8)], excludeKeywords: [], users: [eve.id], caseSensitive: false, diff --git a/packages/backend/test/unit/RoleService.ts b/packages/backend/test/unit/RoleService.ts index 907f1f2edc..6979f23e0c 100644 --- a/packages/backend/test/unit/RoleService.ts +++ b/packages/backend/test/unit/RoleService.ts @@ -4,7 +4,6 @@ import { jest } from '@jest/globals'; import { ModuleMocker } from 'jest-mock'; import { Test } from '@nestjs/testing'; import * as lolex from '@sinonjs/fake-timers'; -import rndstr from 'rndstr'; import { GlobalModule } from '@/GlobalModule.js'; import { RoleService } from '@/core/RoleService.js'; import type { Role, RolesRepository, RoleAssignmentsRepository, UsersRepository, User } from '@/models/index.js'; @@ -14,6 +13,7 @@ import { genAid } from '@/misc/id/aid.js'; import { CacheService } from '@/core/CacheService.js'; import { IdService } from '@/core/IdService.js'; import { GlobalEventService } from '@/core/GlobalEventService.js'; +import { secureRndstr } from '@/misc/secure-rndstr.js'; import { sleep } from '../utils.js'; import type { TestingModule } from '@nestjs/testing'; import type { MockFunctionMetadata } from 'jest-mock'; @@ -30,7 +30,7 @@ describe('RoleService', () => { let clock: lolex.InstalledClock; function createUser(data: Partial = {}) { - const un = rndstr('a-z0-9', 16); + const un = secureRndstr(16); return usersRepository.insert({ id: genAid(new Date()), createdAt: new Date(), @@ -106,19 +106,19 @@ describe('RoleService', () => { }); describe('getUserPolicies', () => { - test('instance default policies', async () => { + test('instance default policies', async () => { const user = await createUser(); metaService.fetch.mockResolvedValue({ policies: { canManageCustomEmojis: false, }, } as any); - + const result = await roleService.getUserPolicies(user.id); - + expect(result.canManageCustomEmojis).toBe(false); }); - + test('instance default policies 2', async () => { const user = await createUser(); metaService.fetch.mockResolvedValue({ @@ -126,12 +126,12 @@ describe('RoleService', () => { canManageCustomEmojis: true, }, } as any); - + const result = await roleService.getUserPolicies(user.id); - + expect(result.canManageCustomEmojis).toBe(true); }); - + test('with role', async () => { const user = await createUser(); const role = await createRole({ @@ -150,9 +150,9 @@ describe('RoleService', () => { canManageCustomEmojis: false, }, } as any); - + const result = await roleService.getUserPolicies(user.id); - + expect(result.canManageCustomEmojis).toBe(true); }); @@ -185,9 +185,9 @@ describe('RoleService', () => { driveCapacityMb: 50, }, } as any); - + const result = await roleService.getUserPolicies(user.id); - + expect(result.driveCapacityMb).toBe(100); }); @@ -226,7 +226,7 @@ describe('RoleService', () => { canManageCustomEmojis: false, }, } as any); - + const user1Policies = await roleService.getUserPolicies(user1.id); const user2Policies = await roleService.getUserPolicies(user2.id); expect(user1Policies.canManageCustomEmojis).toBe(false); @@ -251,7 +251,7 @@ describe('RoleService', () => { canManageCustomEmojis: false, }, } as any); - + const result = await roleService.getUserPolicies(user.id); expect(result.canManageCustomEmojis).toBe(true); diff --git a/packages/backend/test/unit/activitypub.ts b/packages/backend/test/unit/activitypub.ts index 146998937e..7cd740a2fa 100644 --- a/packages/backend/test/unit/activitypub.ts +++ b/packages/backend/test/unit/activitypub.ts @@ -1,7 +1,6 @@ process.env.NODE_ENV = 'test'; import * as assert from 'assert'; -import rndstr from 'rndstr'; import { Test } from '@nestjs/testing'; import { jest } from '@jest/globals'; @@ -13,13 +12,14 @@ import { CoreModule } from '@/core/CoreModule.js'; import { FederatedInstanceService } from '@/core/FederatedInstanceService.js'; import { LoggerService } from '@/core/LoggerService.js'; import type { IActor } from '@/core/activitypub/type.js'; -import { MockResolver } from '../misc/mock-resolver.js'; import { Note } from '@/models/index.js'; +import { secureRndstr } from '@/misc/secure-rndstr.js'; +import { MockResolver } from '../misc/mock-resolver.js'; const host = 'https://host1.test'; function createRandomActor(): IActor & { id: string } { - const preferredUsername = `${rndstr('A-Z', 4)}${rndstr('a-z', 4)}`; + const preferredUsername = secureRndstr(8); const actorId = `${host}/users/${preferredUsername.toLowerCase()}`; return { @@ -61,7 +61,7 @@ describe('ActivityPub', () => { const post = { '@context': 'https://www.w3.org/ns/activitystreams', - id: `${host}/users/${rndstr('0-9a-z', 8)}`, + id: `${host}/users/${secureRndstr(8)}`, type: 'Note', attributedTo: actor.id, to: 'https://www.w3.org/ns/activitystreams#Public', @@ -94,7 +94,7 @@ describe('ActivityPub', () => { test('Truncate long name', async () => { const actor = { ...createRandomActor(), - name: rndstr('0-9a-z', 129), + name: secureRndstr(129), }; resolver._register(actor.id, actor); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 97f9a9583c..893e5409b1 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -314,9 +314,6 @@ importers: rename: specifier: 1.0.4 version: 1.0.4 - rndstr: - specifier: 1.0.0 - version: 1.0.0 rss-parser: specifier: 3.13.0 version: 3.13.0