[chore] JWT Authentication review (#396)

* fixes for the auth flow

* add isStaff

* added configuration flag controlling JWT strategy

README.md

@@ -198,6 +198,22 @@ When a schema is updated, the typescript types should be regenerated. This can b
 npm run generate-types
 ```
 
+## Configuration
+
+### Public API
+
+There are several configuration options that should be set differently in Public API:
+
+ * `isPublicApi` - set to `true` to enable the public API. This configures openapi schema, validation, REST transport.
+ * `rateLimiter` - `enabled` must be set to `true` to enable rate limiting. 
+   capacity and refill rate should be adjusted too.
+ * `authentication.jwtOptions`:
+   * `audience` - should be set to the public API URL. This must be different
+      from the internal API URL to make sure tokens from one could not be used
+      in another.
+   * `expiresIn` - should be set to a reasonable value for the public API (e.g. `8h` for 8 hours)
+ * `authentication.cookie.enabled` set to `false` - cookies are not used in the public API
+
 ## Help
 
 For more information on all the things you can do with Feathers visit [docs.feathersjs.com](http://docs.feathersjs.com).

src/app.ts

@@ -17,6 +17,8 @@ import { ensureServiceIsFeathersCompatible } from './util/feathers'
 import channels from './channels'
 import { ImpressoApplication } from './types'
 import { Application } from '@feathersjs/express'
+import bodyParser from 'body-parser'
+import authentication from './authentication'
 
 const path = require('path')
 const compress = require('compression')
@@ -29,8 +31,6 @@ const middleware = require('./middleware')
 // const services = require('./services');
 const appHooks = require('./app.hooks')
 
-const authentication = require('./authentication')
-
 const multer = require('./multer')
 const cache = require('./cache')
 const cachedSolr = require('./cachedSolr')
@@ -62,6 +62,8 @@ app.use('cachedSolr', ensureServiceIsFeathersCompatible(cachedSolr(app)), {
 app.use(helmet())
 app.use(compress())
 app.use(cookieParser())
+// needed to access body in non-feathers middlewares, like openapi validator
+app.use(bodyParser.json({ limit: '50mb' }))
 
 // configure local multer service.
 app.configure(multer)
@@ -80,8 +82,6 @@ app.configure(media)
 app.configure(proxy)
 app.configure(schemas)
 
-app.configure(errorHandling)
-
 // Enable Swagger and API validator if needed
 app.configure(swagger)
 app.configure(openApiValidator)
@@ -105,4 +105,6 @@ app.configure(appHooks)
 // because one of the services is used in the channels.
 app.configure(channels)
 
+app.configure(errorHandling)
+
 module.exports = app

src/authentication.ts

@@ -0,0 +1,116 @@
+import {
+  AuthenticationParams,
+  AuthenticationRequest,
+  AuthenticationResult,
+  AuthenticationService,
+  JWTStrategy,
+} from '@feathersjs/authentication'
+import { LocalStrategy } from '@feathersjs/authentication-local'
+import { NotAuthenticated } from '@feathersjs/errors'
+import initDebug from 'debug'
+import { createSwaggerServiceOptions } from 'feathers-swagger'
+import User from './models/users.model'
+import { docs } from './services/authentication/authentication.schema'
+import { ImpressoApplication } from './types'
+import { ServiceOptions } from '@feathersjs/feathers'
+
+const debug = initDebug('impresso/authentication')
+
+class CustomisedAuthenticationService extends AuthenticationService {
+  async getPayload(authResult: AuthenticationResult, params: AuthenticationParams) {
+    const payload = await super.getPayload(authResult, params)
+    const { user } = authResult as { user: User }
+    if (user) {
+      payload.userId = user.uid
+      if (user.groups.length) {
+        payload.userGroups = user.groups.map(d => d.name)
+      }
+      payload.isStaff = user.isStaff
+    }
+    return payload
+  }
+}
+
+class HashedPasswordVerifier extends LocalStrategy {
+  comparePassword(user: User, password: string) {
+    return new Promise((resolve, reject) => {
+      if (!(user instanceof User)) {
+        debug('_comparePassword: user is not valid', user)
+        return reject(new NotAuthenticated('Login incorrect'))
+      }
+
+      const isValid = User.comparePassword({
+        encrypted: user.password,
+        password,
+      })
+
+      if (!isValid) {
+        return reject(new NotAuthenticated('Login incorrect'))
+      }
+      return resolve({
+        ...user,
+      })
+    })
+  }
+}
+
+export interface SlimUser {
+  uid: string
+  id: number
+  isStaff: boolean
+}
+
+/**
+ * A custom JWT strategy that does not load the user from the database.
+ * Instead, it uses the payload from the JWT token to create a slim user object
+ * which is enough for most of the use cases across the codebase.
+ * Where a full user object is required, it is requested explicitly.
+ */
+class NoDBJWTStrategy extends JWTStrategy {
+  async authenticate(authentication: AuthenticationRequest, params: AuthenticationParams) {
+    const { accessToken } = authentication
+    const { entity } = this.configuration
+    if (!accessToken) {
+      throw new NotAuthenticated('No access token')
+    }
+    const payload = await this.authentication?.verifyAccessToken(accessToken, params.jwt)
+    const result = {
+      accessToken,
+      authentication: {
+        strategy: 'jwt',
+        accessToken,
+        payload,
+      },
+    }
+    if (entity === null) {
+      return result
+    }
+
+    const slimUser: SlimUser = {
+      uid: payload.userId,
+      id: parseInt(payload.sub),
+      isStaff: payload.isStaff ?? false,
+    }
+    return {
+      ...result,
+      [entity]: slimUser,
+    }
+  }
+}
+
+export default (app: ImpressoApplication) => {
+  const isPublicApi = app.get('isPublicApi')
+  const useDbUserInRequestContext = app.get('useDbUserInRequestContext')
+  const authentication = new CustomisedAuthenticationService(app)
+
+  const jwtStrategy = useDbUserInRequestContext ? new JWTStrategy() : new NoDBJWTStrategy()
+
+  authentication.register('jwt', jwtStrategy)
+  authentication.register('local', new HashedPasswordVerifier())
+
+  app.use('/authentication', authentication, {
+    methods: isPublicApi ? ['create'] : undefined,
+    events: [],
+    docs: createSwaggerServiceOptions({ schemas: {}, docs }),
+  } as ServiceOptions)
+}

src/configuration.ts

@@ -27,6 +27,7 @@ export interface Configuration {
   redis?: RedisConfiguration
   rateLimiter?: RateLimiterConfiguration & { enabled?: boolean }
   publicApiPrefix?: string
+  useDbUserInRequestContext?: boolean
 
   // TODO: move to services:
   sequelizeClient?: Sequelize
@@ -39,17 +40,39 @@ const configurationSchema: JSONSchemaDefinition = {
   $id: 'configuration',
   type: 'object',
   properties: {
-    isPublicApi: {
-      type: 'boolean',
-      description: 'If `true`, the app serves a public API',
-    },
+    isPublicApi: { type: 'boolean', description: 'If `true`, the app serves a public API' },
     allowedCorsOrigins: {
       type: 'array',
       items: {
         type: 'string',
       },
       description: 'List of allowed origins for CORS',
     },
+    redis: {
+      type: 'object',
+      properties: {
+        enable: { type: 'boolean', description: 'Enable Redis' },
+        brokerUrl: { type: 'string', description: 'URL of the Redis broker' },
+      },
+      description: 'Redis configuration',
+    },
+    rateLimiter: {
+      type: 'object',
+      properties: {
+        enabled: { type: 'boolean', description: 'Enable rate limiter' },
+        capacity: { type: 'number', description: 'Capacity of the rate limiter' },
+        refillRate: { type: 'number', description: 'Refill rate of the rate limiter' },
+      },
+      description: 'Rate limiter configuration',
+      required: ['capacity', 'refillRate'],
+    },
+    publicApiPrefix: { type: 'string', description: 'Prefix for the public API' },
+    useDbUserInRequestContext: {
+      type: 'boolean',
+      description:
+        'If `true`, the user object is loaded from the db on every request. ' +
+        'If `false` (default), the user object is created from the JWT token',
+    },
   },
 } as const
 

src/hooks/authenticate.ts

@@ -43,6 +43,8 @@ interface AuthenticateAroundOptions {
 export const authenticateAround =
   ({ strategy = 'jwt', allowUnauthenticated = false }: AuthenticateAroundOptions = {}) =>
   async (context: HookContext<ImpressoApplication>, next: NextFunction) => {
+    if (context.type !== 'around') throw new Error('authenticateAround must be used in "around" hooks')
+
     const isPublicApi = context.app.get('isPublicApi')
     // only allow unauthenticated in non-public API
     const doAllowUnauthenticated = isPublicApi ? false : allowUnauthenticated

src/middleware/openApiValidator.ts

@@ -3,11 +3,18 @@ import type { Application } from '@feathersjs/express'
 import { HookContext, NextFunction } from '@feathersjs/hooks'
 import convertSchema from '@openapi-contrib/json-schema-to-openapi-schema'
 import * as OpenApiValidator from 'express-openapi-validator'
+import { HttpError as OpenApiHttpError } from 'express-openapi-validator/dist/framework/types'
 import type { OpenAPIV3, OpenApiValidatorOpts, ValidationError } from 'express-openapi-validator/dist/framework/types'
 import fs from 'fs'
 import { logger } from '../logger'
 import type { ImpressoApplication } from '../types'
 import { parseFilters } from '../util/queryParameters'
+import type {
+  NextFunction as ExpressNextFunction,
+  Request as ExpressRequest,
+  Response as ExpressResponse,
+} from 'express'
+import { FeathersError } from '@feathersjs/errors'
 
 export default (app: ImpressoApplication & Application) => {
   installMiddleware(app)
@@ -66,7 +73,6 @@ const installMiddleware = (app: ImpressoApplication & Application) => {
     next()
   })
 
-  // app.use(middlewares as any)
   // app.use(middlewares as any)
   middlewares.forEach((middleware, index) => {
     logger.debug('Install', middleware)
@@ -75,6 +81,20 @@ const installMiddleware = (app: ImpressoApplication & Application) => {
       handler(req, res, next)
     })
   })
+
+  app.use((error: any, req: ExpressRequest, res: ExpressResponse, next: ExpressNextFunction) => {
+    if (error instanceof OpenApiHttpError) {
+      next(convertOpenApiError(error))
+    } else {
+      next(error)
+    }
+  })
+}
+
+const convertOpenApiError = (error: OpenApiHttpError): FeathersError => {
+  const newError = new FeathersError(error, 'OpenApiError', error.status, error.constructor.name, {})
+  newError.stack = error.stack
+  return newError
 }
 
 /**

src/services/authentication/authentication.schema.ts

@@ -17,6 +17,7 @@ export const docs: ServiceSwaggerOptions = {
       requestBody: {
         content: getRequestBodyContent('AuthenticationCreateRequest'),
       },
+      security: [],
       responses: {
         201: {
           description: 'Authentication successful',