Merge pull request #4310 from omnivore-app/fix/verification-token

fix/verification token

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<ApiKeysSuccess, ApiKeysError>(
@@ -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',

packages/api/src/resolvers/types.ts

@@ -24,6 +24,7 @@ export interface Claims {
   exp?: number
   email?: string
   system?: boolean
+  destroyAfterUse?: boolean
 }
 
 export type ClaimsToSet = {

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,
         })

packages/api/src/services/send_emails.ts

@@ -10,7 +10,7 @@ export const sendNewAccountVerificationEmail = async (user: {
   email: string
 }): Promise<boolean> => {
   // 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<boolean> => {
   // 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<boolean> => {
   // 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 = {

packages/api/src/utils/auth.ts

@@ -3,12 +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'
 
@@ -23,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<Claims> => {
-  const hashedKey = hashApiKey(key)
-
+export const claimsFromApiKey = async (hashedKey: string): Promise<Claims> => {
   const apiKeyRepo = getRepository(ApiKey)
 
   const apiKey = await apiKeyRepo
@@ -63,6 +66,34 @@ export const claimsFromApiKey = async (key: string): Promise<Claims> => {
   }
 }
 
+const claimsCacheKey = (hashedKey: string) => `api-key-hash:${hashedKey}`
+
+const getCachedClaims = async (
+  hashedKey: string
+): Promise<Claims | undefined> => {
+  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
@@ -74,37 +105,68 @@ 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
 }
 
-export const generateVerificationToken = (
+const verificationTokenKey = (token: string) => `verification:${token}`
+
+export const verifyToken = async (token: string): Promise<Claims> => {
+  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<string> => {
   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 (

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,8 +259,8 @@ describe('auth router', () => {
     })
 
     context('when token is valid', () => {
-      before(() => {
-        token = generateVerificationToken({ id: user.id })
+      beforeEach(async () => {
+        token = await generateVerificationToken({ id: user.id })
       })
 
       it('set auth token in cookie', async () => {
@@ -292,8 +293,17 @@ describe('auth router', () => {
     })
 
     context('when token is expired', () => {
-      before(() => {
-        token = generateVerificationToken({ id: user.id }, -1)
+      let clock: SinonFakeTimers
+
+      before(async () => {
+        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 () => {
@@ -305,9 +315,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 +429,8 @@ describe('auth router', () => {
     })
 
     context('when token is valid', () => {
-      before(() => {
-        token = generateVerificationToken({ id: user.id })
+      beforeEach(async () => {
+        token = await generateVerificationToken({ id: user.id })
       })
 
       context('when password is not empty', () => {
@@ -464,8 +474,17 @@ describe('auth router', () => {
       })
 
       context('when token is expired', () => {
-        before(() => {
-          token = generateVerificationToken({ id: user.id }, -1)
+        let clock: SinonFakeTimers
+
+        before(async () => {
+          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 () => {