From 1fb61a3763fe1598a95a6d95f146e8213b1288b2 Mon Sep 17 00:00:00 2001 From: Carmine DiMascio Date: Fri, 11 Oct 2019 20:21:10 -0400 Subject: [PATCH] security cleanup --- src/middlewares/openapi.security.ts | 105 +++++++++++++++++----------- test/resources/security.yaml | 4 +- 2 files changed, 66 insertions(+), 43 deletions(-) diff --git a/src/middlewares/openapi.security.ts b/src/middlewares/openapi.security.ts index 3d914834..3cd46da3 100644 --- a/src/middlewares/openapi.security.ts +++ b/src/middlewares/openapi.security.ts @@ -3,6 +3,10 @@ import { OpenAPIV3, OpenApiRequest } from '../framework/types'; import { validationError } from './util'; import { OpenApiContext } from '../framework/openapi.context'; +interface SecurityHandlerResult { + success: boolean; + error: string; +} export function security( context: OpenApiContext, securityHandlers: SecurityHandlers, @@ -15,63 +19,94 @@ export function security( return next(); } - const securitySchema = ( + const securities = ( req.openapi.schema.security ); const path: string = req.openapi.openApiRoute; - if (!path || !Array.isArray(securitySchema)) { + if (!path || !Array.isArray(securities)) { return next(); } const securitySchemes = context.apiDoc.components && context.apiDoc.components.securitySchemes; + if (!securitySchemes) { const message = `security referenced at path ${path}, but not defined in components.securitySchemes`; return next(validationError(500, path, message)); } - // TODO security could be boolean or promise bool, handle both - const promises = securitySchema.map(s => { + try { + const results = await new SecuritySchemes( + securitySchemes, + securityHandlers, + securities, + ).executeHandlers(req); + + // TODO handle AND'd and OR'd security + // This assumes OR only! i.e. at least one security passed authentication + let firstError = null; + for (var r of results) { + if (!r.success) { + firstError = r.error || Error('unauthorized'); + break; + } + } + if (firstError) throw firstError; + else next(); + } catch (e) { + const message = (e && e.message) || 'unauthorized'; + const err = validationError(401, path, message); + next(err); + } + }; +} + +class SecuritySchemes { + private securitySchemes; + private securityHandlers: SecurityHandlers; + private security; + constructor(securitySchemes, securityHandlers: SecurityHandlers, security) { + this.securitySchemes = securitySchemes; + this.securityHandlers = securityHandlers; + this.security = security; + } + + executeHandlers(req: OpenApiRequest): Promise { + const promises = this.security.map(async s => { try { const securityKey = Object.keys(s)[0]; - const scopes = Array.isArray(s) ? s : [] - const scheme: any = securitySchemes[securityKey]; - const handler = securityHandlers[securityKey]; + const scopes = Array.isArray(s) ? s : []; + const scheme: any = this.securitySchemes[securityKey]; + const handler = this.securityHandlers[securityKey]; if (!scheme) { const message = `components.securitySchemes.${securityKey} does not exist`; - throw validationError(401, path, message); + throw Error(message); } if (!scheme.type) { const message = `components.securitySchemes.${securityKey} must have property 'type'`; - throw validationError(401, path, message); + throw Error(message); } if (!handler) { const message = `a handler for ${securityKey} does not exist`; - throw validationError(401, path, message); + throw Error(message); } new AuthValidator(req, scheme).validate(); - return Promise.resolve(handler(req, scopes, securitySchemes)); + const success = await handler(req, scopes, this.securitySchemes); + return { success }; } catch (e) { - return Promise.reject(e); + return { + success: false, + error: e, + }; } }); - - try { - const results = await Promise.all(promises); - const authFailed = results.filter(b => !b).length > 0; - if (authFailed) throw validationError(401, path, 'unauthorized'); - else next(); - } catch (e) { - const message = (e && e.message) || 'unauthorized'; - const err = validationError(401, path, message); - next(err); - } - }; + return Promise.all(promises); + } } class AuthValidator { @@ -115,24 +150,16 @@ class AuthValidator { req.headers['authorization'].toLowerCase(); if (!authHeader) { - throw validationError(401, path, `Authorization header required.`); + throw Error(`Authorization header required.`); } const type = scheme.scheme && scheme.scheme.toLowerCase(); if (type === 'bearer' && !authHeader.includes('bearer')) { - throw validationError( - 401, - path, - `Authorization header with scheme 'Bearer' required.`, - ); + throw Error(`Authorization header with scheme 'Bearer' required.`); } if (type === 'basic' && !authHeader.includes('basic')) { - throw validationError( - 401, - path, - `Authorization header with scheme 'Basic' required.`, - ); + throw Error(`Authorization header with scheme 'Basic' required.`); } } } @@ -142,15 +169,11 @@ class AuthValidator { if (scheme.type === 'apiKey') { if (scheme.in === 'header') { if (!req.headers[scheme.name.toLowerCase()]) { - throw validationError(401, path, `'${scheme.name}' header required.`); + throw Error(`'${scheme.name}' header required.`); } } else if (scheme.in === 'query') { if (!req.headers[scheme.name]) { - throw validationError( - 401, - path, - `query parameter '${scheme.name}' required.`, - ); + throw Error(`query parameter '${scheme.name}' required.`); } } // TODO scheme in cookie diff --git a/test/resources/security.yaml b/test/resources/security.yaml index 567423e2..ee9e376e 100644 --- a/test/resources/security.yaml +++ b/test/resources/security.yaml @@ -17,8 +17,8 @@ paths: description: OK '400': description: Bad Request - #'401': - # description: unauthorized + '401': + description: unauthorized /bearer: get: