As promised on the WG call, I’ve gone through the 2.1 document and I’ve made
some notes and suggestions on my way through. A big thanks to the editors for
putting this together, and particularly for Aaron who did the early heavy
lifting on getting a reasonable start on this important work!
But first, a note: I realize that many portions of this are simply copied from
6749 or related specifications, and I do not fault the editors for that. Even
so, there are some places where the old language should be updated in this
draft, since we have an opportunity to fix things and make them more readable
on our way through. There are also a number of places that are redundant with
each other as they clearly come from different source documents. A major goal
of this work is to coalesce these differences into a single and easily
understandable framework.
§Abstract
I think we should update “interaction between the RO and the HTTP Service” to
be “the RO and an authorization service” or something like that instead, since
“the HTTP service” referenced elsewhere in this paragraph is more precisely the
RS and not the AS, yet the interaction and approval happens at the AS. OAuth
2.0 allowed the AS and RS to be separate, and 2.1 should go further to admit
that in a lot of cases today, they are separate.
There’s been debate on the “replaces and obsoletes” language already, and I
think there’s a lot of IETF process that we’ll need to sort out to get that
language right.
§1: We should add a note on password use to this list:
- Often the resource owner’s password is used with other unrelated services,
and the additional exposure of sharing the password among components in one
domain lowers the possible security in other domains where the password is
shared.
¶3/4 can probably be collapsed to read better, and some of the language
cleaned up. Recommend:
OAuth addresses these issues by introducing an authorization layer and
separating the role of the client from that of the resource owner. In OAuth,
the client requests access to resources controlled by the resource owner and
hosted by the resource server. Instead of using the resource owner's
credentials to access protected resources, the client obtains an access token -
a credential representing a specific set of access attributes such as scope and
lifetime. Access tokens are issued to clients by an authorization server with
the approval of the resource owner. The client uses the access token to access
the protected resources hosted by the resource server.
¶6: (and throughout document) avoid use of gendered pronouns in favor of
singular “they” unless needed for clearer reading (such as an Alice and Bob
scenario)
§1.2: This diagram could use an update to not show the client talking directly
to the RO in the first step, especially because the ROPC grant has been removed
here. The way it’s written now makes it look like the user gives something to
the client which it then trades for a token directly, which doesn’t happen
quite like that anywhere.
§1.3: Having taught many OAuth 2 classes to hundreds of people over the years,
I can say with confidence that this definition of “grant” as a credential has
historically been problematic and confusing to most people. And in particular,
it doesn’t even really apply to the client credentials flow listed here: there
is not a separate "credential representing authorization", just the client’s
own authentication. Suggest new text to more accurately reflect what a “grant”
is in the OAuth reality:
An authorization grant is a process by which a client obtains authorization
from a resource owner to obtain an access token to act on that resource owner’s
behalf. This specification defines...
Changing this would require a consensus call
§1.3.X: I don’t see why we shouldn’t list the Device Flow here in this spec.
It’s mentioned as an extension later, but we might as well list it here as a
known and accepted grant type.
§1.4: This section should mention introspection explicitly. Recommend rewriting
the intro to ¶3:
The token make be used by the RS to retrieve the authorization information, or
the token may self-contain the authorization information in a verifiable manner
(i.e., a token string consisting of a signed data payload). One example of a
token retrieval mechanism is Token Introspection [RFC7662], in which the RS
calls an endpoint on the AS to validate the token presented by the client. One
example of a structured token …
We may also want to mention CWT (RFC8392) here in passing.
§1.5: “The string is usually opaque” should be “the string is opaque"
§1.6: this should probably refer to the TLS BCP195 here instead of the RFC and
version.
§1.8: This should have explicit links to introspection (RFC7662), registration
(RFC7591, RFC7592), and discovery (RFC8414) inline as they’re given as
examples, instead of putting them all in the appendix. Perhaps also bring up
JWTs for token formats here as well.
§1.X: Suggest adding a section on compatibility with OAuth 2.0, either as part
of §1.8 or in a new section injected right after it. Some suggested starter
text:
OAuth 2.1 is compatible with OAuth 2.0 with the extensions and restrictions
from known best current practices applied. Specifically, features not available
in OAuth 2.0 core, such as PKCE, are required in OAuth 2.1. Additionally,
features available in OAuth 2.0, such as the implicit or resource owner
credentials grant types, are not available in OAuth 2.1. Furthermore, some
behaviors allowed in OAuth 2.0 are restricted in OAuth 2.1, such as the strict
string matching of redirect URIs required by OAuth 2.1.
§2: the client needs only provide redirect URIs if it uses them — this
registration requirement is meaningless for device flow and client credentials
grants, suggest changing second bullet to something like:
- provide client details needed by the grant type in use, such as
redirect URIs as described in section 3.1.2, and
§2.1: The first sentence on client IDs seems out of place here, and it should
probably be in 2.2 or 2 itself. It might read better to move §2.1 after §2.3 to
allow getting away from that problem
§2.2: While it’s usually the case that the AS issues the client_id, it’s
increasingly common in profiles of OAuth 2 for that to no longer be true.
Ultimately, the AS needs to be able to :recognize: the client software
identified by the client ID, and issuing an ID associated with some known set
of policies (like which redirect URIs are allowed) is just one way to do it. We
should be more precise in the definition and allow for the more general use
cases that we already know are in the wild. Additionally, fixed client IDs are
a thing, and so the restriction of it being unique to the AS is also untrue.
Now allowing client_id choice is important mostly for registration-based
clients, where the AS does issue the ID, and not for other cases where the AS
“recognizes” and processes the client ID, like in OIDC’s Federation profile and
some other specs.
§2.3.1: Can we excise the term “client password” since nobody in the wild uses
it, in my experience at least?
§2.3.X: We should mention other common client authentication methods here in
the core, including private_key_jwt and MTLS. Both have stable implementations
and are registered, we should call them out directly.
§2.4: We need to define what exactly an “unregistered client” is if we’re going
to refer to it here. I think rewriting of §2.2 could help address a lot of this.
§3.1: Since the “resources” parameter spec (RFC8707) breaks the “not more than
once” requirement, can we change that here? Or at least put in an escape valve
like “extensions to this specification MAY define parameters that can be
included more than once”.
§3.1.2: Comparing “two URIs” is talked about here, but only one is explicitly
mentioned. I assume it means comparing the registered redirect URI vs. any
specified in the request, but that’s not mentioned. This is misplaced — maybe
move to §3.1.2.3 or the information needs to be inlined up here.
§3.1.2.5: If we’re going to mention this kind of attack here (and we should),
then we should also talk about exposure of the auth code in referrer headers,
especially to elements loaded within the page (such as images and other
non-script data).
§3.2: Again the use of “grant” as a noun here largely leads to confusion. This
could be changed to just “The token endpoint is used by the client to obtain an
access token using a grant, such as those described in §4 and §6 of this
specification.”
¶3: The client shouldn’t ever be adding query parameters to the token
endpoint, so this requirement is not meaningful.
¶5: We should specify form encoding as the input body here.
We should be explicitly recommending CORS support on the token endpoint here,
especially if we expect SPA’s to use the auth code flow going forward.
§3.3: Can we start to mandate or at least recommend that scope is always
returned and not just when it differs? Yes this is a breaking change for the AS
but it doesn’t affect clients negatively, and it’s similar in requirement to
doing exact redirect URI matching which is already done in this spec that OAuth
2.0 didn’t do.
§4: As above, stating that the authorization is “in the form of a grant” is
confusing as the authorization might be just the existence of the client itself.
§4.1: We should remove the “flow” based language throughout the document. This
seems to have never quite gotten cleaned up previously.
§4.1: The requests aren’t coming “From the authorization server”, suggest
reword: … capable of receiving incoming requests (via redirection from the
authorization server).
§4.1.1.1: The first code-block callout is unhelpful here, suggest inlining it
to the preceding paragraph, or remove it. The content is largely already
covered by the ABNF and the NOTE.
§4.1.1.2: Reverse the order of the plain and S256 examples to imply preference
to S256. Remove line about “existing deployments”, but constrained environments
is still a potentially reasonable reason to keep “plain” so add that to the
preceding paragraph (though you’d be hard pressed to find an embedded system
that can’t do that these days).
§4.1.2: the last few paragraphs on storing code_challenge are awkward to read,
suggest reordering to clarify:
The authorization server MUST associate the code_challenge and
code_challenge_method values with the issued authorization code so the code
challenge can be verified later.
The exact method that the server uses to associate the code_challenge with the
issued code is out of scope for this specification. The code challenge could be
stored on the server and associated with the code there. The code_challenge and
code_challenge_method values may be stored in encrypted form in the code
itself, but the server MUST NOT include the code_challenge value in a response
parameter in a form that other entities can extract.
§4.1.2.1: We should state here, or somewhere in §4.1, that a client might send
a request and never get a response back on the front channel, and they have to
be prepared for that.
¶3: This should be folded into the error definition below instead of its
own paragraph.
Should we state in here the “unsupported_response_mode” error or other
now-registered errors from the registry?
§4.2.1: This section should be dropped, it doesn’t add anything to the
conversation. It was meant to keep things parallel to the other three grants
listed, but now that there’s only one it is just superfluous.
§4.X: Again I think we might want to list the device flow here as more than an
example. If this is meant to be one-stop-shopping then we should have
everything reasonable in the list.
§5.1: Move the content type to the opening sentence instead of below, but keep
the discussion of serialization and structure down where it is in ¶3_
The JSON reference should be updated to RFC8259
§6: I wonder if now we should move this back to a “grant” type in §4. It
originally was but the editor moved it out to its own section, but I have found
that it confuses developers who are looking for it to have it on its own.
Semantically it’s a grant (process to get an access token), but it feels like
something “special” where it isn’t really.
I don’t understand the allowance for allowing clients to refresh a token
with alternate grant types here. In particular, the example of a long-lived
authorization code being used again is illegal since an auth code is
one-time-use as per §9.8 and elsewhere. If the client is just getting another
access token by doing a new grant process, this isn’t “refreshing” the access
token, it’s getting a new one from scratch. What is this new allowance trying
to say or enable?
§6.1: I’m not sure of the value of including the “implementation note” listed
here, especially because “grant” is now being used as an ongoing entity and not
as currently defined in the document nor as I’m proposing we more-accurately
define it here. This seems like general advice for not trusting values in
tokens unless you can validate the integrity of the token, and should go in
another section.
§7.2.2: some leftover language from this being standalone: “All challenges
defined by this specification MUST use the auth-scheme value Bearer”. Change to
“all challenges for this token type…”
§7.3: This section should probably discuss the fact that a given API can have
its own error codes and responses and doesn’t have to use the OAuth responses.
This isn’t clear by the text, and if the intention of the original text was to
have everyone return the same JSON error, that’s violating design principles of
OAuth as a security layer on top of an API (and we should fix that). Either way
we should be clear about the place of OAuth’s error messages.
§7.X I would like to see some discussion on other forms of tokens, especially
sender-constrained token methods such as OAuth PoP, Token Binding, MTLS, and
DPoP.
§7.4: This section feels like it should be its own top level, or integrated
into §9, and not under “accessing protected resources”.
§7.4.2: The beginning should explicitly reference introspection and JWT as
possible solutions.
The TLS recommendation should point to the BCP195.
I’d rather see these recommendations listed out instead of mashed together
in §7.4.2 and listed again in §7.4.3 — it’s hard to read this way.
The recommendations here are listed for bearer tokens but largely apply to
sender constrained tokens as well.
§8.3: The pointer to registering new parameters should point to §8.2 internally
instead of directly to RFC6749
§9: There are several broken/missing links throughout this section, evidenced
by surviving text like (#pop_tokens) and (#mix_up).
§9.1: This probably means “should require clients to authenticate” not “use
client authentication”
¶3 is confusing, suggested rewording: The process of issuance/registration
and distribution of the underlying credentials MUST ensures the confidentiality
of the credentials for an authorization server to rely on authentication with
those credentials.
¶4: This advice should apply even when clients can authenticate, it’s only
especially applicable when the client hasn’t authenticated.
¶6: Should back away from value-judgment language like “more sensible
services”. Also, realize that client instances can be strongly associated even
when using dynamic registration, so this advice falls flat in many cases in the
wild.
§9.1.1 and §9.2 feel like they’re more closely related than their relative
positions in the text would indicate. Should they be §9.2.1/9.2 or §9.2/9.3
respectively instead? I can’t tell what the intent is.
§9.2: This discussion on redirect URIs for native apps should be its own
section (§10.3 has this kind of detail), and it should also include captured
remote URLs (registering an https URL on the app developer’s site that the app
listens for) (this is in §10.3.2 but not mentioned here)
§9.4: Not all access token attributes should be shared with the client. In many
cases, the user’s identity or, say, medical record number might be unknown to
the client but known to the RS to look up information when a call is made. It
might also be necessary to hide some attributes from some RS’s when a token is
used for multiple RS’s in a system.
§9.4.2: The first paragraph isn’t about replay and seems to belong in §9.4.1
instead
I think we should have more detail on sender constrained tokens, and that
the token value vs. the keys used to present the token can be handled
differently by clients. Also discussion of an attack where the RS replays the
access token would fit under such an umbrella.
§9.5: I believe “...guessed to produce valid refresh tokens” in the last
paragraph is supposed to be “…guessed to prevent valid refresh tokens”, or
something similar.
§9.6: The advice here should also indicate that an AS can (should?) record
other details with the token, including which grant type the token was issued
under. The AS can also limit which scopes are available to which grant types
and circumstances.
§9.7: While PAR doesn’t seem to be in scope (should it be?), it does seem that
the use of PAR allows an AS to do something other than direct string matching
for the redirect URI, since the client can present a redirect URI in an
authenticated state in the back channel first, and the AS can make a more
robust decision on those grounds. I think we might want to consider that here.
¶4: The “same user agent” advice only applies to web-based applications,
and this should be made more clear. Otherwise the user isn’t interacting with
the client via an agent, and the launch/return can be separated.
§9.9: The advice for state and scope also applies to anything sent in the front
channel, and that should be called out more specifically here with these as key
examples.
§9.11: What’s the goal with the “other means” requirement, and what means are
intended here (and how do they differ?)?
§9.14: We should be more clear that this isn’t the same as registering to
handle a :single: https URI, which is now a common and accepted pattern.
§9.16: There’s a formatting error on the example here (if I had to guess,
someone used ``` instead of ~~~ in the markdown source)
§9.18.1: We probably want to discuss clients putting target URIs (ie, where to
redirect to after processing the code) into the “state” value in a way that
someone can manipulate before the callback occurs.
§9.19: This uniqueness requirement is stronger than the suggestion earlier in
the document. I think it should still be guidance, not a requirement, as there
are other methods of preventing the mixup attack (like properly using
PKCE/state together).
§9.20: Some of this seems to be a repeat of previous sections, maybe combine
them all into a larger discussion on user agents?
This section should probably explicitly discuss secure browser tabs. (They
are mentioned in passing in §10.2)
§10: Much of the content of this section is covered in §9, and these need to be
shuffled together appropriately. (I get that they come from different specs,
and the plan is for this spec should combine them intelligently into a
navigable set of risks and requirements and not just paste them back to back.)
¶5: This is specifically talking about “The system browser”, and not just
any external browser. This is further muddied by the fact that the user might
not have all their sessions in the default/normal browser on the platform, a
topic which should be discussed somewhere in here.
§10.3: I don’t understand the MUST on the three different options. Are there
any teeth to this requirement? If a platform restricts one or more of these
options is it not in compliance with OAuth 2.1?
§11: This section would be a good place for the discussion on the need for CORS
support on the token endpoint.
§12: The discussion on compatibility expectations can be expanded into this
section here.
§13: It’s not exactly traditional to do so, but it might be worthwhile to list
out the various OAuth registries here with explanations for what they provide.
In particular, the client metadata registry gives a client model, the AS
discovery registry gives a server model, introspection and JWT give a token
model, etc.
§B: The claims in the intro are no longer true as of 2014: it’s been registered
in https://www.iana.org/assignments/media-types/media-types.xhtml
<https://www.iana.org/assignments/media-types/media-types.xhtml> and links to
the WHATWG spec for it.
_______________________________________________
OAuth mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/oauth
