authts / oidc-client-ts

OpenID Connect (OIDC) and OAuth2 protocol support for browser-based JavaScript applications
https://authts.github.io/oidc-client-ts/
Apache License 2.0
1.42k stars 214 forks source link

Don't only allow "code" as response_type #1028

Open ch-lepp opened 1 year ago

ch-lepp commented 1 year ago

In your migration guide you write that

restricted response_type to code flow only. As per OAuth 2.1: PKCE is required for all OAuth clients using the authorization code flow

as in oidc-client 1.x, OAuth 2.0 hybrid flows are not supported

That is implemented in OidcClient.ts line 109 if (response_type !== "code") { throw new Error("Only the Authorization Code flow (with PKCE) is supported"); }

I think this approach of yours is not right for several reasons.

  1. You're not writing an OAuth library. You're writing an OIDC library. And OIDC as of now is still defined as 'based on OAuth2.0' not 'based on OAuth2.1'

  2. OIDC itself defines flows that go beyond the authorization-code-flow. Namely the implicit flow and the hybrid flow.

  3. Even if you attempt to implement OAuth2.1, you are still missing flows (aka grants). OAuth2.1 for example defines the client credentials grant.

However you view it, just allowing for the authorization code flow (aka authorization code grant) seems incorrect to me. So would suggest changing that.

Side node: Your readme still states that

Implements the following OAuth 2.0 protocols and supports OpenID Connect Core 1.0:

Authorization Code Grant with PKCE Resource Owner Password Credentials Grant; however, read the security concerns before using this flow Refresh Token Grant

pamapa commented 1 year ago

This library follows the recommendation of the upcoming OAuth2.1 (https://oauth.net/2.1/):

The major differences from OAuth 2.0 are listed below.
- PKCE is required for all OAuth clients using the authorization code flow
- ...

The implicit flow and the hybrid flow are legacy flows and gone for good! However I accept merge request, which improve the documentation...

ch-lepp commented 1 year ago

As far as I know, the OIDC specs dont say that the implicit flow and the hybrid flow are gone for good.

Again, you have your lib called "oidc-client-ts", so I think you should implement the oidc specs. Otherwise your lib should probably be called "oauth-client-ts" or something like that...

And if you want to implement the OAuth2.1 specs, you are still missing the client credentials grant.