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

[SDK-2677] Support node-oidc-provider #768

Merged
merged 18 commits into from
Jul 29, 2021
Merged

Conversation

stevehobbsdev
Copy link
Contributor

@stevehobbsdev stevehobbsdev commented Jul 28, 2021

Description

This PR adds support for using node-oidc-provider in the playground as an alternative for hitting real Auth0. The motivation is that, in another PR that's coming, we can add support for running E2E tests against this provider instead of Auth0. Additionally, we can run the auth server on the same origin as the application being tested, and avoid cross-origin issues when using Cypress.

To make this work, a number of things had to be changed:

  • Now uses rollup-plugin-dev to serve the playground instead of rollup-plugin-serve, as the former has better extensibility points and proxy support (should we need it)
  • Integrates node-oidc-provider into the application by providing the appropriate mount points to the underlying Koa app that is created by rollup-plugin-dev. This means that the auth server can run on the same origin as the app
  • SDK can now accept a domain with an http or https scheme, and will not try to automatically prefix with https. This allows us to use http://localhost:3000 as the domain when using the OIDC provider
  • Since node-oidc-provider only supports form data on the token endpoint (as per the spec), this PR also adds a new config option useFormData to the SDK where the developer can opt-in to using form data instead of JSON. We'd like to move to using this as the default later, as it's faster (no CORS pre-flight check)
  • Upgrades to Cypress 7.2.0
  • Adds tests
  • Adds a new switch to the playground where form data can be turned on

Summary

  • Swap rollup-plugin-serve for rollup-plugin-dev
  • Add ability to use form post to get tokens
  • Add logout support
  • Fix domain URL
  • Add switch to the playground to enable form data
  • Prevent useCookiesForTransactions from being sent to token endpoint
  • Add tests for domain config
  • Added tests for sending form data when appropriate
  • Added form data tests for token worker
  • Upgrade to Cypress 7.2.0 and fix tests
  • Add E2E tests for form post data

@stevehobbsdev stevehobbsdev requested a review from a team as a code owner July 28, 2021 16:03
@stevehobbsdev stevehobbsdev added CH: Added PR is adding feature or functionality review:medium Medium review labels Jul 28, 2021
@stevehobbsdev stevehobbsdev marked this pull request as draft July 28, 2021 16:09
@lgtm-com
Copy link
Contributor

lgtm-com bot commented Jul 28, 2021

This pull request introduces 1 alert when merging 66a12fe into 30e674c - view on LGTM.com

new alerts:

  • 1 for Hard-coded credentials

@stevehobbsdev
Copy link
Contributor Author

Marked as draft as I want to do a little more testing.

@stevehobbsdev stevehobbsdev added review:large Large review and removed review:medium Medium review labels Jul 28, 2021
@lgtm-com
Copy link
Contributor

lgtm-com bot commented Jul 28, 2021

This pull request introduces 1 alert when merging 5227c26 into 30e674c - view on LGTM.com

new alerts:

  • 1 for Hard-coded credentials

@stevehobbsdev stevehobbsdev marked this pull request as ready for review July 28, 2021 17:45
frederikprijck
frederikprijck previously approved these changes Jul 29, 2021
Copy link
Member

@frederikprijck frederikprijck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, one small question.

src/worker/token.worker.ts Outdated Show resolved Hide resolved
@lgtm-com
Copy link
Contributor

lgtm-com bot commented Jul 29, 2021

This pull request introduces 1 alert when merging 6740c33 into 30e674c - view on LGTM.com

new alerts:

  • 1 for Hard-coded credentials

frederikprijck
frederikprijck previously approved these changes Jul 29, 2021
scripts/oidc-provider.js Outdated Show resolved Hide resolved
src/Auth0Client.ts Show resolved Hide resolved
src/worker/token.worker.ts Outdated Show resolved Hide resolved
src/worker/token.worker.ts Outdated Show resolved Hide resolved
@lgtm-com
Copy link
Contributor

lgtm-com bot commented Jul 29, 2021

This pull request introduces 1 alert when merging 15fb94c into 30e674c - view on LGTM.com

new alerts:

  • 1 for Hard-coded credentials

@stevehobbsdev stevehobbsdev enabled auto-merge (squash) July 29, 2021 11:35
@stevehobbsdev stevehobbsdev merged commit d0578f6 into master Jul 29, 2021
@stevehobbsdev stevehobbsdev deleted the sdk-2677/oidc-provider branch July 29, 2021 11:39
@frederikprijck frederikprijck added this to the v1.16.2 milestone Aug 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CH: Added PR is adding feature or functionality review:large Large review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants