-
Notifications
You must be signed in to change notification settings - Fork 5.5k
Devise is hard-coded to OmniAuth 1.x.x #5326
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
Comments
@BobbyMcWho hey, I'm happy to help enable OmniAuth 2.x support in Devise. Is there any possible breaking change we should be aware of that you could point me to? I'll look into allowing 2.x so we can run the build and see if it all passes. Thanks for reporting. |
I have #5327 to allow 2.x to be installed (so we won't blow up in that case), but I'm locked to try upgrading the gem in our Gemfile due to omniauth-oauth2 requiring OmniAuth 1.x still. I think our tests are using that, I'll look into that later, but this should at least let someone test out this new branch with OmniAuth 2. |
Thanks @carlosantoniodasilva, I do have a branch under my own repo that allows that version https://github.com/omniauth/omniauth-oauth2/pull/134/files, but I will be releasing 2.0 when I have a chance as well. |
Here are the release notes for the release candidate, I am not super familiar with how Devise uses OmniAuth, so unsure if it will affect the typical configuration https://github.com/omniauth/omniauth/releases/tag/v2.0.0-rc1 |
I have released OmniAuth-oauth2 v1.7.1 that allows v2.0.0 to be used, and I have also released omniauth v2.0.0 |
Well that was fast @BobbyMcWho, thanks for the info here. I'll double check our tests are okay with the new version and if it's all good I can look into releasing a patch update enabling that. |
Yeah, I had the release candidate discussion open for 31 days, so I figured it was time. I'm happy to help out if it ends up that Devise needs any changes for it |
@BobbyMcWho no worries, I'm behind on things here. It looks like omniauth-openid is also set to OmniAuth ~> 1.0: https://github.com/omniauth/omniauth-openid/blob/8a46fa98dabe3ea7ced336fef1201ab4870adb15/omniauth-openid.gemspec#L5, is that something on your radar? It's just prevented me from updating the bundle and test against 2.0, but I think only our test suite relies on it and I can remove and/or move it to something else probably. |
I'll have to take a look, it looks like that library hasn't had activity in 3 years |
No worries, thanks for your help... if bumping that doesn't really work/make sense I can move to something else for our tests. I commented those out to run the suite now and got a handful of failures, I'll take a closer look at those in the meantime. |
@BobbyMcWho hopefully last question for you before I work on the release of a new version soon :) It sounds like it's extremely discouraged, but still possible, to use GET requests to initiate the OmniAuth flow, right? Devise generate both routes allowing GET/POST, and I'm inclined to keep that for now. Alternatively I guess I could look at That only applies to the initial request, right? |
Sorry, I've had quite the busy day. Right, if folks want to override the default POST only and use GET as well they can, they'll be met with a big ol' warning in their logs on every auth request unless they config to shut it off and acknowledge it. And correct, the allowed methods are checked during the request phase check |
Perfect, thanks again for the help @BobbyMcWho. And don't worry, I can totally relate on the busy days. ;) I'm hoping to get a new release out at some point this week with the changes included. |
Update: Saw your PR + comments. Thanks again for your updates! |
This is on master branch now, please feel free to bundle from there while we prepare a new release soon. (there's a few other things to wrap up before it.) Thanks all. |
@carlosantoniodasilva is there an ETA for the new release? |
@emancu no ETA yet, but I hope to be releasing it at some point this month, just need to take a look at a couple other things first that may go with it. |
Awesome, thanks for the quick reply. No rush, just wondering :) |
Hello, maintainer of OmniAuth here.
I’ve been floating a release candidate of OmniAuth 2.0.0, and had a user report this
omniauth/omniauth#1017 (comment)
Any chance a maintainer here would be willing to assist me in relaxing that restriction and testing it with the release candidate?
The text was updated successfully, but these errors were encountered: