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

Bugfix for WebExtension #1054

Merged
merged 2 commits into from
Dec 3, 2019
Merged

Bugfix for WebExtension #1054

merged 2 commits into from
Dec 3, 2019

Conversation

STK913
Copy link
Contributor

@STK913 STK913 commented Nov 12, 2019

Can you approve this change to handle WebExtensions, please? The auth0-lock and auth0-js projects are affected by this change. I used auth0-chrome which is supposed to work for the extensions, but this library is not adapted and poses problems of ergonomics.
Thanks in advance.

Can you approve this change to handle WebExtensions, please? The auth0-lock and auth0-js projects are affected by this change. I used auth0-chrome which is supposed to work for the extensions, but this library is not adapted and poses problems of ergonomics.
Thanks in advance.
@STK913 STK913 requested a review from a team November 12, 2019 16:47
@lbalmaceda
Copy link
Contributor

@STK913 I'm not seeing any mention or usage of Auth0 JS or Lock JS on that library's code. If this affecting auth0-chrome, shouldn't this issue be created in that repository instead?

@STK913
Copy link
Contributor Author

STK913 commented Nov 13, 2019

@STK913 I'm not seeing any mention or usage of Auth0 JS or Lock JS on that library's code. If this affecting auth0-chrome, shouldn't this issue be created in that repository instead?

Hello, this does not concern auth0-chrome, only auth0-js and auth0-lock.

I have not managed to compile the Auth0 project, so I modified the getLocationFromUrl method directly in the following scripts:
node_modules/auth0-js/dist/auth0.min.esm.js
node_modules/auth0-js/dist/cordova-auth0-plugin.min.js
node_modules/auth0-lock/lib/utils/url_utils.js

After parsing Github projects, this method is declared in the object.js file of the auth0-js library and the url_utils.js file of the auth0-lock library.

@STK913
Copy link
Contributor Author

STK913 commented Nov 14, 2019

I created an example project here: https://github.com/STK913/WebExtensionForAuth0

Edit the AUTH0_CLIENT_ID and AUTH0_DOMAIN variables in variables.ts.
Download NPM packages: npm i
Since this is an Ionic/Angular project, you will need to run this command: npm install -g ionic
Then start the project: ionic serve
The project should be compiled and accessible from: http://localhost:8100
But what interests us is the extension, so go to the list of extensions in Google Chrome: chrome://extensions/
Display developer mode with the check box at the top right.
Load the unpackaged extension that corresponds to this folder: www
Get the extension ID from chrome://extensions/ to add it to the Auth0 application configuration:

  • Add in "Allowed Callback URLs": chrome-extension://EXTENSION_ID/index.html
  • Add in "Allowed Web Origins": chrome-extension://EXTENSION_ID
  • Add in "Allowed Origins (CORS)": chrome-extension://EXTENSION_ID

Open the extension from the browser icon (next to the address bar).
Try to log in with your Auth0 credentials and you will see that the spinner is running in a loop because of the following error:
ERROR TypeError: Cannot read property 'protocol' of null
at Object.getOriginFromUrl (vendor.js:100277)

Modify directly www/vendor.js and apply the patch in the getLocationFromUrl method as I did in my commit (in 3 places).
Reinstall the extension again and you will be able to connect.

Ps: I also created a request here: auth0/lock#1750

@techdragon
Copy link

This looks like it also affects Electron applications built using ember.js, and ember-electron. By default The plugin configures the serving protocol to be serve:// in order to avoid some of the issues that result from using file:// based URIs. The error seems to be in getLocationFromURL which is called inside of getOriginFromUrl.

@stevehobbsdev stevehobbsdev requested review from stevehobbsdev and removed request for a team November 28, 2019 10:42
@stevehobbsdev
Copy link
Contributor

This looks like it also affects Electron applications built using ember.js, and ember-electron.

@techdragon Does that mean to say that there are further changes to be made to the PR here?

@techdragon
Copy link

techdragon commented Dec 1, 2019

@stevehobbsdev Yes, if the regex is going to be a whitelist, then it either needs to be somehow deliberately extensible by users, or should be expanded to include serve like so (https?:|file:|serve:|chrome-extension:) and this change is also necessary in lock.js (https://github.com/auth0/lock/blob/master/src/utils/url_utils.js#L3), similar to how auth0/lock#1750 and this change as currently written, adds chrome-extension: to the regex.

@stevehobbsdev stevehobbsdev added this to the vNext milestone Dec 3, 2019
@stevehobbsdev
Copy link
Contributor

@techdragon Thanks. What I would suggest is to raise another PR with those changes and we can bring them in as well.

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

Successfully merging this pull request may close these issues.

None yet

4 participants