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

oAuth Sign In possibly broken on front ends with Webpack 5 #153

Open
shaunanoordin opened this issue Mar 16, 2022 · 3 comments
Open

oAuth Sign In possibly broken on front ends with Webpack 5 #153

shaunanoordin opened this issue Mar 16, 2022 · 3 comments
Labels

Comments

@shaunanoordin
Copy link
Member

shaunanoordin commented Mar 16, 2022

Functionality Issue

If a Custom Front End (e.g. Classrooms) upgrades from Webpack 4 to Webpack 5, there's a chance that Sign In via oAuth is broken.

Symptoms:

  • A user might click the Sign In link, sign in on the oAuth page, but when they're redirected back to the front end, they're still not signed in.
    • Sanity check: prior to Webpack 5, sign ins worked fine, and the oAuth application is set up correctly.
  • when the CFE loading, it no longer checks zooniverse.org/api/me to find out who's signed in.

This can be a result of Webpack 5 removing default polyfills for the Node.js url library, which causes a hidden chain of failures when signing in.

Probable triggers:

  • Front end project upgraded from Webpack 4 to Webpack 5, AND...
  • ...the Webpack config contains resolve.fallback = { url: false }, which is usually added to solve build issues on webpack-dev-server 4 onwards.

Dev Notes

Full failure chain:

  1. Webpack 5 removes fallbacks for Node.js's url module
  2. panoptes-client uses json-api-client uses normalizeurl 0.1.3
  3. normalizeurl 0.1.3 expects the Node.js url module. If that doesn't exist, it falls back to its own parsing code.
  4. normalizeurl's fallback parsing code is limited and throws a 'Cannot normalize absolute and protocol-relative urls passed as a string without the node.js url module' error whenever you try to get zooniverse.org/api/me
  5. panoptes-client hides that error, so it just looks like you can't login for some mysterious reason

normalizeurl 1.0.0 fixes the fallback parsing code that was borked in 0.1.3.

Status

Solution in progress. Solution cannot be applied at either PJC or json-api-client, AFAICT.

  • Workaround (if you need things fixed right now): 1. add resolve.fallback = { url: require.resolve("url") } and 2. add url to package.json's dependencies
  • Long term (by end of week): I'm in the process of upgrading panoptes-client and json-api-client, so a few version bumps should fix the problem for all projects.

Solution must be performed at the front end project which uses Webpack 5

  1. add resolve.fallback = { url: require.resolve("url") } to the Webpack config.
  2. add url to package.json's dependencies
@shaunanoordin
Copy link
Member Author

PR Update

After publishing json-api-client 5.0.4, I realise that normalizeurl 1.0.0 does NOT fix the issue. v1.0.0 actually STILL expects the Node.js url module, and the normalizeurl's fallback parsing code still borks the same as v0.1.3

I've updated the original post ☝️ with the following information:

Solution cannot be applied at either PJC or json-api-client, AFAICT. (This is because the problem stems from Webpack 5, and neither use Webpack 5.) Solution must therefore be performed at the front end project which uses Webpack 5.

  1. add resolve.fallback = { url: require.resolve("url") } to the Webpack config.
  2. add url to package.json's dependencies

@eatyourgreens
Copy link
Contributor

This is actually a bug in json-api-client, which uses normalizeurl here:
https://github.com/zooniverse/json-api-client/blob/95268b151acc852fb289e758f24a02abb9c06ef7/src/make-http-request.coffee#L33

normalizeurl is a node package (last maintained 6 years ago!), which requires url to be polyfilled if you want to run it in the browser.

@shaunanoordin shaunanoordin removed their assignment Jul 25, 2022
@eatyourgreens
Copy link
Contributor

We can fix this, here:

The native URL interface is supported in Node since Node 10, so remove normalizeurl and use that instead, maybe?

This note is actually in the Readme for normalizeurl.

The module works also works in a browser, but only for url fragments for now. It'll throw an exception if you attempt to normalize an absolute or protocol-relative url, as that currently relies on require('url').parse.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants