cmd-johnson / deno-oauth2-client

Minimalistic OAuth 2.0 client for Deno.
MIT License
45 stars 9 forks source link

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

Open cmd-johnson opened 11 months ago

cmd-johnson commented 11 months ago

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

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.

christian-hackyourshack commented 10 months ago

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 athash 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 commented 10 months ago

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.