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.
_______________________________________________
OAuth mailing list
OAuth@ietf.org
https://www.ietf.org/mailman/listinfo/oauth
