From 4f822356307813fc5fc143dbf2cb9a022a891125 Mon Sep 17 00:00:00 2001 From: Edouard CHIN Date: Thu, 7 Jul 2022 11:24:19 +0200 Subject: [PATCH] Use Omniauth.allowed_methods' as routing verbs for the auth path: - ### Context Since version 2.0.0, Omniauth no longer recognizes `GET` request on the auth path (`/users/auth/`). `POST` is the only verb that is by default recognized in order to mitigate CSRF attack. https://github.com/omniauth/omniauth/blob/66110da85e3106d9c9b138d384267a9397c75fe7/lib/omniauth/strategy.rb#L205 Ultimatelly, when a user try to access `GET /users/auth/facebook`, Devise [passthru action](https://github.com/heartcombo/devise/blob/6d32d2447cc0f3739d9732246b5a5bde98d9e032/app/controllers/devise/omniauth_callbacks_controller.rb#L6) will be called which just return a raw 404 page. ### Problem There is no problem per se and everything work. However the advantage of not matching GET request at the router layer allows to get that same 404 page stylized for "free" (Rails ending up rendering the 404 page of the app). I believe it's also more consistent and less surprising for users if this passthru action don't get called. ### Drawback An application can no longer override the `passthru` to perform the logic it wants (i.e. redirect the user). If this is a dealbreaker, feel free to close this PR :). --- lib/devise/rails/routes.rb | 2 +- test/integration/omniauthable_test.rb | 22 ++++++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/lib/devise/rails/routes.rb b/lib/devise/rails/routes.rb index 004b985746..f58c9fdc48 100644 --- a/lib/devise/rails/routes.rb +++ b/lib/devise/rails/routes.rb @@ -447,7 +447,7 @@ def devise_omniauth_callback(mapping, controllers) #:nodoc: match "#{path_prefix}/#{provider}", to: "#{controllers[:omniauth_callbacks]}#passthru", as: "#{provider}_omniauth_authorize", - via: [:get, :post] + via: OmniAuth.config.allowed_request_methods match "#{path_prefix}/#{provider}/callback", to: "#{controllers[:omniauth_callbacks]}##{provider}", diff --git a/test/integration/omniauthable_test.rb b/test/integration/omniauthable_test.rb index db3d0871c1..72a59dbfbf 100644 --- a/test/integration/omniauthable_test.rb +++ b/test/integration/omniauthable_test.rb @@ -126,6 +126,28 @@ def stub_action!(name) end end + test "authorization path via GET when Omniauth allowed_request_methods includes GET" do + original_allowed = OmniAuth.config.allowed_request_methods + OmniAuth.config.allowed_request_methods = [:get, :post] + + get "/users/auth/facebook" + + assert_response(:redirect) + ensure + OmniAuth.config.allowed_request_methods = original_allowed + end + + test "authorization path via GET when Omniauth allowed_request_methods doesn't include GET" do + original_allowed = OmniAuth.config.allowed_request_methods + OmniAuth.config.allowed_request_methods = [:post] + + assert_raises(ActionController::RoutingError) do + get "/users/auth/facebook" + end + ensure + OmniAuth.config.allowed_request_methods = original_allowed + end + test "generates a link to authenticate with provider" do visit "/users/sign_in" assert_select "form[action=?][method=post]", "/users/auth/facebook" do