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

Lock not showing rule errors in redirect mode #637

Closed
rashfael opened this issue Oct 10, 2016 · 16 comments
Closed

Lock not showing rule errors in redirect mode #637

rashfael opened this issue Oct 10, 2016 · 16 comments
Assignees
Labels
bug This points to a verified bug in the code
Milestone

Comments

@rashfael
Copy link

Lock version: 10.4.0
Browser: Chromium 53 / Firefox 49

Throwing an UnauthorizedError in a rule does not display the error message in redirect mode.
Works in popup mode.

Reproduction with the simple login example:
https://github.com/rashfael/auth0-javascript-spa/tree/master/01-Login

and this simple rule:

function (user, context, callback) {
  return callback(new UnauthorizedError("This is an UnauthorizedError."));
}

In redirect mode, the authorization_error event receives this object:

{error: "unauthorized", error_description: "This is an UnauthorizedError."}

In popup mode, the event receives another object:

{code: "rule_error", error: "rule_error", description: "This is an UnauthorizedError."}

Working popup mode:
auth0-popup error
No error in redirect mode:
auth0-redirect-no-error

@rashfael
Copy link
Author

Is the solution now to manually feed the error thrown by the lock, back into the lock, to fix inconsistent behaviour between modes? Any plans to properly fix that bug or remove automatic error handling completely?

@glena
Copy link
Contributor

glena commented Oct 18, 2016

Yes, we added a way to let you show error messages in Lock. This way you can easily do the following

lock.on("authorization_error", function(error) {

  lock.show({
    flashMessage:{
      type: 'error',
      text: error.error_description
    }
  });

});

to show the error in lock, or you are free to handle on your app.

@robert-weissgraeber
Copy link

@glena I would not consider your workaround a valid solution to the problem. Could this be fixed more thoroughly, please?

@glena
Copy link
Contributor

glena commented Oct 18, 2016

@robert-weissgraeber It is not a workaround but the suggested approach to handle this case. Since the way people use Lock depends on each project and there are tons of different ways, we do not want to force a unflexible solution. What you want is that we force lock to be shown with the error message and we do not think it is a proper solution, that is why we allow you to handle the error as you want and if you prefer to show it inside lock, you can do it using the flashMessage.

At the end, for us to handle it as you want, ends up abstracting this same behaviour inside lock and I can bet that it wont work for other customers and there will be a feature request to change it. This way, we avoid forcing that solution and provides flexibility to handle it as you prefer.

@rashfael
Copy link
Author

Showing the error messages by default is already a feature, it's just not working in redirect mode. Errors not from rules are already shown the "unflexible" way.
This feels rather inconsistent, especially as a response to a bug.

Additionally, even a flexible solution should provide usable defaults, especially for such user critical behaviour like displaying errors. The default "errors are just ignored" (which, if we'd follow the paradigm of flexibility, while staying consistent, would affect all errors) impacts the ability to simply drop in the auth0-lock with a handful lines of code and have it just work, without having to wire default behaviours together.

If you do not want to force an unflexible solution, at least provide sensible defaults which work for the majority of use cases.

@m3co-code
Copy link

I also don't see this is as a proper solution. Your are outsourcing the inconsistency of Locks behaviour to the developers using it. I do understand your concern regarding backwards compatibility and flexibility, though.

Maybe a configuration flag, e.g. showRuleErrors: true with a default value of falsefor backwards compatibility would be a way to go?

@roni-frantchi
Copy link

Yes, we added a way to let you show error messages in Lock. This way you can easily do the following

  lock.show({
    flashMessage:{
      type: 'error',
      text: error.error_description
    }
  });

});

to show the error in lock, or you are free to handle on your app.

@glena This won't work for me... i'm using redirect and "Hosted Pages"... any ideas?..

@eroncastro
Copy link

Could you please provide an example of using this feature with Hosted Pages?

@albayraktaroglu
Copy link

yes having the same problem with @eroncastro

Could you please provide an example of using this feature with Hosted Pages?

@Hendrixer
Copy link

I don't think there is a solution for hosted pages

@lucasklaassen
Copy link

Do we have an issue logged for hosted pages?

@luisrudge
Copy link
Contributor

the hosted login page will redirect back to your app with the error, right? Can't you catch the error there and handle it?

@tejanium
Copy link

@luisrudge I did this before by do workaround and put the error message inside redirect_uri

the login page URL used to be looked like this

https://<REDACTED>/login?client=<REDACTED>&protocol=oauth2&redirect_uri=http%3A%2F%2F<REDACTED>%2Fauth%2Fmindvalley%2Fcallback%3Forigin%3Dhttp%253A%252F%252F<REDACTED>%252F&auth0Client=<REDACTED>%3D%3D&response_type=code&scope=openid%20email&state=oVrQAtao2hMWGGUHWbIiqRW7vHeHf3jU

but now it changed, I noticed this about 2-3 days ago, now it only become

https://<REDACTED>/login?state=WGRKbMRiRFVpEycAs0ZK-Hsm1kzz-bP3

@adrogon
Copy link

adrogon commented Apr 17, 2018

Anyone coming here for Lock, errorDescription is in camelCase instead of underscores:

lock.on('authorization_error', error => {
  lock.show({
    flashMessage: {
      type: 'error',
      text: error.errorDescription
    }
  });
});

The documentation has not been updated accordingly ; opened auth0/docs#6045 to fix it.

@luisrudge
Copy link
Contributor

@adrogon thanks for the PR! 🎉

@adrogon
Copy link

adrogon commented Oct 9, 2018

Here is a solution for Hosted pages.

Given the following rule:

function (user, context, callback) {
  return callback(new UnauthorizedError("This is an UnauthorizedError."));
}

Your call to /login/callback should receive a 302, redirected to https://yourtenant.com/callback#error=unauthorized&error_description=This%20is%20an%20UnauthorizedError.

Your app should already handle the /callback route for successful logins, by storing access_tokens or id_tokens for later use.
Extend this by also handling errors received on that route. This is language dependent so good luck :)

Invoke webAuth.authorize as you would do for regular logins (https://auth0.com/docs/libraries/auth0js/v9#webauth-authorize-), passing it an extra option containing the received error message:

webAuth.authorize({
  errorDescription: 'This is an UnauthorizedError.'
});

Even though the documentation states //Any additional options can go here, the endpoint oddly complains about the extra parameter, as follows:
Following parameters are not allowed on the /authorize endpoint: [error_description]
We chose to disregard this warning.

You may now catch that extra parameter in your Hosted Page to show a warning on the Lock dialog.

Replace the default lock.show() by:

var options = {};
    
if (config.extraParams.error_description) {
  options = {
    flashMessage: {
      type: 'error',
      text: config.extraParams.error_description
    }
  };
}

lock.show(options);

Expected result:
auth0lock_flashmessage

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This points to a verified bug in the code
Projects
None yet
Development

No branches or pull requests