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

NoAuthorizationCodeError when user cancels authorization with FB #192

Closed
joker-777 opened this issue Mar 30, 2015 · 17 comments · May be fixed by #309
Closed

NoAuthorizationCodeError when user cancels authorization with FB #192

joker-777 opened this issue Mar 30, 2015 · 17 comments · May be fixed by #309

Comments

@joker-777
Copy link

The user can login via fb without problems but when he cancels the authorization on the fb popup we see the following unicorn log entries

I, [2015-03-30T14:13:21.820244 #4511]  INFO -- omniauth: (facebook_login) Request phase initiated.
I, [2015-03-30T14:14:31.162164 #4511]  INFO -- omniauth: (facebook_login) Callback phase initiated.
E, [2015-03-30T14:14:31.162877 #4511] ERROR -- omniauth: (facebook_login) Authentication failure! no_authorization_code: OmniAuth::Strategies::Facebook::NoAuthorizationCodeError, must pass either a `code` (via URL or by an `fbsr_XXX` signed request cookie)

We are confused about this error entry.

@nbrustein
Copy link

Getting this as well using rails and devise_token_auth with omniauth-facebook

@simi
Copy link
Owner

simi commented Jul 17, 2015

And what's expected? Redirect to /auth/failure?

@simi
Copy link
Owner

simi commented Jul 18, 2015

Hello @joker-777 @nbrustein!

Actually it works when RACK_ENV is not set to development.

see https://github.com/intridea/omniauth/blob/master/lib/omniauth.rb#L37, https://github.com/intridea/omniauth/blob/master/lib/omniauth.rb#L36

You can override that via OmniAuth.config block. You can override on_failure also to provide custom behaviour.

@nbrustein
Copy link

In my opinion, it does not work when RACK_ENV is not set to development, because the error message passed through to the failure endpoint is no_authorization_code. This isn't really what went wrong. The real problem is that the user denied the authorization request. The fact that there is no authorization_code is just a symptom of that. It can have other reasons (for example, if someone goes to /auth/failure directly in a browser.

I think that expected behavior should be to get forwarded to auth/failure with the error message saying user_denied.

We're doing this in omniauth.rb right now to produce this behavior:

Rails.application.config.middleware.use OmniAuth::Builder do

    # # uncomment this if you want to test error handling in dev mode
    # configure do |config|
    #     config.failure_raise_out_environments = []
    # end

    on_failure do |env|

        begin

            exception = env['omniauth.error']  
            expected_exception = false

            # This means that the user pressed no when asked if 
            # we could have their e-mail address.
            if (
                    env['omniauth.strategy'].is_a?(OmniAuth::Strategies::Facebook) && 
                    (query = env['rack.request.query_hash']) &&
                    query['error'] == 'access_denied' && 
                    query['error_description'] == 'Permissions error' && 
                    query['error_reason'] == 'user_denied'
                )

                # this will be passed along to the custom_omniauth_callbacks_controller#omniauth_failure, 
                # which will show a nice message to the user
                env['omniauth.error.type'] = "user_denied"
                expected_exception = true

            end

            # we log exceptions using Raven, so we do that here
            # in all cases except with this facebook exception
            Raven.capture_exception(exception) unless expected_exception

        # We don't expect any errors above, but even if there
        # is one, we still want to go ahead to the failure
        # endpoint.  So catch all errors here.
        rescue Exception => err
            Raven.capture_exception(err)
        end

        # do the default behavior
        OmniAuth::FailureEndpoint.call(env)


    end
end

@simi
Copy link
Owner

simi commented Jul 20, 2015

You can try example application in production mode. That works for me.

$ rackup -E production

Open http://localhost:9292/server-side and try to reject. I'm redirected to auth/failure.

@nbrustein
Copy link

But what is the error message? is it no_authorization_code? To me, that's a problem. That is a bad description of what went wrong. It's not specific enough. I think that the error code should tell you that the problem is that the user denied authorization, so that you can handle that specially if you want to.

@simi
Copy link
Owner

simi commented Jul 20, 2015

Why? I think no_authorization_code is the exactly name for this problem. What do you expect? Probably it is possible to raise new exception in with_authorization_code! when error=access_denied is in query string. But this will break backward compatibility. :(

@nbrustein
Copy link

To me, no_authorization_code is not good enough because it can come from other reasons (like going directly to /auth/failure). And because when I, as a developer, read it, it took me hours of reading through code and issues online before I understood that what that really meant was that the user had clicked "Cancel" when I asked them to give me their e-mail address. That's not clear at all.

Let me say it another way. Facebook responded to my api request with a clear error message: user_denied. The facebook omniauth-gem, rather then sending that error message along to me, ate it and sent me a different error message, no_authorization_code. The original error message from facebook would have been a lot more clear and informative.

That make sense?

@simi
Copy link
Owner

simi commented Jul 20, 2015

Sure, I understand you. Do you have any idea how similar omniauth libraries are handling this?

@nbrustein
Copy link

With google_oauth2, I get to the the failure endpoint with params['message'] equal to "access_denied"'

@nbrustein
Copy link

If you'll accept a PR for this, let me know and I'll get one together.

@mkdynamic
Copy link
Collaborator

@nbrustein PR welcome.

@timherby
Copy link

timherby commented Nov 3, 2015

Experiencing the same thing. This would be very valuable to us as we want to log the actual errors, and this is a red herring when troubleshooting oauth issues.

@JoaoLopes
Copy link

This looked like a quick fix, using the same logic as in google_oauth2. Any particular reason it wasn't fixed (e.g. any other issues, backward compatibility)?

@joker-777
Copy link
Author

@mkdynamic Hi, it would be nice if we could fix this issue.

@AmirolAhmad
Copy link

looking for this fix

@simi
Copy link
Owner

simi commented Jan 24, 2020

There is pull request opened for this (#309), we will continue discussion in there. I'm closing this for now. Thanks everyone for your comments here.

@simi simi closed this as completed Jan 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants