From ec07bdb315966de84686f7499828dd8f890e371a Mon Sep 17 00:00:00 2001 From: Lucas Mazza Date: Mon, 7 Mar 2016 11:19:27 -0300 Subject: [PATCH] Do not use the dynamic `:action` segment on Omniauth routes. This was deprecated on rails/rails#23980. We now generate scope and provider specific routes, like `user_facebook_omniauth_callback` or `user_github_omniauth_callback`. We could deprecate the `omniauth_authorize_path` in favor of the generated routes, but the `shared/links.html.erb` depends on it to generate all omniauth links at once. Closes #3983. --- lib/devise/omniauth/url_helpers.rb | 8 ++++---- lib/devise/rails/routes.rb | 22 ++++++++++------------ test/omniauth/url_helpers_test.rb | 10 ++++------ test/routes_test.rb | 4 ++-- 4 files changed, 20 insertions(+), 24 deletions(-) diff --git a/lib/devise/omniauth/url_helpers.rb b/lib/devise/omniauth/url_helpers.rb index dd123f6a..cc0eaee9 100644 --- a/lib/devise/omniauth/url_helpers.rb +++ b/lib/devise/omniauth/url_helpers.rb @@ -4,14 +4,14 @@ module Devise def self.define_helpers(mapping) end - def omniauth_authorize_path(resource_or_scope, *args) + def omniauth_authorize_path(resource_or_scope, provider, *args) scope = Devise::Mapping.find_scope!(resource_or_scope) - _devise_route_context.send("#{scope}_omniauth_authorize_path", *args) + _devise_route_context.send("#{scope}_#{provider}_omniauth_authorize_path", *args) end - def omniauth_callback_path(resource_or_scope, *args) + def omniauth_callback_path(resource_or_scope, provider, *args) scope = Devise::Mapping.find_scope!(resource_or_scope) - _devise_route_context.send("#{scope}_omniauth_callback_path", *args) + _devise_route_context.send("#{scope}_#{provider}_omniauth_callback_path", *args) end end end diff --git a/lib/devise/rails/routes.rb b/lib/devise/rails/routes.rb index e52b3062..73c5ceef 100644 --- a/lib/devise/rails/routes.rb +++ b/lib/devise/rails/routes.rb @@ -441,19 +441,17 @@ ERROR set_omniauth_path_prefix!(path_prefix) - providers = Regexp.union(mapping.to.omniauth_providers.map(&:to_s)) + mapping.to.omniauth_providers.each do |provider| + match "#{path_prefix}/#{provider}", + to: "#{controllers[:omniauth_callbacks]}#passthru", + as: "#{provider}_omniauth_authorize", + via: [:get, :post] - match "#{path_prefix}/:provider", - constraints: { provider: providers }, - to: "#{controllers[:omniauth_callbacks]}#passthru", - as: :omniauth_authorize, - via: [:get, :post] - - match "#{path_prefix}/:action/callback", - constraints: { action: providers }, - to: "#{controllers[:omniauth_callbacks]}#:action", - as: :omniauth_callback, - via: [:get, :post] + match "#{path_prefix}/#{provider}/callback", + to: "#{controllers[:omniauth_callbacks]}##{provider}", + as: "#{provider}_omniauth_callback", + via: [:get, :post] + end ensure @scope = current_scope end diff --git a/test/omniauth/url_helpers_test.rb b/test/omniauth/url_helpers_test.rb index 7dfc6280..21bf9d3a 100644 --- a/test/omniauth/url_helpers_test.rb +++ b/test/omniauth/url_helpers_test.rb @@ -1,23 +1,21 @@ require 'test_helper' class OmniAuthRoutesTest < ActionController::TestCase - ExpectedUrlGeneratiorError = ActionController::UrlGenerationError - tests ApplicationController def assert_path(action, provider, with_param=true) # Resource param assert_equal @controller.send(action, :user, provider), - @controller.send("user_#{action}", provider) + @controller.send("user_#{provider}_#{action}") # With an object assert_equal @controller.send(action, User.new, provider), - @controller.send("user_#{action}", provider) + @controller.send("user_#{provider}_#{action}") if with_param # Default url params assert_equal @controller.send(action, :user, provider, param: 123), - @controller.send("user_#{action}", provider, param: 123) + @controller.send("user_#{provider}_#{action}", param: 123) end end @@ -32,7 +30,7 @@ class OmniAuthRoutesTest < ActionController::TestCase test 'should generate authorization path' do assert_match "/users/auth/facebook", @controller.omniauth_authorize_path(:user, :facebook) - assert_raise ExpectedUrlGeneratiorError do + assert_raise NoMethodError do @controller.omniauth_authorize_path(:user, :github) end end diff --git a/test/routes_test.rb b/test/routes_test.rb index 4c6a3a1f..9589d605 100644 --- a/test/routes_test.rb +++ b/test/routes_test.rb @@ -96,12 +96,12 @@ class DefaultRoutingTest < ActionController::TestCase test 'map omniauth callbacks' do assert_recognizes({controller: 'users/omniauth_callbacks', action: 'facebook'}, {path: 'users/auth/facebook/callback', method: :get}) assert_recognizes({controller: 'users/omniauth_callbacks', action: 'facebook'}, {path: 'users/auth/facebook/callback', method: :post}) - assert_named_route "/users/auth/facebook/callback", :user_omniauth_callback_path, :facebook + assert_named_route "/users/auth/facebook/callback", :user_facebook_omniauth_callback_path # named open_id assert_recognizes({controller: 'users/omniauth_callbacks', action: 'google'}, {path: 'users/auth/google/callback', method: :get}) assert_recognizes({controller: 'users/omniauth_callbacks', action: 'google'}, {path: 'users/auth/google/callback', method: :post}) - assert_named_route "/users/auth/google/callback", :user_omniauth_callback_path, :google + assert_named_route "/users/auth/google/callback", :user_google_omniauth_callback_path assert_raise ExpectedRoutingError do assert_recognizes({controller: 'ysers/omniauth_callbacks', action: 'twitter'}, {path: 'users/auth/twitter/callback', method: :get})