Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve OmniAuth version check to allow anything from 1.0 forward #5327

Merged
merged 9 commits into from
Feb 1, 2021
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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
Expand Down
2 changes: 1 addition & 1 deletion Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down
17 changes: 10 additions & 7 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -212,7 +215,7 @@ DEPENDENCIES
activemodel-serializers-xml!
devise!
mocha (~> 1.1)
omniauth (~> 1.0)
omniauth
omniauth-facebook
omniauth-oauth2
omniauth-openid
Expand Down
2 changes: 1 addition & 1 deletion app/views/devise/shared/_links.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -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 %>
2 changes: 1 addition & 1 deletion gemfiles/Gemfile-rails-4-1
Original file line number Diff line number Diff line change
Expand Up @@ -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+.
Expand Down
2 changes: 1 addition & 1 deletion gemfiles/Gemfile-rails-4-2
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
2 changes: 1 addition & 1 deletion gemfiles/Gemfile-rails-5-0
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down
2 changes: 1 addition & 1 deletion gemfiles/Gemfile-rails-5-1
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down
2 changes: 1 addition & 1 deletion gemfiles/Gemfile-rails-5-2
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down
2 changes: 1 addition & 1 deletion gemfiles/Gemfile-rails-6-0
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down
7 changes: 2 additions & 5 deletions lib/devise/omniauth.rb
Original file line number Diff line number Diff line change
@@ -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

Expand Down
44 changes: 26 additions & 18 deletions test/integration/omniauthable_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)

Expand All @@ -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"]
Expand All @@ -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"]
Expand All @@ -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"]
Expand All @@ -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
Expand All @@ -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