From 37762358fca7337d446ff476db398b12e0e60939 Mon Sep 17 00:00:00 2001 From: Nitwel Date: Wed, 15 Jun 2022 19:27:59 +0200 Subject: [PATCH] Add tfa enforce flow (#7805) * add tfa enforce flow * add 'tfa-secret' to recommended permissions * fix if theme if user has dark mode * oas: rename 'enable-2fa' to 'enable-tfa' * Add required user fields * Uniformize styling * Fix direct and invalid routing * Add required permission docs * Fix typescript warnings * Fix typescript warnings 2 * Allow auto theme * Nest duplicate condition check * Fix routing for users without role * Follow page redirects * Reduce the use of redirect query * Improve error UX * Allow admins to disable 2FA * Improve autofocus UX * Override update permission for 'tfa_secret' when role enforces TFA * Remove permission requirements from docs Co-authored-by: Jose Varela Co-authored-by: rijkvanzanten Co-authored-by: ian --- api/src/controllers/users.ts | 87 +++++++++- app/src/app.vue | 15 +- app/src/composables/use-tfa-setup.ts | 117 +++++++++++++ .../system-mfa-setup/system-mfa-setup.vue | 158 +++++++++--------- app/src/lang/translations/en-US.yaml | 2 + .../settings/routes/roles/app-permissions.ts | 1 + app/src/router.ts | 54 ++++-- app/src/routes/tfa-setup/index.ts | 4 + app/src/routes/tfa-setup/tfa-setup.vue | 136 +++++++++++++++ app/src/stores/user.ts | 2 + packages/shared/src/types/users.ts | 4 +- packages/specs/src/paths/roles/role.yaml | 2 +- packages/specs/src/paths/roles/roles.yaml | 2 +- 13 files changed, 479 insertions(+), 105 deletions(-) create mode 100644 app/src/composables/use-tfa-setup.ts create mode 100644 app/src/routes/tfa-setup/index.ts create mode 100644 app/src/routes/tfa-setup/tfa-setup.vue diff --git a/api/src/controllers/users.ts b/api/src/controllers/users.ts index b9c91d2673..2f6f7436cf 100644 --- a/api/src/controllers/users.ts +++ b/api/src/controllers/users.ts @@ -4,9 +4,10 @@ import { InvalidCredentialsException, ForbiddenException, InvalidPayloadExceptio import { respond } from '../middleware/respond'; import useCollection from '../middleware/use-collection'; import { validateBatch } from '../middleware/validate-batch'; -import { AuthenticationService, MetaService, UsersService, TFAService } from '../services'; +import { AuthenticationService, MetaService, UsersService, RolesService, TFAService } from '../services'; import { PrimaryKey } from '../types'; import asyncHandler from '../utils/async-handler'; +import { Role } from '@directus/shared/types'; const router = express.Router(); @@ -354,6 +355,37 @@ router.post( throw new InvalidPayloadException(`"otp" is required`); } + // Override permissions only when enforce TFA is enabled in role + if (req.accountability.role) { + const rolesService = new RolesService({ + schema: req.schema, + }); + const role = (await rolesService.readOne(req.accountability.role)) as Role; + + if (role && role.enforce_tfa) { + const existingPermission = await req.accountability.permissions?.find( + (p) => p.collection === 'directus_users' && p.action === 'update' + ); + + if (existingPermission) { + existingPermission.fields = ['tfa_secret']; + existingPermission.permissions = { id: { _eq: req.accountability.user } }; + existingPermission.presets = null; + existingPermission.validation = null; + } else { + (req.accountability.permissions || (req.accountability.permissions = [])).push({ + action: 'update', + collection: 'directus_users', + fields: ['tfa_secret'], + permissions: { id: { _eq: req.accountability.user } }, + presets: null, + role: req.accountability.role, + validation: null, + }); + } + } + } + const service = new TFAService({ accountability: req.accountability, schema: req.schema, @@ -377,6 +409,37 @@ router.post( throw new InvalidPayloadException(`"otp" is required`); } + // Override permissions only when enforce TFA is enabled in role + if (req.accountability.role) { + const rolesService = new RolesService({ + schema: req.schema, + }); + const role = (await rolesService.readOne(req.accountability.role)) as Role; + + if (role && role.enforce_tfa) { + const existingPermission = await req.accountability.permissions?.find( + (p) => p.collection === 'directus_users' && p.action === 'update' + ); + + if (existingPermission) { + existingPermission.fields = ['tfa_secret']; + existingPermission.permissions = { id: { _eq: req.accountability.user } }; + existingPermission.presets = null; + existingPermission.validation = null; + } else { + (req.accountability.permissions || (req.accountability.permissions = [])).push({ + action: 'update', + collection: 'directus_users', + fields: ['tfa_secret'], + permissions: { id: { _eq: req.accountability.user } }, + presets: null, + role: req.accountability.role, + validation: null, + }); + } + } + } + const service = new TFAService({ accountability: req.accountability, schema: req.schema, @@ -394,4 +457,26 @@ router.post( respond ); +router.post( + '/:pk/tfa/disable', + asyncHandler(async (req, _res, next) => { + if (!req.accountability?.user) { + throw new InvalidCredentialsException(); + } + + if (!req.accountability.admin || !req.params.pk) { + throw new ForbiddenException(); + } + + const service = new TFAService({ + accountability: req.accountability, + schema: req.schema, + }); + + await service.disableTFA(req.params.pk); + return next(); + }), + respond +); + export default router; diff --git a/app/src/app.vue b/app/src/app.vue index 07d67e4c86..b20d92cad5 100644 --- a/app/src/app.vue +++ b/app/src/app.vue @@ -22,12 +22,13 @@ + + diff --git a/app/src/stores/user.ts b/app/src/stores/user.ts index 2c3c11cbe1..55beec8535 100644 --- a/app/src/stores/user.ts +++ b/app/src/stores/user.ts @@ -42,10 +42,12 @@ export const useUserStore = defineStore({ 'email', 'last_page', 'theme', + 'tfa_secret', 'avatar.id', 'role.admin_access', 'role.app_access', 'role.id', + 'role.enforce_tfa', ]; const { data } = await api.get(`/users/me`, { params: { fields } }); diff --git a/packages/shared/src/types/users.ts b/packages/shared/src/types/users.ts index be9711413d..ba14b8355d 100644 --- a/packages/shared/src/types/users.ts +++ b/packages/shared/src/types/users.ts @@ -3,7 +3,7 @@ export type Role = { name: string; description: string; icon: string; - enforce_2fa: null | boolean; + enforce_tfa: null | boolean; external_id: null | string; ip_whitelist: string[]; app_access: boolean; @@ -27,7 +27,7 @@ export type User = { last_login: string; last_page: string; external_id: string; - '2fa_secret': string; + tfa_secret: string; theme: 'auto' | 'dark' | 'light'; role: Role; password_reset_token: string | null; diff --git a/packages/specs/src/paths/roles/role.yaml b/packages/specs/src/paths/roles/role.yaml index ae4a3377db..a43b87e06b 100644 --- a/packages/specs/src/paths/roles/role.yaml +++ b/packages/specs/src/paths/roles/role.yaml @@ -37,7 +37,7 @@ patch: description: description: Description of the role. type: string - enforce_2fa: + enforce_tfa: description: Whether or not this role enforces the use of 2FA. type: boolean external_id: diff --git a/packages/specs/src/paths/roles/roles.yaml b/packages/specs/src/paths/roles/roles.yaml index bd69e4e6c2..4288bafb05 100644 --- a/packages/specs/src/paths/roles/roles.yaml +++ b/packages/specs/src/paths/roles/roles.yaml @@ -48,7 +48,7 @@ post: description: description: Description of the role. type: string - enforce_2fa: + enforce_tfa: description: Whether or not this role enforces the use of 2FA. type: boolean external_id: