From 904c05425bf6e6306bb4b34333816971d557fc1c Mon Sep 17 00:00:00 2001 From: daffl Date: Mon, 4 Apr 2022 12:06:45 -0700 Subject: [PATCH 1/2] feat(configuration): Allow app configuration to be validated against a schema --- packages/authentication/src/core.ts | 4 +- packages/authentication/src/index.ts | 1 + packages/authentication/src/options.ts | 107 ++++++++++++++++++- packages/authentication/test/service.test.ts | 2 +- packages/configuration/package.json | 1 + packages/configuration/src/index.ts | 14 ++- packages/configuration/test/index.test.ts | 47 +++++++- packages/schema/package.json | 3 +- packages/schema/src/resolver.ts | 5 +- packages/schema/src/schema.ts | 16 ++- 10 files changed, 182 insertions(+), 18 deletions(-) diff --git a/packages/authentication/src/core.ts b/packages/authentication/src/core.ts index 3b27deeba4..d5e5dcc3b1 100644 --- a/packages/authentication/src/core.ts +++ b/packages/authentication/src/core.ts @@ -5,7 +5,7 @@ import { NotAuthenticated } from '@feathersjs/errors'; import { createDebug } from '@feathersjs/commons'; import { Application, Params } from '@feathersjs/feathers'; import { IncomingMessage, ServerResponse } from 'http'; -import defaultOptions from './options'; +import { defaultOptions } from './options'; const debug = createDebug('@feathersjs/authentication/base'); @@ -167,7 +167,7 @@ export class AuthenticationBase { /** * Returns a single strategy by name - * + * * @param name The strategy name * @returns The authentication strategy or undefined */ diff --git a/packages/authentication/src/index.ts b/packages/authentication/src/index.ts index c634a3f7a5..5ec3d7349c 100644 --- a/packages/authentication/src/index.ts +++ b/packages/authentication/src/index.ts @@ -10,3 +10,4 @@ export { export { AuthenticationBaseStrategy } from './strategy'; export { AuthenticationService } from './service'; export { JWTStrategy } from './jwt'; +export { authenticationSettingsSchema } from './options'; diff --git a/packages/authentication/src/options.ts b/packages/authentication/src/options.ts index ee869f2b05..a040b96e71 100644 --- a/packages/authentication/src/options.ts +++ b/packages/authentication/src/options.ts @@ -1,5 +1,5 @@ -export default { - authStrategies: [], +export const defaultOptions = { + authStrategies: [] as string[], jwtOptions: { header: { typ: 'access' }, // by default is an access token but can be any type audience: 'https://yourdomain.com', // The resource server where the token is processed @@ -8,3 +8,106 @@ export default { expiresIn: '1d' } }; + +export const authenticationSettingsSchema = { + type: 'object', + required: ['secret', 'entity', 'authStrategies'], + properties: { + secret: { + type: 'string', + description: 'The JWT signing secret' + }, + entity: { + oneOf: [{ + type: 'null' + }, { + type: 'string' + }], + description: 'The name of the authentication entity (e.g. user)' + }, + entityId: { + type: 'string', + description: 'The name of the authentication entity id property' + }, + service: { + type: 'string', + description: 'The path of the entity service' + }, + authStrategies: { + type: 'array', + items: { type: 'string' }, + description: 'A list of authentication strategy names that are allowed to create JWT access tokens' + }, + parseStrategies: { + type: 'array', + items: { type: 'string' }, + description: 'A list of authentication strategy names that should parse HTTP headers for authentication information (defaults to `authStrategies`)' + }, + jwtOptions: { + type: 'object' + }, + jwt: { + type: 'object', + header: { + type: 'string', + default: 'Authorization', + description: 'The HTTP header containing the JWT' + }, + schemes: { + type: 'array', + items: { type: 'string' }, + description: 'An array of schemes to support' + } + }, + local: { + type: 'object', + required: ['usernameField', 'passwordField'], + properties: { + usernameField: { + type: 'string', + description: 'Name of the username field (e.g. `email`)' + }, + passwordField: { + type: 'string', + description: 'Name of the password field (e.g. `password`)' + }, + hashSize: { + type: 'number', + description: 'The BCrypt salt length' + }, + errorMessage: { + type: 'string', + default: 'Invalid login', + description: 'The error message to return on errors' + }, + entityUsernameField: { + type: 'string', + description: 'Name of the username field on the entity if authentication request data and entity field names are different' + }, + entityPasswordField: { + type: 'string', + description: 'Name of the password field on the entity if authentication request data and entity field names are different' + } + } + }, + oauth: { + type: 'object', + properties: { + redirect: { + type: 'string' + }, + origins: { + type: 'array', + items: { type: 'string' } + }, + defaults: { + type: 'object', + properties: { + key: { type: 'string' }, + secret: { type: 'string' } + } + } + } + } + } +} as const; diff --git a/packages/authentication/test/service.test.ts b/packages/authentication/test/service.test.ts index e5ed22d817..acddbbe8b5 100644 --- a/packages/authentication/test/service.test.ts +++ b/packages/authentication/test/service.test.ts @@ -4,7 +4,7 @@ import jwt from 'jsonwebtoken'; import { feathers, Application } from '@feathersjs/feathers'; import { memory, Service as MemoryService } from '@feathersjs/memory'; -import defaultOptions from '../src/options'; +import { defaultOptions } from '../src/options'; import { AuthenticationService } from '../src'; import { Strategy1 } from './fixtures'; diff --git a/packages/configuration/package.json b/packages/configuration/package.json index 8f17f4be16..f651ee3f3f 100644 --- a/packages/configuration/package.json +++ b/packages/configuration/package.json @@ -59,6 +59,7 @@ "dependencies": { "@feathersjs/commons": "^5.0.0-pre.17", "@feathersjs/feathers": "^5.0.0-pre.17", + "@feathersjs/schema": "^5.0.0-pre.17", "@types/config": "^0.0.41", "config": "^3.3.7" }, diff --git a/packages/configuration/src/index.ts b/packages/configuration/src/index.ts index d425a7150f..adf115bf69 100644 --- a/packages/configuration/src/index.ts +++ b/packages/configuration/src/index.ts @@ -1,10 +1,11 @@ -import { Application } from '@feathersjs/feathers'; +import { Application, ApplicationHookContext, NextFunction } from '@feathersjs/feathers'; import { createDebug } from '@feathersjs/commons'; +import { Schema } from '@feathersjs/schema' import config from 'config'; const debug = createDebug('@feathersjs/configuration'); -export = function init () { +export = function init (schema?: Schema) { return (app?: Application) => { if (!app) { return config; @@ -18,6 +19,15 @@ export = function init () { app.set(name, value); }); + if (schema) { + app.hooks({ + setup: [async (context: ApplicationHookContext, next: NextFunction) => { + await schema.validate(context.app.settings); + await next(); + }] + }) + } + return config; }; } diff --git a/packages/configuration/test/index.test.ts b/packages/configuration/test/index.test.ts index 780c9c4421..7d586e336c 100644 --- a/packages/configuration/test/index.test.ts +++ b/packages/configuration/test/index.test.ts @@ -1,9 +1,10 @@ import { strict as assert } from 'assert'; import { feathers, Application } from '@feathersjs/feathers'; -import plugin from '../src'; +import { Ajv, schema } from '@feathersjs/schema'; +import configuration from '../src'; describe('@feathersjs/configuration', () => { - const app: Application = feathers().configure(plugin()); + const app: Application = feathers().configure(configuration()); it('initialized app with default.json', () => { assert.equal(app.get('port'), 3030); @@ -15,9 +16,49 @@ describe('@feathersjs/configuration', () => { }); it('works when called directly', () => { - const fn = plugin(); + const fn = configuration(); const conf = fn() as any; assert.strictEqual(conf.port, 3030); }); + + it('errors on .setup when a schema is passed and the configuration is invalid', async () => { + const configurationSchema = schema({ + $id: 'ConfigurationSchema', + additionalProperties: false, + type: 'object', + properties: { + port: { type: 'number' }, + deep: { + type: 'object', + properties: { + base: { + type: 'boolean' + } + } + }, + array: { + type: 'array', + items: { type: 'string' } + }, + nullish: { + type: 'string' + } + } + } as const, new Ajv()); + + const schemaApp = feathers().configure(configuration(configurationSchema)) + + await assert.rejects(() => schemaApp.setup(), { + data: [{ + instancePath: '/nullish', + keyword: 'type', + message: 'must be string', + params: { + type: 'string' + }, + schemaPath: '#/properties/nullish/type' + }] + }); + }); }); diff --git a/packages/schema/package.json b/packages/schema/package.json index 91b0bcd656..292b21a2e2 100644 --- a/packages/schema/package.json +++ b/packages/schema/package.json @@ -43,7 +43,8 @@ "scripts": { "prepublish": "npm run compile", "compile": "shx rm -rf lib/ && tsc", - "test": "mocha --config ../../.mocharc.json --recursive test/**.test.ts test/**/*.test.ts" + "mocha": "mocha --config ../../.mocharc.json --recursive test/**.test.ts test/**/*.test.ts", + "test": "npm run compile && npm run mocha" }, "directories": { "lib": "lib" diff --git a/packages/schema/src/resolver.ts b/packages/schema/src/resolver.ts index 6e1789c55d..00bb156d70 100644 --- a/packages/schema/src/resolver.ts +++ b/packages/schema/src/resolver.ts @@ -1,4 +1,5 @@ import { BadRequest } from '@feathersjs/errors'; +import { Schema } from './schema'; export type PropertyResolver = ( value: V|undefined, @@ -14,7 +15,7 @@ export type PropertyResolverMap = { export interface ResolverConfig { // TODO this should be `Schema` but has recently produced an error, see // https://github.com/ThomasAribart/json-schema-to-ts/issues/53 - schema?: any, + schema?: Schema, validate?: 'before'|'after'|false, properties: PropertyResolverMap } @@ -71,7 +72,7 @@ export class Resolver { // Not the most elegant but better performance await Promise.all(propertyList.map(async name => { - const value = data[name]; + const value = (data as any)[name]; if (resolvers[name]) { try { diff --git a/packages/schema/src/schema.ts b/packages/schema/src/schema.ts index 329aa73e49..459c5d6b3d 100644 --- a/packages/schema/src/schema.ts +++ b/packages/schema/src/schema.ts @@ -2,18 +2,24 @@ import Ajv, { AsyncValidateFunction, ValidateFunction } from 'ajv'; import { FromSchema, JSONSchema } from 'json-schema-to-ts'; import { BadRequest } from '@feathersjs/errors'; -export const AJV = new Ajv({ +export const DEFAULT_AJV = new Ajv({ coerceTypes: true }); +export { Ajv }; + export type JSONSchemaDefinition = JSONSchema & { $id: string, $async?: boolean }; -export class Schema { +export interface Schema { + validate (...args: Parameters>): Promise; +} + +export class SchemaWrapper implements Schema> { ajv: Ajv; validator: AsyncValidateFunction; readonly _type!: FromSchema; - constructor (public definition: S, ajv: Ajv = AJV) { + constructor (public definition: S, ajv: Ajv = DEFAULT_AJV) { this.ajv = ajv; this.validator = this.ajv.compile({ $async: true, @@ -36,6 +42,6 @@ export class Schema { } } -export function schema (definition: S, ajv: Ajv = AJV) { - return new Schema(definition, ajv); +export function schema (definition: S, ajv: Ajv = DEFAULT_AJV) { + return new SchemaWrapper(definition, ajv); } From 463f3a29946a236246f8e9706be12b55d5f7f433 Mon Sep 17 00:00:00 2001 From: daffl Date: Mon, 4 Apr 2022 12:21:29 -0700 Subject: [PATCH 2/2] Add tests for authentication schema, improve code coverage --- packages/authentication/package.json | 1 + packages/authentication/src/options.ts | 20 +++++++++++--------- packages/authentication/test/core.test.ts | 17 +++++++++++++++++ packages/schema/src/resolver.ts | 2 -- packages/schema/test/fixture.ts | 4 +++- 5 files changed, 32 insertions(+), 12 deletions(-) diff --git a/packages/authentication/package.json b/packages/authentication/package.json index f358fb8130..5dbf60de6c 100644 --- a/packages/authentication/package.json +++ b/packages/authentication/package.json @@ -64,6 +64,7 @@ }, "devDependencies": { "@feathersjs/memory": "^5.0.0-pre.17", + "@feathersjs/schema": "^5.0.0-pre.17", "@types/lodash": "^4.14.181", "@types/mocha": "^9.1.0", "@types/node": "^17.0.23", diff --git a/packages/authentication/src/options.ts b/packages/authentication/src/options.ts index a040b96e71..d63298a507 100644 --- a/packages/authentication/src/options.ts +++ b/packages/authentication/src/options.ts @@ -48,15 +48,17 @@ export const authenticationSettingsSchema = { }, jwt: { type: 'object', - header: { - type: 'string', - default: 'Authorization', - description: 'The HTTP header containing the JWT' - }, - schemes: { - type: 'array', - items: { type: 'string' }, - description: 'An array of schemes to support' + properties: { + header: { + type: 'string', + default: 'Authorization', + description: 'The HTTP header containing the JWT' + }, + schemes: { + type: 'array', + items: { type: 'string' }, + description: 'An array of schemes to support' + } } }, local: { diff --git a/packages/authentication/test/core.test.ts b/packages/authentication/test/core.test.ts index 3471a0caeb..0863b01323 100644 --- a/packages/authentication/test/core.test.ts +++ b/packages/authentication/test/core.test.ts @@ -1,8 +1,10 @@ import assert from 'assert'; import { feathers, Application } from '@feathersjs/feathers'; import jwt from 'jsonwebtoken'; +import { Infer, schema } from '@feathersjs/schema'; import { AuthenticationBase, AuthenticationRequest } from '../src/core'; +import { authenticationSettingsSchema } from '../src/options'; import { Strategy1, Strategy2, MockRequest } from './fixtures'; import { ServerResponse } from 'http'; @@ -31,6 +33,21 @@ describe('authentication/core', () => { }); describe('configuration', () => { + it('infers configuration from settings schema', async () => { + const settingsSchema = schema({ + $id: 'AuthSettingsSchema', + ...authenticationSettingsSchema + } as const); + type Settings = Infer; + const config: Settings = { + entity: 'user', + secret: 'supersecret', + authStrategies: [ 'some', 'thing' ] + } + + await settingsSchema.validate(config); + }); + it('throws an error when app is not provided', () => { try { // @ts-ignore diff --git a/packages/schema/src/resolver.ts b/packages/schema/src/resolver.ts index 00bb156d70..ad6126bd7a 100644 --- a/packages/schema/src/resolver.ts +++ b/packages/schema/src/resolver.ts @@ -13,8 +13,6 @@ export type PropertyResolverMap = { } export interface ResolverConfig { - // TODO this should be `Schema` but has recently produced an error, see - // https://github.com/ThomasAribart/json-schema-to-ts/issues/53 schema?: Schema, validate?: 'before'|'after'|false, properties: PropertyResolverMap diff --git a/packages/schema/test/fixture.ts b/packages/schema/test/fixture.ts index cbebc0bfa2..509eded507 100644 --- a/packages/schema/test/fixture.ts +++ b/packages/schema/test/fixture.ts @@ -6,7 +6,7 @@ import { GeneralError } from '@feathersjs/errors'; import { schema, resolve, Infer, resolveResult, - queryProperty, resolveQuery, resolveData + queryProperty, resolveQuery, resolveData, validateData, validateQuery } from '../src'; export const userSchema = schema({ @@ -148,6 +148,7 @@ const app = feathers() .use('messages', memory()); app.service('messages').hooks([ + validateQuery(messageQuerySchema), resolveQuery(messageQueryResolver), resolveResult(messageResultResolver) ]); @@ -158,6 +159,7 @@ app.service('users').hooks([ app.service('users').hooks({ create: [ + validateData(userSchema), resolveData(userDataResolver) ] });