From 5912a8011b05f36077f0147cdf7403a8e7626216 Mon Sep 17 00:00:00 2001 From: Tom Hughes Date: Tue, 2 Feb 2021 19:34:05 +0000 Subject: [PATCH] Update to Omniauth 2.x This requires converting all use of the /auth endpoints to use the POST method as GET is no longer supported. --- .rubocop_todo.yml | 2 +- Gemfile | 3 +- Gemfile.lock | 17 ++++++--- app/controllers/users_controller.rb | 2 +- app/helpers/user_helper.rb | 1 + config/routes.rb | 2 +- test/helpers/user_helper_test.rb | 4 +-- test/integration/user_creation_test.rb | 36 +++++++++---------- test/integration/user_login_test.rb | 50 +++++++++++++------------- 9 files changed, 63 insertions(+), 54 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 872a03d8b8..219135dfff 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -18,7 +18,7 @@ require: # Configuration parameters: AutoCorrect, AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, IgnoredPatterns. # URISchemes: http, https Layout/LineLength: - Max: 248 + Max: 254 # Offense count: 36 # Configuration parameters: AllowSafeAssignment. diff --git a/Gemfile b/Gemfile index 703c68eaa6..b80771a28a 100644 --- a/Gemfile +++ b/Gemfile @@ -61,12 +61,13 @@ gem "quad_tile", "~> 1.0.1" gem "rack-uri_sanitizer" # Omniauth for authentication -gem "omniauth", "~> 1.9.1" +gem "omniauth", "~> 2.0.2" gem "omniauth-facebook" gem "omniauth-github" gem "omniauth-google-oauth2", ">= 0.6.0" gem "omniauth-mediawiki", ">= 0.0.4" gem "omniauth-openid" +gem "omniauth-rails_csrf_protection", "~> 1.0" gem "omniauth-windowslive" # Markdown formatting support diff --git a/Gemfile.lock b/Gemfile.lock index 81df6c0cf7..a9b901a8be 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -287,14 +287,15 @@ GEM multi_json (~> 1.3) multi_xml (~> 0.5) rack (>= 1.2, < 3) - omniauth (1.9.1) + omniauth (2.0.2) hashie (>= 3.4.6) rack (>= 1.6.2, < 3) + rack-protection omniauth-facebook (8.0.0) omniauth-oauth2 (~> 1.2) - omniauth-github (1.4.0) - omniauth (~> 1.5) - omniauth-oauth2 (>= 1.4.0, < 2.0) + omniauth-github (2.0.0) + omniauth (~> 2.0) + omniauth-oauth2 (~> 1.7.1) omniauth-google-oauth2 (0.8.1) jwt (>= 2.0) oauth2 (~> 1.1) @@ -312,6 +313,9 @@ GEM omniauth-openid (2.0.1) omniauth (>= 1.0, < 3.0) rack-openid (~> 1.4.0) + omniauth-rails_csrf_protection (1.0.0) + actionpack (>= 4.2) + omniauth (~> 2.0) omniauth-windowslive (0.0.12) multi_json (~> 1.12) omniauth-oauth2 (~> 1.4) @@ -334,6 +338,8 @@ GEM rack-openid (1.4.2) rack (>= 1.1.0) ruby-openid (>= 2.1.8) + rack-protection (2.1.0) + rack rack-test (1.1.0) rack (>= 1.0, < 3) rack-uri_sanitizer (0.0.2) @@ -510,12 +516,13 @@ DEPENDENCIES mini_magick minitest (~> 5.1) oauth-plugin (>= 0.5.1) - omniauth (~> 1.9.1) + omniauth (~> 2.0.2) omniauth-facebook omniauth-github omniauth-google-oauth2 (>= 0.6.0) omniauth-mediawiki (>= 0.0.4) omniauth-openid + omniauth-rails_csrf_protection (~> 1.0) omniauth-windowslive openstreetmap-deadlock_retry (>= 1.3.0) pg diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index b274b18084..a1129339eb 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -260,7 +260,7 @@ def create elsif current_user.auth_provider.present? # Verify external authenticator before moving on session[:new_user] = current_user - redirect_to auth_url(current_user.auth_provider, current_user.auth_uid) + redirect_to auth_url(current_user.auth_provider, current_user.auth_uid), :status => :temporary_redirect else # Save the user record session[:new_user] = current_user diff --git a/app/helpers/user_helper.rb b/app/helpers/user_helper.rb index 5b33b57b33..39532572e8 100644 --- a/app/helpers/user_helper.rb +++ b/app/helpers/user_helper.rb @@ -60,6 +60,7 @@ def auth_button(name, provider, options = {}) link_to( image_tag("#{name}.svg", :alt => t("users.login.auth_providers.#{name}.alt"), :class => "rounded-lg"), auth_path(options.merge(:provider => provider)), + :method => :post, :class => "auth_button", :title => t("users.login.auth_providers.#{name}.title") ) diff --git a/config/routes.rb b/config/routes.rb index 8a1ea8846f..bd6f3034df 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -173,7 +173,7 @@ # omniauth get "/auth/failure" => "users#auth_failure" match "/auth/:provider/callback" => "users#auth_success", :via => [:get, :post], :as => :auth_success - match "/auth/:provider" => "users#auth", :via => [:get, :post], :as => :auth + post "/auth/:provider" => "users#auth", :as => :auth # permalink get "/go/:code" => "site#permalink", :code => /[a-zA-Z0-9_@~]+[=-]*/, :as => :permalink diff --git a/test/helpers/user_helper_test.rb b/test/helpers/user_helper_test.rb index 340d2494d4..6170e3a489 100644 --- a/test/helpers/user_helper_test.rb +++ b/test/helpers/user_helper_test.rb @@ -73,10 +73,10 @@ def test_openid_logo def test_auth_button button = auth_button("google", "google") - assert_equal("\"Login", button) + assert_equal("\"Login", button) button = auth_button("yahoo", "openid", :openid_url => "yahoo.com") - assert_equal("\"Login", button) + assert_equal("\"Login", button) end private diff --git a/test/integration/user_creation_test.rb b/test/integration/user_creation_test.rb index f331a4b981..138911bb12 100644 --- a/test/integration/user_creation_test.rb +++ b/test/integration/user_creation_test.rb @@ -243,7 +243,7 @@ def test_user_create_openid_success :pass_crypt_confirmation => "" } } assert_response :redirect assert_redirected_to auth_path(:provider => "openid", :openid_url => "http://localhost:1123/new.tester", :origin => "/user/new") - follow_redirect! + post response.location assert_response :redirect assert_redirected_to auth_success_path(:provider => "openid", :openid_url => "http://localhost:1123/new.tester", :origin => "/user/new") follow_redirect! @@ -289,7 +289,7 @@ def test_user_create_openid_failure :pass_crypt_confirmation => "" } } assert_response :redirect assert_redirected_to auth_path(:provider => "openid", :openid_url => "http://localhost:1123/new.tester", :origin => "/user/new") - follow_redirect! + post response.location assert_response :redirect assert_redirected_to auth_success_path(:provider => "openid", :openid_url => "http://localhost:1123/new.tester", :origin => "/user/new") follow_redirect! @@ -328,7 +328,7 @@ def test_user_create_openid_redirect :referer => referer } assert_response :redirect assert_redirected_to auth_path(:provider => "openid", :openid_url => "http://localhost:1123/new.tester", :origin => "/user/new") - follow_redirect! + post response.location assert_response :redirect assert_redirected_to auth_success_path(:provider => "openid", :openid_url => "http://localhost:1123/new.tester", :origin => "/user/new") follow_redirect! @@ -397,7 +397,7 @@ def test_user_create_google_success :pass_crypt_confirmation => "" } } assert_response :redirect assert_redirected_to auth_path(:provider => "google", :origin => "/user/new") - follow_redirect! + post response.location assert_response :redirect assert_redirected_to auth_success_path(:provider => "google") follow_redirect! @@ -442,7 +442,7 @@ def test_user_create_google_failure :pass_crypt_confirmation => "" } } assert_response :redirect assert_redirected_to auth_path(:provider => "google", :origin => "/user/new") - follow_redirect! + post response.location assert_response :redirect assert_redirected_to auth_success_path(:provider => "google") follow_redirect! @@ -482,7 +482,7 @@ def test_user_create_google_redirect :referer => referer } assert_response :redirect assert_redirected_to auth_path(:provider => "google", :origin => "/user/new") - follow_redirect! + post response.location assert_response :redirect assert_redirected_to auth_success_path(:provider => "google") follow_redirect! @@ -549,7 +549,7 @@ def test_user_create_facebook_success :pass_crypt_confirmation => "" } } assert_response :redirect assert_redirected_to auth_path(:provider => "facebook", :origin => "/user/new") - follow_redirect! + post response.location assert_response :redirect assert_redirected_to auth_success_path(:provider => "facebook") follow_redirect! @@ -594,7 +594,7 @@ def test_user_create_facebook_failure :pass_crypt_confirmation => "" } } assert_response :redirect assert_redirected_to auth_path(:provider => "facebook", :origin => "/user/new") - follow_redirect! + post response.location assert_response :redirect assert_redirected_to auth_success_path(:provider => "facebook") follow_redirect! @@ -632,7 +632,7 @@ def test_user_create_facebook_redirect :referer => referer } assert_response :redirect assert_redirected_to auth_path(:provider => "facebook", :origin => "/user/new") - follow_redirect! + post response.location assert_response :redirect assert_redirected_to auth_success_path(:provider => "facebook") follow_redirect! @@ -699,7 +699,7 @@ def test_user_create_windowslive_success :pass_crypt_confirmation => "" } } assert_response :redirect assert_redirected_to auth_path(:provider => "windowslive", :origin => "/user/new") - follow_redirect! + post response.location assert_response :redirect assert_redirected_to auth_success_path(:provider => "windowslive") follow_redirect! @@ -744,7 +744,7 @@ def test_user_create_windowslive_failure :pass_crypt_confirmation => "" } } assert_response :redirect assert_redirected_to auth_path(:provider => "windowslive", :origin => "/user/new") - follow_redirect! + post response.location assert_response :redirect assert_redirected_to auth_success_path(:provider => "windowslive") follow_redirect! @@ -782,7 +782,7 @@ def test_user_create_windowslive_redirect :referer => referer } assert_response :redirect assert_redirected_to auth_path(:provider => "windowslive", :origin => "/user/new") - follow_redirect! + post response.location assert_response :redirect assert_redirected_to auth_success_path(:provider => "windowslive") follow_redirect! @@ -849,7 +849,7 @@ def test_user_create_github_success :pass_crypt_confirmation => "" } } assert_response :redirect assert_redirected_to auth_path(:provider => "github", :origin => "/user/new") - follow_redirect! + post response.location assert_response :redirect assert_redirected_to auth_success_path(:provider => "github") follow_redirect! @@ -895,7 +895,7 @@ def test_user_create_github_failure :pass_crypt_confirmation => "" } } assert_response :redirect assert_redirected_to auth_path(:provider => "github", :origin => "/user/new") - follow_redirect! + post response.location assert_response :redirect assert_redirected_to auth_success_path(:provider => "github") follow_redirect! @@ -933,7 +933,7 @@ def test_user_create_github_redirect :referer => referer } assert_response :redirect assert_redirected_to auth_path(:provider => "github", :origin => "/user/new") - follow_redirect! + post response.location assert_response :redirect assert_redirected_to auth_success_path(:provider => "github") follow_redirect! @@ -1001,7 +1001,7 @@ def test_user_create_wikipedia_success :pass_crypt_confirmation => "" } } assert_response :redirect assert_redirected_to auth_path(:provider => "wikipedia", :origin => "/user/new") - follow_redirect! + post response.location assert_response :redirect assert_redirected_to auth_success_path(:provider => "wikipedia", :origin => "/user/new") follow_redirect! @@ -1047,7 +1047,7 @@ def test_user_create_wikipedia_failure :pass_crypt_confirmation => "" } } assert_response :redirect assert_redirected_to auth_path(:provider => "wikipedia", :origin => "/user/new") - follow_redirect! + post response.location assert_response :redirect assert_redirected_to auth_success_path(:provider => "wikipedia", :origin => "/user/new") follow_redirect! @@ -1085,7 +1085,7 @@ def test_user_create_wikipedia_redirect :referer => referer } assert_response :redirect assert_redirected_to auth_path(:provider => "wikipedia", :origin => "/user/new") - follow_redirect! + post response.location assert_response :redirect assert_redirected_to auth_success_path(:provider => "wikipedia", :origin => "/user/new") follow_redirect! diff --git a/test/integration/user_login_test.rb b/test/integration/user_login_test.rb index b83b90acf1..149875defb 100644 --- a/test/integration/user_login_test.rb +++ b/test/integration/user_login_test.rb @@ -359,7 +359,7 @@ def test_login_openid_success follow_redirect! assert_response :success assert_template "users/login" - get auth_path(:provider => "openid", :openid_url => "http://localhost:1123/john.doe", :origin => "/login?referer=%2Fhistory", :referer => "/history") + post auth_path(:provider => "openid", :openid_url => "http://localhost:1123/john.doe", :origin => "/login?referer=%2Fhistory", :referer => "/history") assert_response :redirect assert_redirected_to auth_success_path(:provider => "openid", :openid_url => "http://localhost:1123/john.doe", :origin => "/login?referer=%2Fhistory", :referer => "/history") follow_redirect! @@ -380,7 +380,7 @@ def test_login_openid_connection_failed follow_redirect! assert_response :success assert_template "users/login" - get auth_path(:provider => "openid", :openid_url => user.auth_uid, :origin => "/login?referer=%2Fhistory", :referer => "/history") + post auth_path(:provider => "openid", :openid_url => user.auth_uid, :origin => "/login?referer=%2Fhistory", :referer => "/history") assert_response :redirect assert_redirected_to auth_success_path(:provider => "openid", :openid_url => user.auth_uid, :origin => "/login?referer=%2Fhistory", :referer => "/history") follow_redirect! @@ -405,7 +405,7 @@ def test_login_openid_invalid_credentials follow_redirect! assert_response :success assert_template "users/login" - get auth_path(:provider => "openid", :openid_url => user.auth_uid, :origin => "/login?referer=%2Fhistory", :referer => "/history") + post auth_path(:provider => "openid", :openid_url => user.auth_uid, :origin => "/login?referer=%2Fhistory", :referer => "/history") assert_response :redirect assert_redirected_to auth_success_path(:provider => "openid", :openid_url => user.auth_uid, :origin => "/login?referer=%2Fhistory", :referer => "/history") follow_redirect! @@ -429,7 +429,7 @@ def test_login_openid_unknown follow_redirect! assert_response :success assert_template "users/login" - get auth_path(:provider => "openid", :openid_url => "http://localhost:1123/fred.bloggs", :origin => "/login?referer=%2Fhistory", :referer => "/history") + post auth_path(:provider => "openid", :openid_url => "http://localhost:1123/fred.bloggs", :origin => "/login?referer=%2Fhistory", :referer => "/history") assert_response :redirect assert_redirected_to auth_success_path(:provider => "openid", :openid_url => "http://localhost:1123/fred.bloggs", :origin => "/login?referer=%2Fhistory", :referer => "/history") follow_redirect! @@ -452,7 +452,7 @@ def test_login_google_success follow_redirect! assert_response :success assert_template "users/login" - get auth_path(:provider => "google", :origin => "/login?referer=%2Fhistory", :referer => "/history") + post auth_path(:provider => "google", :origin => "/login?referer=%2Fhistory", :referer => "/history") assert_response :redirect assert_redirected_to auth_success_path(:provider => "google") follow_redirect! @@ -472,7 +472,7 @@ def test_login_google_connection_failed follow_redirect! assert_response :success assert_template "users/login" - get auth_path(:provider => "google", :origin => "/login?referer=%2Fhistory", :referer => "/history") + post auth_path(:provider => "google", :origin => "/login?referer=%2Fhistory", :referer => "/history") assert_response :redirect assert_redirected_to auth_success_path(:provider => "google") follow_redirect! @@ -496,7 +496,7 @@ def test_login_google_invalid_credentials follow_redirect! assert_response :success assert_template "users/login" - get auth_path(:provider => "google", :origin => "/login?referer=%2Fhistory", :referer => "/history") + post auth_path(:provider => "google", :origin => "/login?referer=%2Fhistory", :referer => "/history") assert_response :redirect assert_redirected_to auth_success_path(:provider => "google") follow_redirect! @@ -522,7 +522,7 @@ def test_login_google_unknown follow_redirect! assert_response :success assert_template "users/login" - get auth_path(:provider => "google", :origin => "/login?referer=%2Fhistory", :referer => "/history") + post auth_path(:provider => "google", :origin => "/login?referer=%2Fhistory", :referer => "/history") assert_response :redirect assert_redirected_to auth_success_path(:provider => "google") follow_redirect! @@ -545,7 +545,7 @@ def test_login_google_upgrade follow_redirect! assert_response :success assert_template "users/login" - get auth_path(:provider => "google", :origin => "/login?referer=%2Fhistory", :referer => "/history") + post auth_path(:provider => "google", :origin => "/login?referer=%2Fhistory", :referer => "/history") assert_response :redirect assert_redirected_to auth_success_path(:provider => "google") follow_redirect! @@ -570,7 +570,7 @@ def test_login_facebook_success follow_redirect! assert_response :success assert_template "users/login" - get auth_path(:provider => "facebook", :origin => "/login?referer=%2Fhistory", :referer => "/history") + post auth_path(:provider => "facebook", :origin => "/login?referer=%2Fhistory", :referer => "/history") assert_response :redirect assert_redirected_to auth_success_path(:provider => "facebook") follow_redirect! @@ -590,7 +590,7 @@ def test_login_facebook_connection_failed follow_redirect! assert_response :success assert_template "users/login" - get auth_path(:provider => "facebook", :origin => "/login?referer=%2Fhistory", :referer => "/history") + post auth_path(:provider => "facebook", :origin => "/login?referer=%2Fhistory", :referer => "/history") assert_response :redirect assert_redirected_to auth_success_path(:provider => "facebook") follow_redirect! @@ -614,7 +614,7 @@ def test_login_facebook_invalid_credentials follow_redirect! assert_response :success assert_template "users/login" - get auth_path(:provider => "facebook", :origin => "/login?referer=%2Fhistory", :referer => "/history") + post auth_path(:provider => "facebook", :origin => "/login?referer=%2Fhistory", :referer => "/history") assert_response :redirect assert_redirected_to auth_success_path(:provider => "facebook") follow_redirect! @@ -638,7 +638,7 @@ def test_login_facebook_unknown follow_redirect! assert_response :success assert_template "users/login" - get auth_path(:provider => "facebook", :origin => "/login?referer=%2Fhistory", :referer => "/history") + post auth_path(:provider => "facebook", :origin => "/login?referer=%2Fhistory", :referer => "/history") assert_response :redirect assert_redirected_to auth_success_path(:provider => "facebook") follow_redirect! @@ -659,7 +659,7 @@ def test_login_windowslive_success follow_redirect! assert_response :success assert_template "users/login" - get auth_path(:provider => "windowslive", :origin => "/login?referer=%2Fhistory", :referer => "/history") + post auth_path(:provider => "windowslive", :origin => "/login?referer=%2Fhistory", :referer => "/history") assert_response :redirect assert_redirected_to auth_success_path(:provider => "windowslive") follow_redirect! @@ -679,7 +679,7 @@ def test_login_windowslive_connection_failed follow_redirect! assert_response :success assert_template "users/login" - get auth_path(:provider => "windowslive", :origin => "/login?referer=%2Fhistory", :referer => "/history") + post auth_path(:provider => "windowslive", :origin => "/login?referer=%2Fhistory", :referer => "/history") assert_response :redirect assert_redirected_to auth_success_path(:provider => "windowslive") follow_redirect! @@ -703,7 +703,7 @@ def test_login_windowslive_invalid_credentials follow_redirect! assert_response :success assert_template "users/login" - get auth_path(:provider => "windowslive", :origin => "/login?referer=%2Fhistory", :referer => "/history") + post auth_path(:provider => "windowslive", :origin => "/login?referer=%2Fhistory", :referer => "/history") assert_response :redirect assert_redirected_to auth_success_path(:provider => "windowslive") follow_redirect! @@ -727,7 +727,7 @@ def test_login_windowslive_unknown follow_redirect! assert_response :success assert_template "users/login" - get auth_path(:provider => "windowslive", :origin => "/login?referer=%2Fhistory", :referer => "/history") + post auth_path(:provider => "windowslive", :origin => "/login?referer=%2Fhistory", :referer => "/history") assert_response :redirect assert_redirected_to auth_success_path(:provider => "windowslive") follow_redirect! @@ -748,7 +748,7 @@ def test_login_github_success follow_redirect! assert_response :success assert_template "users/login" - get auth_path(:provider => "github", :origin => "/login?referer=%2Fhistory", :referer => "/history") + post auth_path(:provider => "github", :origin => "/login?referer=%2Fhistory", :referer => "/history") assert_response :redirect assert_redirected_to auth_success_path(:provider => "github") follow_redirect! @@ -768,7 +768,7 @@ def test_login_github_connection_failed follow_redirect! assert_response :success assert_template "users/login" - get auth_path(:provider => "github", :origin => "/login?referer=%2Fhistory", :referer => "/history") + post auth_path(:provider => "github", :origin => "/login?referer=%2Fhistory", :referer => "/history") assert_response :redirect assert_redirected_to auth_success_path(:provider => "github") follow_redirect! @@ -792,7 +792,7 @@ def test_login_github_invalid_credentials follow_redirect! assert_response :success assert_template "users/login" - get auth_path(:provider => "github", :origin => "/login?referer=%2Fhistory", :referer => "/history") + post auth_path(:provider => "github", :origin => "/login?referer=%2Fhistory", :referer => "/history") assert_response :redirect assert_redirected_to auth_success_path(:provider => "github") follow_redirect! @@ -816,7 +816,7 @@ def test_login_github_unknown follow_redirect! assert_response :success assert_template "users/login" - get auth_path(:provider => "github", :origin => "/login?referer=%2Fhistory", :referer => "/history") + post auth_path(:provider => "github", :origin => "/login?referer=%2Fhistory", :referer => "/history") assert_response :redirect assert_redirected_to auth_success_path(:provider => "github") follow_redirect! @@ -837,7 +837,7 @@ def test_login_wikipedia_success follow_redirect! assert_response :success assert_template "users/login" - get auth_path(:provider => "wikipedia", :origin => "/login?referer=%2Fhistory", :referer => "/history") + post auth_path(:provider => "wikipedia", :origin => "/login?referer=%2Fhistory", :referer => "/history") assert_response :redirect assert_redirected_to auth_success_path(:provider => "wikipedia", :origin => "/login?referer=%2Fhistory", :referer => "/history") follow_redirect! @@ -857,7 +857,7 @@ def test_login_wikipedia_connection_failed follow_redirect! assert_response :success assert_template "users/login" - get auth_path(:provider => "wikipedia", :origin => "/login?referer=%2Fhistory", :referer => "/history") + post auth_path(:provider => "wikipedia", :origin => "/login?referer=%2Fhistory", :referer => "/history") assert_response :redirect assert_redirected_to auth_success_path(:provider => "wikipedia", :origin => "/login?referer=%2Fhistory", :referer => "/history") follow_redirect! @@ -881,7 +881,7 @@ def test_login_wikipedia_invalid_credentials follow_redirect! assert_response :success assert_template "users/login" - get auth_path(:provider => "wikipedia", :origin => "/login?referer=%2Fhistory", :referer => "/history") + post auth_path(:provider => "wikipedia", :origin => "/login?referer=%2Fhistory", :referer => "/history") assert_response :redirect assert_redirected_to auth_success_path(:provider => "wikipedia", :origin => "/login?referer=%2Fhistory", :referer => "/history") follow_redirect! @@ -905,7 +905,7 @@ def test_login_wikipedia_unknown follow_redirect! assert_response :success assert_template "users/login" - get auth_path(:provider => "wikipedia", :origin => "/login?referer=%2Fhistory", :referer => "/history") + post auth_path(:provider => "wikipedia", :origin => "/login?referer=%2Fhistory", :referer => "/history") assert_response :redirect assert_redirected_to auth_success_path(:provider => "wikipedia", :origin => "/login?referer=%2Fhistory", :referer => "/history") follow_redirect!