openapi-backend
openapi-backend copied to clipboard
SecurityHandlers aren't fed the required scopes, and don't properly handlemultiple requirements with the same scheme
An operation may have multiple security requirements, where each requirement may require multiple security schemes. This means that the same scheme could in theory be present in multiple requirements, but openapi-backend treats each scheme as unique.
The effect is that if an operation looks like this:
security:
- Foo: [scope1, scope2]
Bar: []
- Foo: [scope3]
Baz: []
... it means that this operation is authorized if any of the following is true:
- The
Fooscheme is authorized and is granted at leastscope1orscope2, and theBarscope is authorized
or
- The
Fooscheme is authorized and is granted at leastscope3, and theBazscope is authorized
However, openapi-backend will find that there are four schemes to authorize: Foo, Bar, Foo (again!) and Baz. And since they are performed in parallel using Promise.all, the results for Foo will be calculated twice and is subject to concurrency issues. Furthermore, the implementation of the Foo security handler has no way of knowing which set of scopes is required in each of the two calls.
I see two possible improvements here: Either the security handlers are be invoked as today (Foo invoked twice) but with the required scopes array as a parameter; or Foo is only invoked once and security handlers are required to return the granted scopes as their result, and then some additional logic is implemented to handle scope checking outside of the security handlers. But that limits the freedom of what security handlers may do - perhaps they need to return something else alongside just the granted scopes. Currently, security handlers are very general; they return any and if the return value is truthy it's considered to "pass". With such a change, a security handler would be required to return an array of granted scopes, which limits flexibility.
Actually, even without multiple colliding schemes, a security handler needs to manually iterate context.operation.security to get the relevant scopes to check for, so also in a more normal scenario it would be beneficial if the required scopes would be passed to the security handler. This would be a bit difficult to implement in a backwards compatible way though, since all handlers are basically (context, ...args). But the other approach has even more side-effects.
Thoughts?