From ae3c155490d9b5a574c45309744ba2a0cbe78932 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=81=8B=E3=81=A3=E3=81=93=E3=81=8B=E3=82=8A?= <67428053+kakkokari-gtyih@users.noreply.github.com> Date: Sat, 5 Oct 2024 12:03:47 +0900 Subject: [PATCH] =?UTF-8?q?fix:=20signin=20=E3=81=AE=E8=B3=87=E6=A0=BC?= =?UTF-8?q?=E6=83=85=E5=A0=B1=E3=81=8C=E8=B6=B3=E3=82=8A=E3=81=AA=E3=81=84?= =?UTF-8?q?=E3=81=A0=E3=81=91=E3=81=AE=E5=A0=B4=E5=90=88=E3=81=AF=E3=82=A8?= =?UTF-8?q?=E3=83=A9=E3=83=BC=E3=81=AB=E3=81=9B=E3=81=9A200=E3=82=92?= =?UTF-8?q?=E8=BF=94=E3=81=99=E3=82=88=E3=81=86=E3=81=AB=20(#14700)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: signin の資格情報が足りないだけの場合はエラーにせず200を返すように * run api extractor * fix * fix * fix test * /signin -> /signin-flow * fix * fix lint * rename * fix * fix --- cypress/e2e/basic.cy.ts | 2 +- cypress/support/commands.ts | 2 +- .../src/server/api/ApiServerService.ts | 2 +- .../src/server/api/SigninApiService.ts | 66 ++--- .../backend/src/server/api/SigninService.ts | 6 +- packages/backend/test/e2e/2fa.ts | 71 +++--- packages/backend/test/e2e/endpoints.ts | 8 +- packages/frontend/src/components/MkSignin.vue | 236 +++++++++--------- .../src/components/MkSignupDialog.form.vue | 11 +- .../src/components/MkSignupDialog.vue | 4 +- packages/misskey-js/etc/misskey-js.api.md | 24 +- packages/misskey-js/src/api.types.ts | 10 +- packages/misskey-js/src/entities.ts | 22 +- 13 files changed, 230 insertions(+), 234 deletions(-) diff --git a/cypress/e2e/basic.cy.ts b/cypress/e2e/basic.cy.ts index c9d7e0a24a..d2efbf709c 100644 --- a/cypress/e2e/basic.cy.ts +++ b/cypress/e2e/basic.cy.ts @@ -120,7 +120,7 @@ describe('After user signup', () => { it('signin', () => { cy.visitHome(); - cy.intercept('POST', '/api/signin').as('signin'); + cy.intercept('POST', '/api/signin-flow').as('signin'); cy.get('[data-cy-signin]').click(); diff --git a/cypress/support/commands.ts b/cypress/support/commands.ts index ed5cda31b0..197ff963ac 100644 --- a/cypress/support/commands.ts +++ b/cypress/support/commands.ts @@ -55,7 +55,7 @@ Cypress.Commands.add('registerUser', (username, password, isAdmin = false) => { Cypress.Commands.add('login', (username, password) => { cy.visitHome(); - cy.intercept('POST', '/api/signin').as('signin'); + cy.intercept('POST', '/api/signin-flow').as('signin'); cy.get('[data-cy-signin]').click(); cy.get('[data-cy-signin-page-input]').should('be.visible', { timeout: 1000 }); diff --git a/packages/backend/src/server/api/ApiServerService.ts b/packages/backend/src/server/api/ApiServerService.ts index 356e145681..6b760c258b 100644 --- a/packages/backend/src/server/api/ApiServerService.ts +++ b/packages/backend/src/server/api/ApiServerService.ts @@ -133,7 +133,7 @@ export class ApiServerService { 'turnstile-response'?: string; 'm-captcha-response'?: string; }; - }>('/signin', (request, reply) => this.signinApiService.signin(request, reply)); + }>('/signin-flow', (request, reply) => this.signinApiService.signin(request, reply)); fastify.post<{ Body: { diff --git a/packages/backend/src/server/api/SigninApiService.ts b/packages/backend/src/server/api/SigninApiService.ts index 81684beb3c..0d24ffa56a 100644 --- a/packages/backend/src/server/api/SigninApiService.ts +++ b/packages/backend/src/server/api/SigninApiService.ts @@ -5,8 +5,8 @@ import { Inject, Injectable } from '@nestjs/common'; import bcrypt from 'bcryptjs'; -import * as OTPAuth from 'otpauth'; import { IsNull } from 'typeorm'; +import * as Misskey from 'misskey-js'; import { DI } from '@/di-symbols.js'; import type { MiMeta, @@ -26,27 +26,9 @@ 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, PublicKeyCredentialRequestOptionsJSON } from '@simplewebauthn/types'; +import type { AuthenticationResponseJSON } from '@simplewebauthn/types'; import type { FastifyReply, FastifyRequest } from 'fastify'; -/** - * 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( @@ -101,7 +83,7 @@ export class SigninApiService { const password = body['password']; const token = body['token']; - function error(status: number, error: SigninErrorResponse) { + function error(status: number, error: { id: string }) { reply.code(status); return { error }; } @@ -152,21 +134,17 @@ export class SigninApiService { const securityKeysAvailable = await this.userSecurityKeysRepository.countBy({ userId: user.id }).then(result => result >= 1); if (password == null) { - reply.code(403); + reply.code(200); if (profile.twoFactorEnabled) { return { - error: { - id: '144ff4f8-bd6c-41bc-82c3-b672eb09efbf', - next: 'password', - }, - } satisfies { error: SigninErrorResponse }; + finished: false, + next: 'password', + } satisfies Misskey.entities.SigninFlowResponse; } else { return { - error: { - id: '144ff4f8-bd6c-41bc-82c3-b672eb09efbf', - next: 'captcha', - }, - } satisfies { error: SigninErrorResponse }; + finished: false, + next: 'captcha', + } satisfies Misskey.entities.SigninFlowResponse; } } @@ -178,7 +156,7 @@ export class SigninApiService { // Compare password const same = await bcrypt.compare(password, profile.password!); - const fail = async (status?: number, failure?: SigninErrorResponse) => { + const fail = async (status?: number, failure?: { id: string; }) => { // Append signin history await this.signinsRepository.insert({ id: this.idService.gen(), @@ -268,27 +246,23 @@ export class SigninApiService { const authRequest = await this.webAuthnService.initiateAuthentication(user.id); - reply.code(403); + reply.code(200); return { - error: { - id: '06e661b9-8146-4ae3-bde5-47138c0ae0c4', - next: 'passkey', - authRequest, - }, - } satisfies { error: SigninErrorResponse }; + finished: false, + next: 'passkey', + authRequest, + } satisfies Misskey.entities.SigninFlowResponse; } else { if (!same || !profile.twoFactorEnabled) { return await fail(403, { id: '932c904e-9460-45b7-9ce6-7ed33be7eb2c', }); } else { - reply.code(403); + reply.code(200); return { - error: { - id: '144ff4f8-bd6c-41bc-82c3-b672eb09efbf', - next: 'totp', - }, - } satisfies { error: SigninErrorResponse }; + finished: false, + next: 'totp', + } satisfies Misskey.entities.SigninFlowResponse; } } // never get here diff --git a/packages/backend/src/server/api/SigninService.ts b/packages/backend/src/server/api/SigninService.ts index 4b041f373f..640356b50c 100644 --- a/packages/backend/src/server/api/SigninService.ts +++ b/packages/backend/src/server/api/SigninService.ts @@ -4,6 +4,7 @@ */ import { Inject, Injectable } from '@nestjs/common'; +import * as Misskey from 'misskey-js'; import { DI } from '@/di-symbols.js'; import type { SigninsRepository, UserProfilesRepository } from '@/models/_.js'; import { IdService } from '@/core/IdService.js'; @@ -57,9 +58,10 @@ export class SigninService { reply.code(200); return { + finished: true, id: user.id, - i: user.token, - }; + i: user.token!, + } satisfies Misskey.entities.SigninFlowResponse; } } diff --git a/packages/backend/test/e2e/2fa.ts b/packages/backend/test/e2e/2fa.ts index 88c32b4346..48e1bababb 100644 --- a/packages/backend/test/e2e/2fa.ts +++ b/packages/backend/test/e2e/2fa.ts @@ -136,7 +136,7 @@ describe('2要素認証', () => { keyName: string, credentialId: Buffer, requestOptions: PublicKeyCredentialRequestOptionsJSON, - }): misskey.entities.SigninRequest => { + }): misskey.entities.SigninFlowRequest => { // AuthenticatorAssertionResponse.authenticatorData // https://developer.mozilla.org/en-US/docs/Web/API/AuthenticatorAssertionResponse/authenticatorData const authenticatorData = Buffer.concat([ @@ -196,22 +196,21 @@ describe('2要素認証', () => { }, alice); assert.strictEqual(doneResponse.status, 200); - const signinWithoutTokenResponse = await api('signin', { + const signinWithoutTokenResponse = await api('signin-flow', { ...signinParam(), }); - assert.strictEqual(signinWithoutTokenResponse.status, 403); + assert.strictEqual(signinWithoutTokenResponse.status, 200); assert.deepStrictEqual(signinWithoutTokenResponse.body, { - error: { - id: '144ff4f8-bd6c-41bc-82c3-b672eb09efbf', - next: 'totp', - }, + finished: false, + next: 'totp', }); - const signinResponse = await api('signin', { + const signinResponse = await api('signin-flow', { ...signinParam(), token: otpToken(registerResponse.body.secret), }); assert.strictEqual(signinResponse.status, 200); + assert.strictEqual(signinResponse.body.finished, true); assert.notEqual(signinResponse.body.i, undefined); // 後片付け @@ -252,29 +251,23 @@ describe('2要素認証', () => { assert.strictEqual(keyDoneResponse.body.id, credentialId.toString('base64url')); assert.strictEqual(keyDoneResponse.body.name, keyName); - const signinResponse = await api('signin', { + const signinResponse = await api('signin-flow', { ...signinParam(), }); - const signinResponseBody = signinResponse.body as unknown as { - error: { - id: string; - next: 'passkey'; - authRequest: PublicKeyCredentialRequestOptionsJSON; - }; - }; - assert.strictEqual(signinResponse.status, 403); - assert.strictEqual(signinResponseBody.error.id, '06e661b9-8146-4ae3-bde5-47138c0ae0c4'); - assert.strictEqual(signinResponseBody.error.next, 'passkey'); - assert.notEqual(signinResponseBody.error.authRequest.challenge, undefined); - assert.notEqual(signinResponseBody.error.authRequest.allowCredentials, undefined); - assert.strictEqual(signinResponseBody.error.authRequest.allowCredentials && signinResponseBody.error.authRequest.allowCredentials[0]?.id, credentialId.toString('base64url')); + assert.strictEqual(signinResponse.status, 200); + assert.strictEqual(signinResponse.body.finished, false); + assert.strictEqual(signinResponse.body.next, 'passkey'); + assert.notEqual(signinResponse.body.authRequest.challenge, undefined); + assert.notEqual(signinResponse.body.authRequest.allowCredentials, undefined); + assert.strictEqual(signinResponse.body.authRequest.allowCredentials && signinResponse.body.authRequest.allowCredentials[0]?.id, credentialId.toString('base64url')); - const signinResponse2 = await api('signin', signinWithSecurityKeyParam({ + const signinResponse2 = await api('signin-flow', signinWithSecurityKeyParam({ keyName, credentialId, - requestOptions: signinResponseBody.error.authRequest, + requestOptions: signinResponse.body.authRequest, })); assert.strictEqual(signinResponse2.status, 200); + assert.strictEqual(signinResponse2.body.finished, true); assert.notEqual(signinResponse2.body.i, undefined); // 後片付け @@ -320,32 +313,26 @@ describe('2要素認証', () => { assert.strictEqual(iResponse.status, 200); assert.strictEqual(iResponse.body.usePasswordLessLogin, true); - const signinResponse = await api('signin', { + const signinResponse = await api('signin-flow', { ...signinParam(), password: '', }); - const signinResponseBody = signinResponse.body as unknown as { - error: { - id: string; - next: 'passkey'; - authRequest: PublicKeyCredentialRequestOptionsJSON; - }; - }; - assert.strictEqual(signinResponse.status, 403); - assert.strictEqual(signinResponseBody.error.id, '06e661b9-8146-4ae3-bde5-47138c0ae0c4'); - assert.strictEqual(signinResponseBody.error.next, 'passkey'); - assert.notEqual(signinResponseBody.error.authRequest.challenge, undefined); - assert.notEqual(signinResponseBody.error.authRequest.allowCredentials, undefined); + assert.strictEqual(signinResponse.status, 200); + assert.strictEqual(signinResponse.body.finished, false); + assert.strictEqual(signinResponse.body.next, 'passkey'); + assert.notEqual(signinResponse.body.authRequest.challenge, undefined); + assert.notEqual(signinResponse.body.authRequest.allowCredentials, undefined); - const signinResponse2 = await api('signin', { + const signinResponse2 = await api('signin-flow', { ...signinWithSecurityKeyParam({ keyName, credentialId, - requestOptions: signinResponseBody.error.authRequest, + requestOptions: signinResponse.body.authRequest, } as any), password: '', }); assert.strictEqual(signinResponse2.status, 200); + assert.strictEqual(signinResponse2.body.finished, true); assert.notEqual(signinResponse2.body.i, undefined); // 後片付け @@ -450,11 +437,12 @@ describe('2要素認証', () => { assert.strictEqual(afterIResponse.status, 200); assert.strictEqual(afterIResponse.body.securityKeys, false); - const signinResponse = await api('signin', { + const signinResponse = await api('signin-flow', { ...signinParam(), token: otpToken(registerResponse.body.secret), }); assert.strictEqual(signinResponse.status, 200); + assert.strictEqual(signinResponse.body.finished, true); assert.notEqual(signinResponse.body.i, undefined); // 後片付け @@ -485,10 +473,11 @@ describe('2要素認証', () => { }, alice); assert.strictEqual(unregisterResponse.status, 204); - const signinResponse = await api('signin', { + const signinResponse = await api('signin-flow', { ...signinParam(), }); assert.strictEqual(signinResponse.status, 200); + assert.strictEqual(signinResponse.body.finished, true); assert.notEqual(signinResponse.body.i, undefined); // 後片付け diff --git a/packages/backend/test/e2e/endpoints.ts b/packages/backend/test/e2e/endpoints.ts index 5aaec7f6f9..b91d77c398 100644 --- a/packages/backend/test/e2e/endpoints.ts +++ b/packages/backend/test/e2e/endpoints.ts @@ -66,9 +66,9 @@ describe('Endpoints', () => { }); }); - describe('signin', () => { + describe('signin-flow', () => { test('間違ったパスワードでサインインできない', async () => { - const res = await api('signin', { + const res = await api('signin-flow', { username: 'test1', password: 'bar', }); @@ -77,7 +77,7 @@ describe('Endpoints', () => { }); test('クエリをインジェクションできない', async () => { - const res = await api('signin', { + const res = await api('signin-flow', { username: 'test1', // @ts-expect-error password must be string password: { @@ -89,7 +89,7 @@ describe('Endpoints', () => { }); test('正しい情報でサインインできる', async () => { - const res = await api('signin', { + const res = await api('signin-flow', { username: 'test1', password: 'test1', }); diff --git a/packages/frontend/src/components/MkSignin.vue b/packages/frontend/src/components/MkSignin.vue index 03dd61f6c6..26e1ac516c 100644 --- a/packages/frontend/src/components/MkSignin.vue +++ b/packages/frontend/src/components/MkSignin.vue @@ -83,7 +83,7 @@ import type { AuthenticationPublicKeyCredential } from '@github/webauthn-json/br import type { OpenOnRemoteOptions } from '@/scripts/please-login.js'; const emit = defineEmits<{ - (ev: 'login', v: Misskey.entities.SigninResponse): void; + (ev: 'login', v: Misskey.entities.SigninFlowResponse): void; }>(); const props = withDefaults(defineProps<{ @@ -212,23 +212,63 @@ async function onTotpSubmitted(token: string) { } } -async function tryLogin(req: Partial): Promise { +async function tryLogin(req: Partial): Promise { const _req = { username: req.username ?? userInfo.value?.username, ...req, }; - function assertIsSigninRequest(x: Partial): x is Misskey.entities.SigninRequest { + function assertIsSigninFlowRequest(x: Partial): x is Misskey.entities.SigninFlowRequest { return x.username != null; } - if (!assertIsSigninRequest(_req)) { + if (!assertIsSigninFlowRequest(_req)) { throw new Error('Invalid request'); } - return await misskeyApi('signin', _req).then(async (res) => { - emit('login', res); - await onLoginSucceeded(res); + return await misskeyApi('signin-flow', _req).then(async (res) => { + if (res.finished) { + emit('login', res); + await onLoginSucceeded(res); + } else { + switch (res.next) { + case 'captcha': { + needCaptcha.value = true; + page.value = 'password'; + break; + } + case 'password': { + needCaptcha.value = false; + page.value = 'password'; + break; + } + case 'totp': { + page.value = 'totp'; + break; + } + case 'passkey': { + if (webAuthnSupported()) { + credentialRequest.value = parseRequestOptionsFromJSON({ + publicKey: res.authRequest, + }); + page.value = 'passkey'; + } else { + page.value = 'totp'; + } + break; + } + } + + if (doingPasskeyFromInputPage.value === true) { + doingPasskeyFromInputPage.value = false; + page.value = 'input'; + password.value = ''; + } + passwordPageEl.value?.resetCaptcha(); + nextTick(() => { + waiting.value = false; + }); + } return res; }).catch((err) => { onSigninApiError(err); @@ -236,7 +276,7 @@ async function tryLogin(req: Partial): Promise(); @@ -269,14 +269,19 @@ async function onSubmit(): Promise { }); emit('signupEmailPending'); } else { - const res = await misskeyApi('signin', { + const res = await misskeyApi('signin-flow', { username: username.value, password: password.value, }); emit('signup', res); - if (props.autoSet) { + if (props.autoSet && res.finished) { return login(res.i); + } else { + os.alert({ + type: 'error', + text: i18n.ts.somethingHappened, + }); } } } catch { diff --git a/packages/frontend/src/components/MkSignupDialog.vue b/packages/frontend/src/components/MkSignupDialog.vue index 97310d32a6..4cccd99492 100644 --- a/packages/frontend/src/components/MkSignupDialog.vue +++ b/packages/frontend/src/components/MkSignupDialog.vue @@ -47,7 +47,7 @@ const props = withDefaults(defineProps<{ }); const emit = defineEmits<{ - (ev: 'done', res: Misskey.entities.SigninResponse): void; + (ev: 'done', res: Misskey.entities.SigninFlowResponse): void; (ev: 'closed'): void; }>(); @@ -55,7 +55,7 @@ const dialog = shallowRef>(); const isAcceptedServerRule = ref(false); -function onSignup(res: Misskey.entities.SigninResponse) { +function onSignup(res: Misskey.entities.SigninFlowResponse) { emit('done', res); dialog.value?.close(); } diff --git a/packages/misskey-js/etc/misskey-js.api.md b/packages/misskey-js/etc/misskey-js.api.md index 9ad784c296..732352abd8 100644 --- a/packages/misskey-js/etc/misskey-js.api.md +++ b/packages/misskey-js/etc/misskey-js.api.md @@ -1158,9 +1158,9 @@ export type Endpoints = Overwrite> = T[keyof T];