Hello Dick! Unless the two typos that I have mentioned should be updated beforehand , no, I do not.
Thank you, Sascha On Tue, 7 Jul 2020 at 16:36, Dick Hardt <dick.ha...@gmail.com> wrote: > Thanks Sascha and Vladimir for the feedback! > > Sascha: did you have a concern with the document being adopted by the WG? > > ᐧ > > On Tue, Jul 7, 2020 at 4:04 PM Sascha Preibisch <saschapreibi...@gmail.com> > wrote: > >> 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 >> >
_______________________________________________ OAuth mailing list OAuth@ietf.org https://www.ietf.org/mailman/listinfo/oauth
