deno-oauth2-client icon indicating copy to clipboard operation
deno-oauth2-client copied to clipboard

Allow for easier extension of OAuth2 grants and add support for OpenID Connect Authorization Code Flow

Open cmd-johnson opened this issue 1 year ago • 2 comments

There have been a few requests to add support for OpenID Connect (OIDC) related extensions to the OAuth2 protocol (#28, #29, #32). Instead of adding these extensions to the OAuth2 client (making it more complex than it needs to be), I've decided to add a separate OIDC Client, living in its own subdirectory. That way, if you only need the OAuth2 client you can just continue to import it as is. If you do want to use OIDC specific features and APIs you'll be able to import those from https://deno.land/x/oauth2_client/oidc.ts. The OAuth2 and OIDC clients mostly share the same interface, with the OIDC client marking a few fields of the OAuth2 config required and adding a few extra options (like the URI of the userinfo endpoint).

This PR isn't quite ready to be merged yet, but we're getting there!

To Do

  • [ ] Fix OAuth2 tests (they're not yet adapted to the refactored implementation of OAuth2 grant classes)
  • [ ] Add Tests for the OIDC implementation

Future Work

This is a pretty basic implementation of an OIDC client so far. Apart from the userinfo endpoint (#29), there's additional APIs that OIDC specifies, like

And then there's APIs that are often used with OIDC, but also apply to OAuth2, like

When this PR is merged I'll create a separate issue to track all these.

cmd-johnson avatar Sep 20 '23 06:09 cmd-johnson

I tried your OIDCClient and found out, that it throws an error if I reduce the scope to "openid", because it tries to match the at_hash with a non-existing access_token, ~because the OpenID provider (at least Google) then just provides the id_token (which is completely fine, if you are only interested in authentication)~:

EDIT: Debugged a bit more... There still is an access_token and there is an at_hash, but there are special characters in there, that do not match: a - in the at_hash is a + in the jose.base64EncodedHash, rest is equal. Special character = is already replaced with an empty string. Besides that I found other solutions (https://stackoverflow.com/a/52298549) that try to make the base64 encoding url-safe by also replacing the following "+" -> "-" and "/" -> "_".

An error occurred during route handling or page rendering. Error: Invalid token response: id_token at_hash claim does not match access_token hash at AuthorizationCodeFlow.validateAccessToken (https://raw.githubusercontent.com/cmd-johnson/deno-oauth2-client/feature/oidc/src/oidc/authorization_code_flow.ts:246:13) at eventLoopTick (ext:core/01_core.js:183:11) at async AuthorizationCodeFlow.getToken (https://raw.githubusercontent.com/cmd-johnson/deno-oauth2-client/feature/oidc/src/oidc/authorization_code_flow.ts:206:7)

~Should I open a seperate issue for this?~ I could provide a PR, if you like, although you might want to change it yourself:

Add a function to authorization_code_flow.ts

function makeUrlSafe(hash: string) {
  return hash.replaceAll("+", "-").replaceAll("/", "_").replaceAll("=", "");
}

Use that in line 245 instead of manipulating the hash in line 243

    const base64EncodedHash = base64Encode(leftHalf);

    if (makeUrlSafe(base64EncodedHash) !== makeUrlSafe(atHash)) {

christian-hackyourshack avatar Oct 07 '23 13:10 christian-hackyourshack

Another finding: When using OpenID with Facebook, the id_token comes back with an empty string as nonce property, even when nonce option was not used, causing the authorization_code_flow to throw in line 290. You should check for a non-empty string there instead.

christian-hackyourshack avatar Oct 09 '23 11:10 christian-hackyourshack