Reviewer: Michael Jones Review result: Has Issues Comments on substantive issues with the specification: In 4.15. Client Impersonating Resource Owner, the sentence “If the resource server cannot properly distinguish between access tokens issued to clients and access tokens issued to end-users, the client may then be able to access resource of the end-user.” has two problems. (1) End-users are people. Therefore access tokens are not issued to them. Please rework so this paragraph means what you actually intend. (2) I suspect that you want to change “the client may then be able to access resource of the end-user” to something like “the client may then be able to access resources belonging to the end-user”. In 4.15.1. Countermeasures, it talks about “the resource server to be unable to distinguish an access token authorized by a resource owner from an access token authorized by a client itself”. I’m sure I don’t understand the distinction or the scenarios here, and if I don’t, others likely won’t either. This is probably related to the confused language cited in the previous paragraph. In my maybe overly simplistic view, all access tokens are authorized by authorization servers and issued to clients. Clients don’t authorize access tokens. Please rework these paragraphs so that the distinctions you’re trying to make will be clear to readers. Other, mostly editorial, review comments: In the Abstract, change “It further deprecates some modes of operation” to “Further, it deprecates some modes of operation”. In 1. Introduction add “the” before “client” in “OAuth initially assumed static relationships between client”. The Introduction says “OAuth 2.1, under developement as [I-D.ietf-oauth-v2-1], will incorporate the security recommendations of this document.” Is the plan for it to incorporate all the security recommendations in this document verbatim (possibly rendering this document largely obsolete)? And all the ones in [RFC6819]? Or might it be more accurate to say “will incorporate security recommendations from this document”? I’d suggest the latter. Also, correct “developement” to “development”. In 2. Best Practices, change “considers best practices” to “considers to be best practices”. In 2.1. Protecting Redirect-Based Flows, I’d suggest changing “, see Section 4.1.3” to “(see Section 4.1.3)” to be consistent with the syntax used in the following sentences. That, or change it to “, per Section 4.1.3”. In 2.1. Protecting Redirect-Based Flows, change “(open redirector)” to “(open redirectors)”. In 2.1.2. Implicit Grant, change “aka” to “a.k.a.,”. In 2.6. Other Recommendations, change “However, CORS MUST NOT be supported at the Authorization Endpoint as the client does not access this endpoint directly, instead, the client redirects the user agent to it.” to “However, CORS MUST NOT be supported at the Authorization Endpoint, as the client does not access this endpoint directly; instead, the client redirects the user agent to it.” In 3. The Updated OAuth 2.0 Attacker Model, change “as good as practically possible” to “as well as practically possible”. In 3. The Updated OAuth 2.0 Attacker Model, change “See also Section 4.9.2.” to “Also see Section 4.9.2.”. In 4.1.1. Redirect URI Validation Attacks on Authorization Code Grant, in “For a client using the grant type code”, put backquotes around “code”. In 4.2.1. Leakage from the OAuth Client, change “ (and potentially access token)” to “ (and potentially access_token)”. In 4.5.3.1. PKCE, this sentence is awkward “PKCE is a deployed OAuth feature, although its originally intended use was solely focused on securing native apps, not the broader use recommended by this document.” and verges on being a run-on sentence. Please update. In 4.5.3.2. Nonce, change “The nonce value is a one-time use” to “The nonce value is one-time use”. In 4.5.3.2. Nonce, the meaning of “where the attacker has stolen the respective authorization code” in “The assumption is that an attacker cannot get hold of the user agent state on the victim's device, where the attacker has stolen the respective authorization code.” isn't clear. This sounds like the attacker stole the code from the user agent state, but that’s probably not what’s meant. Please clarify. In 4.5.3.3. Other Solutions, this sentence is missing two commas “PKCE is the most obvious solution for OAuth clients as it is available today (originally intended for OAuth native apps) whereas nonce is appropriate for OpenID Connect clients.” In 4.9.3. Countermeasures, this sentence leaves me wanting more: “Depending on the severity of the penetration, sender-constrained access tokens will also prevent replay on the compromised system.” In particularly, please characterize when sender-constrained access tokens will and will not prevent replay on the compromised system. In 4.10.1. Sender-Constrained Access Tokens, change “The approach as specified in this document” to “The approach specified in this document”. In 4.10.2. Audience-Restricted Access Tokens, change “the fingerprint of the resource server's X.509 certificate as audience value” to “the fingerprint of the resource server's X.509 certificate as the audience value”. In 4.10.2. Audience-Restricted Access Tokens, change “crypto” to “cryptography”. In 4.10.2. Audience-Restricted Access Tokens, given the effective demise of Token Binding, I suggest changing “[I-D.ietf-oauth-token-binding] has the same property since different token-binding IDs must be associated with the access token.” to “[I-D.ietf-oauth-token-binding] had the same property since different token-binding IDs would be associated with the access token.” In 4.11.2. Authorization Server as Open Redirector, put backquotes around “prompt=none”. In 4.14. Refresh Token Protection, the sentence “Refresh tokens are a convenient and user-friendly way to obtain new access tokens after the expiration of access tokens.” is a somewhat inaccurate characterization, since refresh tokens can also be used to obtain new access tokens before the expiration of access tokens. Please revise. In 4.14.1. Discussion, the description “overall grant” seemed like it could be improved. I don’t think the word “overall” conveys all that you mean readers to understand about this scenario. Please revise. In 4.17. Authorization Server Redirecting to Phishing Site, put backquotes around “prompt=none”. There’s a lot of duplicated text between 4.11.2. Authorization Server as Open Redirector and 4.17. Authorization Server Redirecting to Phishing Site. Consider refactoring to eliminate or reduce the duplication. Update [OpenID.Core] to OpenID Connect Core 1.0 incorporating errata set 2, December 15, 2023. Update [OpenID.Discovery] to OpenID Connect Discovery 1.0 incorporating errata set 2, December 15, 2023. Update [W3C.WebAuthn] to reference https://www.w3.org/TR/2021/REC-webauthn-2-20210408/ (including updating editors, date, etc.). Change to <?rfc tocdepth="4" ?> (or whatever its equivalent is in Markdown) so that sections such as 4.5.3.1. PKCE appear in the table of contents. -- last-call mailing list last-call@xxxxxxxx https://www.ietf.org/mailman/listinfo/last-call
