Skip to content

Commit

Permalink
Add additional check to Two-Factor Authentication (by @masterwendu) (d…
Browse files Browse the repository at this point in the history 
…irectus#6187)

* 6113 add tfa generate endpoint

* 6113 implement tfa otp check in user settings

* 6113 add autfocus for otp field on login form

* update package-lock

* improve TFA uri with user email

* fix vue 3 compatibility

* reduce code duplication for enableTFA

* Remove unnecessary parameters

* Use project name in otp url when available

* Update docs/reference/api/system/users.md

Co-authored-by: Wendelin Peleska <wendu@pm.me>
  • Loading branch information
@rijkvanzanten
rijkvanzanten and Wendelin Peleska authored Jun 10, 2021
1 parent f55a207 commit 4cb8d1f
Show file tree
Hide file tree
Showing 7 changed files with 238 additions and 54 deletions.
31 changes: 29 additions & 2 deletions api/src/controllers/users.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,7 @@ router.post(
);

router.post(
'/me/tfa/enable/',
'/me/tfa/generate/',
asyncHandler(async (req, res, next) => {
if (!req.accountability?.user) {
throw new InvalidCredentialsException();
Expand All @@ -317,14 +317,41 @@ router.post(
});
await authService.verifyPassword(req.accountability.user, req.body.password);

const { url, secret } = await service.enableTFA(req.accountability.user);
const { url, secret } = await service.generateTFA(req.accountability.user);

res.locals.payload = { data: { secret, otpauth_url: url } };
return next();
}),
respond
);

router.post(
'/me/tfa/enable/',
asyncHandler(async (req, res, next) => {
if (!req.accountability?.user) {
throw new InvalidCredentialsException();
}

if (!req.body.secret) {
throw new InvalidPayloadException(`"secret" is required`);
}

if (!req.body.otp) {
throw new InvalidPayloadException(`"otp" is required`);
}

const service = new UsersService({
accountability: req.accountability,
schema: req.schema,
});

await service.enableTFA(req.accountability.user, req.body.otp, req.body.secret);

return next();
}),
respond
);

router.post(
'/me/tfa/disable',
asyncHandler(async (req, res, next) => {
Expand Down
23 changes: 14 additions & 9 deletions api/src/services/authentication.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -248,20 +248,25 @@ export class AuthenticationService {
}

async generateOTPAuthURL(pk: string, secret: string): Promise<string> {
const user = await this.knex.select('first_name', 'last_name').from('directus_users').where({ id: pk }).first();
const name = `${user.first_name} ${user.last_name}`;
return authenticator.keyuri(name, 'Directus', secret);
const user = await this.knex.select('email').from('directus_users').where({ id: pk }).first();
const project = await this.knex.select('project_name').from('directus_settings').limit(1).first();
return authenticator.keyuri(user.email, project?.project_name || 'Directus', secret);
}

async verifyOTP(pk: string, otp: string): Promise<boolean> {
const user = await this.knex.select('tfa_secret').from('directus_users').where({ id: pk }).first();
async verifyOTP(pk: string, otp: string, secret?: string): Promise<boolean> {
let tfaSecret: string;
if (!secret) {
const user = await this.knex.select('tfa_secret').from('directus_users').where({ id: pk }).first();

if (!user.tfa_secret) {
throw new InvalidPayloadException(`User "${pk}" doesn't have TFA enabled.`);
if (!user.tfa_secret) {
throw new InvalidPayloadException(`User "${pk}" doesn't have TFA enabled.`);
}
tfaSecret = user.tfa_secret;
} else {
tfaSecret = secret;
}

const secret = user.tfa_secret;
return authenticator.check(otp, secret);
return authenticator.check(otp, tfaSecret);
}

async verifyPassword(pk: string, password: string): Promise<boolean> {
Expand Down
23 changes: 20 additions & 3 deletions api/src/services/graphql.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1481,9 +1481,9 @@ export class GraphQLService {
return true;
},
},
users_me_tfa_enable: {
users_me_tfa_generate: {
type: new GraphQLObjectType({
name: 'users_me_tfa_enable_data',
name: 'users_me_tfa_generate_data',
fields: {
secret: { type: GraphQLString },
otpauth_url: { type: GraphQLString },
Expand All @@ -1503,10 +1503,27 @@ export class GraphQLService {
schema: this.schema,
});
await authService.verifyPassword(this.accountability.user, args.password);
const { url, secret } = await service.enableTFA(this.accountability.user);
const { url, secret } = await service.generateTFA(this.accountability.user);
return { secret, otpauth_url: url };
},
},
users_me_tfa_enable: {
type: GraphQLBoolean,
args: {
otp: GraphQLNonNull(GraphQLString),
secret: GraphQLNonNull(GraphQLString),
},
resolve: async (_, args) => {
if (!this.accountability?.user) return null;
const service = new UsersService({
accountability: this.accountability,
schema: this.schema,
});

await service.enableTFA(this.accountability.user, args.otp, args.secret);
return true;
},
},
users_me_tfa_disable: {
type: GraphQLBoolean,
args: {
Expand Down
29 changes: 26 additions & 3 deletions api/src/services/users.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
ForbiddenException,
InvalidPayloadException,
UnprocessableEntityException,
InvalidCredentialsException,
} from '../exceptions';
import { RecordNotUniqueException } from '../exceptions/database/record-not-unique';
import logger from '../logger';
Expand Down Expand Up @@ -347,7 +348,7 @@ export class UsersService extends ItemsService {
}
}

async enableTFA(pk: string): Promise<Record<string, string>> {
async generateTFA(pk: string): Promise<Record<string, string>> {
const user = await this.knex.select('tfa_secret').from('directus_users').where({ id: pk }).first();

if (user?.tfa_secret !== null) {
Expand All @@ -361,14 +362,36 @@ export class UsersService extends ItemsService {
});
const secret = authService.generateTFASecret();

await this.knex('directus_users').update({ tfa_secret: secret }).where({ id: pk });

return {
secret,
url: await authService.generateOTPAuthURL(pk, secret),
};
}

async enableTFA(pk: string, otp: string, secret: string): Promise<void> {
const authService = new AuthenticationService({
schema: this.schema,
});

if (!pk) {
throw new InvalidCredentialsException();
}

const otpValid = await authService.verifyOTP(pk, otp, secret);

if (otpValid === false) {
throw new InvalidPayloadException(`"otp" is invalid`);
}

const userSecret = await this.knex.select('tfa_secret').from('directus_users').where({ id: pk }).first();

if (userSecret?.tfa_secret !== null) {
throw new InvalidPayloadException('TFA Secret is already set for this user');
}

await this.knex('directus_users').update({ tfa_secret: secret }).where({ id: pk });
}

async disableTFA(pk: string): Promise<void> {
await this.knex('directus_users').update({ tfa_secret: null }).where({ id: pk });
}
Expand Down
103 changes: 72 additions & 31 deletions app/src/interfaces/_system/system-mfa-setup/system-mfa-setup.vue
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@
</template>
</v-checkbox>

<v-dialog persistent v-model="enableActive" @esc="enableActive = false">
<v-dialog persistent v-model="enableActive" @esc="cancelAndClose">
<v-card>
<template v-if="tfaEnabled === false && loading === false">
<form @submit.prevent="generateTFA" v-if="tfaEnabled === false && tfaGenerated === false && loading === false">
<v-card-title>
{{ t('enter_password_to_enable_tfa') }}
</v-card-title>
Expand All @@ -20,42 +20,49 @@
<v-error v-if="error" :error="error" />
</v-card-text>
<v-card-actions>
<v-button @click="enableActive = false" secondary>{{ t('cancel') }}</v-button>
<v-button @click="enableTFA" :loading="loading">{{ t('next') }}</v-button>
<v-button type="button" @click="cancelAndClose" secondary>{{ t('cancel') }}</v-button>
<v-button type="submit" :loading="loading">{{ t('next') }}</v-button>
</v-card-actions>
</template>
</form>

<v-progress-circular class="loader" indeterminate v-else-if="loading === true" />

<div v-show="tfaEnabled && loading === false">
<div v-show="tfaEnabled === false && tfaGenerated === true && loading === false">
<form @submit.prevent="enableTFA">
<v-card-title>
{{ t('tfa_scan_code') }}
</v-card-title>
<v-card-text>
<canvas class="qr" :id="canvasID" />
<output class="secret selectable">{{ secret }}</output>
<v-input type="text" :placeholder="t('otp')" v-model="otp" :nullable="false" />
<v-error v-if="error" :error="error" />
</v-card-text>
<v-card-actions>
<v-button type="button" @click="cancelAndClose" secondary>{{ t('cancel') }}</v-button>
<v-button type="submit" @click="enableTFA" :disabled="otp.length !== 6">{{ t('done') }}</v-button>
</v-card-actions>
</form>
</div>
</v-card>
</v-dialog>

<v-dialog v-model="disableActive" @esc="disableActive = false">
<v-card>
<form @submit.prevent="disableTFA">
<v-card-title>
{{ t('tfa_scan_code') }}
{{ t('enter_otp_to_disable_tfa') }}
</v-card-title>
<v-card-text>
<canvas class="qr" :id="canvasID" />
<output class="secret selectable">{{ secret }}</output>
<v-input type="text" :placeholder="t('otp')" v-model="otp" :nullable="false" />
<v-error v-if="error" :error="error" />
</v-card-text>
<v-card-actions>
<v-button @click="enableActive = false">{{ t('done') }}</v-button>
<v-button type="submit" class="disable" :loading="loading" :disabled="otp.length !== 6">
{{ t('disable_tfa') }}
</v-button>
</v-card-actions>
</div>
</v-card>
</v-dialog>

<v-dialog v-model="disableActive">
<v-card>
<v-card-title>
{{ t('enter_otp_to_disable_tfa') }}
</v-card-title>
<v-card-text>
<v-input type="text" :placeholder="t('otp')" v-model="otp" :nullable="false" />
<v-error v-if="error" :error="error" />
</v-card-text>
<v-card-actions>
<v-button class="disable" :loading="loading" @click="disableTFA" :disabled="otp.length !== 6">
{{ t('disable_tfa') }}
</v-button>
</v-card-actions>
</form>
</v-card>
</v-dialog>
</div>
Expand Down Expand Up @@ -85,6 +92,7 @@ export default defineComponent({
const userStore = useUserStore();
const tfaEnabled = ref(!!props.value);
const tfaGenerated = ref(false);
const enableActive = ref(false);
const disableActive = ref(false);
const loading = ref(false);
Expand All @@ -110,6 +118,9 @@ export default defineComponent({
return {
t,
tfaEnabled,
tfaGenerated,
generateTFA,
cancelAndClose,
enableTFA,
toggle,
password,
Expand All @@ -132,17 +143,46 @@ export default defineComponent({
}
}
async function enableTFA() {
async function generateTFA() {
if (loading.value === true) return;
loading.value = true;
try {
const response = await api.post('/users/me/tfa/enable', { password: password.value });
const response = await api.post('/users/me/tfa/generate', { password: password.value });
const url = response.data.data.otpauth_url;
secret.value = response.data.data.secret;
await qrcode.toCanvas(document.getElementById(canvasID), url);
tfaGenerated.value = true;
error.value = null;
} catch (err) {
error.value = err;
} finally {
loading.value = false;
}
}
function cancelAndClose() {
tfaGenerated.value = false;
enableActive.value = false;
password.value = '';
otp.value = '';
secret.value = '';
}
async function enableTFA() {
if (loading.value === true) return;
loading.value = true;
try {
await api.post('/users/me/tfa/enable', { otp: otp.value, secret: secret.value });
tfaEnabled.value = true;
tfaGenerated.value = false;
enableActive.value = false;
password.value = '';
otp.value = '';
secret.value = '';
error.value = null;
} catch (err) {
error.value = err;
Expand All @@ -159,6 +199,7 @@ export default defineComponent({
tfaEnabled.value = false;
disableActive.value = false;
otp.value = '';
} catch (err) {
error.value = err;
} finally {
Expand Down Expand Up @@ -189,7 +230,7 @@ export default defineComponent({
.secret {
display: block;
margin: 0 auto;
margin: 0 auto 16px auto;
color: var(--foreground-subdued);
font-family: var(--family-monospace);
letter-spacing: 2.6px;
Expand Down
2 changes: 1 addition & 1 deletion app/src/routes/login/components/login-form/login-form.vue
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
<v-input type="password" autocomplete="current-password" v-model="password" :placeholder="t('password')" />

<transition-expand>
<v-input type="text" :placeholder="t('otp')" v-if="requiresTFA" v-model="otp" />
<v-input type="text" :placeholder="t('otp')" v-if="requiresTFA" v-model="otp" autofocus />
</transition-expand>

<v-notice type="warning" v-if="error">
Expand Down
Loading

0 comments on commit 4cb8d1f

Please sign in to comment.