Hi all! Here is the rest of my feedback. At the end I also included a list of typos and the summary of changes that I have found between RFC 6739 and the current draft.
Regards, Sascha Section 2.3. Client Authentication ------------------------- Draft and current: - both documents contain this description: "... the authorization server (e.g., password, public/private key pair)" - since the client usually uses a 'client secret', maybe this could be worded as such: - suggestion: "... the authorization server (e.g., client secret, public/private key pair)" Draft: "The authorization server MAY establish a client authentication method with public clients, which converts them to credentialed clients. However, the authorization server MUST NOT rely on credentialed client authentication for the purpose of identifying the client." - Does this mean that credentialed clients are as trustworthy/ not trustworthy as public clients? Draft: "Clients in possession of a client password, also known as a client secret, ..." - Maybe this could simply be changed to: "Clients in possession of a client secret ..." Section 3.1.2.2 Registration Requirements ------------------------- Draft: "Lack of requiring registration of redirect URIs enables an attacker to use the authorization endpoint as an open redirector as described in <a href="#section-9.18">Section 9.18</a>." - is that still required since redirect_uris have to be pre-registered now? Section 4.1. Authorization Code Grant ------------------------- Draft: - Figure 3, step (1), does not include 'code_challenge_method' Is that intentionally? - I am suggesting to include it to avoid potential questions and confusion. It could listed as 'optional' as 'scope' is - In addition, when referencing parameters, they should be spelled as used in the protocol, i.e.: 'code_challenge' instead of 'code_challenge' Section 4.1.1 Authorization Request ------------------------- Draft: "Clients use a unique secret per authorization request to protect .... " - It would be less confusing if this section would start of with "PKCE is required" - Introducing a '... unique secret per ...' sounds like something very new although this is referencing PKCE - Suggestion (along the lines of): "Clients MUST leverage PKCE per authorization request to protect ..." Section 4.1.2.1 Error Response ------------------------- Draft: "An AS MUST reject requests without a "code_challenge" from public clients, and MUST reject such requests from other clients unless there is reasonable assurance that ..." - These statements are the ones that cause discussions between developers and/ or third parties - ' ... unless ...' is very difficult to grasp, even when looking into section 9.8 - I would suggest to make it required Section 5.1 Successful Response ------------------------- Draft and current: - both documents describe the refresh_token response parameter and describe it as such: "OPTIONAL. The refresh token, which can be used to obtain new access tokens using the same authorization grant as described in <a href="#section-6">Section 6</a>" - As an enhancement, I suggest this update: "OPTIONAL. The refresh token, which can be used to obtain new access tokens using the grant type "refresh_token" as described in <a href="#section-6">Section 6</a>" Section 6. Refreshing an Access Token ------------------------- Draft: Authorization servers SHOULD determine, based on a risk assessment, whether to issue refresh tokens to a certain client. If the authorization server decides not to issue refresh tokens, the client MAY refresh access tokens by utilizing other grant types, such as the authorization code grant type. In such a case, the authorization server may utilize cookies and persistent grants to optimize the user experience. - That paragraph sounds like a general advice for web developers and should appear in an appendix for my taste - ' ... based on risk assessment ... ' may exclude any implementation that does not have such capabilities === Draft: - this section includes this statement: "Confidential or credentialed clients MUST authenticate with the authorization server ..." - section 2.3 includes this statement and makes me wonder how confident an authorization server can be when working with 'credentialed' clients': "However, the authorization server MUST NOT rely on credentialed client authentication for the purpose of identifying the client." - Any clarification, I would say about the client type 'credentialed' in general, would be helpful ------------------------- Typos: ------------------------- Section 2.1. Client Types ------------------------- Draft: "credentialed": Clients that have credentials and their identity has been not been confirmed by the AS are designated as "credentialed clients" - I believe it should be: "credentialed": Clients that have credentials and their identity has not been confirmed by the AS are designated as "credentialed clients" Section 3.2.1 Client Authentication ------------------------- Draft: "Confidential or credentialed clients client MUST authenticate with..." - I believe it should be: "Confidential or credentialed clients MUST authenticate with..." ------------------------- Summary of changes between draft and current: ------------------------- - no more implicit - no more response_type=token - no more ropc - no more redirect code 307 - no more open redirect_uri - new client type 'credentialed' - must use PKCE (with few exceptions) - AS must provide a way to show their support for 'code_challenge_method' - refresh token should expire - description for client type 'confidential' got updated - clients should not be able to choose their client_id - no reference to 'mac' token profile anymore - section 7.2 details on Bearer token - resource server must include 'WWW-Authenticate: Bearer realm="example"' header for failing authorization - extended list of security threats - discussion on native apps removed - recommended bindings between access_token and resource_server - recommended refresh_token rotation or sender-constraints - recommended to use '127.0.0.1' instead of 'localhost' On Tue, 7 Jul 2020 at 15:21, Vladimir Dzhuvinov <vladi...@connect2id.com> wrote: > > I find 03 well structured, well written and it shows that a lot of thought > and work has gone into it. > > If this is a formal call for adoption - I support it. > > > - defined new client type - credentialed clients - a client that has > credentials, but the AS has not confirmed the identity of the client. > Confidential clients have had their identity confirmed by the AS. We talked > about changing the names of confidential and public, but thought that would > be confusing. This new definition cleans up the text substantially. > > I understand why this new client type was introduced. For the reader who is > not familiar with the recent OAuth RFCs and drafts - I suspect this can still > be confusing. There will likely be questions -- Why does this difference > between confidential and credentialed matter? What is a concrete example of a > credentialed client? > > Also, people will likely ask themselves - what does the confirmation of a > (credentialed) client's identity by the AS actually mean and do? (section 2.1) > > > Authorization servers SHOULD consider the level of confidence in a > client's identity when deciding whether they allow such a client > access to more critical functions, such as the Client Credentials > grant type. > > Again, normative text that relies on the implementer being able to assign > levels of confidence in the client's identity, but is not immediately obvious > how to go about this. > > > There is mention in 9.1 about "enlisting the resource owner to confirm > identity" and "if there is a web application whose developer's identity was > verified...". But this talk about client identity is secondary and happens in > the context of client authentication. > > Perhaps it will make sense to promote the discussion about identity to a new > 9.x section "Client identity" or "Client Identification", before "Client > Authentication". Addressing the topics what client identity is, why does it > matter (especially for security), and the "confirmation by the AS". Then > proceed with "Client Authentication" which now is freed to focus on the > credentials / auth methods aspects. > > Such credentials are either issued by the > authorization server or registered by the developer of the client > with the authorization server. > > Credentials (public key) can also be registered by a client performing > dynamic registration (section 2.1) > > > Vladimir > > _______________________________________________ > 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
