suggestion: `getUserInfo()`

[iuioiua]

trafficstars

Or, more specifically, something like OAuth2Client.getUserInfo(accessToken: string): Promise<unknown>. This would require adding a userInfoUri?: string property to OAuth2ClientConfig. I'd be happy to submit a PR.

P.S. We use this module for the official Deno KV OAuth module. Thank you for this great module. If you have any ideas on improving this module (deno-oauth2-client) or ours (deno_kv_oauth), I'd be happy to help!

[cmd-johnson]

I'm not quite sure how to move forward on this one. The userinfo endpoint isn't really part of the OAuth 2.0 spec and lives in the OpenID Connect spec instead.

While I originally intended this module to be a pure OAuth 2.0 client, I see the potential in extending its functionality to cover OpenID Connect as well. Although I'd rather separate OpenID Connect related code into its own class (e.g. OIDCClient instead of OAuth2Client) to make it easier to follow the specs in the code.

Basing the OIDCClient on OAuth2Client requires a few additional changes to how OAuth 2.0 is handled right now though. For example, there's currently no way to access the Access Token Response in the Authorization Code Grant which for OIDC would additionally include the id_token field.

This in turn means that we'd have to find a nice(ish) way to extend/augment parts of the grant logic in quite a few places. This is especially true when considering that not all Authorization Servers follow the OAuth 2.0 spec to the letter (e.g. returning an array of scopes instead of a space-concatenated string #26).

Maybe the easiest way forward is this:

• change the Grant classes to directly accept a config object instead of the OAuth2Client (I borrowed that architecture from https://github.com/mulesoft/js-client-oauth2/, but I don't see a reason why passing the OAuth2Client instance to its grants is actually necessary). This would open up the possibility of extending the Grant classes themselves, allowing to override parts of their behaviour
• create a new OIDCClient class (maybe located in src/oidc/oidc_client.ts to keep it separate from the "pure" OAuth 2.0 code)
• create a new AuthorizationCodeFlow class in the oidc directory, extending the OAuth 2.0 AuthorizationCodeGrant class (e.g. returning the additional id_token from the getToken method)
• things like the proposed getUserInfo() could be added to the new OIDCClient class instead (or maybe a OIDCUserInfo class, an instance of which is then attached to the OIDCClient class to stick with the current pattern)
• to allow others to extend the grants to fit their needs, export the *Grant classes from mod.ts as actual classes and not only their types

[cmd-johnson]

Although at that point I'm not really sure if we even need the OAuth2Client class anymore because you can just directly create an instance of the grants you're actually using and be done with it :shrug:

[cmd-johnson]

I started working on an OIDC implementation within this module that includes a few of the changes proposed above, as well as a getUserInfo(…) method: https://github.com/cmd-johnson/deno-oauth2-client/tree/feature/oidc/src/oidc

The implementation is lacking tests and proper documentation, but it'd be great if you could give it a try and report any shortcomings or bugs that you encounter!

The new OIDCClient class works mostly like the OAuth2Client class does, but it only supports the OIDC Authorization Code Flow for now. It does require a bit of additional configuration though:

• the redirectUri is now required (as per the OIDC spec)
• a verifyJwt(jwt: string) => Promise<JWTVerifyResult> function that parses and more importantly validates the signature of JWTs passed to it. The return type is compatible with jose, so that's what I would recommend using
• you can optionally configure a userInfoEndpoint URL (required when calling oidcClient.getUserInfo(…))
  • getUserInfo() takes an access token, ID token (required for validating that the aud claims of the ID token and userinfo response match) and an optional options object (for now you can only add additional headers to the request)

[iuioiua]

I'm sorry I didn't reply to you about the last comments! It's very exciting to see you're giving OIDC a crack! It'd be a big, once done, for this module and other projects that use this module.

I'll give this a try as soon as I'm able. I've only skimmed the code so far. Perhaps, what might help, both you and me is an example like this one. Perhaps, writing the code in action might help write the implementation 🙂

I'll let you know how I go.

[adoublef]

I'm looking forward to this myself too. I like the idea of using a different class for OIDC as likely gives us, the end user for flexibility. OIDC will definitely solve a query I've been having for a while and so all for it. Thanks

[iuioiua]

Sorry for the delay, @cmd-johnson. I've started tinkering with your OIDC implementation (https://github.com/denoland/deno_kv_oauth/tree/oidc). I'm stuck on defining the correct verifyJwt parameter. Again, perhaps, having an example app like your examples/http.ts would help.

[iuioiua]

Gentle ping, @cmd-johnson

[cmd-johnson]

Oh right, sorry! I designed the type of verifyJwt to be compatible with jose's jwtVerify function, so your code in demo_oidc.ts doesn't look too wrong.

I've added two OIDC examples to the OIDC branch. One for http and one for oak, just like with the OAuth2 examples. I also found and fixed two small errors in my userInfo and OIDC access token validation logic.

[iuioiua]

Thanks! Now, with my current demo, I'm getting the following error:

Error: Invalid token response: id_token is already expired
    at AuthorizationCodeFlow.assertIsValidIDToken (https://raw.githubusercontent.com/cmd-johnson/deno-oauth2-client/feature/oidc/src/oidc/authorization_code_flow.ts:275:13)
    at AuthorizationCodeFlow.getToken (https://raw.githubusercontent.com/cmd-johnson/deno-oauth2-client/feature/oidc/src/oidc/authorization_code_flow.ts:203:10)
    at eventLoopTick (ext:core/01_core.js:183:11)
    at async handleCallback (file:///Users/asher/GitHub/deno_kv_oauth/src/handle_callback.ts:70:18)
    at async handler (file:///Users/asher/GitHub/deno_kv_oauth/demo_oidc.ts:80:30)
    at async ext:deno_http/00_serve.js:436:22

I'll do some further troubleshooting and get back to you.

[cmd-johnson]

Sounds like a bug that I pushed a fix for together with the examples You might just have to run your code with --reload to fix this

[iuioiua]

A little hacky, but it works! Thank you. I've updated the code. WDYT?

[iuioiua]

@cmd-johnson, would you be happy to review the PR if we implement something that uses getUserInfo(), once ready, in Deno KV OAuth? All good if you're unable 🙂

[iuioiua]

Gently pinging @cmd-johnson

[cmd-johnson]

Sure, just point me to the right direction and I can take a look :slightly_smiling_face: Did you encounter anything missing in the implementation? Otherwise I'll go forward and open up a PR to get this into main as soon as I've written tests for the OIDC code

[iuioiua]

I'll create a PR in the coming days and ping you 🙂

I didn't notice anything in the implementation missing.