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 !hostedLoginPage condition to redirect check #1876

Merged
merged 4 commits into from
Jun 3, 2020
Merged

Conversation

stevehobbsdev
Copy link
Contributor

@stevehobbsdev stevehobbsdev commented Jun 2, 2020

Changes

This PR forces popup mode + sso mode for Electron and Cordova apps only when not on the hosted login page. This means that Electron apps where the login page is opened inside the app would not try and do a cross-origin authentication request when using the hosted ULP.

References

Raised by internal ESD.

Testing

Tested and verified through manual testing with a customer using an Electron app.

  • This change adds unit test coverage
  • This change adds integration test coverage
  • This change has been tested on the latest version of the platform/language

Checklist

@stevehobbsdev stevehobbsdev added this to the vNext milestone Jun 2, 2020
@stevehobbsdev stevehobbsdev requested a review from a team June 2, 2020 17:34
@lbalmaceda
Copy link
Contributor

@stevehobbsdev Looks good. I noticed the branch wasn't tested since the build didn't break. Is there any reason not to add tests for this change?

@stevehobbsdev stevehobbsdev marked this pull request as draft June 2, 2020 18:19
@stevehobbsdev stevehobbsdev removed the request for review from a team June 2, 2020 18:19
@stevehobbsdev
Copy link
Contributor Author

stevehobbsdev commented Jun 2, 2020

@lbalmaceda Sorry about that, I had marked the PR as draft while i was adding tests as they somehow did not get included in the commit, then i noticed a problem but didn't have time to fix immediately.

The tests are there now but weirdly the build is breaking on Circle (not locally). Let me look into it.

@stevehobbsdev
Copy link
Contributor Author

Ok, tests sorted!

@stevehobbsdev stevehobbsdev marked this pull request as ready for review June 2, 2020 22:11
@stevehobbsdev stevehobbsdev requested a review from a team June 2, 2020 22:11
@stevehobbsdev stevehobbsdev merged commit eed5526 into master Jun 3, 2020
@stevehobbsdev stevehobbsdev deleted the fix/electron branch June 3, 2020 13:09
davidpatrick pushed a commit to davidpatrick/lock that referenced this pull request Jun 12, 2020
* Add !hostedLoginPage condition to redirect check

* Add tests for core/web_api

* Use global.window instead of window in web_api.test

Co-authored-by: Luciano Balmaceda <balmacedaluciano@gmail.com>
jfromaniello pushed a commit to jfromaniello/auth0-lock that referenced this pull request Jul 23, 2020
* Add !hostedLoginPage condition to redirect check

* Add tests for core/web_api

* Use global.window instead of window in web_api.test

Co-authored-by: Luciano Balmaceda <balmacedaluciano@gmail.com>
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.

3 participants