Better handle errors that come from the actual app.

As a consequence of the changes that were merged in #689, errors
thrown by strategies that utilize other_phase (or more specifically
call_app!), would be caught by omniauth, causing headaches for folks
looking to have those errors handled by their application. This
should allow for errors that come from the app to pass through, while
passing errors that come from the authentication phases to the fail!
handler.
This commit is contained in:
Bobby McDonald 2021-02-12 12:17:04 -05:00
parent 0d533c3615
commit 6f4cdb08f4
No known key found for this signature in database
GPG Key ID: CAD931A49619329A
2 changed files with 37 additions and 0 deletions

View File

@ -193,6 +193,8 @@ module OmniAuth
return callback_call if on_callback_path?
return other_phase if respond_to?(:other_phase)
rescue StandardError => e
raise e if env.delete('omniauth.error.app')
return fail!(e.message, e)
end
@ -302,6 +304,8 @@ module OmniAuth
return mock_request_call if on_request_path? && OmniAuth.config.allowed_request_methods.include?(request.request_method.downcase.to_sym)
return mock_callback_call if on_callback_path?
rescue StandardError => e
raise e if env.delete('omniauth.error.app')
return fail!(e.message, e)
end
@ -462,6 +466,9 @@ module OmniAuth
def call_app!(env = @env)
@app.call(env)
rescue StandardError => e
env['omniauth.error.app'] = true
raise e
end
def full_host

View File

@ -629,6 +629,36 @@ describe OmniAuth::Strategy do
expect(strategy.callback_url).to eq('http://example.com/myapp/override/callback')
end
end
context 'error during call_app!' do
class OtherPhaseStrategy < ExampleStrategy
def other_phase
call_app!
end
end
class AppError < StandardError; end
let(:app) { ->(_env) { raise(AppError.new('Some error in the app!')) } }
let(:strategy) { OtherPhaseStrategy.new(app) }
it 'raises the application error' do
expect { strategy.call(make_env('/some/path')) }.to raise_error(AppError, 'Some error in the app!')
end
end
context 'error during auth phase' do
class SomeStrategy < ExampleStrategy
def request_phase
raise AuthError.new('Some error in authentication')
end
end
class AuthError < StandardError; end
let(:strategy) { SomeStrategy.new(app) }
it 'passes the error to fail!()' do
expect(strategy).to receive(:fail!).with('Some error in authentication', kind_of(AuthError))
strategy.call(make_env('/auth/test', props))
end
end
end