enhance(frontend): サインイン画面の改善 (misskey-dev#14658)

* wip * Update MkSignin.vue * Update MkSignin.vue * wip * Update CHANGELOG.md * enhance(frontend): サインイン画面の改善 * Update Changelog * 14655の変更取り込み * spdx * fix * fix * fix * 🎨 * 🎨 * 🎨 * 🎨 * Captchaがリセットされない問題を修正 * 次の処理をsignin apiから読み取るように * Add Comments * fix * fix test * attempt to fix test * fix test * fix test * fix test * fix * fix test * fix: 一部のエラーがちゃんと出るように * Update Changelog * 🎨 * 🎨 * remove border --------- Co-authored-by: syuilo <[email protected]>
liumingye · Nov 1, 2024 · ef19325 · ef19325
1 parent 5f02271
commit ef19325
Show file tree

Hide file tree

Showing 18 changed files with 1,187 additions and 529 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,8 @@
 - サーバー初期設定時に使用する初期パスワードを設定できるようになりました。今後Misskeyサーバーを新たに設置する際には、初回の起動前にコンフィグファイルの`setupPassword`をコメントアウトし、初期パスワードを設定することをおすすめします。（すでに初期設定を完了しているサーバーについては、この変更に伴い対応する必要はありません）  
   - ホスティングサービスを運営している場合は、コンフィグファイルを構築する際に`setupPassword`をランダムな値に設定し、ユーザーに通知するようにシステムを更新することをおすすめします。
   - なお、初期パスワードが設定されていない場合でも初期設定を行うことが可能です（UI上で初期パスワードの入力欄を空欄にすると続行できます）。
+- ユーザーデータを読み込む際の型が一部変更されました。
+	- `twoFactorEnabled`, `usePasswordLessLogin`, `securityKeys`: 自分とモデレーター以外のユーザーからは取得できなくなりました
 
 ### General
 - Feat: サーバー初期設定時に初期パスワードを設定できるように
@@ -14,9 +16,11 @@
 
 ### Client
 - Enhance: デザインの調整
+- Enhance: ログイン画面の認証フローを改善
 
 ### Server
 - Enhance: セキュリティ向上のため、ログイン時にメール通知を行うように
+- Enhance: 自分とモデレーター以外のユーザーから二要素認証関連のデータが取得できないように
 
 
 ## 2024.9.0

diff --git a/cypress/e2e/basic.cy.ts b/cypress/e2e/basic.cy.ts
@@ -123,8 +123,13 @@ describe('After user signup', () => {
 		cy.intercept('POST', '/api/signin').as('signin');
 
 		cy.get('[data-cy-signin]').click();
-		cy.get('[data-cy-signin-username] input').type('alice');
-		// Enterキーでサインインできるかの確認も兼ねる
+
+		cy.get('[data-cy-signin-page-input]').should('be.visible', { timeout: 1000 });
+		// Enterキーで続行できるかの確認も兼ねる
+		cy.get('[data-cy-signin-username] input').type('alice{enter}');
+
+		cy.get('[data-cy-signin-page-password]').should('be.visible', { timeout: 10000 });
+		// Enterキーで続行できるかの確認も兼ねる
 		cy.get('[data-cy-signin-password] input').type('alice1234{enter}');
 
 		cy.wait('@signin');
@@ -139,8 +144,9 @@ describe('After user signup', () => {
 		cy.visitHome();
 
 		cy.get('[data-cy-signin]').click();
-		cy.get('[data-cy-signin-username] input').type('alice');
-		cy.get('[data-cy-signin-password] input').type('alice1234{enter}');
+
+		cy.get('[data-cy-signin-page-input]').should('be.visible', { timeout: 1000 });
+		cy.get('[data-cy-signin-username] input').type('alice{enter}');
 
 		// TODO: cypressにブラウザの言語指定できる機能が実装され次第英語のみテストするようにする
 		cy.contains(/アカウントが凍結されています|This account has been suspended due to/gi);

diff --git a/cypress/support/commands.ts b/cypress/support/commands.ts
@@ -58,7 +58,9 @@ Cypress.Commands.add('login', (username, password) => {
 	cy.intercept('POST', '/api/signin').as('signin');
 
 	cy.get('[data-cy-signin]').click();
-	cy.get('[data-cy-signin-username] input').type(username);
+	cy.get('[data-cy-signin-page-input]').should('be.visible', { timeout: 1000 });
+	cy.get('[data-cy-signin-username] input').type(`${username}{enter}`);
+	cy.get('[data-cy-signin-page-password]').should('be.visible', { timeout: 10000 });
 	cy.get('[data-cy-signin-password] input').type(`${password}{enter}`);
 
 	cy.wait('@signin').as('signedIn');

diff --git a/packages/backend/src/core/entities/UserEntityService.ts b/packages/backend/src/core/entities/UserEntityService.ts
@@ -597,11 +597,6 @@ export class UserEntityService implements OnModuleInit {
 				publicReactions: this.isLocalUser(user) ? profile!.publicReactions : false, // https://github.com/misskey-dev/misskey/issues/12964
 				followersVisibility: profile!.followersVisibility,
 				followingVisibility: profile!.followingVisibility,
-				twoFactorEnabled: profile!.twoFactorEnabled,
-				usePasswordLessLogin: profile!.usePasswordLessLogin,
-				securityKeys: profile!.twoFactorEnabled
-					? this.userSecurityKeysRepository.countBy({ userId: user.id }).then(result => result >= 1)
-					: false,
 				roles: this.roleService.getUserRoles(user.id).then(roles => roles.filter(role => role.isPublic).sort((a, b) => b.displayOrder - a.displayOrder).map(role => ({
 					id: role.id,
 					name: role.name,
@@ -616,6 +611,14 @@ export class UserEntityService implements OnModuleInit {
 				moderationNote: iAmModerator ? (profile!.moderationNote ?? '') : undefined,
 			} : {}),
 
+			...(isDetailed && (isMe || iAmModerator) ? {
+				twoFactorEnabled: profile!.twoFactorEnabled,
+				usePasswordLessLogin: profile!.usePasswordLessLogin,
+				securityKeys: profile!.twoFactorEnabled
+					? this.userSecurityKeysRepository.countBy({ userId: user.id }).then(result => result >= 1)
+					: false,
+			} : {}),
+
 			...(isDetailed && isMe ? {
 				avatarId: user.avatarId,
 				bannerId: user.bannerId,

diff --git a/packages/backend/src/models/json-schema/user.ts b/packages/backend/src/models/json-schema/user.ts
@@ -392,21 +392,6 @@ export const packedUserDetailedNotMeOnlySchema = {
 			nullable: false, optional: false,
 			enum: ['public', 'followers', 'private'],
 		},
-		twoFactorEnabled: {
-			type: 'boolean',
-			nullable: false, optional: false,
-			default: false,
-		},
-		usePasswordLessLogin: {
-			type: 'boolean',
-			nullable: false, optional: false,
-			default: false,
-		},
-		securityKeys: {
-			type: 'boolean',
-			nullable: false, optional: false,
-			default: false,
-		},
 		roles: {
 			type: 'array',
 			nullable: false, optional: false,
@@ -428,6 +413,18 @@ export const packedUserDetailedNotMeOnlySchema = {
 			type: 'string',
 			nullable: false, optional: true,
 		},
+		twoFactorEnabled: {
+			type: 'boolean',
+			nullable: false, optional: true,
+		},
+		usePasswordLessLogin: {
+			type: 'boolean',
+			nullable: false, optional: true,
+		},
+		securityKeys: {
+			type: 'boolean',
+			nullable: false, optional: true,
+		},
 		//#region relations
 		isFollowing: {
 			type: 'boolean',
@@ -689,6 +686,21 @@ export const packedMeDetailedOnlySchema = {
 			nullable: false, optional: false,
 			ref: 'RolePolicies',
 		},
+		twoFactorEnabled: {
+			type: 'boolean',
+			nullable: false, optional: false,
+			default: false,
+		},
+		usePasswordLessLogin: {
+			type: 'boolean',
+			nullable: false, optional: false,
+			default: false,
+		},
+		securityKeys: {
+			type: 'boolean',
+			nullable: false, optional: false,
+			default: false,
+		},
 		//#region secrets
 		email: {
 			type: 'string',

diff --git a/packages/backend/src/server/api/SigninApiService.ts b/packages/backend/src/server/api/SigninApiService.ts
@@ -13,6 +13,7 @@ import type {
 	MiMeta,
 	SigninsRepository,
 	UserProfilesRepository,
+	UserSecurityKeysRepository,
 	UsersRepository,
 } from '@/models/_.js';
 import type { Config } from '@/config.js';
@@ -26,11 +27,29 @@ import { CaptchaService } from '@/core/CaptchaService.js';
 import { FastifyReplyError } from '@/misc/fastify-reply-error.js';
 import { RateLimiterService } from './RateLimiterService.js';
 import { SigninService } from './SigninService.js';
-import type { AuthenticationResponseJSON } from '@simplewebauthn/types';
+import type { AuthenticationResponseJSON, PublicKeyCredentialRequestOptionsJSON } from '@simplewebauthn/types';
 import type { FastifyReply, FastifyRequest } from 'fastify';
 import { isSystemAccount } from '@/misc/is-system-account.js';
 import type { MiMeta } from '@/models/_.js';
 
+/**
+ * next を指定すると、次にクライアント側で行うべき処理を指定できる。
+ *
+ * - `captcha`: パスワードと、（有効になっている場合は）CAPTCHAを求める
+ * - `password`: パスワードを求める
+ * - `totp`: ワンタイムパスワードを求める
+ * - `passkey`: WebAuthn認証を求める（WebAuthnに対応していないブラウザの場合はワンタイムパスワード）
+ */
+
+type SigninErrorResponse = {
+	id: string;
+	next?: 'captcha' | 'password' | 'totp';
+} | {
+	id: string;
+	next: 'passkey';
+	authRequest: PublicKeyCredentialRequestOptionsJSON;
+};
+
 @Injectable()
 export class SigninApiService {
 	constructor(
@@ -46,6 +65,9 @@ export class SigninApiService {
 		@Inject(DI.userProfilesRepository)
 		private userProfilesRepository: UserProfilesRepository,
 
+		@Inject(DI.userSecurityKeysRepository)
+		private userSecurityKeysRepository: UserSecurityKeysRepository,
+
 		@Inject(DI.signinsRepository)
 		private signinsRepository: SigninsRepository,
 
@@ -63,7 +85,7 @@ export class SigninApiService {
 		request: FastifyRequest<{
 			Body: {
 				username: string;
-				password: string;
+				password?: string;
 				token?: string;
 				credential?: AuthenticationResponseJSON;
 				'hcaptcha-response'?: string;
@@ -82,7 +104,7 @@ export class SigninApiService {
 		const password = body['password'];
 		const token = body['token'];
 
-		function error(status: number, error: { id: string }) {
+		function error(status: number, error: SigninErrorResponse) {
 			reply.code(status);
 			return { error };
 		}
@@ -106,11 +128,6 @@ export class SigninApiService {
 			return;
 		}
 
-		if (typeof password !== 'string') {
-			reply.code(400);
-			return;
-		}
-
 		if (token != null && typeof token !== 'string') {
 			reply.code(400);
 			return;
@@ -141,6 +158,31 @@ export class SigninApiService {
 		}
 
 		const profile = await this.userProfilesRepository.findOneByOrFail({ userId: user.id });
+		const securityKeysAvailable = await this.userSecurityKeysRepository.countBy({ userId: user.id }).then(result => result >= 1);
+
+		if (password == null) {
+			reply.code(403);
+			if (profile.twoFactorEnabled) {
+				return {
+					error: {
+						id: '144ff4f8-bd6c-41bc-82c3-b672eb09efbf',
+						next: 'password',
+					},
+				} satisfies { error: SigninErrorResponse };
+			} else {
+				return {
+					error: {
+						id: '144ff4f8-bd6c-41bc-82c3-b672eb09efbf',
+						next: 'captcha',
+					},
+				} satisfies { error: SigninErrorResponse };
+			}
+		}
+
+		if (typeof password !== 'string') {
+			reply.code(400);
+			return;
+		}
 
 		if (!user.approved && this.meta.approvalRequiredForSignup) {
 			reply.code(403);
@@ -156,7 +198,7 @@ export class SigninApiService {
 		// Compare password
 		const same = await argon2.verify(profile.password!, password) || bcrypt.compareSync(password, profile.password!);
 
-		const fail = async (status?: number, failure?: { id: string }) => {
+		const fail = async (status?: number, failure?: SigninErrorResponse) => {
 			// Append signin history
 			await this.signinsRepository.insert({
 				id: this.idService.gen(),
@@ -254,7 +296,7 @@ export class SigninApiService {
 					id: '93b86c4b-72f9-40eb-9815-798928603d1e',
 				});
 			}
-		} else {
+		} else if (securityKeysAvailable) {
 			if (!same && !profile.usePasswordLessLogin) {
 				return await fail(403, {
 					id: '932c904e-9460-45b7-9ce6-7ed33be7eb2c',
@@ -263,8 +305,28 @@ export class SigninApiService {
 
 			const authRequest = await this.webAuthnService.initiateAuthentication(user.id);
 
-			reply.code(200);
-			return authRequest;
+			reply.code(403);
+			return {
+				error: {
+					id: '06e661b9-8146-4ae3-bde5-47138c0ae0c4',
+					next: 'passkey',
+					authRequest,
+				},
+			} satisfies { error: SigninErrorResponse };
+		} else {
+			if (!same || !profile.twoFactorEnabled) {
+				return await fail(403, {
+					id: '932c904e-9460-45b7-9ce6-7ed33be7eb2c',
+				});
+			} else {
+				reply.code(403);
+				return {
+					error: {
+						id: '144ff4f8-bd6c-41bc-82c3-b672eb09efbf',
+						next: 'totp',
+					},
+				} satisfies { error: SigninErrorResponse };
+			}
 		}
 		// never get here
 	}