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

/auth/failure never reached due to warden throw #10

Open
sbauch opened this issue Sep 19, 2020 · 2 comments
Open

/auth/failure never reached due to warden throw #10

sbauch opened this issue Sep 19, 2020 · 2 comments

Comments

@sbauch
Copy link

sbauch commented Sep 19, 2020

My understanding of omniauth is that, when a strategy failure occurs, strategy.fail! should be called such that the user is redirected to /auth/failure.

I've looked through a handful of strategies, and haven't seen a throw :warden like we see here -

https://github.com/salsify/omniauth-multi-provider/blob/master/lib/omni_auth/multi_provider/handler.rb#L67

Should throwing a warden error be the responsibility of the consumer of this lib?

I can't find a way to rescue this error (I am not using warden, nor devise) and as such haven't found a reasonable way to get the user to /auth/failure when there's an error in the identity_provider_options_generator block.

The only thing I can really do is something like this, which results in invalid ticket errors, which isn't really what's going on here:

do |identity_provider_id, env|
  
  Models::IdentityProvider.find(identity_provider_id).
    saml_options

  rescue ActiveRecord::RecordNotFound
    {}
end

If my block throws an error, the user should be redirected to /auth/failure per OmniAuth. Instead, i get an UncaughtThrowError, which in prod is not going be something i can really rescue to display a useful message.

@jturkel
Copy link
Member

jturkel commented Oct 5, 2020

Hmm. That does sound like a problem that we've let an implicit warden dependency sneak into this gem. Ideally this gem would work with plain omniauth or omniauth + warden + devise. Unfortunately I don't have the bandwidth to investigate this right not but PRs are always welcome!

@sbauch
Copy link
Author

sbauch commented Oct 5, 2020

I'm happy to take a stab, especially if it comes with a FREE TEE SHIRT 👻 😱 🎃

any thoughts on how you would like to approach backwards compatibility? I'm not terribly familiar with warden, it might be nice to be able to still throw the warden error if warden is used? kinda seems like env['warden'] might tell us if this strategy is being used with warden?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants