-
Notifications
You must be signed in to change notification settings - Fork 9.1k
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
Initial proposal for New Security Schemes #2582
Conversation
c450fa2
to
0631cb9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Basically I feel this proposal is attempting to do several things (and gets caught up trying to be backwards compatible as well):
- allow (some) credentials in
body
andpath
- allow JWTs with some
type
-specific config - genericise credential locations and config
It's not clear to me why all these things are or need to be combined.
|
||
[Security Schemes](https://github.com/OAI/OpenAPI-Specification/blob/main/versions/3.1.0.md#security-scheme-object) | ||
_(formerly known as "Security Definitions")_ provide a way to describe the security scheme(s) used by your API. | ||
Unfortunately, the _types_ supported by OpenAPI are not only limited but they often tie the credential location to the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately, the types supported by OpenAPI ... often tie the credential location to the Security Scheme type
Can you expand on why this is "unfortunate"? Type apiKey
can already be located in
any of query
, header
,cookie
, and arguably if you try to put your Bearer
token anywhere other than in an Authorization
header, you are not doing oAuth2.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The point was more about separating the credential location(s) from the Security Scheme type. API Key is the closest to allowing free-form credential locations while others do not. I can see where some Security Schemes where these are locked down, like OAuth 2.0 (while it uses Bearer Tokens, I don't think it supports them being in all locations a Bearer Token could be represented), but this separation is really a flexibility thing than just trying to support JWT.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would also be nice to have a way to describe a type not supported by OAS, where our locked-down types don't fit and a customer wants to still describe their security needs.
* `type: http`: This will only work if your JWT is in a header and in a supported | ||
[HTTP Auth](https://datatracker.ietf.org/doc/html/rfc7235#section-5.1) format | ||
|
||
OpenAPI being so opinionated about these kinds things does not serve any real benefit to anyone involved, and this |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OpenAPI being so opinionated about these kinds things does not serve any real benefit to anyone involved
Unsure this is totally true. Tooling authors may appreciate the limited set of possibilities they have to support?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's subjective for sure but is focusing on the tooling authors more important than the consumers of OpenAPI? I think with a clean implementation, it shouldn't be too hard to support something like this as a fellow tooling author to another.
The bulk of this proposal is defining a way to separate the properties used to describe the credential(s) for a Security | ||
Scheme from the root properties of the Security Scheme. This proposal will also allow you to define multiple | ||
credentials _(multiple locations, formats, ...)_ for the same Security Scheme for cases where you might support | ||
multiple formats/locations for effectively the same credential. The new objects being suggested as part of this |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These multiple locations, are they AND or OR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would suggest we do something like how the Security Requirements work now, which would allow for an AND or an OR. But in the case of having an AND of the same credential location, I can't think of a case where you'd say I want the same to credential to be supplied in multiple places at the same time.
| Field Name | Type | Applies To | Description | | ||
| ---------- | ---- | ---------- | ----------- | | ||
| in | `string` | Any | **REQUIRED.** Specifies where in the request the credential is provided. Valid values are: `body`, `cookie`, `header`, `path` and `query` | | ||
| format | `regex` | Any | Specifies the format of the credential. Since in many cases, the credential is a subset of the raw value, `format` must be a regular expression that matches the complete value, and contains a single capturing group to match the actual value within the raw value. | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rather than express how to retrieve the credential using a regex, it may be more OAS-like to provide a template for how the credential should be constructed. Thus
format: ^[B|b][E|e][A|a][R|r][E|e][R|r] (.*)$
would become something like:
format: Bearer {credential}
PS: Header names are case-insensitive, but the "Bearer" string is not, so this regex can be simplified.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't disagree. The main reason for using a regex example was to show how one might pick out a portion of a larger HTTP value, I'm not married to any specific implementation.
This comment was marked as abuse.
This comment was marked as abuse.
This comment was marked as abuse.
This comment was marked as abuse.
As a result of #2604 the filename for this proposal should change to use a dated prefix. Please could you update this PR to match? Apologies for the extra work. |
2671aff
to
6d9ba00
Compare
6d9ba00
to
0b586c8
Compare
A few notes after presenting this to the Security SIG:
|
I am migrating this PR from the spec repo to allow it to be iterated upon within the security SIG. [Original PR](OAI/OpenAPI-Specification#2582)
I have republished @whitlockjc proposal within the security SIG, and copied the conversation thread from here over to an issue. We are going to work on the proposal over there in the security SIG repo, and when ready republish here as a formal proposal. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added some preliminary notes.
credentials: | ||
- in: header | ||
name: authorization | ||
format: ^[B|b][E|e][A|a][R|r][E|e][R|r] (.*)$ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if it's a regexp, I'd use pattern
.
I've dropped the ball on this, but since it's never been reasoned about, what should we do here? |
Filed #3257 to revive and clarify our proposals process. Once we figure it out, we'll resolve this PR accordingly. |
Closing because the conversation is now happening in Moonwalk discussions OAI/sig-moonwalk#46 |
This purpose of this proposal is to rethink how Security Schemes are defined in OpenAPI. This proposal would separate the credential location(s) from the Security Scheme type and would allow for complete control over dictating where the credential(s) are provided for a Security Scheme. This proposal will add a generic way to retrieve a credential from anywhere in a request, and allow any Security Scheme to dictate where its supported credential(s) are instead of relying on OpenAPI-supported conventions/opinions. This proposal also separates the credential location(s) details from the other configurations associated with the Security Scheme (like JWKS details for JWT).