[OAUTH-WG] WGLC Review of PAR

[Justin Richer <[e-mail]>]

I’ve done a full read through of the PAR specification, and here are my notes on it.

For additional context, I’ve implemented this specification for both a client and a server in a couple of languages. Overall, I think it’s in good shape and it makes sense from a developer’s perspective. I’ve got a few comments, some small and some that might need more conversation within the WG,



Throughout: Suggest using “credentialed” instead of “confidential” client, as introduced in OAuth 2.1 draft.

§1: Suggest the problems list start with changing scopes or swapping client IDs as scenarios in the first bullet, ACR is an esoteric use case for many and not in OAuth 2 core, either remove it or put it at the end of the bullet.

   Suggest the second bullet note who the information needs to be protected from, at least in passing. It’s not clear from this setup why the parameters should be confidential, and this is a major motivation for this work.

   Avoid use of phrase “so-called” and just give the name “Request Object”.

   ¶4: Perhaps overly pedantic but I suggest extending: “in exchange for a request_uri value usable at the authorization server”. 

   ¶4/5: Alternatively, suggest combining these paragraphs: “This document complements JAR by providing an interoperable way for the client to push its authorization request parameters to the authorization server in exchange for a request_uri usable at the authorization server. The document further allows the client to push request objects as specified in JAR in exchange for a request_uri usable at the authorization server.”

    ¶12: “This is directly utilized” is a little ambiguous into what it’s referring to. Would suggest rewording the start as: “This early stage client authentication is used by this draft to allow confidential clients…” or something of that sort.

    ¶13: Not only is POST much harder to use, it’s also optional for the AS to implement so it can’t be counted on by a client to be available generally. (To be honest in retrospect we shouldn’t have included it in OAuth 2.)

§2: Please provide a reference to JWT client assertion auth here (either the assertion RFC or OIDC’s definition of the client auth methods mentioned). I would also phrase this as direct guidance instead of a note/aside.

§2.1: There’s some potential weirdness about client_id here. Since the authz request was designed around not having client authentication, that request requires client_id. However, here the client is authenticating, and the client_id might be included elsewhere like the Basic header. A developer might be curious about whether they need to include them twice.

    ¶2: Of necessity, this spec mixes parameters in the authorization endpoint and token endpoint registries into a single request. Is there any danger of conflict between them? The registry holds them in one list but they could possibly have different semantics in both places.

    ¶6: Does the AS really have "the ability to authenticate and authorize clients”? I think what we mean here is "the ability to authenticate clients and validate client requests”, but I’m not positive of the intent. 

    ¶7: I’m not sure I buy this example. Even if the clientID is managed externally, the association with a set or pattern of allowed redirect URIs is still important, and the AS will need to know what that is. I think this example could lead an AS developer to (erroneously and dangerously) conclude that they don’t have to check any other values in a request, including scope and redirect URI. It’s important that DynReg doesn’t alleviate that issue, but removal of DynReg doesn’t really change things in that regard. Suggest removing example or reworking paragraph.

§2.2: Is “expires_in” required? If so, can an AS decide that a request URI doesn’t expire after a certain amount of time? Related, what does a “0” or negative value mean, if anything? 

    I don’t see why a request URI with unguessable values isn’t a MUST for one-time-use, is there a reason?

§2.3: Are the HTTP status codes a normative MAY or just an informative “can” as written?

§3: This bit should be normative as follows: "he authorization server MUST take the following steps beyond the processing rules…"

    Clean up the tenses and voice of the numbered list, which are currently inconsistent.

§7.2: The AS should also make sure that the new redirect URI is applicable within that client’s domain or policies. So you wouldn’t want a legit-but-rogue client registering for someone else’s redirect URI in order to start an attack, for example. I think overall we might need better guidance around the redirect URI variability feature (which, to be clear, I’m hugely in favor of — but OAuth’s assumptions in the client model make this trickier to talk about and implement safely, so we need to be extra careful).

§7.3: As above, is there a reason that this isn’t a MUST? It’s a temporary credential representing a request, much like an authorization code in many ways. I don’t see why a legitimate client would use it more than once, or why a server would want to allow it for AS-generated values. For client-supplied request URIs, sure, but not here.

§X: Missing privacy considerations section. If anything this spec helps alleviate some privacy concerns by trading values for a reference, but the section is still needed.