Comments (5)
Yes, although I'd suggest if there's at least one set of invalid credentials, we should still fail, even if there's also some valid passing credentials. I'm working on something - hopefully I'll have a fix for this after the weekend.
from exegesis.
This is confusing.
So, let's say you have an operation that has a security requirement like:
{
"sessionKey": [],
"basicAuth": []
}
Now, if someone authenticates with a good sessionKey, we're never going to even try to authenticate against basicAuth. If someone authenticates with a bad sessionKey, though, should we continue on to try basicAuth, or should we reject immediately? What if they have good basicAuth credentials and a bad sessionKey?
It's tempting to say we should reject immediately, but then, if you have a good basicAuth and a bad sessionKey and you get this security requirement:
{
"basicAuth": [],
"sessionKey": []
}
where basicAuth and sessionKey are reversed, then we'd accept this (since we'd pass the basicAuth, and never bother to check the sessionKey). It's kind of weird that we pass in the one case, and fail in the other, just because the order of security requirements is different.
Maybe what we should really be doing here is not short-circuiting on success - always run all the authenticators, and if any of them returns "invalid" we fail. The only real downside to that is that exegesis-passport usually doesn't know the difference between a missing result and an invalid result, but so long as it errors on the side of "missing" it's not any worse than what we have now.
from exegesis.
I think the invalid credentials scenario is going to be more common with the scenario of multiple "credentials" being an edge case. The multiple credentials being passed would be also easier to spot from the request.
It's tempting to say we should reject immediately, but then, if you have a good basicAuth and a bad sessionKey ... It's kind of weird that we pass in the one case, and fail in the other, just because the order of security requirements is different
That a very good point. Could this flow be a possible solution?
For each authenticator
- check the security requirement
- collect in array if invalid
- return on the first success
- If there was no successful result but one or more invalid results then return an error listing the schemes(s) that were invalid
{message: "Authentication was invalid for the the following schemes: sessionKey." errors: [{scheme: result[x].failedSchemeName, message: result[x].message, ... }] with the detailed messages of the invalid results }
- If all are missing then return
{ "message":"Must authenticate using one of the following schemes: sessionKey." }
Obviously setting the headers appropriately.
I think the consumer of the API would have a clear understanding what occurred. An update to the developer documentation would be needed to call out that the order that the authenticators are run in and that the process is short-circuited on a success.
Maybe what we should really be doing here is not short-circuiting on success - always run all the authenticators, and if any of them returns "invalid" we fail. The only real downside to that is that exegesis-passport usually doesn't know the difference between a missing result and an invalid result, but so long as it errors on the side of "missing" it's not any worse than what we have now.
Agreed. The passport flow would be no worse than the current flow, but things would be improved for other integrations.
from exegesis.
Hi Jason,
Some interesting documentation in the specification under Using Multiple Authentication Types
...
Some REST APIs support several authentication types. The security section lets you combine the security requirements using logical OR and AND to achieve the desired result.
OR case
security: # A OR B
- A
- B
- Successfully authenticate if either A or B returns a success.
- A 'invalid' or 'missing' result in only one will be ignored.
AND case
security: # A AND B
- A
B
- Successfully authenticate only if both A or B returns a success.
- A 'invalid' or 'missing' result in either will cause authentication to fail.
It makes sense to follow the spec but I can see that it wouldn't be a small change. It would also have an impact on the authenticator results, as they would need merged together when there is multiple results.
from exegesis.
🎉 This issue has been resolved in version 1.0.7 🎉
The release is available on:
Your semantic-release bot 📦🚀
from exegesis.
Related Issues (20)
- Validation error of request body when oneOf is used HOT 5
- Crash with recursive definition in referenced OpenAPI file is back again
- Does not work well with newest json-ptr (1.3.1) HOT 2
- Your .dependabot/config.yml contained invalid details HOT 1
- Export class from controller HOT 2
- Could not find controller if controller is a .ts file HOT 2
- Can we access "ExegesisContext" at express middleware(app.use) not controllers/*.ts HOT 1
- How to filter the request body by only passing the defined requestBody schema? HOT 2
- Security vulnerability in `json-ptr` HOT 4
- Exegesis 3.0.0 using updated ajv outputs issues with OpenAPI spec, but 2.0.0 did not HOT 5
- res.redirect not supported HOT 3
- Is OpenAPI 3.1 supported?
- Bump json-ptr to fix security vulnerability HOT 1
- Support for additional string formats HOT 2
- json-schema-ref-parser dependency breaks node engine < 17 HOT 3
- How to use exegesis with multipart/form-data?
- Mime-types HOT 2
- Query parameters not being validated. HOT 1
- Loading is too slow due to schema compilation HOT 2
- Skip/bypass response validation HOT 2
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from exegesis.