Re: [OAUTH-WG] Detailed review of OAuth2.1

[Aaron Parecki <[e-mail]>]

I've continued processing this feedback through section 6. Below are
responses to your comments that I either already incorporated or did not
think needed any changes. Any of your comments not mentioned below have
been opened as GitHub issues here:
https://github.com/aaronpk/oauth-v2-1/issues

Feedback from Justin:


> §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

Great, this also makes it applicable to further extensions that might
require other things in the client registration.


> §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

Vittorio also caught this. I rephrased 2.1 to not mention client_id yet.


> §2.3.1: Can we excise the term “client password” since nobody in the wild
uses it, in my experience at least?

I am in favor of renaming this header to "Client Secret", but I do want to
keep a reference to the term "client password" as a secondary term for it.
I think "Clients in possession of a client secret, sometimes known as a
client password..." captures both goals.


> §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.

For now I've added explicit references to mTLS and private_key_jwt as
examples. I'm not sure there should be a whole section about them but I'm
willing to be convinced.


> §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”.

I think this is intending to say that only the parameters defined in
RFC6749 can't be repeated. It's not meant to cover parameters defined in
extensions.


> §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.

I think this actually makes more sense in the Authorization Request section
since it's part of the steps of validating that request.

> §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).

There isn't any recourse for the client developer to take here, but I do
feel like it's worth mentioning anyway.

> §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.”

Agreed.

>   ¶3: The client shouldn’t ever be adding query parameters to the token
endpoint, so this requirement is not meaningful.

Agreed, I'll remove that.

>   ¶5: We should specify form encoding as the input body here.

Not sure what you mean, I don't see any reference to the input body here at
all, I'm not sure it's worth adding that just to point out the form
encoding.

>   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.

I agree but I think that would be better suited for section 3.2.

> §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.

Agreed, I'll just remove this sentence.

> §4.1: The requests aren’t coming “From the authorization server”, suggest
reword: … capable of receiving incoming requests (via redirection from the
authorization server).

I agree this is misleading, but I'd like to go even further to avoid the
term "receiving incoming requests" entirely, to make it more clear that
it's a redirect based flow.

> §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.

I've collapsed this into the previous paragraph.

> §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).

Agreed on all counts.

> §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.

Sounds great, thanks.

> §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

This makes sense, thanks.

> §6: 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?

I think this is implying that the client can get a new access token by
starting the authorization code flow again, not suggesting that the
authorization code itself would be long lived. I'll rephrase this to be
clearer.

> §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.


Feedback from Vittorio:

> §2.1
> “browser-based application”
> I am not convinced this is so much easier than the original
“user-agent-based”. I understand the advantages (dovetails w the BCP, more
precise given that apps can be user agent as wells nowadays, more familiar)
however the break w 2.0 terminology is jarring. I don’t feel very strongly
about it but enough to type it.

I do think this is a much more common term for it these days. I was
considering going one step further and calling it "single-page
application", but that felt like a step too far.

> §2.2
> It’s a bit odd to define the client identifier like it’s something brand
new when §2.1 already introduced it. This language from the original 2.0
might need to be revised to accommodate that change.

Good catch. I'm revising section 2.1 to not mention client_id, same as in
RFC6749.

> §2.3
> P1
> That sounds vaguely circular, given that being assigned credentials might
be considered part of the “establish a client authentication method” task
listed there. I’d simply say “if the client is confidential or
credentialed”.

Agreed.

> P2
> I’d add “by the authorization server” for good measure.

My understanding is the "(or establish)" parenthetical there was to allow
clients to bring their own private key proof as a form of client
authentication, in which case it is not the authorization server issuing
the credentials.

> §2.3.1
> We no longer mentioned the empty client secret, but we don’t forbid it
either. What’s our stance?

I'm not sure anyone reading this would assume you could use an empty string
as a client secret anymore. Is this really necessary to spell out
explicitly?

> §2.3.2
> In §2.3 we mention MTLS, private_key_jwt, but here we just point the
reader to IANA. It looks like echoing those methods here might help clarity.

Done.

> §3.1
> Last paragraph
> I have been in discussions where readers interpreted this as “you cannot
send custom parameters to the authorization server”. To preempt that
mistake, we mighr consider calling out that custom extensions _are_
permitted as long as the AS supports them. I know that’s what the current
language says already.

For now I've moved the sentence "The authorization server MUST ignore
unrecognized request parameters." into its own paragraph so that it's more
obvious. I struggled to find a way to write a sentence capturing the idea
of allowing extensions.

> §3.1.1
> Wondering if referring to some specific, well known extensions (like
OIDC) might help readers to better understand this point.

I don't want to mention any specific response type values, but I don't mind
mentioning OpenID Connect defines some additional ones.

> §3.1.2.2
> P3
> “lack of requiring” doesn’t sound proper.

Agreed, I've rephrased this.

> §3.2
> P2
> Should we also say that the spec doesn’t care about _when_ the client
obtains the endpoint?

I like the idea of making this a little more specific so I've expanded the
mentions of the service documentation and metadata document to include an
element of time.

> Last P
> Same considerations as §3.1

I've moved the "MUST ignore" to a separate paragraph like in 3.1.

> §3.3
> Wondering if the “scope strings order does not matter” point should be
somehow emphasized or clarified. I know of implementations who considered
heuristics such as “if the scopes requested correspond to multiple
resources, I’ll show consent for all byt the token eventually issued when
redeeming the code will have as audience the resource corresponding to the
FIRST requested scope”, which would violate the order invariant requirement.

I don't know how to make this more explicit, but suggestions are welcome!

> §4
> Potentially VERY confusing. I would recommend to be more specific and
state that “OAuth 2.1 defines two grant types”.

> Step 5
> “optionally, a refresh token” is too vague IMO. I will look for
opportunities to clarify later in the spec, given that this might not be
the best place to go in details.

I think this is fine for the summary, and the specifics of whether refresh
tokens are returned can be discussed later in the appropriate section.

> §4.1.1
> Overall: a high summary of the steps in this preamble might help. The
current denormalization in subsection can be pretty hard to follow for
someone seeing this for the first time.
> Also: creating challenge and verifier BEFORE assembling the request seems
profoundly counterintuitive to me, as it emphasizes a security measure over
the core function of this leg of the flow. Unless there’s a crypto reason
for this current sequencing that I can’t see, I recommend first creating
the core request (what’s now 4.1.1.3) and then attaching challenge and
verifier. Also, sending the message can be its own subsection rathe than
being conflated with the last message composition subsection.

I suppose you're right. My thought was that this was the order in which
someone's implementation would execute, but I can see how this is less
intuitive when reading the spec. I'll rearrange this a bit to hopefully
flow better.

> P1
> “to begin” remains a bit suspended, given that there’s no obvious segue
on what constitutes the steps after the beginning.

I've added a summary of the following steps to this sentence.

> P2
> “later use with the authorization code” could be clearer, e.g. “at
authorization code redemption time”. At this point that might still not be
obvious for the reader.

Agreed, thanks.

> Mentioning the provenance of properties (parameters?) code_challenge and
code_verifier without first having introduced them might confuse people not
already familiar with them and the request process in general, as their
function will not be obvious not naturally map with the preceding sentencer.

I've moved this description lower in the document after they are defined.

> P3
> Imposing a MUST before knowing what those this are yet is not as clear as
it would be if this would be stated after their use and function has been
explained.

This has also been moved to after the description of the PKCE mechanism.

> §4.1.1.3
> On state. Given the change vs OAuth2, I think it might be helpful to call
out the relevant section on the appendix about differences to help people
familiar w 2.0 not to miss this important change and avoid doing work twice.

I am not sure exactly what the suggestion here is.

> P8
> “he server MUST NOT  include the "code_challenge" value in client
requests”, was that meant to be “responses”?
> Qualifying “other entities” (anyone but the AS?) might make this point
clearer.

Yeah I guess this is a client request from the point of view of the AS, but
I agree it's terribly confusing this way. Justin had some good suggested
text that addresses this.

> §6
> P1
> I think the risk assessment is just one of the factors an AS might use to
decide whether to issue an RT or not. The current language suggests risk is
the only determinant in that decision and that doesn’t seem right.

Agreed, I'll add "and their own policies" to that, if you think it needs
more than that we can continue the discussion.

> Saying that one might refresh tokens using other grants seems odd. A new
authorization code grant gets me a new token and offers me the opportunity
to describe what token I want (scopes etc), the fact that I might choose to
ask the exact same things I asked in the original request is expediency. I
would rather phrase this as the fact that the client can simply repeat the
original request, and external factors such as cookies, sessions and other
auth method specific options may allow the client to do so without
prompting the user.

Agreed, Justin pointed this out too so I've rephrased this to hopefully be
clearer.

> §6.1
> It’s a bit confusing that half of the RT use requirements is in §6 (the
requirement to authenticate confidential/credentialed clients) and half is
in here, with the only differentiator being the nature of the client. This
is pretty minor, but I think I would personally find clearer if all the
requirements for the use of an RT would be consolidated in a single place.
It’s true that the public client reqs are a SHOULD, but still.

Agreed, I've restructured this into refresh token request and response
sections.

> Rotation.
> Wondering whether it would be wise to advise the reader to have their AS
revoke all the still valid ATs issued by the AS from the same
session/family of RTs upon detection of a RT reuse. It is not uncommon for
clients to request new ATs before their projected expiration.


> P6
> I think the “MAY” here might be confusing when applied to the rotation,
as in either the AS does it, or the scenario won’t work. I understand this
is formally correct, but perhaps explicitly calling out some cases in which
the AS might decide to do otherwise and acknowledge that in that case the
client will be stuck might help clarify. Also, if the public client
protection measures were in §6 instead of here, there would be less
opportunities for confusion as it would be easier to grok that this doesn’t
apply to the rotation case only (now adjacent) but to other RT reissuance
cases as well (eg sliding expiration).

I believe this is clarified by splitting and merging sections 6 and 6.1
into the new request/response sections.



Thanks again for the detailed reviews, and I will continue processing the
rest of the feedback!

Aaron



On Mon, Feb 1, 2021 at 6:10 PM Aaron Parecki <[e-mail]> wrote:

> Thank you both for the detailed review! I'm responding inline to the
> things I've gone ahead and updated the draft to reflect. There's a lot
> going on here, so I'm only touching the introduction section for now. I've
> opened issues on the github repo to track the other items that will take
> some more discussion to work out.
>
> ---
>
>
>> §1.1
>> In the RS definition, wondering whether including the word “API” would
>> help to clarify what an RS is in practice.
>
>
> I agree, I added the sentence "The resource server is often accessible via
> an API."
>
> §1.3.1
>> We don’t say anywhere here that the authorization code can be exchanged
>> for an access token. It can be somewhat inferred from 1.2, but it’s a bit
>> of an intelligence test (one needs to infer from authorization grant).
>>
>
> Agreed, I've updated this with some language to clarify that the
> authorization code is later exchanged for an access token.
>
>
>> P2
>> “obtains authorization” could be more specific, to reinforce that we are
>> doing a delegated flow. “Obtains” seem to suggest that we are talking about
>> consent, rather than AS side rules. If that’s the case, calling it out
>> might make the scenario clearer.
>>
>
> I'm not quite sure what you're getting at here, but I agree this paragraph
> could be improved, so I've added some context to it around consent and
> MFA/delegated accounts.
>
>
>> P3
>> Both the benefits listed apply to confidential clients only. Not sure
>> whether calling it out here would help prevent confusion later on (eg
>> people thinking that public clients can prove their identity) or would
>> bring confusion on now (given that we didn’t differentiate between client
>> types yet). Either ways, formally we are OK here; I am just thinking how to
>> make things clearer. Perhaps defining client types before grants might help
>> being clearer here.
>
>
> I think this is fine as is, technically the access token doesn't go
> through the user agent in mobile apps since it's delivered directly to the
> native app code in response to an HTTP request (which may even use
> certificate pinning if you want to be really paranoid).
>
>
> §1.3.2
>> A concrete example of credential (eg shared secret) might help clarify
>> things here. Also, the fact that client credentials indicate both a grant
>> in itself and an artifact (which participates in other grants) is a well
>> know source of confusion. Wondering if calling this out here might help.
>
>
> I've added this: "(e.g. a `client_secret` or a private key used to sign a
> JWT)"
>
>
> §1.4
>> In general, we use “access token” and “token” interchangeably- perhaps
>> pedantic, but I would suggest we always use “access token” to prevent
>> confusion with refresh tokens later on, and other token types in other
>> contexts (eg think ID tokens).
>
>
> I went through and updated every use of "token" that refers to an access
> token to actually mention access token.
>
>
> P1
>> The client should treat the AT string as opaque, but that doesn’t
>> necessarily means it is: in some cases the client CAN see inside the token,
>> and with the current language they might interpret it as “in this case,
>> it’s OK to look- otherwise they would have made it opaque, per the spec”.
>
>
> How about this: "The string is considered opaque to the client, even if it
> has a structure. Depending on the authorization server, the access token
> string may be parseable by the resource server."
>
>
> I didn't think it was appropriate to add a "MUST NOT" or something here
> given that this is still the introduction.
>
>
> §1.5
>> The first phrase of P1 is wonderfully clear. We should have the
>> equivalent in §1.3.1
>
>
> Yes! "An authorization code is a temporary credential used to obtain an
> access token." I've updated the rest of the paragraph as well.
>
>
> Not having defined a mechanism for requesting a RT here, leaving it to
>> ASes to decide when and where, created the situation in which some AS only
>> issues RTs when they get the offline_access scope, with all the unfortunate
>> consequences about RT lifetime vs session lifetime etc… I know we can’t
>> really change this now as we don’t want to break existing AS
>> implementations, but wondering if there’s anything we can say to further
>> clarify/give readers a headsup about the ambiguity/diversity of behaviors
>> they’ll encounter here.
>
>
> Yeah this is tricky, I've added this to emphasize all the different
> reasons why a RT may or may not be returned: "... and may be issued based
> on properties of the client, properties of the request, policies within the
> authorization server, or any other criteria."
>
> P2
>> It’s odd that we say “usually opaque to the client”  for the RT while we
>> decisively said opaque for the AT. Also, the client shouln’t do anything w
>> the RT content hence I think the same considerations done for §1.4P2 apply
>> here.
>
>
> Good catch. I'm fixing it the same way as the AT definition above.
>
> “The token denotes an identifier used to retrieve the authorization
>> information” gets into the specifics of the implementation and it’s not
>> universally true (some AS encrypt/sign the authz info in the RT itself and
>> have no server state whatsoever.
>
>
> Yep, how about: "The refresh token may be an identifier used to retrieve
> the authorization information or may encode this information into the
> string itself."
>
> Step 3
>> Should we add a reference to RFC6750 here?
>
>
> RFC6750 is being folded into this spec, so no I don't think so.
>
> §2
>> “end-user interaction with an HTML registration form” is oddly specific
>> 😊 in particular, I think “end user” might be misleading. We can either say
>> “interactive” or refer “the client app developer” or equivalent.
>> Overkill but I’ll mention it anyway. Should we say that typically the
>> client registration in the non-dynamic scenario occurs in authenticated
>> settings? Not strictly necessary but might help the reader to tie what we
>> say in this section with their concrete experience.
>
>
> Yeah it does seem like it would be better to say "client developer" than
> "end user". Also "the client registers with the authorization server" is
> only true with dynamic client registration. I've updated the whole section
> to hopefully make more sense.
>
> I'm going to save processing the feedback from here out for a separate
> email.
>
> ---
>
> Now I'm going to respond to Justin's feedback on the introduction section
> in the same way here.
>
> §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.
>
>
> I like it, it's a small change but I think it makes it clearer.
>
> §1: We should add a note on password use to this list:
>
>
> Thanks! This is a great consideration, I've added it to the list.
>
> ¶3/4 can probably be collapsed to read better, and some of the language
>> cleaned up. Recommend:
>
>
> Thanks, looks great, I've incorporated this.
>
> ¶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)
>
>
> Thank you! I've replaced a few of them I found, and I will replace the
> rest if I find more later.
>
> §1.4: This section should mention introspection explicitly. Recommend
>> rewriting the intro to ¶3:
>
>
> Looks great, thanks.
>
> §1.5: “The string is usually opaque” should be “the string is opaque"
>
>
> Updated to "the string is considered opaque" based on Vittorio's
> suggestion as well.
>
> §1.6: this should probably refer to the TLS BCP195 here instead of the RFC
>> and version.
>
>
> Great, thanks.
>
> §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.
>
>
> Great idea, thanks.
>
> §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:
>
>
> Great idea, I've incorporated your initial suggested and added a link to
> the existing "differences from OAuth 2.0" section. We can expand further
> later as well.
>
> ---
>
> I'm tracking the feedback that I didn't respond to here from the
> Introduction section 1 on GitHub, (
> https://github.com/aaronpk/oauth-v2-1/issues) so feel free to continue
> the discussion in the appropriate thread there, or respond on the mailing
> list with a reference to the github issue so it's easier to manage.
>
> I'll continue working on this feedback from the next sections as well.
>
> Thanks!
>
> Aaron Parecki
>
>
>
> On Tue, Dec 8, 2020 at 3:22 PM <[e-mail]> wrote:
>
>> Dear authors,
>>
>> It took ages but I finally managed to go thru a full review of the
>> current OAuth2.1 draft. Apologies for the delay.
>>
>> Metacomments:
>>
>>    - The VAST majority of the comments are suggestions for improving
>>    clarity, mostly on historical language coming from 2.0 that I found myself
>>    having to clarify to customers and colleagues over and over thru the years.
>>    None of those are critical.
>>    - There are a few places where 2.1 requires a MUST I believe to be
>>    unwarranted/too restrictive. For each of those I did my best to provide
>>    context and concrete examples.
>>    - A sizeable category of comments and disagreements on MUST come from
>>    treating mobile and desktop apps as largely equivalent under the “native
>>    app” umbrella, despite of the vast gulf that separates the two both in
>>    terms of security posture and user experience. Again, I tried to be as
>>    matter of fact as possible in there.
>>    - The main reason for which I spoke up during the IETF interim on
>>    oauth2.1 was the confusion the omission f the implicit grant caused among
>>    the devs using implicit in OIDC for obtaining ID_tokens. I suggested some
>>    language to pre-empt the issue, but I expect some iteration there.
>>
>> Thanks,
>>
>> V
>>
>>
>>
>> §1
>>
>> I wonder whether we should take the opportunity offered by OAuth2.1 to
>> clarify frequent points of confusion about OAuth, by explicitly calling out
>> in the introduction what is out of scope.
>>
>> For example: OAuth is not an identity protocol, as it doesn’t concern
>> itself with how resource owners are authenticated; OAuth isn’t meant to
>> address 1st party scenarios, although the reader is free to use it in
>> that context as well; and so on.
>>
>> I believe there is value in adding this in the introduction rather than
>> relegating it in some later considerations section, as the people who need
>> this information the most rarely read past this point.
>>
>>
>>
>> §1.1
>>
>> In the RS definition, wondering whether including the word “API” would
>> help to clarify what an RS is in practice.
>>
>>
>>
>> §1.2
>>
>> I always found this part extraordinarily difficult to decipher. I get
>> that this is the first description and doesn’t have to be exhaustive and
>> consider all cases (eg it’s ok if step 3 claims that the client
>> authenticates w the AS even tho that’s only for confidential clients), but
>> I think it could be much clearer than it is today.
>>
>> Step 1 says
>>
>> The client requests authorization from the resource owner.  The
>> authorization request can be made directly to the resource owner (as
>> shown), or preferably indirectly via the authorization server as an
>> intermediary.
>>
>> Besides the fact that “requests authorization” is a bit vague, this step
>> and the corresponding diagram leg does not correspond at all to what
>> normally happens- to get a code, the client does need to hit the AS and the
>> mention in passing in the text isn’t enough to figure that out. Also, with
>> the omission of ROPG there really isn’t any way of asking anything to the
>> RO directly (the client creds doesn’t involve the RO).
>>
>> I would recommend updating that diagram to be more descriptive of the
>> canonical scenario.
>>
>> Step 2
>>
>> mentions the 2 grants defined in the spec, but only one of them
>> represents the RO’s authorization. Claiming that the client itself is the
>> RO is a formalism that doesn’t meet the reader’s intuition at this point.
>>
>> Step 5
>>
>> The language here triggered multiple discussions, in particular on
>> whether the AT can actually be used to ascertain the identity of the client
>> – that isn’t the case for public clients, for example; besides, that’s not
>> really the highest order bit of the AT. If it is, it seems that the spec
>> should be more explicit about how client identification from the RS by
>> means of an AT works. If it isn’t, perhaps we should change the language to
>> omit authenticate.
>>
>> The last paragraph is emblematic IMO – if the preferred method is very
>> different from the diagram here, and if the abstraction presented here is
>> not terribly useful (given that we no longer have multiple RO based grants,
>> excluding the extension grants that are still too far at this point to
>> warrant a cognitive downpayment for the reader) I wonder whether we’d be
>> better off doing the authz code diagram directly (and mention that we also
>> have the client creds grant separately).
>>
>>
>>
>> §1.3
>>
>> I understand that we can’t really change this because we inherit from
>> OAuth 2 but I’ll mention it anyway- modeling clients as ROs is problematic,
>> as it doesn’t often match what happens in practice. A confidential client
>> might batch-read a user’s inbox searching for ad words, but the resource
>> owner remains the user.
>>
>> I know we straighten things up in 1.3.2, but the positioning here is
>> confusing.
>>
>> Also: isn’t the refresh token grant a core-specified grant as well? I
>> know I am nitpicking.
>>
>> §1.3.1
>>
>> We don’t say anywhere here that the authorization code can be exchanged
>> for an access token. It can be somewhat inferred from 1.2, but it’s a bit
>> of an intelligence test (one needs to infer from authorization grant).
>>
>> P2
>>
>> “obtains authorization” could be more specific, to reinforce that we are
>> doing a delegated flow. “Obtains” seem to suggest that we are talking about
>> consent, rather than AS side rules. If that’s the case, calling it out
>> might make the scenario clearer.
>>
>> P3
>>
>> Both the benefits listed apply to confidential clients only. Not sure
>> whether calling it out here would help prevent confusion later on (eg
>> people thinking that public clients can prove their identity) or would
>> bring confusion on now (given that we didn’t differentiate between client
>> types yet). Either ways, formally we are OK here; I am just thinking how to
>> make things clearer. Perhaps defining client types before grants might help
>> being clearer here.
>>
>>
>>
>> §1.3.2
>>
>> A concrete example of credential (eg shared secret) might help clarify
>> things here. Also, the fact that client credentials indicate both a grant
>> in itself and an artifact (which participates in other grants) is a well
>> know source of confusion. Wondering if calling this out here might help.
>>
>>
>>
>> §1.4
>>
>> In general, we use “access token” and “token” interchangeably- perhaps
>> pedantic, but I would suggest we always use “access token” to prevent
>> confusion with refresh tokens later on, and other token types in other
>> contexts (eg think ID tokens).
>>
>> P1
>>
>> The client should treat the AT string as opaque, but that doesn’t
>> necessarily means it is: in some cases the client CAN see inside the token,
>> and with the current language they might interpret it as “in this case,
>> it’s OK to look- otherwise they would have made it opaque, per the spec”.
>>
>>
>>
>> §1.5
>>
>> The first phrase of P1 is wonderfully clear. We should have the
>> equivalent in §1.3.1
>>
>> Not having defined a mechanism for requesting a RT here, leaving it to
>> ASes to decide when and where, created the situation in which some AS only
>> issues RTs when they get the offline_access scope, with all the unfortunate
>> consequences about RT lifetime vs session lifetime etc… I know we can’t
>> really change this now as we don’t want to break existing AS
>> implementations, but wondering if there’s anything we can say to further
>> clarify/give readers a headsup about the ambiguity/diversity of behaviors
>> they’ll encounter here.
>>
>> P2
>>
>> It’s odd that we say “usually opaque to the client”  for the RT while we
>> decisively said opaque for the AT. Also, the client shouln’t do anything w
>> the RT content hence I think the same considerations done for §1.4P2 apply
>> here.
>>
>> “The token denotes an identifier used to retrieve the authorization information” gets into the specifics of the implementation and it’s not universally true (some AS encrypt/sign the authz info in the RT itself and have no server state whatsoever.
>>
>> Step 3
>>
>> Should we add a reference to RFC6750 here?
>>
>>
>>
>> §1.8
>>
>> Should we say rich **delegated** authorization framework?
>>
>>
>>
>> §2
>>
>> “end-user interaction with an HTML registration form” is oddly specific 😊 in particular, I think “end user” might be misleading. We can either say “interactive” or refer “the client app developer” or equivalent.
>>
>> Overkill but I’ll mention it anyway. Should we say that typically the client registration in the non-dynamic scenario occurs in authenticated settings? Not strictly necessary but might help the reader to tie what we say in this section with their concrete experience.
>>
>>
>>
>> §2.1
>>
>> P4
>>
>> “Authorization servers SHOULD consider the level of confidence in a client's identity when deciding whether they allow such a client access to more critical functions, such as the Client Credentials grant type.”
>>
>> I don’t understand this sentence. Is the client credentials grant type a “more critical function”? Or is it a level of confidence? Either ways, I think it needs clarifying.
>>
>> P5
>>
>> IMPORTANT: this is going to break many OAuth implementations with significant adoption. Auth0 is fine (each client_id is tied to a single client type) but I know of others that will break.
>>
>> I suggest softening to a SHOULD NOT.
>>
>> “browser-based application”
>>
>> I am not convinced this is so much easier than the original “user-agent-based”. I understand the advantages (dovetails w the BCP, more precise given that apps can be user agent as wells nowadays, more familiar) however the break w 2.0 terminology is jarring. I don’t feel very strongly about it but enough to type it.
>>
>>
>>
>> §2.2
>>
>> It’s a bit odd to define the client identifier like it’s something brand new when §2.1 already introduced it. This language from the original 2.0 might need to be revised to accommodate that change.
>>
>> Wondering whether a warning against structured client_ids (eg identifiers assembled thru some string template, like developer name+region+serial) would be in order. Perhaps in the security considerations?
>>
>>
>>
>> §2.3
>>
>> P1
>>
>> That sounds vaguely circular, given that being assigned credentials might be considered part of the “establish a client authentication method” task listed there. I’d simply say “if the client is confidential or credentialed”.
>>
>> P2
>>
>> I’d add “by the authorization server” for good measure.
>>
>> P3
>>
>> That sounds vague. Shouldn’t it be mandatory for the AS to require client auth for the client types who have creds? “if possible” seem to open the possibility of circumstances where that’s not he case.
>>
>> P5
>>
>> I think that this idea of identifying the client will need to be fleshed out more for people to fully understand it. Credentialed clients can prove that they are the same client instance across multiple transactions, which some might consider a weak form of identification. To rule that out, it has to be mentioned upfront IMO. If not here, in some of the considerations section… with a forward reference here.
>>
>> P6
>>
>> Do we say why anywhere? If yes, we should reference it. If not, perhaps we should.
>>
>>
>>
>> §2.3.1
>>
>> We no longer mentioned the empty client secret, but we don’t forbid it either. What’s our stance?
>>
>>
>>
>> §2.3.2
>>
>> In §2.3 we mention MTLS, private_key_jwt, but here we just point the reader to IANA. It looks like echoing those methods here might help clarity.
>>
>>
>>
>> §3.1
>>
>> Last paragraph
>>
>> I have been in discussions where readers interpreted this as “you cannot send custom parameters to the authorization server”. To preempt that mistake, we mighr consider calling out that custom extensions _are_ permitted as long as the AS supports them. I know that’s what the current language says already.
>>
>>
>>
>> §3.1.1
>>
>> Wondering if referring to some specific, well known extensions (like OIDC) might help readers to better understand this point.
>>
>>
>>
>> §3.1.2
>>
>> RFC3986 6.2.1 talks about character by character comparison, but doesn’t mention case sensitivity. I am sure it does elsewhere in the spec, but for clarify and readability I recommend specifying the desired behavior directly here.
>>
>>
>>
>> §3.1.2.1
>>
>> Personally, I would advocate for a MUST here. True, lots of people won’t comply at development time, but I think that’s OK as long as they do use TLS when going in production.
>>
>> Also, SameSite changes are making the use of HTTPS at dev time more and more common. If OAuth2.1 is about picking the best of the security practices, this seems like a an obvious candidate.
>>
>>
>>
>> §3.1.2.2
>>
>> P3
>>
>> “lack of requiring” doesn’t sound proper.
>>
>>
>>
>> §3.2
>>
>> P2
>>
>> Should we also say that the spec doesn’t care about _when_ the client obtains the endpoint?
>>
>> Last P
>>
>> Same considerations as §3.1
>>
>>
>>
>> §3.2.1
>>
>> P1
>>
>> That’s stricter than §2.3P3 – I think the language there should be tweaked to be coherent with the one here.
>>
>>
>>
>> §3.3
>>
>> Wondering if the “scope strings order does not matter” point should be somehow emphasized or clarified. I know of implementations who considered heuristics such as “if the scopes requested correspond to multiple resources, I’ll show consent for all byt the token eventually issued when redeeming the code will have as audience the resource corresponding to the FIRST requested scope”, which would violate the order invariant requirement.
>>
>>
>>
>> §4
>>
>> Potentially VERY confusing. I would recommend to be more specific and state that “OAuth *2.1* defines two grant types”.
>>
>>
>>
>> §4.1
>>
>> Diagram
>>
>> Not critical. But I want to point it out. The first time I saw this diagram I found it confusing. The fact that the same numeral is assigned to multiple legs is just odd for anyone not already familiar with the flow, possibly still struggling to understand the client as a service side component.
>>
>> Also, now that we have mighty SVG support, I would strongly advocate for a modern version of this diagram (there lines perhaps don’t need to be broken into segments).
>>
>> Step 5
>>
>> “optionally, a refresh token” is too vague IMO. I will look for opportunities to clarify later in the spec, given that this might not be the best place to go in details.
>>
>>
>>
>> §4.1.1
>>
>> Overall: a high summary of the steps in this preamble might help. The current denormalization in subsection can be pretty hard to follow for someone seeing this for the first time.
>>
>> Also: creating challenge and verifier BEFORE assembling the request seems profoundly counterintuitive to me, as it emphasizes a security measure over the core function of this leg of the flow. Unless there’s a crypto reason for this current sequencing that I can’t see, I recommend first creating the core request (what’s now 4.1.1.3) and then attaching challenge and verifier. Also, sending the message can be its own subsection rathe than being conflated with the last message composition subsection.
>>
>> P1
>>
>> “to begin” remains a bit suspended, given that there’s no obvious segue on what constitutes the steps after the beginning.
>>
>> P2
>>
>> “later use with the authorization code” could be clearer, e.g. “at authorization code redemption time”. At this point that might still not be obvious for the reader.
>>
>> Mentioning the provenance of properties (parameters?) code_challenge and code_verifier without first having introduced them might confuse people not already familiar with them and the request process in general, as their function will not be obvious not naturally map with the preceding sentencer.
>>
>> P3
>>
>> Imposing a MUST before knowing what those this are yet is not as clear as it would be if this would be stated after their use and function has been explained.
>>
>>
>>
>> §4.1.1.3
>>
>> On state. Given the change vs OAuth2, I think it might be helpful to call out the relevant section on the appendix about differences to help people familiar w 2.0 not to miss this important change and avoid doing work twice.
>>
>>
>>
>> §4.1.2
>>
>> P2
>>
>> Should we say that the code should be opaque to the client, to discourage the use of structured code templates that can be partially manufactured?
>>
>> P8
>>
>> “he server MUST NOT  include the "code_challenge" value in client requests”, was that meant to be “responses”?
>>
>> Qualifying “other entities” (anyone but the AS?) might make this point clearer.
>>
>>
>>
>> §4.3
>>
>> We mentioned extension grants in passing, but I don’t recall seeing a definition/description of their function in the context of the framework. Even a short sentence to that effect here would help, given that the section title names them explicitly. Also, stressing that the device flow is just one example and other extensions might differ (for example in their logic to establish whether an access token request is valid and authorized) would go a long way in helping the reader put this section in better focus.
>>
>>
>>
>> §5.1
>>
>> On the access_token parameter. Given the discussions we had for the JWT AT profile draft, I am wondering whether it should be called out here that the AT recipient is the RS, that the client should not expect to be able to parse the access_token, and that the AS is under no obligation to use a consistent AT encoding outside of what is negotiated with the RS. I don’t feel very strongly about this, or about where in the spec this should be called out, but it sure would have made life easier in those discussions- hence the comment.
>>
>> On the refresh_token parameter. The lack of details in how OAuth2 describes how/when an AS returns refresh tokens led to today’s complicated situation in which many implementations issue RTs only when OIDC’s offline_access is received in the scopes, as it was the only mention in public specs describing a concrete behavior. See the associated online_access discussion on the OIDC list, as RTs gain importance as session artifacts of sort for SPAs now that implicit is dead and ITP makes iframe renewals problematic.
>>
>> Unfortunately it is too late to be prescriptive here, as we cannot break compatibility with whatever choices existing AS implementations made. However we can be more descriptive and give the reader a better idea of what’s the range of possibilities. Some nonnormative examples of how existing AS determine whether to issue an RT or not (eg as an option determined at client registration time, or any other heuristic you guys encountered in the wild) might help people to better understand their options and the intent of the specification here.
>>
>>  §5.2
>>
>> It might help to remind the reader here that extensions to the core spec might specify or further specialize circumstances in which the errors mentioned here are returned (for example, see the validation errors in the JWT AT profile). There’s a mention of that in §7.3.1 but that’s pretty far, and having even brief language here might be handy for people reading the spec for reference rather than cover to cover.
>>
>>
>>
>> §6
>>
>> P1
>>
>> I think the risk assessment is just one of the factors an AS might use to decide whether to issue an RT or not. The current language suggests risk is the only determinant in that decision and that doesn’t seem right.
>>
>> Saying that one might refresh tokens using other grants seems odd. A new authorization code grant gets me a new token and offers me the opportunity to describe what token I want (scopes etc), the fact that I might choose to ask the exact same things I asked in the original request is expediency. I would rather phrase this as the fact that the client can simply repeat the original request, and external factors such as cookies, sessions and other auth method specific options may allow the client to do so without prompting the user.
>>
>> P2
>>
>> We might need to be more precise here. Do we mean the scopes consented by the RO in the request that led to the issuance of the RT being used? Just saying consented by the RO for the client does not exclude cases in which there are more instances of the client in operation. Say that I am running uber on phone 1 and I consent to read my google calendar, getting AT1 and RT1. Say that on phone 2 I also run the uber app, and this time I consent to write my google calendar, obtaining AT2 and RT2 on this new device. Now consider the various combinations here. Should RT2 allow me to get calendar:read too, given that it was already consented by RO for this client? Should RT1 allow me to get AT1’ containing calendar:write, given that RO consented for it when using a different instance of the same client? Whatever is the answer you want to the questions above, I think the spec should have language clear enough to unambiguously determine the desired behavior.
>>
>>
>>
>> §6.1
>>
>> It’s a bit confusing that half of the RT use requirements is in §6 (the requirement to authenticate confidential/credentialed clients) and half is in here, with the only differentiator being the nature of the client. This is pretty minor, but I think I would personally find clearer if all the requirements for the use of an RT would be consolidated in a single place. It’s true that the public client reqs are a SHOULD, but still.
>>
>> Rotation.
>>
>> Wondering whether it would be wise to advise the reader to have their AS revoke all the still valid ATs issued by the AS from the same session/family of RTs upon detection of a RT reuse. It is not uncommon for clients to request new ATs before their projected expiration.
>>
>> P6
>>
>> I think the “MAY” here might be confusing when applied to the rotation, as in either the AS does it, or the scenario won’t work. I understand this is formally correct, but perhaps explicitly calling out some cases in which the AS might decide to do otherwise and acknowledge that in that case the client will be stuck might help clarify. Also, if the public client protection measures were in §6 instead of here, there would be less opportunities for confusion as it would be easier to grok that this doesn’t apply to the rotation case only (now adjacent) but to other RT reissuance cases as well (eg sliding expiration).
>>
>> On the identical scopes requirement. Say that after obtaining RT1, which includes scopes s1 and s2 for client c1, the RO revokes authorization for c1 to use s2. Should the AS fail the RT redemption, or return an AT with only s1 and a scopes parameter informing the client of the change? As developer I would prefer the latter, to preserve the experience: but if we are adamant about the current language, I think it might be useful to explicitly call out that any changes to the grant on the AS side should result in failure of the RT redemption.
>>
>> P7
>>
>> Calling out deprovisioning of RO might be useful as well.
>>
>> On “the logout at the AS” event, that’s definitely a valid case but I worry about how presenting that alone might reinforce misunderstandings that equate RTs with sessions. There are certainly times where we want that (see mentions earlier of the online_access discussions) but there are also cases where the ability of a client to refresh ATs needs to survive session boundaries (offline_access) and confusing the two are problematic. I don’t have a clear solution here, just pointing out a potential point of confusion. Maybe there will be more opportunities to clarify later in the spec.
>>
>>
>>
>> §7
>>
>> P1
>>
>> An important case to call out is the AS-RS colocation, where neither introspection nor token format agreements are necessary. I suggest mentioning it openly.
>>
>>
>>
>> §7.2
>>
>> “bearer tokens may be extended to include proof-of-possession  techniques by other specifications” sounds like an oxymoron. Wouldn’t PoP make the token no longer bearer by the very definition above?
>>
>> It looks like we might need a term for simply “token”.
>>
>>
>>
>> §7.2.1
>>
>> Do we also want to forbid two tokens in the same request, using different methods? Current language only constraints the behavior of one token.
>>
>>
>>
>> §7.2.3
>>
>> The “insufficient_scope” description here is problematic. The privileges the AT carries/points to are not necessarily (or exclusively) represented by the included scopes (eg the RO might have granted document:read to the client, but RO might have no privileges for the particular document being requested in this particular call). It might be useful to specify that “invalid_scope” should be used for authorization errors that can be actually expressed in terms of delegated authorization, leaving to RS implementers the freedom to handle other authorization issues (eg user privileges, RBAC, etc) with a different error code. Or at least, we should be clear that authorization logic not expressed via scopes is out of scope (pun not intended) for this specification.
>>
>> Note, this isn’t an abstract problem: there are SDKs out there that use “invalid_scope” for every permission issues. Very confusing.
>>
>>
>>
>> §7.3.1
>>
>> We rewrite portions of 6750 in oauth2.1, but here we refer to it as its own spec… that could be confusing as it’s unclear which parts of 6750 are overridden by oauth2.1 (eg no more querystring) and what parts remain normative. Perhaps we can call those things out in the sections meant to replace the corresponding sections in 6750.
>>
>>
>>
>> §7.4.2
>>
>> Pedantic: although the title of the section states it, wondering whether every instance of “token” here should be “access token” instead. Think of cases in which the spec is quoted in discussions and disputes, where snippets can be pasted and mentioned outside of the context of this section.
>>
>> P2
>>
>> Referencing the JWT AT profile as an example of extension providing the info out of scope for the core might help the reader grok a concrete example.
>>
>>
>>
>> §7.4.3.5
>>
>> “one hour or less” seems very arbitrary, and breaks step in respect to what the spec does elsewhere (eg we don’t give any indication of how long an AS should wait to invalidate an RT for inactivity, but we do say the AS should do so). I would actually not provide any reference value here.
>>
>>
>>
>> §7.4.3.6
>>
>> Another opportunity of referencing the JWT AT profile for a concrete example of detailed audience restriction guidance in ATs.
>>
>>
>>
>> §7.4.3.7
>>
>> Besides the indications given to clients here, should we also give guidance to an RS to ignore tokens passed that way?
>>
>>
>>
>> §7.4.5
>>
>> Along the same lines of the comments about delegated authorization earlier for §7.2.3. I think it would be useful to acknowledge here that ATs might carry, and RSs might expect, authorization information that go beyond the delegated authorization for 3rd party API case that is core to OAuth- and remind the reader that those mechanisms are out of scope for oauth hence they shouldn’t expect those aspects to be addressed/handled/regulated by this specification.
>>
>>
>>
>> §8
>>
>> As mentioned earlier, it seems a potentially confusing to reference the section of a document being superseded. I do see an issue in redefining here something already established and in use, hence I am not expecting this to change. Just wondering whether we need to provide a more explicit map of the sections in 6749 that are being updated by oauth2.1.
>>
>>
>>
>> §9
>>
>> Should we also say something about the scenario being chiefly 3rd party clients? We know lots of people use OAuth2 for 1st party scenarios and some considerations might differ. This might be an opportunity to finally make that clear.
>>
>>
>>
>> §9.1
>>
>> P1
>>
>> Well, the AS doesn’t really use client auth... the active part here is the client itself. Perhaps the AS can require it, when possible.
>>
>> P2
>>
>> Unclear. Are we saying that if it is possible to safely distribute keys to client, the AS MUST use client auth? That seems odd, there might be other reasons coming into play (cost, security posture) making that choice not viable. Or is the intent to say that the AS should not use client auth if the key distribution cannot be trusted? That sounds far more realistic, but then the language should be tweaked or the reader might pick the former interpretation.
>>
>> P3
>>
>> The AS can’t really PREVENT creds forwarding as the RO machine might still have funny business going on (eg DNS attacks). Some softer language might be more accurate there.
>>
>> P4
>>
>> That sounds very abstract. What does it mean? That the AS should consider to issuing RTs to public clients? If that’s the case, we should just say so… tho without more details, I don’t know how actionable the guidance here will be. I can see BoFa requiring a user to reauth in their iPhone app after inactivity, but I don’t see Uber doing so for their app.. unless they produce long lived ATs, which isn’t what we want either.
>>
>> P5
>>
>> This could be clearer. The dynamic client registration case just flesh out the confidence level an AS can have in its identity, but does not offer a corresponding privilege level to it – whereas the second case does mention assigned privileges explicitly. Also, pitting “dynamically registered client” vs “web application” might suggest the app type is a factor, whereas AFAIK 7591 can be used for registering web apps too (whether that s wise or not is immaterial here).
>>
>> §9.2
>>
>> Should we say something about whether native clients should be allowed to be “upgraded” as confidential clients in the future, or the other way round? I know of scenarios where people did that to preserve consent info, but that seem sketchy security wise.
>>
>> P3
>>
>> The SHOULD here refers to a requirement that in 10.3.1 is a MUST. I don’t think the MUST is warranted (more about that in the 10.3.1 comments) but if we do keep it, it looks like the level should be coherent here.
>>
>> P4
>>
>> That example is compatible with a SHOULD- works now, but would look odd if we’d upgrade P3 with a MUST for coherence with 10.3.1.
>>
>> P5
>>
>> Inclusion where/how? We should be precise IMO. If it’s just registration material (eg not part of the redirect URI), we should mention how we expect it to be used in the context of OAuth- and if we don’t know, perhaps we should not mention it here.
>>
>>
>>
>> §9.3
>>
>> P1
>>
>> I know it becomes clearer later on, but I think it would help here to explicitly call out confidential and credentialed client as the subject of this sentence. Those are the only client types with credentials, hence the current language is formally correct. This is just for clarity.
>>
>> P2
>>
>> I thought we require redirect URI registration in all cases? This makes it sound it’s only for public clients.
>>
>> P3
>>
>> I have been in various discussions where people were attempting to interpret what “explicit RO authentication” means in practice. Is it a full credential prompt regardless of whether one session already exists? A selection between existing sessions, if present?
>>
>> P4
>>
>> This is unclear. As it currently reads it seems to prohibit things like getting a new authz code silently via iframe (and prompt=none or equivalent UX suppressing mechanism, please ignore the ITP complications for the sake of argument).
>>
>>
>>
>> §9.3.1
>>
>> P1
>>
>> I don’t follow this sentence. The client identity cannot be proved for public clients (see also next P), and the native apps are public clients unless otherwise specified (eg credentialed).
>>
>> P2
>>
>> I find this misleading. Client side measures such as claimed schemes, domains etc might work to prevent an app impersonating another app on the same device/OS, but they aren’t guaranteed to be honored on other operating systems. The AS has no way of knowing whether those measures have been enforced on the client, hence it should not accept them as proof.
>>
>>
>>
>> §9.4.1
>>
>> This is another place where a reference to the JWT AT profile would provide a concrete example of the conditions set forth here (eg RS guidance for audience validation).
>>
>> Also, as mentioned earlier, it might be useful to remind the reader that the AS might include in the token authorization information that go beyond the delegated authz scenario OAuth2.x concerns itself with, and that those aspects are beyond the scope of this specification.  That would truly go a long way preventing people from abusing and overextending the spec on scenarios it is not meant to address.
>>
>> And even for the canonical scenarios, it might be useful to remind the reader that the RS might have extra logic not described in this spec that determines whether the call will be authorized or otherwise- to dispel the notion that the AS is always the sole source of truth for authorization.
>>
>>
>>
>> §9.4.2
>>
>> P2
>>
>> Clarifying that MTLS is one instance of the sender constraint methods just mentioned would prevent some readers considering that an independent, additional constraint.
>>
>>
>>
>>  §9.5
>>
>> In §6.1 we give more details about protection, should we have a backward reference to that section here?
>>
>> P2
>>
>> Is it worth to specify that it doesn’t matter how the AS tracks the binding? (eg server side or embedded in the RT bits themselves).
>>
>>
>>
>> §9.6
>>
>> P1
>>
>> This language makes the assumption that for client cred grants, the sub will take on the client_id value. That it certainly possible, but not a given. As such, the language here should be explicit abouty the fact that it’s what we are expecting to happen in this particular scenario.
>>
>> P2
>>
>> Referring to the client as the actor here is confusing as soon as you move beyond client_id. When you talk about rhe sub or any other value, it’s not as much the client as it is the developer who owns the client. The difference is subtle but might be a source of confusion.
>>
>>  §9.7
>>
>> P3
>>
>> I thought that we were going to make PKCE or nonce the only two mandatory alternatives? Nothing against supporting state as an accepted way to achieve this, just surprised as I recall people being quite adamant about pushing PKCE.
>>
>> P4
>>
>> Looks like the iss response parameter might make the distinct redirect URIs unnecessary?
>>
>>
>>
>> §9.7.1
>>
>> The guidance in this section isn’t likely to be widely followed, but I understand the rationale behind it.
>>
>>
>>
>> §9.8
>>
>> P2
>>
>> That’s probably obvious, but I think we should specify that those multiple attempts should come with the expected code verifier as well for the revocation logic to be triggered- if it’s just the code without verifier, it doesn’t look like the leak went far enough to warrant that intervention.
>>
>> P4
>>
>> If this holds, then the remark about the use of state for CSRF in §9.7 P3 seems unnecessary.
>>
>> P9
>>
>> The use of MUST here seems incompatible with the concession of using nonce instead of PKCE. Either we allow it or we don’t…
>>
>>  §9.11
>>
>> P1
>>
>> Now that we no longer trade in RO passwords, should we mention them here? Sure, if the AS uses passwords they do need protection, but so do lots of other things the AS might use as part of auth that we don’t mention here for Occam’s razor.
>>
>> P3
>>
>> What other means are we referring to?
>>
>>
>>
>> §9.12
>>
>> “service providers” occurs for the first time in the document here. Tying the guidance to entities already mentioned in the doc (clients, AS) might make things clearer/more actionable.
>>
>>
>>
>> §9.13
>>
>> I understand that some of the comments in this area should be done on the BCP rather than here, but here we are. That said…
>>
>> Blanket comment: this section seems to assume native app == mobile app, and that’s not strictly the case. Desktop apps have different characteristics and desktop OSes different capabilities. Would be clearer to make a distinction when relying on mobile OS capabilitesas we  appear to do here.
>>
>> P2
>>
>> Embedded user-agent != fake external user agent, but P2 uses that interchangeably. A desktop app might use an embedded browser for UX reasons and making no attempt whatsoever to disguise that as an external agent. And I would be pretty surprised to see a malware remover get rid of google drive for Mac, or Adobe products, or Office- all native apps using embedded user-agents for authentication.
>>
>> P3
>>
>> It’s unclear how the AS would do that, given that user agent strings can be faked. Google has its ML based secret sauce, but that might not be accessible by everyone.
>>
>> P4
>>
>> This is mobile specific, doesn’t apply to desktop apps as readily. Also unclear how that would change for the end user, given that pixel perfect replicas can easily include fake address bars. More details on how a user would detect an actual browser (presence of an existing session, access to bookmarks etc) might add enough color to help the reader truly understand the extent of the remedy power (or its true limits).
>>
>>
>>
>> §9.14
>>
>> There’s no action for the reader here. If that reflects the actual situation (none of the roles described in this spec can do anything to mitigate or contain the damage, and solutions to prevent the situation lie outside its purview eg use MDM software on your corporate devices) we should explicitly say so.
>>
>>
>>
>> §9.15
>>
>> A bit odd that we used CSRF throughout the document assuming the reader was familiar with it, but here we attempt a definition.
>>
>> Still confused on how we admit state as a valid mechanism, as opposed to limiting to nonce and PKCE. Also note the potential discrepancy called out earlier in which at code redemption time we appear to require PKCE, in contrast to admitting nonce/state here.
>>
>>
>>
>> §9.18.2
>>
>> Wondering if here we should go as far as recommending the AS keeps dynamic client registration OFF when it’s not needed. That might provide good secure by default guidance for AS SDKs and product developers.
>>
>>
>>
>> §9.19
>>
>> I assume we’d add the “iss” response parameter as alternative to the requirements here?
>>
>>
>>
>> §9.20
>>
>> P1, P2, P3
>>
>> The security properties described here do not apply as is to desktop apps using embedded browsers.
>>
>> As mentioned earlier, on the desktop the separation between apps isn’t as stark as on mobile- the keystrokes message pump is potentially accessible at the user session level, encompassing multiple applications. Although process separation mechanisms are in place, the circumstances of the execution and the OS specific features in this respect determine the degree to which entering credentials in one process makes the data inaccessible by another. We should not claim advantages we cannot guarantee, and we should be explicit about what measures specific to dekstops should be considered to mitigate risks eg not executing apps as admins, enable and use UAC at the right level on Windows or equivalent on other OSes, use app sandboxing when the OS supports it, etc. Not suggesting we call out specific technologies like UAC, but at least point at the category of security measures.
>>
>> P4
>>
>> As mentioned above, the presence of the address bar can be easily faked by pixel perfect replicas- relying on those is security theater.
>>
>> P5
>>
>> This is a valid concern, but if we want to frame it in the context of the security considerations section we need to add more color (eg minimizing the situations in which one needs to enter creds is good security practice).
>>
>>
>>
>> Another thing we might add as lowlight for embedded browsers: password managers might not work, making the user’s life difficult and possibly promoting insecure situations (eg placing a random pwd in the clipboard, where other apps might later steal it from).
>>
>> §9.21
>>
>> Do we need this here, given that we cover this in depth earlier?
>>
>>
>>
>> §10
>>
>> I find this subdivision confusing. We have native clients considerations scattered throughout the entire specification, but now we have a dedicated section that somewhat repeats some of the points already made while interleaving new ones. Perhaps having a more specific section title, one that better characterizes the content and intent of this section, would avoid giving the impression that if the reader is interested in native clients, this is the only section they need to read.
>>
>> P2
>>
>> See discussion so far on native vs desktop.
>>
>> Also: I have been in ferocious discussions where people thought the external user agent HAD to be the browser, which we know isn’t the case (hero apps like the FB SDK or OS features like sign in w Apple work just as well). Some language here giving a nod to those non-browser external user agents might help clarify.
>>
>> P4
>>
>> See previous discussion on mobile!=desktop.
>>
>> P5
>>
>> I’d always qualify the “browser” with system, external, device etc given that some reader calls the embedded user-agent “embedded browser”, hence just saying “browser” without modifiers is ambiguous.
>>
>> User authentication state.. on the device. I think specifying it will clarify what this really means.
>>
>>
>>
>> §10.1
>>
>> Unless you scope this statement to MOBILE apps, I think the MUST here should be a SHOULD.
>>
>> The security posture of the desktop is different enough, and the quality of the user experience when using a system browser bad and disomogeneous enough, that a MUST isn’t justified here.
>>
>>
>>
>> §10.2
>>
>> Beware of the native-mobile false equivalence here.
>>
>> P3
>>
>> One or two examples of non-browser external user agents might help.
>>
>> P4
>>
>> Text pasted here is technically not a best practice but core.
>>
>> I see there are examples here, so no need to add them in P3.
>>
>> It’s unclear why the external browser is RECOMMENDED here- if it’s because we can’t go in details of the behavior of non browser apps, it seems like we should say that much rather than making a recommendation. In other words, RECOMMENDED expresses a strong preference for it, but if I am on iOS and I want to sign in with Facebook I am actually better off using their SDK both for security and user experience reasons than to use the system browser.
>>
>>
>>
>> §10.3
>>
>> Requesting a MUST for all 3 methods seems restrictive… why not requesting at least one?
>>
>>
>>
>> §10.3.1
>>
>> P3
>>
>> This seems unnecessarily restrictive. 3.8 in 7595 is mostly SHOULDs, and it refers to organizations rather than individuals. Not every developer owns a domain, not every app goes on an app store and is meant for general public consumption. An organization sideloading apps on managed devices should not be forced to follow those constraints if they control the environment and aren’t worried about other apps competing for the same schema, but placing a MUST here might compel SDK developers to embed validation checks that would make developers on those circumstances deal with complexity without any security upside. In fact, apps should not even be required to have internet access in the general case but this requirement does impose that.
>>
>> I think this is an important best practice that should be encouraged, RECOMMENDED or even SHOULDed , but it shouldn’t be a MUST in the core specification.
>>
>>
>>
>> §10.3.2
>>
>> P4
>>
>> Like everything happening on the client, the AS cannot really take that as guarantee. What might be true for an app running on iOS might not apply if the requests are all manufactured via cURL on a Linux box. The scope of those measures is really limited to one particular device or devices sharing an OS, outside of that cohort there are no guarantees.
>>
>>
>>
>> §10.3.3
>>
>> This section should come with glaring warnings, anything actively listening on the client extends the attack surface- an app listening on the loopback might now get affected by exploits in the local HTTP driver/local network stack and taken over, executing with the same privileges as the user (remember https://www.rapid7.com/db/vulnerabilities/WINDOWS-HOTFIX-MS15-034/). The fact that the loopback is only accessible locally only reduces the risk but doesn’t eliminate it, a local process with lower privileges might use that exploit for elevation of privileges for example. We should at least recommend that the app hosting the loopback adapter runs with low privileges, reiterate the “listen only when you must” and generally warn about the extra attack surface.
>>
>> As AS I might not want to support this method at all, both for security reasons and for the extra logic the wildcard port entails at registration and request serving times, but right now the spec forces me to- more reasons for relaxing the MUST for all 3 methods as mentioned earlier..
>>
>>
>>
>> §12
>>
>> I am not exactly sure where to place the following- this section (or a subsection) might be the best fit.
>>
>> As mentioned during the interim meeting, the omission of the implicit flow in OAuth2.1 has already caused a lot of people to interpret this as an indirect deprecation of the use of implicit flow by OpenID Connect for obtaining ID_tokens, either via traditional  response_modes or via form_post.
>>
>> We already debated and concluded that the reasons that led to the omission of the implicit grant in 2.1 do not apply to ID_tokens, hence there’s no reason for people to stop using OpenID Connect that way.
>>
>> Formally we are in the clear, as OIDC is vased on 2.0 the omission of implicit here does not prevent it as extension grant in OIDC anyway- but the formal stance doesn’t help in preventing the confusion and assuaging the concerns of the developers who aren’t as well versed as the people on this list in all things specifications.
>>
>> Because of this, I recommend we add some language here that prevents that confusion. Something like
>>
>>
>>
>> *  The Implicit grant ("response_type=token") is omitted from this
>>
>>       specification as per Section 2.1.2 of[I-D.ietf-oauth-security-topics <https://tools.ietf.org/html/draft-ietf-oauth-v2-1-00#ref-I-D.ietf-oauth-security-topics>]
>>
>> Please note: the omission of the implicit grant from this specification does not automatically imply that other extension grants obtaining credentials directly from the authorization endpoint should also be discarded. For example, the implicit flow defined in Section 3.2 of [OIDC <https://openid.net/specs/openid-connect-core-1_0.html#ImplicitFlowAuth>] remains valid for all the response_type values not including “token”.
>>
>>
>>
>> Although it might be a bit unusual to refer to details of specs from other entities, this spec already mentions OpenID 9 times even excluding §14.2- and as soon as the browser apps section will be included, that number is certain to rise. And the confusion on this point is truly widespread- adding language along the lines of the above directly in the core would go a long way to save a lot of grief.
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>