Merge pull request #5327 from heartcombo/ca-omniauth-2

Improve OmniAuth version check to allow anything from 1.0 forward
heartcombo · Feb 1, 2021 · 743b693 · 743b693
2 parents aa8d895 + e16d60d
commit 743b693
Show file tree

Hide file tree

Showing 12 changed files with 49 additions and 38 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,6 +1,9 @@
 ### unreleased
 
 * enhancements
+  * Devise now enables the upgrade of OmniAuth 2+. Previously Devise would raise an error if you'd try to upgrade. Please note that OmniAuth 2 is considered a security upgrade and recommended to everyone. You can read more about the details (and possible necessary changes to your app as part of the upgrade) in [their release notes](https://github.com/omniauth/omniauth/releases/tag/v2.0.0). [Devise's OmniAuth Overview wiki](https://github.com/heartcombo/devise/wiki/OmniAuth:-Overview) was also updated to cover OmniAuth 2.0 requirements.
+    - Note that the upgrade required Devise shared links that initiate the OmniAuth flow to be changed to `method: :post`, which is now a requirement for OmniAuth, part of the security improvement. If you have copied and customized the Devise shared links partial to your app, or if you have other links in your app that initiate the OmniAuth flow, they will have to be updated to use `method: :post`, or changed to use buttons (e.g. `button_to`) to work with OmniAuth 2. (if you're using links with `method: :post`, make sure your app has `rails-ujs` or `jquery-ujs` included in order for these links to work properly.)
+    - As part of the OmniAuth 2.0 upgrade you might also need to add the [`omniauth-rails_csrf_protection`](https://github.com/cookpad/omniauth-rails_csrf_protection) gem to your app if you don't have it already. (and you don't want to roll your own code to verify requests.) Check the OmniAuth v2 release notes for more info.
   * Move CI to GitHub Actions.
 
 * deprecations

diff --git a/Gemfile b/Gemfile
@@ -5,7 +5,7 @@ source "https://rubygems.org"
 gemspec
 
 gem "rails", "~> 6.1.0"
-gem "omniauth", "~> 1.0"
+gem "omniauth"
 gem "omniauth-oauth2"
 gem "rdoc"
 

diff --git a/Gemfile.lock b/Gemfile.lock
@@ -130,23 +130,26 @@ GEM
       multi_json (~> 1.3)
       multi_xml (~> 0.5)
       rack (>= 1.2, < 3)
-    omniauth (1.9.1)
+    omniauth (2.0.1)
       hashie (>= 3.4.6)
       rack (>= 1.6.2, < 3)
+      rack-protection
     omniauth-facebook (8.0.0)
       omniauth-oauth2 (~> 1.2)
     omniauth-oauth2 (1.7.1)
       oauth2 (~> 1.4)
       omniauth (>= 1.9, < 3)
-    omniauth-openid (1.0.1)
-      omniauth (~> 1.0)
-      rack-openid (~> 1.3.1)
+    omniauth-openid (2.0.1)
+      omniauth (>= 1.0, < 3.0)
+      rack-openid (~> 1.4.0)
     orm_adapter (0.5.0)
     racc (1.5.2)
     rack (2.2.3)
-    rack-openid (1.3.1)
+    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)
     rails (6.1.1)
@@ -181,7 +184,7 @@ GEM
       actionpack (>= 5.0)
       railties (>= 5.0)
     ruby-openid (2.9.2)
-    ruby2_keywords (0.0.2)
+    ruby2_keywords (0.0.4)
     sprockets (4.0.2)
       concurrent-ruby (~> 1.0)
       rack (> 1, < 3)
@@ -212,7 +215,7 @@ DEPENDENCIES
   activemodel-serializers-xml!
   devise!
   mocha (~> 1.1)
-  omniauth (~> 1.0)
+  omniauth
   omniauth-facebook
   omniauth-oauth2
   omniauth-openid

diff --git a/app/views/devise/shared/_links.html.erb b/app/views/devise/shared/_links.html.erb
@@ -20,6 +20,6 @@
 
 <%- if devise_mapping.omniauthable? %>
   <%- resource_class.omniauth_providers.each do |provider| %>
-    <%= link_to "Sign in with #{OmniAuth::Utils.camelize(provider)}", omniauth_authorize_path(resource_name, provider) %><br />
+    <%= link_to "Sign in with #{OmniAuth::Utils.camelize(provider)}", omniauth_authorize_path(resource_name, provider), method: :post %><br />
   <% end %>
 <% end %>
diff --git a/gemfiles/Gemfile-rails-4-1 b/gemfiles/Gemfile-rails-4-1
@@ -5,7 +5,7 @@ source "https://rubygems.org"
 gemspec path: ".."
 
 gem "rails", github: "rails/rails", branch: "4-1-stable"
-gem "omniauth", "~> 1.0"
+gem "omniauth"
 gem "omniauth-oauth2"
 gem "rdoc", "~> 5.1"
 # Force this version because it's breaking on CI since a higher nokogiri version requires Ruby 2.3+.

diff --git a/gemfiles/Gemfile-rails-4-2 b/gemfiles/Gemfile-rails-4-2
@@ -5,7 +5,7 @@ source "https://rubygems.org"
 gemspec path: ".."
 
 gem "rails", github: "rails/rails", branch: "4-2-stable"
-gem "omniauth", "~> 1.0"
+gem "omniauth"
 gem "omniauth-oauth2"
 gem "rdoc", "~> 5.1"
 gem "nokogiri", "1.9.1"

diff --git a/gemfiles/Gemfile-rails-5-0 b/gemfiles/Gemfile-rails-5-0
@@ -5,7 +5,7 @@ source "https://rubygems.org"
 gemspec path: ".."
 
 gem "rails", '~> 5.0.0'
-gem "omniauth", "~> 1.0"
+gem "omniauth"
 gem "omniauth-oauth2"
 gem "rdoc"
 

diff --git a/gemfiles/Gemfile-rails-5-1 b/gemfiles/Gemfile-rails-5-1
@@ -3,7 +3,7 @@ source "https://rubygems.org"
 gemspec path: ".."
 
 gem "rails", '~> 5.1.0'
-gem "omniauth", "~> 1.0"
+gem "omniauth"
 gem "omniauth-oauth2"
 gem "rdoc"
 

diff --git a/gemfiles/Gemfile-rails-5-2 b/gemfiles/Gemfile-rails-5-2
@@ -3,7 +3,7 @@ source "https://rubygems.org"
 gemspec path: ".."
 
 gem "rails", '~> 5.2.0'
-gem "omniauth", "~> 1.0"
+gem "omniauth"
 gem "omniauth-oauth2"
 gem "rdoc"
 

diff --git a/gemfiles/Gemfile-rails-6-0 b/gemfiles/Gemfile-rails-6-0
@@ -3,7 +3,7 @@ source "https://rubygems.org"
 gemspec path: ".."
 
 gem "rails", '~> 6.0.0'
-gem "omniauth", "~> 1.0"
+gem "omniauth"
 gem "omniauth-oauth2"
 gem "rdoc"
 

diff --git a/lib/devise/omniauth.rb b/lib/devise/omniauth.rb
@@ -1,17 +1,14 @@
 # frozen_string_literal: true
 
 begin
+  gem "omniauth", ">= 1.0.0"
+
   require "omniauth"
-  require "omniauth/version"
 rescue LoadError
   warn "Could not load 'omniauth'. Please ensure you have the omniauth gem >= 1.0.0 installed and listed in your Gemfile."
   raise
 end
 
-unless OmniAuth::VERSION =~ /^1\./
-  raise "You are using an old OmniAuth version, please ensure you have 1.0.0.pr2 version or later installed."
-end
-
 # Clean up the default path_prefix. It will be automatically set by Devise.
 OmniAuth.config.path_prefix = nil
 

diff --git a/test/integration/omniauthable_test.rb b/test/integration/omniauthable_test.rb
@@ -23,6 +23,9 @@ class OmniauthableIntegrationTest < Devise::IntegrationTest
       "extra" => {"user_hash" => FACEBOOK_INFO}
     }
     OmniAuth.config.add_camelization 'facebook', 'FaceBook'
+    if OmniAuth.config.respond_to?(:request_validation_phase)
+      OmniAuth.config.request_validation_phase = ->(env) {}
+    end
   end
 
   teardown do
@@ -45,17 +48,17 @@ def stub_action!(name)
   test "omniauth sign in should not run model validations" do
     stub_action!(:sign_in_facebook) do
       create_user
-      visit "/users/sign_in"
-      click_link "Sign in with FaceBook"
+      post "/users/auth/facebook"
+      follow_redirect!
       assert warden.authenticated?(:user)
 
       refute User.validations_performed
     end
   end
 
   test "can access omniauth.auth in the env hash" do
-    visit "/users/sign_in"
-    click_link "Sign in with FaceBook"
+    post "/users/auth/facebook"
+    follow_redirect!
 
     json = ActiveSupport::JSON.decode(response.body)
 
@@ -68,8 +71,8 @@ def stub_action!(name)
 
   test "cleans up session on sign up" do
     assert_no_difference "User.count" do
-      visit "/users/sign_in"
-      click_link "Sign in with FaceBook"
+      post "/users/auth/facebook"
+      follow_redirect!
     end
 
     assert session["devise.facebook_data"]
@@ -89,8 +92,8 @@ def stub_action!(name)
 
   test "cleans up session on cancel" do
     assert_no_difference "User.count" do
-      visit "/users/sign_in"
-      click_link "Sign in with FaceBook"
+      post "/users/auth/facebook"
+      follow_redirect!
     end
 
     assert session["devise.facebook_data"]
@@ -100,8 +103,8 @@ def stub_action!(name)
 
   test "cleans up session on sign in" do
     assert_no_difference "User.count" do
-      visit "/users/sign_in"
-      click_link "Sign in with FaceBook"
+      post "/users/auth/facebook"
+      follow_redirect!
     end
 
     assert session["devise.facebook_data"]
@@ -110,23 +113,28 @@ def stub_action!(name)
   end
 
   test "sign in and send remember token if configured" do
-    visit "/users/sign_in"
-    click_link "Sign in with FaceBook"
+    post "/users/auth/facebook"
+    follow_redirect!
     assert_nil warden.cookies["remember_user_token"]
 
     stub_action!(:sign_in_facebook) do
       create_user
-      visit "/users/sign_in"
-      click_link "Sign in with FaceBook"
+      post "/users/auth/facebook"
+      follow_redirect!
       assert warden.authenticated?(:user)
       assert warden.cookies["remember_user_token"]
     end
   end
 
+  test "generates a link to authenticate with provider" do
+    visit "/users/sign_in"
+    assert_select "a[href=?][data-method='post']", "/users/auth/facebook", text: "Sign in with FaceBook"
+  end
+
   test "generates a proper link when SCRIPT_NAME is set" do
     header 'SCRIPT_NAME', '/q'
     visit "/users/sign_in"
-    assert_select "a", href: "/q/users/auth/facebook"
+    assert_select "a[href=?][data-method='post']", "/q/users/auth/facebook", text: "Sign in with FaceBook"
   end
 
   test "handles callback error parameter according to the specification" do
@@ -139,10 +147,10 @@ def stub_action!(name)
   test "handles other exceptions from OmniAuth" do
     OmniAuth.config.mock_auth[:facebook] = :invalid_credentials
 
-    visit "/users/sign_in"
-    click_link "Sign in with FaceBook"
+    post "/users/auth/facebook"
+    follow_redirect!
+    follow_redirect!
 
-    assert_current_url "/users/sign_in"
     assert_contain 'Could not authenticate you from FaceBook because "Invalid credentials".'
   end
 end