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

add support for OIDC implicit flow #1879

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

bgerrity
Copy link
Contributor

@bgerrity bgerrity commented Feb 23, 2024

Description & motivation 💭

Work in progress. Addresses #1881.

Adds configurable support for the OIDC Implicit Flow. As that flow is designed for SPAs, this approach shifts auth from the server to the client.

Screenshots (if applicable) 📸

Design Considerations 🎨

Looked at using JS libraries but didn't see a way to mesh it neatly.

Testing 🧪

How was this tested 👻

  • Manual testing
  • E2E tests added
  • Unit tests added

Manual tests

Worked from this guide. Pointed the server to an Okta development app with the configuration below. Can add other users to the app for reproduction.

Screen.Recording.2024-04-01.at.4.07.01.PM.mov
    - label: oidc implicit flow
      type: oidc
      flow: implicit
      # TODO: support optional issuer validation
      authorizationUrl: https://dev-95856544.okta.com/oauth2/v1/authorize  # discovery isn't supported for implicit flow. the endpoint must be provided directly
      clientId: 0oag5vkuazxXyvuel5d7
      scopes:
        - openid
        - profile
        - email

Unit tests

Updated unit tests and added some more. Opened #1915 for some unrelated tests. Happy to take suggestions for additional areas or manual approaches.

Steps for others to test: 🚶🏽‍♂️🚶🏽‍♀️

# unit tests
pnpm vitest

# manual with dev yaml configured
pnpm dev

Checklists

Draft Checklist

  • Add testing with feedback

Merge Checklist

Issue(s) closed

#1881

Docs

Any docs updates needed?

Copy link

vercel bot commented Feb 23, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
holocene ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 9, 2024 10:05pm

@CLAassistant
Copy link

CLAassistant commented Feb 23, 2024

CLA assistant check
All committers have signed the CLA.

@bgerrity bgerrity changed the title OIDC implicit flow [draft] add support for OIDC implicit flow Feb 23, 2024
@bgerrity
Copy link
Contributor Author

bgerrity commented Feb 26, 2024

@stevekinney @Alex-Tideman Still a draft and haven't added tests yet but I'm seeking early feedback and guidance on testing against a public IdP.

utilities/ui-server.ts Outdated Show resolved Hide resolved
@bgerrity bgerrity force-pushed the oidc-implicit-flow branch from 8ec6ebf to 1a24d07 Compare March 18, 2024 22:09
@bgerrity bgerrity changed the title [draft] add support for OIDC implicit flow add support for OIDC implicit flow Mar 18, 2024
@bgerrity bgerrity marked this pull request as ready for review March 20, 2024 14:22
@bgerrity bgerrity requested review from stevekinney, Alex-Tideman and a team as code owners March 20, 2024 14:22
@bgerrity bgerrity force-pushed the oidc-implicit-flow branch from 1a24d07 to fc9399f Compare March 20, 2024 14:25
@bgerrity bgerrity force-pushed the oidc-implicit-flow branch from fc9399f to f2df0ea Compare March 20, 2024 17:20
@bgerrity
Copy link
Contributor Author

Added some unit tests to maintain coverage. Happy to take feedback on additional testing.

@bgerrity
Copy link
Contributor Author

bgerrity commented Apr 1, 2024

Added manual testing against an Okta dev app.

@bgerrity
Copy link
Contributor Author

bgerrity commented Apr 9, 2024

made changes to address issues found in review. also added unit tests to define the behavior for handling the hash fragment from the callback.

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.

4 participants