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/<provider>`). `POST` is the only
  verb that is by default recognized in order to mitigate CSRF
  attack. 66110da85e/lib/omniauth/strategy.rb (L205)

  Ultimatelly, when a user try to access `GET /users/auth/facebook`,
  Devise [passthru action](6d32d2447c/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 :).
This commit is contained in:
Edouard CHIN
2022-07-07 11:24:19 +02:00
committed by Rafael Mendonça França
parent 9be24c0ae4
commit 4f82235630
2 changed files with 23 additions and 1 deletions

View File

@@ -447,7 +447,7 @@ ERROR
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}",

View File

@@ -126,6 +126,28 @@ class OmniauthableIntegrationTest < Devise::IntegrationTest
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