[OAUTH-WG] some WGLC comments on draft-ietf-oauth-jwsreq-06

[Brian Campbell <[e-mail]>]

Sakimura-san, Senior Bradley, and distinguished members of the OAUTH WG,

I re-read draft-ietf-oauth-jwsreq (-06 anyway, it'd been a while) for WGLC
and, while I feel it's in pretty good shape, I did have a few comments on
the draft. Those are listed below in roughly the order I came across things
while reading the document.

To my eye, "oauth-jar" looks a bit awkward. I'd suggest changing the abbrev
to something like
"OAuth JAR".

>From the wording in Section 3
<https://tools.ietf.org/html/draft-ietf-oauth-jwsreq-06#section-3>, it is
unclear whether the Request Object can be a JWE only or if a JWS is always
used (with alg:none for unsigned) and is nested within a JWE when
encryption but not singing is needed. To my reading there is text that
suggest both cases. Which is it? I think some clarification is needed
around this. Section 5.1
<https://tools.ietf.org/html/draft-ietf-oauth-jwsreq-06#section-5.1>
doesn't help me as I'm unsure if "unsigned (plaintext) Request Object" is
an out-of-date usage of the old way to refer to the "none" JWS alg or is
intended to mean the straight up JSON of the request object parameters.

Also in Section 3
<https://tools.ietf.org/html/draft-ietf-oauth-jwsreq-06#section-3>, the
sentence 'The Authorization Request Object MAY alternatively be sent by
reference using the "request_uri" parameter.' reads a little awkward
because the other alternative isn't explicit discussed anywhere nearby.
Perhaps something like "The Authorization Request Object may be sent by
value as described in section 4.1 or by reference as described in section
4.2."?

Also in Section 3
<https://tools.ietf.org/html/draft-ietf-oauth-jwsreq-06#section-3>, the
sentence "If a required parameter is not present in neither the query
parameter nor the Request Object, it forms a malformed request." made my
brain hurt. Maybe reword to something like, "If a required parameter is
missing from both the query parameters and the Request Object, the request
is considered malformed."?

Also in Section 3
<https://tools.ietf.org/html/draft-ietf-oauth-jwsreq-06#section-3>, '...
the Authorization Request Object SHOULD contain the Claims "iss" (issuer)
and "aud" (audience) as members ...', however, that will produce a
parameter name conflict with the "aud" parameter from OAuth 2.0
Proof-of-Possession: Authorization Server to Client Key Distribution.
<https://tools.ietf.org/html/draft-ietf-oauth-pop-key-distribution-02>
Seems like draft-ietf-oauth-pop-key-distribution will need to change its
parameter name (aud in JWT is pretty well established). And shouldn't
draft-ietf-oauth-jwsreq register some of the JWT's Registered Claim Names
<http://tools.ietf.org/html/rfc7519#section-4.1> (at least iss and aud but
maybe exp and others) as authorization request OAuth parameters
<https://www.iana.org/assignments/oauth-parameters/oauth-parameters.xhtml#parameters>
?

Also in Section 3
<https://tools.ietf.org/html/draft-ietf-oauth-jwsreq-06#section-3>, I
personally feel like the "extension variables" detract from the example
being able to easily convey the concept. I would suggest using only
'vanilla' OAuth parameters. Or, at the very least, removing the "claims"
stuff, which doubles the space used by the example and is a very OpenID
Connect specific thing. This applies to the examples throughout the draft
including those that have the encoded JWT Request Object.

In Section 4
<https://tools.ietf.org/html/draft-ietf-oauth-jwsreq-06#section-4>, the
whole "The client constructs the authorization request URI by ..." followed
by the list of parameters is somewhat awkward to me - especially with the
"REQUIRED"s. Why list "state" here and none of the others here? I think I
know why (you expect state to vary on each request but not others) but a
lot of inferring needs to happen from the reader.  For the extension
defined in this document, one uses either request_uri or request (but not
both, right? I'm not sure it's explicitly stated anywhere) and whatever
other parameters are needed in the given situation. Maybe just some text
towards that end rather than the list format?

Also in Section 4
<https://tools.ietf.org/html/draft-ietf-oauth-jwsreq-06#section-4>, the
request_uri in the example is a lot like the URI that's used for
redirect_uri in a lot of places (the /cb path, I think, was intended to
short for callback). I mean, it *could* happen but the example is more
confusing than needs to be. It's also too long/wide for the page - add some
line breaks, which are for display purposes only. Perhaps move this example
to 4.2(.1) somewhere and match the value with the value shown there?

Also in Section 4
<https://tools.ietf.org/html/draft-ietf-oauth-jwsreq-06#section-4>, the
sentence that starts with 'When the "request" parameter is used...'
suggests that the text about JWT parameters superseding others is only
applicable to the pass by value method but it applies to both. Perhaps
change '"request" parameter' to 'Request Object'?

The second paragraph of Section 4.2.1
<https://tools.ietf.org/html/draft-ietf-oauth-jwsreq-06#section-4.2.1>
talks about "requested values for Claims" which is an OpenID Connect thing
that isn't really applicable to this draft. Can this paragraph be dropped
or made more general to be about any potentially sensitive or PII data?

Also in Section 4.2.1
<https://tools.ietf.org/html/draft-ietf-oauth-jwsreq-06#section-4.2.1>, is
"... at a URL the Server can access... " - assume the "Authorization
Sever"?

Isn't it a bit odd in Section 5.2
<https://tools.ietf.org/html/draft-ietf-oauth-jwsreq-06#section-5.2> to
talk about the "request_object_signing_alg" which is defined in OpenID
Connect Dynamic Client Registration
<http://openid.net/specs/openid-connect-registration-1_0.html> (which is
not referenced) and kinda mentioned in OpenID Connect Core
<http://openid.net/specs/openid-connect-core-1_0.html> (informative
referenced)? I don't know that it belongs here? Or if it does, what about
request_object_encryption_alg and request_object_encryption_enc? I suppose
related to that is the question of if/how to use the client_secret for HMAC
and symmetric encryption algorithms and if that needs to be discussed here?
Indicating public key(s) too for that matter. Seems like all the
metadata/registration stuff should be in here. Or it should all be out of
scope. But just the request_object_signing_alg seems odd to me.

Section 6 <https://tools.ietf.org/html/draft-ietf-oauth-jwsreq-06#section-6>
says that "... this document defines additional error values ..." but those
were previously defined in 3.1.2.6 of OpenID Connect Core
<http://openid.net/specs/openid-connect-core-1_0.html#AuthError>. Maybe
just change the wording to reflect the real state of things?

Section 7 <https://tools.ietf.org/html/draft-ietf-oauth-jwsreq-06#section-7>
says that "The request_object_signing_alg OAuth Dynamic Client Registration
Metadata is pending registration by OpenID Connect Dynamic Registration
specification." But is that true? The registry
<https://www.iana.org/assignments/oauth-parameters/oauth-parameters.xhtml#client-metadata>
doesn't have it and Connect's Registration
<http://openid.net/specs/openid-connect-registration-1_0.html#IANA> "makes
no requests of IANA".

As I'm sure reading this has been a ton of fun for the editors, I'm sure
you will be happy to please add me to the Acknowledgements Section
<https://tools.ietf.org/html/draft-ietf-oauth-jwsreq-06#section-9> ;)