Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Security handlers fix for "AND" case #548

Merged
merged 6 commits into from
Jan 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -84,3 +84,6 @@ $RECYCLE.BIN/

# Windows shortcuts
*.lnk

# vscode config
.vscode
11 changes: 11 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,17 @@
## [Unreleased]
### Changed

## [4.4.3] 19-01-2024
### Changed
- fix: securityHandlers fix for "AND" case
- fix: remove last traces of TAP from generator templates
- updated dependencies
- @biomejs/biome ^1.2.2 → ^1.5.2
- @seriousme/openapi-schema-validator ^2.1.2 → ^2.1.5
- c8 ^9.0.0 → ^9.1.0
- fastify ^4.23.2 → ^4.25.2
- fastify-cli ^6.0.0 → ^6.0.1

## [4.4.2] 14-10-2023
### Changed
- fix: add warning for OpenAPI v3 cookie parameters
Expand Down
20 changes: 20 additions & 0 deletions docs/securityHandlers.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,4 +44,24 @@ will return errors originating from the securityHandlers as well in `err.errors`

You can use `err.errors` also to trigger other behaviour in a registered error handler.

Using multiple securityHandlers it is possible to use AND and OR logic:
E.g.:
```json
...
{
"petstore_auth1": [
"read:pets"
],
"petstore_auth2": [
"read:pets"
],
},
{
"petstore_auth3": [
"read:pets"
],
},
```
means that authorization is granted if `((petstore_auth1("read:pets") AND petstore_auth2("read:pets")) OR (petstore_auth3("read:pets")))` returns without error.

For a more elaborate example see the [examples/generated-javascript-project](/examples/generated-javascript-project) folder.
11 changes: 5 additions & 6 deletions examples/generated-javascript-project/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,16 @@
},
"dependencies": {
"fastify-plugin": "^4.5.1",
"@seriousme/openapi-schema-validator": "^2.1.2",
"@seriousme/openapi-schema-validator": "^2.1.5",
"js-yaml": "^4.1.0",
"minimist": "^1.2.8",
"fastify-openapi-glue": "^4.4.2"
},
"devDependencies": {
"fastify": "^4.23.2",
"fastify-cli": "^5.8.0",
"tap": "",
"c8": "^8.0.1",
"@biomejs/biome": "^1.2.2",
"fastify": "^4.25.2",
"fastify-cli": "^6.0.1",
"c8": "^9.1.0",
"@biomejs/biome": "^1.5.2",
"husky": "^8.0.3"
}
}
22 changes: 9 additions & 13 deletions examples/generated-standaloneJS-project/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ async function generateRoutes(fastify, opts) {
},
handler: buildHandler(service, "addPet").bind(Service),
prehandler: buildPreHandler(security, [
{ name: "petstore_auth", parameters: ["write:pets", "read:pets"] },
{ petstore_auth: ["write:pets", "read:pets"] },
]).bind(Security),
});

Expand Down Expand Up @@ -217,7 +217,7 @@ async function generateRoutes(fastify, opts) {
},
handler: buildHandler(service, "updatePet").bind(Service),
prehandler: buildPreHandler(security, [
{ name: "petstore_auth", parameters: ["write:pets", "read:pets"] },
{ petstore_auth: ["write:pets", "read:pets"] },
]).bind(Security),
});

Expand All @@ -244,7 +244,7 @@ async function generateRoutes(fastify, opts) {
},
handler: buildHandler(service, "findPetsByStatus").bind(Service),
prehandler: buildPreHandler(security, [
{ name: "petstore_auth", parameters: ["write:pets", "read:pets"] },
{ petstore_auth: ["write:pets", "read:pets"] },
]).bind(Security),
});

Expand All @@ -269,7 +269,7 @@ async function generateRoutes(fastify, opts) {
},
handler: buildHandler(service, "findPetsByTags").bind(Service),
prehandler: buildPreHandler(security, [
{ name: "petstore_auth", parameters: ["write:pets", "read:pets"] },
{ petstore_auth: ["write:pets", "read:pets"] },
]).bind(Security),
});

Expand All @@ -290,9 +290,7 @@ async function generateRoutes(fastify, opts) {
},
},
handler: buildHandler(service, "getPetById").bind(Service),
prehandler: buildPreHandler(security, [
{ name: "api_key", parameters: [] },
]).bind(Security),
prehandler: buildPreHandler(security, [{ api_key: [] }]).bind(Security),
});

fastify.route({
Expand Down Expand Up @@ -326,7 +324,7 @@ async function generateRoutes(fastify, opts) {
},
handler: buildHandler(service, "updatePetWithForm").bind(Service),
prehandler: buildPreHandler(security, [
{ name: "petstore_auth", parameters: ["write:pets", "read:pets"] },
{ petstore_auth: ["write:pets", "read:pets"] },
]).bind(Security),
});

Expand All @@ -348,7 +346,7 @@ async function generateRoutes(fastify, opts) {
},
handler: buildHandler(service, "deletePet").bind(Service),
prehandler: buildPreHandler(security, [
{ name: "petstore_auth", parameters: ["write:pets", "read:pets"] },
{ petstore_auth: ["write:pets", "read:pets"] },
]).bind(Security),
});

Expand Down Expand Up @@ -383,7 +381,7 @@ async function generateRoutes(fastify, opts) {
},
handler: buildHandler(service, "uploadFile").bind(Service),
prehandler: buildPreHandler(security, [
{ name: "petstore_auth", parameters: ["write:pets", "read:pets"] },
{ petstore_auth: ["write:pets", "read:pets"] },
]).bind(Security),
});

Expand All @@ -392,9 +390,7 @@ async function generateRoutes(fastify, opts) {
url: "/store/inventory",
schema: {},
handler: buildHandler(service, "getInventory").bind(Service),
prehandler: buildPreHandler(security, [
{ name: "api_key", parameters: [] },
]).bind(Security),
prehandler: buildPreHandler(security, [{ api_key: [] }]).bind(Security),
});

fastify.route({
Expand Down
11 changes: 5 additions & 6 deletions examples/generated-standaloneJS-project/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,15 @@
},
"dependencies": {
"fastify-plugin": "^4.5.1",
"@seriousme/openapi-schema-validator": "^2.1.2",
"@seriousme/openapi-schema-validator": "^2.1.5",
"js-yaml": "^4.1.0",
"minimist": "^1.2.8"
},
"devDependencies": {
"fastify": "^4.23.2",
"fastify-cli": "^5.8.0",
"tap": "",
"c8": "^8.0.1",
"@biomejs/biome": "^1.2.2",
"fastify": "^4.25.2",
"fastify-cli": "^6.0.1",
"c8": "^9.1.0",
"@biomejs/biome": "^1.5.2",
"husky": "^8.0.3"
}
}
45 changes: 23 additions & 22 deletions examples/generated-standaloneJS-project/securityHandlers.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@ export default class SecurityHandlers {
/** constructor */
constructor(handlers) {
this.handlers = handlers;
// the specificatio allows for an empty scheme that allows all access
this.handlers[emptyScheme] = async () => {};
this.handlerMap = new Map();
this.missingHandlers = [];
}
Expand All @@ -25,21 +23,17 @@ export default class SecurityHandlers {
}
const mapKey = JSON.stringify(schemes);
if (!this.handlerMap.has(mapKey)) {
const processedSchemes = [];
for (const scheme of schemes) {
// parser returns undefined on empty scheme
if (scheme.name === undefined) {
scheme.name = emptyScheme;
}
if (!(scheme.name in this.handlers)) {
this.handlers[scheme.name] = () => {
throw `Missing handler for "${scheme.name}" validation`;
};
this.missingHandlers.push(scheme.name);
for (const schemeList of schemes) {
for (const name in schemeList) {
if (!(name in this.handlers)) {
this.handlers[name] = () => {
throw `Missing handler for "${name}" validation`;
};
this.missingHandlers.push(name);
}
}
processedSchemes.push(scheme);
}
this.handlerMap.set(mapKey, this._buildHandler(processedSchemes));
this.handlerMap.set(mapKey, this._buildHandler(schemes));
}
return this.handlerMap.has(mapKey);
}
Expand All @@ -62,24 +56,31 @@ export default class SecurityHandlers {
const securityHandlers = this.handlers;
return async (req, reply) => {
const handlerErrors = [];
const schemeList = [];
const schemeListDone = [];
let statusCode = 401;
for (const scheme of schemes) {
for (const schemeList of schemes) {
let name;
const andList = [];
try {
await securityHandlers[scheme.name](req, reply, scheme.parameters);
return; // If one security check passes, no need to try any others
for (name in schemeList) {
const parameters = schemeList[name];
andList.push(name);
// all the handlers in a scheme list must succeed
await securityHandlers[name](req, reply, parameters);
}
return; // If one list of schemes passes, no need to try any others
} catch (err) {
req.log.debug(`Security handler '${scheme.name}' failed: '${err}'`);
req.log.debug(`Security handler '${name}' failed: '${err}'`);
handlerErrors.push(err);
if (err.statusCode !== undefined) {
statusCode = err.statusCode;
}
}
schemeList.push(scheme.name);
schemeListDone.push(andList.toString());
}
// if we get this far no security handlers validated this request
throw new SecurityError(
`None of the security schemes (${schemeList.join(
`None of the security schemes (${schemeListDone.join(
", ",
)}) successfully authenticated this request.`,
statusCode,
Expand Down
16 changes: 1 addition & 15 deletions lib/ParserBase.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,18 +44,6 @@ export class ParserBase {
}
}

parseSecurity(schemes) {
return schemes
? schemes.map((item) => {
const name = Object.keys(item)[0];
return {
name,
parameters: item[name],
};
})
: undefined;
}

removeRecursion(schemas) {
function escapeJsonPointer(str) {
return str.replace(/~/g, "~0").replace(/\//g, "~1");
Expand Down Expand Up @@ -118,9 +106,7 @@ export class ParserBase {
operationId:
operationSpec.operationId || this.makeOperationId(operation, path),
openapiSource: operationSpec,
security: this.parseSecurity(
operationSpec.security || this.spec.security,
),
security: operationSpec.security || this.spec.security,
};

if (operationSpec["x-fastify-config"]) {
Expand Down
45 changes: 23 additions & 22 deletions lib/securityHandlers.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@ export default class SecurityHandlers {
/** constructor */
constructor(handlers) {
this.handlers = handlers;
// the specificatio allows for an empty scheme that allows all access
this.handlers[emptyScheme] = async () => {};
this.handlerMap = new Map();
this.missingHandlers = [];
}
Expand All @@ -25,21 +23,17 @@ export default class SecurityHandlers {
}
const mapKey = JSON.stringify(schemes);
if (!this.handlerMap.has(mapKey)) {
const processedSchemes = [];
for (const scheme of schemes) {
// parser returns undefined on empty scheme
if (scheme.name === undefined) {
scheme.name = emptyScheme;
}
if (!(scheme.name in this.handlers)) {
this.handlers[scheme.name] = () => {
throw `Missing handler for "${scheme.name}" validation`;
};
this.missingHandlers.push(scheme.name);
for (const schemeList of schemes) {
for (const name in schemeList) {
if (!(name in this.handlers)) {
this.handlers[name] = () => {
throw `Missing handler for "${name}" validation`;
};
this.missingHandlers.push(name);
}
}
processedSchemes.push(scheme);
}
this.handlerMap.set(mapKey, this._buildHandler(processedSchemes));
this.handlerMap.set(mapKey, this._buildHandler(schemes));
}
return this.handlerMap.has(mapKey);
}
Expand All @@ -62,24 +56,31 @@ export default class SecurityHandlers {
const securityHandlers = this.handlers;
return async (req, reply) => {
const handlerErrors = [];
const schemeList = [];
const schemeListDone = [];
let statusCode = 401;
for (const scheme of schemes) {
for (const schemeList of schemes) {
let name;
const andList = [];
try {
await securityHandlers[scheme.name](req, reply, scheme.parameters);
return; // If one security check passes, no need to try any others
for (name in schemeList) {
const parameters = schemeList[name];
andList.push(name);
// all the handlers in a scheme list must succeed
await securityHandlers[name](req, reply, parameters);
}
return; // If one list of schemes passes, no need to try any others
} catch (err) {
req.log.debug(`Security handler '${scheme.name}' failed: '${err}'`);
req.log.debug(`Security handler '${name}' failed: '${err}'`);
handlerErrors.push(err);
if (err.statusCode !== undefined) {
statusCode = err.statusCode;
}
}
schemeList.push(scheme.name);
schemeListDone.push(andList.toString());
}
// if we get this far no security handlers validated this request
throw new SecurityError(
`None of the security schemes (${schemeList.join(
`None of the security schemes (${schemeListDone.join(
", ",
)}) successfully authenticated this request.`,
statusCode,
Expand Down
1 change: 0 additions & 1 deletion lib/templates/js/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
"devDependencies": {
"fastify": "",
"fastify-cli": "",
"tap": "",
"c8": ""
}
}
1 change: 0 additions & 1 deletion lib/templates/standaloneJS/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
"devDependencies": {
"fastify": "",
"fastify-cli": "",
"tap": "",
"c8": ""
}
}
Loading