Skip to content

Commit

Permalink
implement and/or logic in security validator (#393)
Browse files Browse the repository at this point in the history 
* implement and/or logic in security validators

* fix

* fix

* fix

* console.logs removed

* add tests for add/or securities

* remove uneeded route

Co-authored-by: Carmine DiMascio <cdimascio@gmail.com>
  • Loading branch information
@balazssoltesz @cdimascio
balazssoltesz and cdimascio authored Oct 24, 2020
1 parent b98b965 commit 683c54b
Show file tree
Hide file tree
Showing 3 changed files with 169 additions and 60 deletions.
148 changes: 89 additions & 59 deletions src/middlewares/openapi.security.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,24 +85,54 @@ export function security(
securitySchemes,
securityHandlers,
securities,
).executeHandlers(req);

).executeHandlers(req);
// TODO handle AND'd and OR'd security
// This assumes OR only! i.e. at least one security passed authentication
let firstError: SecurityHandlerResult = null;
let firstError: SecurityHandlerResult = null;
let success = false;
for (const r of results) {
if (r.success) {
success = true;
break;
} else if (!firstError) {
firstError = r;
}

function checkErrorWithOr(res) {
return res.forEach(r => {
if (r.success) {
success = true;
} else if (!firstError) {
firstError = r;
}
})
}

function checkErrorsWithAnd(res) {
let allSuccess = false;

res.forEach(r => {
if (!r.success) {
allSuccess = false;
if (!firstError) {
firstError = r;
}
} else if (!firstError) {
allSuccess = true;
}
})

if (allSuccess) {
success = true;
}
}

results.forEach(result => {
if (Array.isArray(result) && result.length > 1) {
checkErrorsWithAnd(result);
} else {
checkErrorWithOr(result);
}
});

if (success) {
next();
next();
} else {
throw firstError;
throw firstError;
}
} catch (e) {
const message = e?.error?.message || 'unauthorized';
Expand Down Expand Up @@ -138,62 +168,62 @@ class SecuritySchemes {

public async executeHandlers(
req: OpenApiRequest,
): Promise<SecurityHandlerResult[]> {
): Promise<(SecurityHandlerResult | SecurityHandlerResult[])[]> {
// use a fallback handler if security handlers is not specified
// This means if security handlers is specified, the user must define
// all security handlers
const fallbackHandler = !this.securityHandlers
? defaultSecurityHandler
: null;

const promises = this.securities.map(async (s) => {
try {
if (Util.isEmptyObject(s)) {
// anonumous security
return { success: true };
}
const securityKey = Object.keys(s)[0];
const scheme = this.securitySchemes[securityKey];
const handler = this.securityHandlers?.[securityKey] ?? fallbackHandler;
const scopesTmp = s[securityKey];
const scopes = Array.isArray(scopesTmp) ? scopesTmp : [];

if (!scheme) {
const message = `components.securitySchemes.${securityKey} does not exist`;
throw new InternalServerError({ message });
}
if (!scheme.hasOwnProperty('type')) {
const message = `components.securitySchemes.${securityKey} must have property 'type'`;
throw new InternalServerError({ message });
}
if (!handler) {
const message = `a security handler for '${securityKey}' does not exist`;
throw new InternalServerError({ message });
if (Util.isEmptyObject(s)) {
// anonumous security
return [{ success: true }];
}
return Promise.all(Object.keys(s).map(async (securityKey) => {
var _a, _b, _c;
try {
const scheme = this.securitySchemes[securityKey];
const handler = (_b = (_a = this.securityHandlers) === null || _a === void 0 ? void 0 : _a[securityKey]) !== null && _b !== void 0 ? _b : fallbackHandler;
const scopesTmp = s[securityKey];
const scopes = Array.isArray(scopesTmp) ? scopesTmp : [];
if (!scheme) {
const message = `components.securitySchemes.${securityKey} does not exist`;
throw new InternalServerError({ message });
}
if (!scheme.hasOwnProperty('type')) {
const message = `components.securitySchemes.${securityKey} must have property 'type'`;
throw new InternalServerError({ message });
}
if (!handler) {
const message = `a security handler for '${securityKey}' does not exist`;
throw new InternalServerError({ message });
}
new AuthValidator(req, scheme, scopes).validate();
// expected handler results are:
// - throw exception,
// - return true,
// - return Promise<true>,
// - return false,
// - return Promise<false>
// everything else should be treated as false
const securityScheme = <OpenAPIV3.SecuritySchemeObject>scheme;
const success = await handler(req, scopes, securityScheme);
if (success === true) {
return { success };
}
else {
throw Error();
}
}

new AuthValidator(req, scheme, scopes).validate();

// expected handler results are:
// - throw exception,
// - return true,
// - return Promise<true>,
// - return false,
// - return Promise<false>
// everything else should be treated as false
const securityScheme = <OpenAPIV3.SecuritySchemeObject>scheme;
const success = await handler(req, scopes, securityScheme);
if (success === true) {
return { success };
} else {
throw Error();
catch (e) {
return {
success: false,
status: (_c = e.status) !== null && _c !== void 0 ? _c : 401,
error: e,
};
}
} catch (e) {
return {
success: false,
status: e.status ?? 401,
error: e,
};
}
}));
});
return Promise.all(promises);
}
Expand Down
12 changes: 12 additions & 0 deletions test/resources/security.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,18 @@ servers:
- url: /v1/

paths:
/apikey_and_bearer_or_basic:
get:
security:
- ApiKeyAuth: []
BearerAuth: []
- BasicAuth: []
responses:
"200":
description: OK
"401":
description: unauthorized

/no_security:
get:
responses:
Expand Down
69 changes: 68 additions & 1 deletion test/security.handlers.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,6 @@ describe('security.handlers', () => {
.expect(401)
.then((r) => {
const body = r.body;
console.log(body);
expect(body.errors).to.be.an('array');
expect(body.errors).to.have.length(1);
expect(body.errors[0].message).to.equals('unauthorized');
Expand Down Expand Up @@ -379,3 +378,71 @@ describe('security.handlers', () => {
.expect(200);
});
});

describe('when securities declare: (apikey && bearer) || basic', () => {
let app = null;
let basePath = null;
const eovConf: OpenApiValidatorOpts = {
apiSpec: path.join('test', 'resources', 'security.yaml'),
};
before(async () => {
app = await createApp(eovConf, 3005);
basePath = app.basePath;

app.use(
`${basePath}`,
express
.Router()
.get('/apikey_and_bearer_or_basic', (req, res) =>
res.json({ logged_in: true }),
),
);
});

after(() => {
app.server.close();
});

it('should return 401 if not X-Api-Key is missing', async () =>
request(app)
.get(`${basePath}/apikey_and_bearer_or_basic`)
.set('Authorization', 'Bearer XXXX') // Bearer
.expect(401)
.then((r) => {
const body = r.body;
expect(body.errors).to.be.an('array');
expect(body.errors).to.have.length(1);
expect(body.message).to.include("'X-API-Key' header required");
}));

it('should return 401 if Bearer token is missing', async () => {
eovConf.validateSecurity = true;
return request(app)
.get(`${basePath}/apikey_and_bearer_or_basic`)
.set('X-Api-Key', 'XXX') // api key
.expect(401)
.then((r) => {
const body = r.body;
expect(body.errors).to.be.an('array');
expect(body.errors).to.have.length(1);
expect(body.message).to.include('Authorization header required');
});
});

it('should return 200 when X-Api-Key and Bearer token are present', async () => {
eovConf.validateSecurity = true;
return request(app)
.get(`${basePath}/apikey_and_bearer_or_basic`)
.set('Authorization', 'Bearer XXXX') // Bearer
.set('X-Api-Key', 'XXX') // api key
.expect(200);
});

it('should return 200 when Basic auth is present', async () => {
eovConf.validateSecurity = true;
return request(app)
.get(`${basePath}/apikey_and_bearer_or_basic`)
.set('Authorization', 'Basic XXXX')
.expect(200);
});
});

0 comments on commit 683c54b

Please sign in to comment.