-
Notifications
You must be signed in to change notification settings - Fork 96
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
SSO login failing when not using implicit flow #363
Comments
Thanks @nicosabena |
@nicosabena - Just circling back to this now. So is this just a matter of passing a different I tried your reproduction steps and did not see the same error log entry, with Implicit turned off or turned on. I tried 3.4.0 and the latest branch I'm working on and neither had that issue. Is it possible the Lock or Auth.js upgrade in 3.4.0 addressed this? |
@luisrudge just wanted to run past you ☝️ thx |
yeah, checkSession doesn't support responseType:code, because it uses response_mode:web_message, which renders the response inside the page itself (there's no redirect). |
@luisrudge - Thanks for chiming in here. Is there a |
|
Instead of always using a different response type isn't just a case of removing that snippet? When I enable that option Lock 11 will perform a similar call to what that snippet is trying to perform. |
@nicosabena - The latest version of the plugin is now using Lock 11.1, which might be affecting my ability to test this. I've tried a number of different ways to get this to work, understanding the @luisrudge - Easy enough to change on my end but any thoughts on what @jmangelo is saying above? Do we need to do a I'm curious if this report is related at all: |
@joshcanhelp It depends on what the page is doing. Lock will do a checkSession to see if needs to show the "Last time you logged in with" screen. It won't auto-login the user. If you want to auto-login the user, you need to do that by yourself in that page, yes. |
Pinging @cocojoe and @glena for some context. What's the expected behavior? If it means less code operating in this plugin, I would lean heavily towards removing this code and relying on default Lock behavior.
https://github.com/auth0/wp-auth0/blob/master/templates/auth0-sso-handler-lock10.php ... the latter of which is not included anywhere currently. |
we should remove that since it will be deprecated. Also, we will handle prompts differently and this wont be needed at all if we use the hosted login page/universal login. Ping me tomorrow and I can explain. |
Thanks @glena, will do. @nicosabena - once you’re out of the woods, it would be great to get you a version of the plugin you can test that will include these changes and the Lock upgrade |
ZD # 34725 |
Could we get an update on when this will be fixed? |
@codyayers - This specific fix or SSO in general? We've been talking internally and need to come up with a solution that's going to work long-term. We're aware of what needs to be done here, just want to make sure the solution works well now and going forward. Getting this operating 💯 is our highest priority for the plugin right now. |
@joshcanhelp this specific fix. I'm happy to hear this, just would like to know if any ETA could be provided. Thanks! |
@codyayers - This particular fix won't make much of a difference as the check this is trying to do is now happening in Lock 11 (which the latest plugin version will upgrade you to). That said, here is the one-line change if you want to give it a test on your end: dev...fixed-lock-sso-options (EDIT: updated to release candidate with a few changes) If this has a positive effect, happy to include it in the next release (2 weeks max). |
Checking in again here ... that compare branch above expanded by 4 lines but I've got it working now, auto-logging in if the WP session ends before the Auth0 one does. It would be great to get someone to test this out against another set of config options but it's a simple fix and I can fast-track this into the next release coming very soon. |
@joshcanhelp these changes: https://github.com/auth0/wp-auth0/compare/issue-363-experiment appear to resolve the problem completely for us! 👍 |
Shoot.. I take that back.. worked the first time, but now it doesn't seem to be... Will continue testing. |
@codyayers - Error messages seen, logged in wp-admin, and logged in the dashboard are helpful, along with steps to reproduce, of course. Might be best to start with a clean browser session, i.e. cleared cookies, but not incognito. |
I had some issues with "Unable to automatically update Client Grant Type." but I've been able to resolve those issues by following the documented instructions and can confirm now that this is working as expected for logging users in automatically. However, I can't seem to get the Single Logout to work. Any thoughts there? |
@codyayers - Likely to be a related issue. I had it on my list to walk through that once this process was confirmed. Stay tuned ... and thank you for testing! |
@codyayers - Similar fix for SLO, same compare link as before, single-line fix: dev...fixed-lock-sso-options (EDIT: updated to release candidate with a few changes) I need to setup a network of sites to test this properly so not ready for release but the changes in that compare shouldn't hurt anything or introduce any new issues. |
Tested on my end and confirmed it works as expected. If anyone else has any feedback 👍 👎 on this, that would be great. Otherwise, I'm going to put this into a PR and get it ready for release next week in 3.5.2. Thanks everyone! |
… login; fixing incorrect redirect URL used for SSO; removing related but unused template; fixes #363
… login; fixing incorrect redirect URL used for SSO; removing related but unused template; fixes #363
… login; fixing incorrect redirect URL used for SSO; removing related but unused template; fixes #363
… login; fixing incorrect redirect URL used for SSO; removing related but unused template; fixes #363
When you enable the Single Sign On (SSO) toggle in the Features settings of the plugin, some client javascript is rendered to
checkSession
and see if a valid SSO session is available at Auth0, to log the user in automatically without prompting for credentials.The above flow is failing when there's a valid SSO session at Auth0, causing the plugin code to display:
and logging a "Wrong number of segments" error in the internal plugin logs:
The
checkSession
callback does not have anidToken
in theauthResult
, which causes theWrong number of segments
error described above.This is because the call to
checkSession
usesresponseType: 'code'
when implicit flow is turned off, and that response type is not supported bycheckSession
.results into:
responseType: 'code'
is not valid incheckSession
:https://github.com/auth0/auth0.js/blob/308d55f9f0a8c6fa2f46cd61cf7be2917a8fa647/src/web-auth/index.js#L389-L391
To reproduce:
/wp-login.php
into the WP site. The rendered HTML will do acheckSession
call (which will find a session in Auth0). You'll get "There was a problem with your log in"The text was updated successfully, but these errors were encountered: