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

CallbackError or invalid_credentials rasied by params[:state] check is confusing #33

Closed
goofrider opened this issue Feb 18, 2013 · 2 comments

Comments

@goofrider
Copy link

When params[:state] check fails, omniauth-oauth2 raises a generic CallbackError with no description or labeling.

raise CallbackError.new(nil, :csrf_detected)

The use of CallbackError here is also very counter-intuitive. And in the code above, CallbackError always means an error raised by the OAuth2 provider originally:

CallbackError.new(request.params['error'], request.params['error_description'] || request.params['error_reason'], request.params['error_uri'])

Likewise, "invalid_credentials" would immediately imply authentication failure on the provider-side, since OAuth clients do not perform any user authentication at all during the authorization phase.

The params[:state] check is a client-side validation, at a bare minimum it should be reported as something different from provider-originated errors. It'd be better if a CSRF alert can be included either in the exception or the log on params[:state] check fails.

There's already several issues reported because of this non-descriptive CallbackError:

#20
#32
#24

@bobbytables
Copy link

I came here to write the same issue. Chased this for a few hours until I saw that tiny little if statement tucked away. 👍

@tmilewski
Copy link
Member

A commit was recently added to master that should help out.

Here's the method that was added to the CallbackError class:
https://github.com/intridea/omniauth-oauth2/blob/master/lib/omniauth/strategies/oauth2.rb#L114

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

No branches or pull requests

3 participants