area51 chapters texts in as they are #2521
Conversation
5.0/en/0x51-V51-OAuth2.md
Outdated
|
|
||
| * Access tokens, shall only be consumed by RS and can either be reference tokens, validated using introspection, or self-contained tokens, validated using some key material. | ||
| * Refresh tokens, shall only be consumed by the authorization server who issued the token. | ||
| * OIDC ID Tokens, shall only be consumed by the client who issued the authorization flow (as a proof of user authentication) |
There was a problem hiding this comment.
Perhaps change "(as a proof of user authentication)" to "(as a proof of user authentication for the OIDC RP)"
There was a problem hiding this comment.
I don't know if adding this information is necessary here. Maybe "intended to" is more precise than "for"?
There was a problem hiding this comment.
I think your right, we can skip this (if we add this, "intended" is better)
There was a problem hiding this comment.
What is the expected action from here?
| * Validate the access token, according to token format and protocol specifications | ||
| * If valid, enforce authorization decisions based on information from the access token and granted permissions. For example, the resource server needs to verify that the client (acting on behalf of RO) is authorized to access the requested resource. | ||
|
|
||
| Thus the requirements listed here are OAuth or OIDC specific and should be performed after token integrity validation and before accepting the delegated authorization information from the token. |
There was a problem hiding this comment.
I find "and before accepting the delegated authorization information from the token" a little hard to understand.
To make this clearer, perhaps change to something like this "and before performing authorization based on information from the token"
There was a problem hiding this comment.
Nitpick about:
Thus the requirements listed here are OAuth or OIDC specific and should be performed after token integrity validation
The requirements are to "the verifier must verify that …" therefore they cannot really be performed after "token integrity validation".
I am wondering if what is being expressed in this sentence should not be a requirement instead?
There was a problem hiding this comment.
I am wondering if what is being expressed in this sentence should not be a requirement instead?
Rereading these requirements, I find it strange (somewhat lacking) that there is no requirement in this section requiring the verification of the token's authenticity (same for OIDC ID tokens in the OIDC RP section).
It is implied by "2.11.2" (for the self-encoded cases, but not for the "token-introspection" case) but I am wondering if this is still somehow lacking here.
Strawman proposition of a requirement:
Verify the resource server, checks the validity of the access token before using the associated claims. This is usually done by verification of a digital signature or through token introspection.
There was a problem hiding this comment.
Sounds good to me, what do you think @elarlang ?
There was a problem hiding this comment.
Please open a separate issue for that
There was a problem hiding this comment.
How the proposed requirement could be not duplicate of 52.1.1?
| @@ -57,6 +113,8 @@ Given the complexity of the area, it is vitally important for a secure OAuth or | |||
|
|
|||
| ## V51.5 OIDC Client | |||
|
|
|||
| As the OIDC Relying Party acts as an OAuth client, the requirements from the section "OAuth Client" apply as well. Note that if using the 'id-token' flow (not the 'code' flow), then no access tokens are issued and requirements for OAuth clients are not applicable. | |||
There was a problem hiding this comment.
Perhaps change "As the OIDC Relying Party acts as" to "As the RP acts as", and change the title to "OIDC Relying Party"?
There was a problem hiding this comment.
Note that if using the 'id-token' flow (not the 'code' flow),
This should be "the implicit flow". Is it supposed to be allowed though?
51.4.5 [ADDED] Verify that for a given client, the authorization server only allows the usage of grants that this client needs to use. Note that the grants 'token' (Implicit flow) and 'password' (Resource Owner Password Credentials flow) must no longer be used.
Moreover, in this case, all the requirements would currently in the Oauth client actually would actually apply:
- “51.2.1 [ADDED] Verify that, if the OAuth Client can interact with more than one authorization server, it has a defense against mix-up attacks. For example, it could require that the authorization server returns the 'iss' parameter value and validate it in the authorization response and the token response.” → Still applicable AFAIU.
- “51.2.2 [ADDED] Verify that, if the code flow is used, the OAuth Client has protection against cross-site request forgery (CSRF) attacks which trigger token requests, either by using proof key for code exchange (PKCE) functionality or checking the 'state' parameter that was sent in the authorization request.“ → Validation of the state is still applicable.
- “51.2.3 [ADDED] Verify that the OAuth Client only requests the required scopes (or other authorization parameters) in requests to the authorization server.“ → Still applicable as well.
There was a problem hiding this comment.
This should be "the implicit flow".
No, the implicit flow has 'id-token token'
https://openid.net/specs/openid-connect-core-1_0.html#ImplicitAuthRequest
This is just 'id-token', thus no OAuth access-token is returned, only an OIDC ID token. But the note could be removed anyway now, since 2 of 3 requirements apply and the third one has "if using code" .
There was a problem hiding this comment.
What is the expected action from here?
|
|
||
| ## V51.1 Generic OAuth and OIDC security | ||
|
|
||
| The requirements cover generic architectural requirements that apply to all applications using OAuth or OIDC, such as main architectural patterns available when building browser-based JavaScript applications that rely on OAuth or OIDC for accessing protected resources. |
There was a problem hiding this comment.
"The requirements" → "The following requirements" (same comment in many other places in this PR).
There was a problem hiding this comment.
Can you please update the PR they way you want to see it?
|
Hi @TobiasAhnoff and @randomstuff - I made some updates to the PR, but it is not covering every comment here. If you have agreement, please update the PR as well - it is more effective way than I going to do the find-replace activity. If there is a separate discussion (especially about some requirement wording), please open a separate issue for that. The goal here is to have the chapter text in and to get wider feedback for this. |
|
A public client is not capable of maintaining the confidentiality of credentials or is able to authenticate itself with the authorization server in a secure way. Therefore, a public client can not be trusted with client credentials and can only identify itself using the given client id.
That said, this configuration should still provide zero access until the user authenticates and grants consent. Also, security measures like PKCE is required….
|
Yes, I think we have this covered in requirements for PKCE and consent etc |
Chapter texts for V51 OAuth and OIDC