From 114523e69e30a90bc7bc043254cfc89e3a523c46 Mon Sep 17 00:00:00 2001
From: Satsuki Yanagi <17376330+u1-liquid@users.noreply.github.com>
Date: Fri, 5 Jul 2019 07:48:12 +0900
Subject: [PATCH] Fix WebAuthn login (#5103)

---
 .../app/common/views/components/signin.vue    | 16 +++--
 src/server/api/endpoints/i/2fa/getkeys.ts     | 67 -------------------
 src/server/api/private/signin.ts              | 42 +++++++++++-
 3 files changed, 51 insertions(+), 74 deletions(-)
 delete mode 100644 src/server/api/endpoints/i/2fa/getkeys.ts

diff --git a/src/client/app/common/views/components/signin.vue b/src/client/app/common/views/components/signin.vue
index 53cc62c333..8498a1dc3e 100644
--- a/src/client/app/common/views/components/signin.vue
+++ b/src/client/app/common/views/components/signin.vue
@@ -107,9 +107,8 @@ export default Vue.extend({
 					})),
 					timeout: 60 * 1000
 				}
-			}).catch(err => {
+			}).catch(() => {
 				this.queryingKey = false;
-				console.warn(err);
 				return Promise.reject(null);
 			}).then(credential => {
 				this.queryingKey = false;
@@ -127,8 +126,7 @@ export default Vue.extend({
 				localStorage.setItem('i', res.i);
 				location.reload();
 			}).catch(err => {
-				if(err === null) return;
-				console.error(err);
+				if (err === null) return;
 				this.$root.dialog({
 					type: 'error',
 					text: this.$t('login-failed')
@@ -142,7 +140,7 @@ export default Vue.extend({
 
 			if (!this.totpLogin && this.user && this.user.twoFactorEnabled) {
 				if (window.PublicKeyCredential && this.user.securityKeys) {
-					this.$root.api('i/2fa/getkeys', {
+					this.$root.api('signin', {
 						username: this.username,
 						password: this.password
 					}).then(res => {
@@ -150,6 +148,14 @@ export default Vue.extend({
 						this.signing = false;
 						this.challengeData = res;
 						return this.queryKey();
+					}).catch(() => {
+						this.$root.dialog({
+							type: 'error',
+							text: this.$t('login-failed')
+						});
+						this.challengeData = null;
+						this.totpLogin = false;
+						this.signing = false;
 					});
 				} else {
 					this.totpLogin = true;
diff --git a/src/server/api/endpoints/i/2fa/getkeys.ts b/src/server/api/endpoints/i/2fa/getkeys.ts
deleted file mode 100644
index bb1585d795..0000000000
--- a/src/server/api/endpoints/i/2fa/getkeys.ts
+++ /dev/null
@@ -1,67 +0,0 @@
-import $ from 'cafy';
-import * as bcrypt from 'bcryptjs';
-import * as crypto from 'crypto';
-import define from '../../../define';
-import { UserProfiles, UserSecurityKeys, AttestationChallenges } from '../../../../../models';
-import { ensure } from '../../../../../prelude/ensure';
-import { promisify } from 'util';
-import { hash } from '../../../2fa';
-import { genId } from '../../../../../misc/gen-id';
-
-export const meta = {
-	requireCredential: true,
-
-	secure: true,
-
-	params: {
-		password: {
-			validator: $.str
-		}
-	}
-};
-
-const randomBytes = promisify(crypto.randomBytes);
-
-export default define(meta, async (ps, user) => {
-	const profile = await UserProfiles.findOne(user.id).then(ensure);
-
-	// Compare password
-	const same = await bcrypt.compare(ps.password, profile.password!);
-
-	if (!same) {
-		throw new Error('incorrect password');
-	}
-
-	const keys = await UserSecurityKeys.find({
-		userId: user.id
-	});
-
-	if (keys.length === 0) {
-		throw new Error('no keys found');
-	}
-
-	// 32 byte challenge
-	const entropy = await randomBytes(32);
-	const challenge = entropy.toString('base64')
-		.replace(/=/g, '')
-		.replace(/\+/g, '-')
-		.replace(/\//g, '_');
-
-	const challengeId = genId();
-
-	await AttestationChallenges.save({
-		userId: user.id,
-		id: challengeId,
-		challenge: hash(Buffer.from(challenge, 'utf-8')).toString('hex'),
-		createdAt: new Date(),
-		registrationChallenge: false
-	});
-
-	return {
-		challenge,
-		challengeId,
-		securityKeys: keys.map(key => ({
-			id: key.id
-		}))
-	};
-});
diff --git a/src/server/api/private/signin.ts b/src/server/api/private/signin.ts
index cd9fe5bb9d..bc9346d088 100644
--- a/src/server/api/private/signin.ts
+++ b/src/server/api/private/signin.ts
@@ -9,6 +9,7 @@ import { ILocalUser } from '../../../models/entities/user';
 import { genId } from '../../../misc/gen-id';
 import { ensure } from '../../../prelude/ensure';
 import { verifyLogin, hash } from '../2fa';
+import { randomBytes } from 'crypto';
 
 export default async (ctx: Koa.BaseContext) => {
 	ctx.set('Access-Control-Allow-Origin', config.url);
@@ -99,7 +100,7 @@ export default async (ctx: Koa.BaseContext) => {
 			});
 			return;
 		}
-	} else {
+	} else if (body.credentialId) {
 		const clientDataJSON = Buffer.from(body.clientDataJSON, 'hex');
 		const clientData = JSON.parse(clientDataJSON.toString('utf-8'));
 		const challenge = await AttestationChallenges.findOne({
@@ -131,7 +132,7 @@ export default async (ctx: Koa.BaseContext) => {
 		const securityKey = await UserSecurityKeys.findOne({
 			id: Buffer.from(
 				body.credentialId
-					.replace(/\-/g, '+')
+					.replace(/-/g, '+')
 					.replace(/_/g, '/'),
 					'base64'
 			).toString('hex')
@@ -161,7 +162,44 @@ export default async (ctx: Koa.BaseContext) => {
 			});
 			return;
 		}
+	} else {
+		const keys = await UserSecurityKeys.find({
+			userId: user.id
+		});
+
+		if (keys.length === 0) {
+			await fail(403, {
+				error: 'no keys found'
+			});
+		}
+
+		// 32 byte challenge
+		const challenge = randomBytes(32).toString('base64')
+			.replace(/=/g, '')
+			.replace(/\+/g, '-')
+			.replace(/\//g, '_');
+
+		const challengeId = genId();
+
+		await AttestationChallenges.save({
+			userId: user.id,
+			id: challengeId,
+			challenge: hash(Buffer.from(challenge, 'utf-8')).toString('hex'),
+			createdAt: new Date(),
+			registrationChallenge: false
+		});
+
+		ctx.body = {
+			challenge,
+			challengeId,
+			securityKeys: keys.map(key => ({
+				id: key.id
+			}))
+		};
+		ctx.status = 200;
+		return;
 	}
 
 	await fail();
+	return;
 };