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
