public client: `token_endpoint_auth_method` should be `none`

[uvdsl]

Browser-based applications, which this library is designed for, are to be considered public clients. As such, any client_secret obtained during dynamic client registration is considered non-confidential; it is thus not a secret.

Therefore, the client is considered a Public Client with no Client Secret.
Therefore, token_endpoint_auth_method should be none as recommended by the OIDC spec.

This is then the same token endpoint authentication method as used for a provided client_id that can be dereferenced to a client profile document. In this case, there exists no client_secret at all.

TODO: See...

... in dynamic client registration

solid-oidc-client-browser/src/requestDynamicClientRegistration.ts

Line 17 in 8ee1849

token_endpoint_auth_method: "client_secret_basic", // also works with value "none" if you do not provide "client_secret" on token request

... in authorization code grant

solid-oidc-client-browser/src/AuthorizationCodeGrantFlow.ts

Line 276 in 8ee1849

client_secret: client_secret,

... in refresh token grant

solid-oidc-client-browser/src/RefreshTokenGrant.ts

Line 122 in 8ee1849

authorization: `Basic ${btoa(`${client_id}:${client_secret}`)}`,

and remove all usage of client_secret.

---

Note: This change doesn't reduce security, as the library already correctly implements PKCE (Proof Key for Code Exchange) for the Authorization Code flow. PKCE is the recommended approach for browser-based applications according to OAuth 2.0 Security Best Current Practice. The combination of PKCE with token_endpoint_auth_method: "none" thus represents the current best practice for browser-based clients.

[uvdsl]

Using token_endpoint_auth_method: "client_secret_basic" results in a 401 on Inrupt's ESS token endpoint

{"status":401,"title":"Unauthorized","detail":"Invalid client credentials","instance":"131709de94003...","error":"invalid_client","error_description":"Invalid client credentials"}

at least after dynamic client registration.

Using token_endpoint_auth_method: "none" works with ESS.

[uvdsl]

Using token_endpoint_auth_method: "none" results on a 400 on the RefreshTokenGrant on both CSS/Pivot and ESS - at least after dynamic client registration.

This means that if we refresh the page - or move to a different path on the same origin with a full page reload - we cannot be logged in automatically. Each page reload results in a new prompt to authenticate.

[uvdsl]

As per OIDC Sec. 12.1, Refreshing Access Token:

To refresh an Access Token, the Client MUST authenticate to the Token Endpoint using the authentication method registered for its client_id [...]

As per OIDC Sec. 9, Client Authentication:

none
The Client does not authenticate itself at the Token Endpoint

meaning that a public client simply cannot use the Refresh Token Grant.

[uvdsl]

Using silent authentication via an Authorization Code Grant with prompt=none if idp, authorization_endpoint and client_id are available in sessionStorage seems to work.

Also, this seems to mitigate the risk of a malicious app hijacking the session of an honest app, as discussed in #3:
If a malicious app obtains the client_id of an honest app, the malicious app still needs to provide a redirect_uri to the IDP upon authentication. As this redirect_uri is either the honest app (no problem then) or not in the set of trusted redirect URIs as registered (dynamically or in a client profile document), the IDP will reject the authentication attempt.

Trade-off seems to be a bit of UX - where the user needs to endure an extra redirect before accessing the app - but I believe there are methods to ease the pain.

[jg10-mastodon-social]

Using token_endpoint_auth_method: "none" results on a 400 on the RefreshTokenGrant on both CSS/Pivot and ESS - at least after dynamic client registration.

Do you have a minimal reproducible example you are using for this?
In my experience debugging OIDC is non-trivial and there's a variety of reasons that refresh tokens would be rejected or not created. For CSS, the underlying node-oidc-provider library is quite demanding in how it implements the spec, and that's usually what needs to be debugged.

Refresh tokens generally are widely supported, and are intended to be possible for public clients
https://datatracker.ietf.org/doc/html/draft-ietf-oauth-browser-based-apps#name-browser-based-oauth-20-clie
https://github.com/oauth-wg/oauth-browser-based-apps

Edit: Updated outdated links

[uvdsl]

Thanks, @jg10-mastodon-social, for pushing me to re-read the things on the refresh tokens again. I don't quite know how I missed that (just a note, your link is on the old draft version 19, I think).

Anyway - here is the version that produces the error in a smoke test against solidcommunity.net: https://github.com/uvdsl/solid-oidc-client-browser/tree/11a2a470c8ab4407333b00dc222350238399abcb

I currently just test it interactively in a web app - I probably should just write a test against solidcommunity.net - but then again CSS/Pivot are not implementing Solid-OIDC spec (UMA...) so I dont want to put too much effort in tests that will change anyway at some point. 🤷

But as this also does not work with ESS (not that I care too much), I dont think it is just a bug on the side of CSS/Pivot...

[uvdsl]

I just got it working: token_endpoint_auth_method:"none" and RefreshTokenGrant.
See: https://github.com/uvdsl/solid-oidc-client-browser/tree/feature/preserve_dpop_keypair

It seems that CSS/Pivot and ESS assume that a client will re-use the same DPoP keypair that was used for the AuthorizationCodeGrant also for the RefreshTokenGrant. Therefore, the DPoP keypair needs to be persisted alongside the refresh token to make this work...

I think this is basically equivalent in terms of risk level compared to the current solution with "client_secret_basic" - in that case we were able to bind a new DPoP token after retrieving client_id, client_secret and refresh_token. By storing the DPoP token along side the client_id, this is basically equivalent.

[uvdsl]

And merged with https://github.com/uvdsl/solid-oidc-client-browser/tree/best_practice/token_endpoint_auth_method_none

PR #7
Now, we first try to renew tokens using the RefreshTokenGrant, if that doesnt work, we attempt silent authentication in the AuthorizationCodeGrant once, if that doesnt work, the session remains unauthenticated such that a user needs to manually login again (and be prompted).

Note:
CSS/Pivot does not provide a refresh_token on silent authenticaton (i.e. prompt=none).
ESS does provide a new refresh_token.

[uvdsl]

My current understanding is - revisiting the remark that a public client is unfit to store secret information securely - that a browser-based application should not use the RefreshTokenGrant to re-establish a session. A browser-based app should not (rather: must not) store the refresh_token and DPoP keypair in sessionStorage / localStorage for later re-authentication at the IDP. These items are supposed to be kept secret - which the browser-based app cannot ensure.

Storing these secret information in sessionStorage / localStorage significantly increases the risk of a successful XSS attack. XSS attacks (or injection attacks in general) still hold rank 3 in OWASP Top 10 - 2021, a list of the most critical web application security risks. It also gives rise to the security issue discussed in #3.

Therefore, I currently conclude that a browser-based app should only use the RefreshTokenGrant to refresh the current session's set of tokens - keeping the refresh_token and DPoP keypair only in memory (in JavaScript variables).

For re-establishing a session, silent authentication in the AuthorizationCodeGrant (with PKCE, of course) is to be preferred. No sensitive information needs to be persisted across sessions. Only the idp (and/or related information) would be persisted; but these do not help an attack (to my current knowledge). In the case of dynamic registration, the client_id could be persisted as well. Again, this does not help an attack if obtained as a successful silent authentication is only possible using one of the redirect_uris that were registered for the particular client_id, i.e. the ones that the client expects and which the IDP validates to use.

---

Also, to already address some points:

Of course, the apps using this library are supposed to prevent XSS attacks - but this does not mean that we can assume they do. We should still aim to provide as little of a risk surface as possible.

Of course, refresh token rotation by an IDP helps mitigating the effect of an attacker obtaining a refresh_token - but this does not mean that we can assume all IDPs do rotate tokens or provide a mechanism to revoke leaked refresh_tokens once noticed. We should still aim to provide as little of a risk surface as possible.

---

As a kind reminder: I am considering browser-based applications without a server-side component, e.g. SPAs. I am not considering a Backend-For-Frontend (BFF) pattern. In such BFF case, the server-side component is of course able to store the refresh_token and DPoP keypair securely - then again, this BFF is then not considered public (to my current understanding).

[jg10-mastodon-social]

It seems that CSS/Pivot and ESS assume that a client will re-use the same DPoP keypair

That makes sense because the intention of DPoP is to prove possession of the same private key that was used to obtain the token.
I agree with the equivalence you draw with client_secret_basic.

[jg10-mastodon-social]

keeping the refresh_token and DPoP keypair only in memory (in JavaScript variables).

For re-establishing a session, silent authentication in the AuthorizationCodeGrant (with PKCE, of course) is to be preferred.

It sounds like you're converging on the same solution as Inrupt, with the same problem, particularly regarding CSS silent authentication.

That seems a shame given that https://datatracker.ietf.org/doc/html/draft-ietf-oauth-browser-based-apps#name-browser-based-oauth-20-clie does provide alternatives.

Firstly, note that the tokens are intended to be sender-constrained, and this is specifically meant to be achieved with a non-extractable CryptoKeyPair (which I am not familiar with). Is this already how this is handled here?

"In such an implementation, the private key used to sign DPoP proofs is handled by the browser (a non-extractable [CryptoKeyPair] is stored using [W3C.IndexedDB]). As a result, the use of DPoP effectively prevents scenarios where the attacker exfiltrates the application's tokens"
Source

Secondly, if you consider sessionStorage to still be too high risk, see the advice "Universally accessible storage areas, such as Local Storage [WebStorage], are easier to access from malicious JavaScript than highly isolated storage areas, such as a Web Worker"
Source
"In this scenario, the application's own refresh token is effectively protected against exfiltration, but the access token is not."
Source

Thirdly, if the major concern is XSS, how far would CSP go in mitigating your concerns?

Fourthly, I appreciate the specificity of this point:

this does not mean that we can assume all IDPs do rotate tokens or provide a mechanism to revoke leaked refresh_tokens once noticed.

Perhaps we need to look at what mechanisms there are to discover what features the IDP offers and/or require that the IDP has the feature enabled?

Apologies if this comment is now too far from the original issue's scope.
Thank you also for your efforts in exploring this. It's beyond my confidence level to implement a solution on this, so I'm very grateful to be able to comment on the efforts of someone else! Ultimately the decision on how you wish to proceed is entirely yours!

Edit:
Deleted a link.

This discussion covers more of the considerations around web workers
oauth-wg/oauth-browser-based-apps#62

Note also the conclusion for "Browser-based OAuth 2.0 client":
"To summarize, the architecture of a browser-based OAuth client application is straightforward, but results in a significant increase in the attack surface of the application. The attacker is not only able to hijack the client, but also to extract a full-featured set of tokens from the browser-based application.

This architecture is not recommended for business applications, sensitive applications, and applications that handle personal data."
https://datatracker.ietf.org/doc/html/draft-ietf-oauth-browser-based-apps#:~:text=To%20summarize%2C%20the,handle%20personal%20data.

Based on this, it seems that ultimately without a server component there are limits to reducing the attack surface, so some compromises need to be made.

[uvdsl]

@jg10-mastodon-social, thank you for your input - you raise very specific points and provide references - I appreciate this very much! This ties back to #3.

---

"In such an implementation, the private key used to sign DPoP proofs is handled by the browser (a non-extractable [CryptoKeyPair] is stored using [W3C.IndexedDB]). As a result, the use of DPoP effectively prevents scenarios where the attacker exfiltrates the application's tokens"

This bit in particular helped me understand the interrelation between non-extractable CryptoKeyPairs and the IndexDB in comparison with extracting a keypair and putting it in session or local storage. So, I'd even conclude that storing the non-extractable CryptoKeyPair in the IndexDB is more secure than the previous solution: The keypair can be re-used on the origin but not extracted which means that no token are really usable by an attacker outside of the browser environment. This provides a level up over clientSecret or an extracted keypair in session or local storage alongside the refresh_token. This is good!

What remains, however, is that on the same origin any JavaScript (at any path) can restore a session using the (non-extractable) keypair and the refresh_token from the IndexDB if they also have the client_id. A client_id is basically public and could be guessed based on current origin and path (e.g. for a deferencable client_id like the one of Umai) or obtained from browser storage in case of a dynamic registration. So this is still an issue.

---

Thirdly, if the major concern is XSS, how far would CSP go in mitigating your concerns?

Well, XSS is a concern because this library cannot protect against this threat. As I said:

Of course, the apps using this library are supposed to prevent XSS attacks - but this does not mean that we can assume they do. We should still aim to provide as little of a risk surface as possible.

---

Perhaps we need to look at what mechanisms there are to discover what features the IDP offers and/or require that the IDP has the feature enabled?

In general, I agree. And I would add that we need a discussion of how IDPs exactly handle authentication. For example, I know that CSS/Pivot use session cookies (for silent authentication in the AuthorizationCodeFlow) and these cookies have currently the property SameSite=None. Which they should not (rather: must not) do - instead they should use SameSite=Strict to prevent an attacker to obtain and extract tokens bound to keypairs that are potentially usable outside of the compromised application.

However - as the current status quo shows - this library cannot protect against IDPs not doing their job properly. Therefore, we should aim to provide as little of a risk surface as possible. And this is precisely the reason why I appreciate our discussion.

---

"[...] This architecture is not recommended for business applications, sensitive applications, and applications that handle personal data."
[...]
Based on this, it seems that ultimately without a server component there are limits to reducing the attack surface, so some compromises need to be made.

From my current understanding, I think that I agree with both of these statements.

The point to discuss (which we currently already do) is which approach reduces the risk surface most or sufficiently.