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

Unable to override OmniauthCallbacksController#redirect_callbacks #186

Closed
tbloncar opened this issue Mar 14, 2015 · 11 comments
Closed

Unable to override OmniauthCallbacksController#redirect_callbacks #186

tbloncar opened this issue Mar 14, 2015 · 11 comments

Comments

@tbloncar
Copy link
Contributor

I noticed that my OmniauthCallbacksController#redirect_callbacks override implementation wasn't being used. I dug into this a bit, and it turns out that the override for omniauth_callbacks will never touch this particular route, as it is defined in config/routes.rb (and not lib/devise_token_auth/rails/routes.rb).

I was able to fix this by moving the route definition to the latter file (where the overrides are considered).

match "#{DeviseTokenAuth.omniauth_prefix}/:provider/callback", controller: omniauth_ctrl, action: "redirect_callbacks", via: [:get]

Does this present other issues? Or can we safely move this route definition and remove config/routes.rb?

I can put a PR together with the update if we're okay with moving forward.

@nbrustein
Copy link
Contributor

I think we're succeeding in doing what you're talking about like this in routes.rb:

mount_devise_token_auth_for 'User', at: 'api/auth', controllers: {
  omniauth_callbacks: 'devise_token_auth/custom_omniauth_callbacks',
}

That helpful?

@frankjwu
Copy link

@nbrustein That doesn't override the redirect_callbacks method for me (it works for other methods but not that specific one). Having the same problem as @tbloncar... any updates?

@tbloncar
Copy link
Contributor Author

@nbrustein @frankjwu Right. FWIW, I haven't encountered any issues since implementing the fix I described above.

@nbrustein
Copy link
Contributor

@frankjwu I just noticed that we also have this in our routes. So I guess this is why it's working for us:

post "/omniauth/:provider/callback(.:format)", to: 'devise_token_auth/custom_omniauth_callbacks#redirect_callbacks'

I thought that was just ensuring that we handle posts appropriately (as opposed to gets) when using omniauth-saml, but maybe that's the reason it works at all.

@medaglia
Copy link

@tbloncar i think i'm having the same problem as you. i tried moving the route into lib/devise_token_auth/rails/routes.rb with no luck, however. does your current fork have this change implemented?

@tbloncar
Copy link
Contributor Author

@medaglia I don't have this change on my fork, but it looks like I just added the following beneath this line.

match "#{DeviseTokenAuth.omniauth_prefix}/:provider/callback", controller: omniauth_ctrl, action: "redirect_callbacks", via: [:get]

Is this—save the specific location on line 54—the same change that you implemented?

@medaglia
Copy link

thanks @tbloncar, that resolved my issue.

@tbloncar
Copy link
Contributor Author

Okay. Cool. Welcome.

@frankjwu
Copy link

This seems like it should probably get merged into master, especially if you haven't seen any problems with it (@tbloncar).

@booleanbetrayal
Copy link
Collaborator

@tbloncar - if you have a PR for this already, I'll merge it!

booleanbetrayal added a commit that referenced this issue Aug 20, 2015
Fully support OmniauthCallbacksController action overrides. Fixes #186.
@m2omou
Copy link

m2omou commented Oct 4, 2015

Is the fix from @tbloncar been merged ? @booleanbetrayal thanks.

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

6 participants