[OAUTH-WG] OAuth v.2.1 Readthrough

[Justin Richer <[e-mail]>]

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.