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: Replace passport-openidconnect with openid-clien #4

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

leeomara
Copy link
Member

This is a work in progress which should not be merged yet.

This is some experimentation to investigate the use of OpenID client instead of passport-openidconnect. This relates to #3.

Checklist

  • Authorize redirects to IDP
  • Code authorization flow
  • Authorization code to token
  • User info endpoint
  • Sane structure to code

Overall status

This is somewhat working, though the code is a mess as I don't know TS, nor Node, Express, Passport or modern JS development practices. You've been warned!

Current roadblock

The headers for hitting userinfo endpoint are using lower case. It looks like Hydra/C1 wants "Authorization: Bearer ...". C1 needs a capital "B" for "Bearer" there

I'm unable to untangle the specs on this.
https://tools.ietf.org/html/rfc7230#section-3.2

This suggests that it should be case-sensitive, but isn't 100% clear.
https://tools.ietf.org/html/rfc6750

Many other OpenID Connect/OAuth2 clients and servers have this issue:

Okay, so the token endpoint returns a "token_type" of "bearer", but then requires the userinfo endpoint to use "Bearer"

Some options

  1. Change the token response to send token_type "Bearer" (upper case B).
  2. Change the userinfo authorization to allow Athorization "bearer" (lower case b).
  3. Have the client send "Authorization: Bearer" (upper case B)

Vivvo uses https://github.com/vivvo/hydra

Therese a case-sensitivity dispute between what's returned from the token endpoint
and what's required in the Authorization header for the userinfo endpoint
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.

1 participant