The errata/update version of Connect’s registration spec need to register a bunch of stuff in the Dyn-Reg IANA registry, but I don’t think that’s been done yet.
— Justin > On Oct 27, 2015, at 3:41 PM, Brian Campbell <bcampb...@pingidentity.com> > wrote: > > 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> ;) > > > > > _______________________________________________ > OAuth mailing list > OAuth@ietf.org > https://www.ietf.org/mailman/listinfo/oauth
_______________________________________________ OAuth mailing list OAuth@ietf.org https://www.ietf.org/mailman/listinfo/oauth
