Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft: PoC for OIDC implementation #131

Closed
wants to merge 1 commit into from
Closed

Conversation

tennox
Copy link

@tennox tennox commented Nov 25, 2023

I'm working on adding first-class OIDC support:

const client = new OidcClient({
  // The base URI of your OIDC server
  server: process.env.OIDC_PROVIDER_URL,
  // OAuth2/OIDC client id
  clientId: process.env.OIDC_CLIENT_ID,
})

export const oauthFetchWrapper = new OAuth2Fetch({
  client,
  useIdToken: true,
  //...
})

Related to issues #67 & #68

@tennox
Copy link
Author

tennox commented Nov 26, 2023

I realized a few other things were missing that I didn't want to implement myself (ID token deserialization, user storage, ..)
so I ended up pivoting to oidc-client-ts 💁‍♂️ (although it's less modern and so on 😅

@tennox tennox closed this Nov 26, 2023
@tennox
Copy link
Author

tennox commented Nov 26, 2023

I thought about it - this branch works fine... so it could still be merged as a starting point 🤔

But at your discretion, dear maintainers - and thx anyways :)

@tennox tennox reopened this Nov 26, 2023
@evert
Copy link
Collaborator

evert commented Nov 27, 2023

Hi @tennox ,

It's tricky. Some people requested adding a few more properties (like idToken), which I get, but the question then becomes: how much of OIDC do we implement?

If we do a little, feature requests will continue to roll in. If we do the whole thing, a major issue is that we would need a JWT implementation and this library becomes a lot larger.

So the only 2 ways forward I see are:

  1. Define a very strict useful subset that gets added here, based on some criteria (not sure what!) that ensures that the library stays lean.
  2. Instead of adding openid features here, create a separate package specifically for OpenID Connect features.

so yeah im not really sure what to do with this, but I do really appreciate the work. You could publish this code as your own package and make my library a dependency maybe?

@tennox
Copy link
Author

tennox commented Nov 27, 2023

Hey @evert ,

I see.
I think this lib convinced me by being lean and small, so I think adding a lot doesn't help.

In general, as I said, I ended up using the other lib that is made for oidc, because there was more things it did for me - so I I'm not very invested into this decision about this lib 🤔

My take on it is that it could be really useful that, if you only need Auth without JWT/userinfo stuff (but having id_token), maybe having this lib supporting that (and only that, as a clear scope) would be a good thing - but it's a made-up example, so the realistic scenarios are rather small I think.

So I guess I'll close this and the lib stays with it's current focus.?

Maybe a hint in the readme about this?
"This library focuses on OAuth2 only. If you need OIDC support, check out e.g. oidc-client-ts" or smth like that 🤔

In any case - thanks for your work & responsiveness ♥️

@evert evert closed this Dec 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants