From 54f5081e6946cf172a753ee5689b6cb406c21d36 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Varela?= Date: Tue, 11 Apr 2023 20:49:26 +0100 Subject: [PATCH] Users: Verify JWT on accept invitation (#16711) * Adds ability to verify JWT with meaningful errors * Fix tests * Apply verify JWT to accept invitation * Update per review * Add joselcvarela to contributors He's a core team member; already signed the CLA outside of GH --------- Co-authored-by: Brainslug Co-authored-by: rijkvanzanten --- api/src/services/users.ts | 5 +++-- api/src/utils/jwt.test.ts | 45 ++++++++++++++++++++++++++++++++------- api/src/utils/jwt.ts | 8 +++++-- contributors.yml | 1 + 4 files changed, 47 insertions(+), 12 deletions(-) diff --git a/api/src/services/users.ts b/api/src/services/users.ts index 9382884965..faf3f7595d 100644 --- a/api/src/services/users.ts +++ b/api/src/services/users.ts @@ -6,10 +6,11 @@ import { cloneDeep } from 'lodash-es'; import { performance } from 'perf_hooks'; import getDatabase from '../database/index.js'; import env from '../env.js'; -import { ForbiddenException, InvalidPayloadException, UnprocessableEntityException } from '../exceptions/index.js'; import { RecordNotUniqueException } from '../exceptions/database/record-not-unique.js'; +import { ForbiddenException, InvalidPayloadException, UnprocessableEntityException } from '../exceptions/index.js'; import type { AbstractServiceOptions, Item, MutationOptions, PrimaryKey } from '../types/index.js'; import isUrlAllowed from '../utils/is-url-allowed.js'; +import { verifyJWT } from '../utils/jwt.js'; import { stall } from '../utils/stall.js'; import { Url } from '../utils/url.js'; import { ItemsService } from './items.js'; @@ -349,7 +350,7 @@ export class UsersService extends ItemsService { } async acceptInvite(token: string, password: string): Promise { - const { email, scope } = jwt.verify(token, env['SECRET'] as string, { issuer: 'directus' }) as { + const { email, scope } = verifyJWT(token, env['SECRET'] as string) as { email: string; scope: string; }; diff --git a/api/src/utils/jwt.test.ts b/api/src/utils/jwt.test.ts index 5bb5590e86..df7ff875b8 100644 --- a/api/src/utils/jwt.test.ts +++ b/api/src/utils/jwt.test.ts @@ -6,7 +6,7 @@ import { TokenExpiredException, } from '../../src/exceptions/index.js'; import type { DirectusTokenPayload } from '../../src/types/index.js'; -import { verifyAccessJWT } from '../../src/utils/jwt.js'; +import { verifyAccessJWT, verifyJWT } from '../../src/utils/jwt.js'; const payload: DirectusTokenPayload = { role: null, app_access: false, admin_access: false }; const secret = 'test-secret'; @@ -14,33 +14,62 @@ const options = { issuer: 'directus' }; test('Returns the payload of a correctly signed token', () => { const token = jwt.sign(payload, secret, options); - const result = verifyAccessJWT(token, secret); - expect(result).toEqual(payload); + const result = verifyJWT(token, secret); + + expect(result['admin_access']).toEqual(payload.admin_access); + expect(result['app_access']).toEqual(payload.app_access); + expect(result['role']).toEqual(payload.role); + expect(result['iss']).toBe('directus'); + expect(result['iat']).toBeTypeOf('number'); }); test('Throws TokenExpiredException when token used has expired', () => { const token = jwt.sign({ ...payload, exp: new Date().getTime() / 1000 - 500 }, secret, options); - expect(() => verifyAccessJWT(token, secret)).toThrow(TokenExpiredException); + expect(() => verifyJWT(token, secret)).toThrow(TokenExpiredException); }); const InvalidTokenCases = { 'wrong issuer': jwt.sign(payload, secret, { issuer: 'wrong' }), 'wrong secret': jwt.sign(payload, 'wrong-secret', options), 'string payload': jwt.sign('illegal payload', secret), - 'missing properties in token payload': jwt.sign({ role: null }, secret, options), }; Object.entries(InvalidTokenCases).forEach(([title, token]) => test(`Throws InvalidTokenError - ${title}`, () => { - expect(() => verifyAccessJWT(token, secret)).toThrow(InvalidTokenException); + expect(() => verifyJWT(token, secret)).toThrow(InvalidTokenException); }) ); test(`Throws ServiceUnavailableException for unexpected error from jsonwebtoken`, () => { - vi.spyOn(jwt, 'verify').mockImplementation(() => { + const mock = vi.spyOn(jwt, 'verify').mockImplementation(() => { throw new Error(); }); const token = jwt.sign(payload, secret, options); - expect(() => verifyAccessJWT(token, secret)).toThrow(ServiceUnavailableException); + expect(() => verifyJWT(token, secret)).toThrow(ServiceUnavailableException); + mock.mockRestore(); +}); + +const RequiredEntries: Array = ['role', 'app_access', 'admin_access']; + +RequiredEntries.forEach((entry) => { + test(`Throws InvalidTokenException if ${entry} not defined`, () => { + const { [entry]: _entryName, ...rest } = payload; + const token = jwt.sign(rest, secret, options); + expect(() => verifyAccessJWT(token, secret)).toThrow(InvalidTokenException); + }); +}); + +test('Returns the payload of an access token', () => { + const payload = { id: 1, role: 1, app_access: true, admin_access: true }; + const token = jwt.sign(payload, secret, options); + const result = verifyAccessJWT(token, secret); + expect(result).toEqual({ + id: 1, + role: 1, + app_access: true, + admin_access: true, + share: undefined, + share_scope: undefined, + }); }); diff --git a/api/src/utils/jwt.ts b/api/src/utils/jwt.ts index 21929bc990..aa9a7f50a0 100644 --- a/api/src/utils/jwt.ts +++ b/api/src/utils/jwt.ts @@ -2,7 +2,7 @@ import jwt from 'jsonwebtoken'; import { InvalidTokenException, ServiceUnavailableException, TokenExpiredException } from '../exceptions/index.js'; import type { DirectusTokenPayload } from '../types/index.js'; -export function verifyAccessJWT(token: string, secret: string): DirectusTokenPayload { +export function verifyJWT(token: string, secret: string): Record { let payload; try { @@ -19,7 +19,11 @@ export function verifyAccessJWT(token: string, secret: string): DirectusTokenPay } } - const { id, role, app_access, admin_access, share, share_scope } = payload; + return payload; +} + +export function verifyAccessJWT(token: string, secret: string): DirectusTokenPayload { + const { id, role, app_access, admin_access, share, share_scope } = verifyJWT(token, secret); if (role === undefined || app_access === undefined || admin_access === undefined) { throw new InvalidTokenException('Invalid token payload.'); diff --git a/contributors.yml b/contributors.yml index e36880efb0..34e6e3fe14 100644 --- a/contributors.yml +++ b/contributors.yml @@ -11,3 +11,4 @@ - u12206050 - that1matt - jaads +- joselcvarela