Commit e54d847e authored by Markus Koller's avatar Markus Koller

Update omniauth-oauth2 and remove error verification monkey-patch

The fix for this was submitted upstream at
https://github.com/omniauth/omniauth-oauth2/pull/144

Changelog: changed
parent ff568d09
...@@ -860,7 +860,7 @@ GEM ...@@ -860,7 +860,7 @@ GEM
omniauth-oauth (1.1.0) omniauth-oauth (1.1.0)
oauth oauth
omniauth (~> 1.0) omniauth (~> 1.0)
omniauth-oauth2 (1.7.1) omniauth-oauth2 (1.7.2)
oauth2 (~> 1.4) oauth2 (~> 1.4)
omniauth (>= 1.9, < 3) omniauth (>= 1.9, < 3)
omniauth-oauth2-generic (0.2.2) omniauth-oauth2-generic (0.2.2)
......
...@@ -10,31 +10,11 @@ ...@@ -10,31 +10,11 @@
module OmniAuth module OmniAuth
module Strategies module Strategies
class OAuth2 class OAuth2
alias_method :original_callback_phase, :callback_phase
def callback_phase def callback_phase
error = request.params["error_reason"].presence || request.params["error"].presence original_callback_phase
# Monkey patch #1: # Monkey patch #1:
#
# Swap the order of these conditions around so the `state` param is verified *first*,
# before using the error params returned by the provider.
#
# This avoids content spoofing attacks by crafting a URL with malicious messages,
# because the `state` param is only present in the session after a valid OAuth2 authentication flow.
if !options.provider_ignores_state && (request.params["state"].to_s.empty? || request.params["state"] != session.delete("omniauth.state"))
fail!(:csrf_detected, CallbackError.new(:csrf_detected, "CSRF detected"))
elsif error
fail!(error, CallbackError.new(request.params["error"], request.params["error_description"].presence || request.params["error_reason"].presence, request.params["error_uri"]))
else
self.access_token = build_access_token
self.access_token = access_token.refresh! if access_token.expired?
super
end
rescue ::OAuth2::Error, CallbackError => e
fail!(:invalid_credentials, e)
rescue ::Timeout::Error, ::Errno::ETIMEDOUT => e
fail!(:timeout, e)
rescue ::SocketError => e
fail!(:failed_to_connect, e)
# Monkey patch #2:
# #
# Also catch errors from Faraday. # Also catch errors from Faraday.
# See https://github.com/omniauth/omniauth-oauth2/pull/129 # See https://github.com/omniauth/omniauth-oauth2/pull/129
......
...@@ -2,12 +2,10 @@ ...@@ -2,12 +2,10 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe 'OmniAuth::Strategies::OAuth2', type: :strategy do RSpec.describe 'OmniAuth::Strategies::OAuth2' do
let(:strategy) { [OmniAuth::Strategies::OAuth2] }
it 'verifies the gem version' do it 'verifies the gem version' do
current_version = OmniAuth::OAuth2::VERSION current_version = OmniAuth::OAuth2::VERSION
expected_version = '1.7.1' expected_version = '1.7.2'
expect(current_version).to eq(expected_version), <<~EOF expect(current_version).to eq(expected_version), <<~EOF
New version #{current_version} of the `omniauth-oauth2` gem detected! New version #{current_version} of the `omniauth-oauth2` gem detected!
...@@ -18,39 +16,18 @@ RSpec.describe 'OmniAuth::Strategies::OAuth2', type: :strategy do ...@@ -18,39 +16,18 @@ RSpec.describe 'OmniAuth::Strategies::OAuth2', type: :strategy do
EOF EOF
end end
context 'when a custom error message is passed from an OAuth2 provider' do context 'when a Faraday exception is raised' do
let(:message) { 'Please go to https://evil.com' } where(exception: [Faraday::TimeoutError, Faraday::ConnectionFailed])
let(:state) { 'secret' }
let(:callback_path) { '/users/auth/oauth2/callback' }
let(:params) { { state: state, error: 'evil_key', error_description: message } }
let(:error) { last_request.env['omniauth.error'] }
before do
env('rack.session', { 'omniauth.state' => state })
end
it 'returns the custom error message if the state is valid' do
get callback_path, **params
expect(error.message).to eq("evil_key | #{message}")
end
it 'returns the custom `error_reason` message if the `error_description` is blank' do with_them do
get callback_path, **params.merge(error_description: ' ', error_reason: 'custom reason') it 'passes the exception to OmniAuth' do
instance = OmniAuth::Strategies::OAuth2.new(double)
expect(error.message).to eq('evil_key | custom reason')
end
it 'returns a CSRF error if the state is invalid' do
get callback_path, **params.merge(state: 'invalid')
expect(error.message).to eq('csrf_detected | CSRF detected')
end
it 'returns a CSRF error if the state is missing' do expect(instance).to receive(:original_callback_phase) { raise exception, 'message' }
get callback_path, **params.without(:state) expect(instance).to receive(:fail!).with(:timeout, kind_of(exception))
expect(error.message).to eq('csrf_detected | CSRF detected') instance.callback_phase
end
end end
end end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment