From 5c9816b5b87b538c4962ab1cb656945440e76014 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Thu, 22 Aug 2024 18:04:18 +0800 Subject: [PATCH 1/3] store verification token in redis with exp and destroy after use --- packages/api/src/resolvers/types.ts | 1 + packages/api/src/routers/auth/auth_router.ts | 22 +++-------- packages/api/src/services/send_emails.ts | 9 +++-- packages/api/src/utils/auth.ts | 41 +++++++++++++++++--- packages/api/test/routers/auth.test.ts | 20 +++++----- 5 files changed, 58 insertions(+), 35 deletions(-) diff --git a/packages/api/src/resolvers/types.ts b/packages/api/src/resolvers/types.ts index 47fa91954..6cf19be02 100644 --- a/packages/api/src/resolvers/types.ts +++ b/packages/api/src/resolvers/types.ts @@ -24,6 +24,7 @@ export interface Claims { exp?: number email?: string system?: boolean + destroyAfterUse?: boolean } export type ClaimsToSet = { diff --git a/packages/api/src/routers/auth/auth_router.ts b/packages/api/src/routers/auth/auth_router.ts index 7cb940e78..c34b628ab 100644 --- a/packages/api/src/routers/auth/auth_router.ts +++ b/packages/api/src/routers/auth/auth_router.ts @@ -29,12 +29,13 @@ import { import { analytics } from '../../utils/analytics' import { comparePassword, - getClaimsByToken, hashPassword, setAuthInCookie, + verifyToken, } from '../../utils/auth' import { corsConfig } from '../../utils/corsConfig' import { logger } from '../../utils/logger' +import { DEFAULT_HOME_PATH } from '../../utils/navigation' import { hourlyLimiter } from '../../utils/rate_limit' import { verifyChallengeRecaptcha } from '../../utils/recaptcha' import { createSsoToken, ssoRedirectURL } from '../../utils/sso' @@ -48,7 +49,6 @@ import { } from './google_auth' import { createWebAuthToken } from './jwt_helpers' import { createMobileAccountCreationResponse } from './mobile/account_creation' -import { DEFAULT_HOME_PATH } from '../../utils/navigation' export interface SignupRequest { email: string @@ -582,13 +582,7 @@ export function authRouter() { try { // verify token - const claims = await getClaimsByToken(token) - if (!claims) { - return res.redirect( - `${env.client.url}/auth/confirm-email?errorCodes=INVALID_TOKEN` - ) - } - + const claims = await verifyToken(token) const user = await getRepository(User).findOneBy({ id: claims.uid }) if (!user) { return res.redirect( @@ -710,20 +704,14 @@ export function authRouter() { const { token, password } = req.body try { - // verify token - const claims = await getClaimsByToken(token) - if (!claims) { - return res.redirect( - `${env.client.url}/auth/reset-password/${token}?errorCodes=INVALID_TOKEN` - ) - } - if (!password || password.length < 8) { return res.redirect( `${env.client.url}/auth/reset-password/${token}?errorCodes=INVALID_PASSWORD` ) } + // verify token + const claims = await verifyToken(token) const user = await getRepository(User).findOneBy({ id: claims.uid, }) diff --git a/packages/api/src/services/send_emails.ts b/packages/api/src/services/send_emails.ts index 988ed9183..f1138cc3a 100644 --- a/packages/api/src/services/send_emails.ts +++ b/packages/api/src/services/send_emails.ts @@ -10,7 +10,7 @@ export const sendNewAccountVerificationEmail = async (user: { email: string }): Promise => { // generate confirmation link - const token = generateVerificationToken({ id: user.id }) + const token = await generateVerificationToken({ id: user.id }) const link = `${env.client.url}/auth/confirm-email/${token}` // send email const dynamicTemplateData = { @@ -71,7 +71,10 @@ export const sendAccountChangeEmail = async (user: { email: string }): Promise => { // generate verification link - const token = generateVerificationToken({ id: user.id, email: user.email }) + const token = await generateVerificationToken({ + id: user.id, + email: user.email, + }) const link = `${env.client.url}/auth/reset-password/${token}` // send email const dynamicTemplateData = { @@ -94,7 +97,7 @@ export const sendPasswordResetEmail = async (user: { email: string }): Promise => { // generate link - const token = generateVerificationToken({ id: user.id }) + const token = await generateVerificationToken({ id: user.id }) const link = `${env.client.url}/auth/reset-password/${token}` // send email const dynamicTemplateData = { diff --git a/packages/api/src/utils/auth.ts b/packages/api/src/utils/auth.ts index 2dbc56d1f..67d62b657 100644 --- a/packages/api/src/utils/auth.ts +++ b/packages/api/src/utils/auth.ts @@ -6,6 +6,7 @@ import { promisify } from 'util' import { v4 as uuidv4 } from 'uuid' import { ApiKey } from '../entity/api_key' import { env } from '../env' +import { redisDataSource } from '../redis_data_source' import { getRepository } from '../repository' import { Claims, ClaimsToSet } from '../resolvers/types' import { logger } from './logger' @@ -89,22 +90,52 @@ export const getClaimsByToken = async ( } } -export const generateVerificationToken = ( +const verificationTokenKey = (token: string) => `verification:${token}` + +export const verifyToken = async (token: string): Promise => { + const redisClient = redisDataSource.redisClient + const key = verificationTokenKey(token) + if (redisClient) { + const cachedToken = await redisClient.get(key) + if (!cachedToken) { + throw new Error('Token not found') + } + } + + const claims = jwt.verify(token, env.server.jwtSecret) as Claims + if (claims.destroyAfterUse) { + await redisClient?.del(key) + } + + return claims +} + +export const generateVerificationToken = async ( user: { id: string email?: string }, - expireInSeconds = 60 * 60 * 24 // 1 day -): string => { + expireInSeconds = 60, // 1 minute + destroyAfterUse = true +): Promise => { const iat = Math.floor(Date.now() / 1000) const exp = Math.floor( new Date(Date.now() + expireInSeconds * 1000).getTime() / 1000 ) - return jwt.sign( - { uid: user.id, iat, exp, email: user.email }, + const token = jwt.sign( + { uid: user.id, iat, exp, email: user.email, destroyAfterUse }, env.server.jwtSecret ) + + await redisDataSource.redisClient?.set( + verificationTokenKey(token), + user.id, + 'EX', + expireInSeconds + ) + + return token } export const setAuthInCookie = async ( diff --git a/packages/api/test/routers/auth.test.ts b/packages/api/test/routers/auth.test.ts index e7f93c5b1..b144f77f9 100644 --- a/packages/api/test/routers/auth.test.ts +++ b/packages/api/test/routers/auth.test.ts @@ -258,8 +258,8 @@ describe('auth router', () => { }) context('when token is valid', () => { - before(() => { - token = generateVerificationToken({ id: user.id }) + before(async () => { + token = await generateVerificationToken({ id: user.id }) }) it('set auth token in cookie', async () => { @@ -292,8 +292,8 @@ describe('auth router', () => { }) context('when token is expired', () => { - before(() => { - token = generateVerificationToken({ id: user.id }, -1) + before(async () => { + token = await generateVerificationToken({ id: user.id }, -1) }) it('redirects to confirm-email page with error code TokenExpired', async () => { @@ -305,9 +305,9 @@ describe('auth router', () => { }) context('when user is not found', () => { - before(() => { + before(async () => { const nonExistsUserId = generateFakeUuid() - token = generateVerificationToken({ id: nonExistsUserId }) + token = await generateVerificationToken({ id: nonExistsUserId }) }) it('redirects to confirm-email page with error code UserNotFound', async () => { @@ -419,8 +419,8 @@ describe('auth router', () => { }) context('when token is valid', () => { - before(() => { - token = generateVerificationToken({ id: user.id }) + before(async () => { + token = await generateVerificationToken({ id: user.id }) }) context('when password is not empty', () => { @@ -464,8 +464,8 @@ describe('auth router', () => { }) context('when token is expired', () => { - before(() => { - token = generateVerificationToken({ id: user.id }, -1) + before(async () => { + token = await generateVerificationToken({ id: user.id }, -1) }) it('redirects to reset-password page with error code ExpiredToken', async () => { From 69cb7825a02c298270a84543ec27becb3dd2d4e1 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Thu, 22 Aug 2024 18:33:33 +0800 Subject: [PATCH 2/3] cache api key and invalidate it once api key is revoked --- packages/api/src/resolvers/api_key/index.ts | 8 ++- packages/api/src/utils/auth.ts | 63 +++++++++++++++------ 2 files changed, 54 insertions(+), 17 deletions(-) diff --git a/packages/api/src/resolvers/api_key/index.ts b/packages/api/src/resolvers/api_key/index.ts index a0604f91f..04d11e352 100644 --- a/packages/api/src/resolvers/api_key/index.ts +++ b/packages/api/src/resolvers/api_key/index.ts @@ -16,7 +16,11 @@ import { import { getRepository } from '../../repository' import { findApiKeys } from '../../services/api_key' import { analytics } from '../../utils/analytics' -import { generateApiKey, hashApiKey } from '../../utils/auth' +import { + deleteCachedClaims, + generateApiKey, + hashApiKey, +} from '../../utils/auth' import { authorized } from '../../utils/gql-utils' export const apiKeysResolver = authorized( @@ -92,6 +96,8 @@ export const revokeApiKeyResolver = authorized< const deletedApiKey = await apiRepo.remove(apiKey) + await deleteCachedClaims(deletedApiKey.key) + analytics.capture({ distinctId: uid, event: 'api_key_revoked', diff --git a/packages/api/src/utils/auth.ts b/packages/api/src/utils/auth.ts index 67d62b657..4bd4b21f3 100644 --- a/packages/api/src/utils/auth.ts +++ b/packages/api/src/utils/auth.ts @@ -3,13 +3,12 @@ import crypto from 'crypto' import express from 'express' import * as jwt from 'jsonwebtoken' import { promisify } from 'util' -import { v4 as uuidv4 } from 'uuid' +import { v4 as uuidv4, validate } from 'uuid' import { ApiKey } from '../entity/api_key' import { env } from '../env' import { redisDataSource } from '../redis_data_source' import { getRepository } from '../repository' import { Claims, ClaimsToSet } from '../resolvers/types' -import { logger } from './logger' export const OmnivoreAuthorizationHeader = 'Omnivore-Authorization' @@ -24,17 +23,20 @@ export const comparePassword = async (password: string, hash: string) => { } export const generateApiKey = (): string => { - // TODO: generate random string key + // generate random string key return uuidv4() } +export const isApiKey = (key: string): boolean => { + // check if key in is uuid v4 format + return validate(key) +} + export const hashApiKey = (apiKey: string) => { return crypto.createHash('sha256').update(apiKey).digest('hex') } -export const claimsFromApiKey = async (key: string): Promise => { - const hashedKey = hashApiKey(key) - +export const claimsFromApiKey = async (hashedKey: string): Promise => { const apiKeyRepo = getRepository(ApiKey) const apiKey = await apiKeyRepo @@ -64,6 +66,34 @@ export const claimsFromApiKey = async (key: string): Promise => { } } +const claimsCacheKey = (hashedKey: string) => `api-key-hash:${hashedKey}` + +const getCachedClaims = async ( + hashedKey: string +): Promise => { + const cache = await redisDataSource.redisClient?.get( + claimsCacheKey(hashedKey) + ) + if (!cache) { + return undefined + } + + return JSON.parse(cache) as Claims +} + +const cacheClaims = async (hashedKey: string, claims: Claims) => { + await redisDataSource.redisClient?.set( + claimsCacheKey(hashedKey), + JSON.stringify(claims), + 'EX', + claims.exp ? claims.exp - claims.iat : 3600 * 24 * 365 // default 1 year + ) +} + +export const deleteCachedClaims = async (key: string) => { + await redisDataSource.redisClient?.del(claimsCacheKey(key)) +} + // verify jwt token first // if valid then decode and return claims // if expired then throw error @@ -75,19 +105,20 @@ export const getClaimsByToken = async ( return undefined } - try { - return jwt.verify(token, env.server.jwtSecret) as Claims - } catch (e) { - if ( - e instanceof jwt.JsonWebTokenError && - !(e instanceof jwt.TokenExpiredError) - ) { - logger.info(`not a jwt token, checking api key`, { token }) - return claimsFromApiKey(token) + if (isApiKey(token)) { + const hashedKey = hashApiKey(token) + const cachedClaims = await getCachedClaims(hashedKey) + if (cachedClaims) { + return cachedClaims } - throw e + const claims = await claimsFromApiKey(hashedKey) + await cacheClaims(hashedKey, claims) + + return claims } + + return jwt.verify(token, env.server.jwtSecret) as Claims } const verificationTokenKey = (token: string) => `verification:${token}` From 1485230af0471561bd851397fa923dd46f8c05c2 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Thu, 22 Aug 2024 19:06:50 +0800 Subject: [PATCH 3/3] fix tests --- packages/api/test/routers/auth.test.ts | 27 ++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/packages/api/test/routers/auth.test.ts b/packages/api/test/routers/auth.test.ts index b144f77f9..0aaf801a9 100644 --- a/packages/api/test/routers/auth.test.ts +++ b/packages/api/test/routers/auth.test.ts @@ -1,4 +1,5 @@ import { expect } from 'chai' +import sinon, { SinonFakeTimers } from 'sinon' import supertest from 'supertest' import { StatusType, User } from '../../src/entity/user' import { getRepository } from '../../src/repository' @@ -258,7 +259,7 @@ describe('auth router', () => { }) context('when token is valid', () => { - before(async () => { + beforeEach(async () => { token = await generateVerificationToken({ id: user.id }) }) @@ -292,8 +293,17 @@ describe('auth router', () => { }) context('when token is expired', () => { + let clock: SinonFakeTimers + before(async () => { - token = await generateVerificationToken({ id: user.id }, -1) + clock = sinon.useFakeTimers() + token = await generateVerificationToken({ id: user.id }) + // advance time by 1 hour + clock.tick(60 * 60 * 1000) + }) + + after(() => { + clock.restore() }) it('redirects to confirm-email page with error code TokenExpired', async () => { @@ -419,7 +429,7 @@ describe('auth router', () => { }) context('when token is valid', () => { - before(async () => { + beforeEach(async () => { token = await generateVerificationToken({ id: user.id }) }) @@ -464,8 +474,17 @@ describe('auth router', () => { }) context('when token is expired', () => { + let clock: SinonFakeTimers + before(async () => { - token = await generateVerificationToken({ id: user.id }, -1) + clock = sinon.useFakeTimers() + token = await generateVerificationToken({ id: user.id }) + // advance time by 1 hour + clock.tick(60 * 60 * 1000) + }) + + after(() => { + clock.restore() }) it('redirects to reset-password page with error code ExpiredToken', async () => {