Messages - Amanda Anganes' review issues

[Michael Jones]

From: [e-mail] [mailto:[e-mail]] On Behalf Of Anganes, Amanda L Sent: Friday, January 25, 2013 12:12 PM To: [e-mail] Subject: [Openid-specs-ab] openid-connect-messages-1_0-15 review

Messages release candidate review. Comments are ordered by section number.

1.2 Authentication context definition: "…before it makes an entitlement_s_ decision" => "…before it makes an entitlement decision"

1.3 step 5: "The UserInfo Endpoint…" All other steps start with "the".

2. opening sentence is awkward, suggest rewording: "The OpenID Connect protocol defines several endpoints, which the RP interacts with in order to accomplish its goal of obtaining claims from the OP."

3. step 1 last sentence: "When id_token was specified" => "If id_token was specified" OR "When id_token is specified"

2.1.1 3rd sentence "Section 4.1.1 and 4.2.2 of OAuth 2.0 defines …" => "Sections 4.11 and 4.22 of OAuth 2.0 define"

2.1.1.1.2 2nd paragraph "Following Claims" => "The following Claims"

2.1.1.1.3 "The Client may request additional Claims on voluntary basis that it requires to perform other tasks offered to the user." What is this sentence adding? Grammar is off but I'm not sure what it is trying to add to the section. Remove it?

Formatting for member values of "null" or "A JSON Object" is off. Not formatted like other definition lists in the document.

2.1.2.1 auth_time definition mentions Request Object semantics (If requested with {"essential" : true} then the claim is REQUIRED). Why does this optional claim get this clause while none of the other optional claims do? Pull it out or add it to all other OPTIONAL Claims.

2.2 "..to obtain Access Token Response" => "…to obtain an Access Token Response"

2.2.1 client_secret_jwt definition, 3rd sentence "The Client Authenticates" => "The Client authenticates"

The paragraph at the end of client_secret_jwt definition specifies value of "client_assertion_type" and "client_assertion", which are NOT included in the list of required claims in this document. The claims listing currently states "The JWT MUST contain the following REQUIRED Claims and MAY contain the following OPTIONAL Claims:" but should instead say "In addition to any REQUIRED and OPTIONAL claims specified by OAuth JWT Bearer Token Profiles and OAuth 2.0 Assertion Profile, the JWT MUST contain the following…" It might be worthwhile to additionally specify on the "client_assertion" and "client_assertion_type" claim requirements that those claims are specified in the referenced docs.

The private_key_jwt section has the same problem.

2.4 offline_access definition "…access token that grants access to the End-User's UserInfo endpoint…"

3.1 "Each parameter MAY have JSON Structure as its value." Should this be "Each parameter MAY have a JSON structure as its value"?

4.2 Delete fragment "The related elements are:" from the first line. Provider x509_url, x509_encryption_url, and Client x509_encryption_url definitions are all missing end periods.

5.1.1 "MUST decode the JWT in accordance with the JSON Web Encryption specification"

5.1.3 step 1 change "the unsupported Claims" to "any unsupported Claims".

5.2 step 10 "…a nonce Claim MUST be present and its value of the checked to verify" => "a nonce Claim MUST be present and its value checked to verify"

6. "The user MUST always explicitly consent to the return of a Refresh Token that enables offline access", but the Authorization Server "SHOULD explicitly receive user consent for all clients when the registered application_type is native". This seems contradictory; am I missing something?

9.1 "…a request may be disclosed to an attacker posing security and privacy threat" => "…a request may be disclosed to an attacker, posing a security and privacy threat"

"This works even against a compromised user-agent in the case of indirect request." => "This protects against even a compromised user-agent in the case of an indirect request."

9.6 "Since…malicious Client to send a request to a wrong party" "To mitigate…require that the request to be digitally signed…using" => "To mitigate…require that the request be digitally signed…using"

9.8 is an Authorization Code really an example of possible token reuse, as it is not a token (maybe a question of semantics) and is already required by OAuth 2.0 section 10.5 to be single-use only?

9.9 OAuth SC needs to be called out with a proper reference/hyperlink. "it" in the first sentence should be replaced with "an authorization code", and/or the whole thing could be rewritten. It reads like an unfinished thought.

9.10 2nd paragraph "Responses to token requests is bound" => "Responses to token requests are bound"

9.11 "A timing attack is an attack that allows the attacker to obtain an unnecessary large amount of information" => "A timing attack allows an attacker to obtain an unnecessarily large amount of information"

2nd paragraph, should "instance of the finding error" be "instance of finding the error"?

9.16 comma in title is unnecessary

If providing refresh token revocation (which is not mentioned anywhere else in this document) is required, shouldn't the revocation document be referenced? It feels odd for a MUST to be placed here w/o further information, as things like a revocation endpoint and revocation request message(s) are not described by this document. Maybe it should have the phrase "The details of such a mechanism are out of scope of this document"?

Appendix A: Breno's name is mis-placed, should be above Casper for alphabetical order by first name

--Amanda

[Michael Jones]

As for this clause = 6. "The user MUST always explicitly consent to the return of a Refresh Token that enables offline access", but the Authorization Server "SHOULD explicitly receive user consent for all clients when the registered application_type is native". This seems contradictory; am I missing something? - it seems to me that both can be true.

As for you comment on token revocation, it's not clear that that it's necessary for the security considerations to describe how the refresh token can be revoked. I don't know that the revocation draft is far along enough that we should reference it, at least at present.

I believe I've addressed all the other comments in your note. Thanks for sending them!

[Michael Jones]

Fixed #723 - Messages - Amanda Anganes' review issues

→ <<cset bdcb3b613f7d>>