build_access_token raises un-rescued Faraday error that bubble to apps using shopify-app
ewalk153 opened this issue · 0 comments
ewalk153 commented
We expect that the underlying omniauth gem will reliably raise a connection errors with a fail
when we call build_access_token
:
def callback_phase # rubocop:disable AbcSize, CyclomaticComplexity, MethodLength, PerceivedComplexity
error = request.params["error_reason"] || request.params["error"]
if error
fail!(error, CallbackError.new(request.params["error"], request.params["error_description"] || request.params["error_reason"], request.params["error_uri"]))
elsif !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"))
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)
end
The underlying connection can close, which normally raises a Faraday ConnectionFailed error.
vendor/ruby-2.5.3/lib/ruby/2.5.0/net/protocol.rb:189:in `rbuf_fill': end of file reached (Faraday::ConnectionFailed)
from vendor/ruby-2.5.3/lib/ruby/2.5.0/net/protocol.rb:157:in `readuntil'
from vendor/ruby-2.5.3/lib/ruby/2.5.0/net/protocol.rb:167:in `readline'
from vendor/ruby-2.5.3/lib/ruby/2.5.0/net/http/response.rb:40:in `read_status_line'
from vendor/ruby-2.5.3/lib/ruby/2.5.0/net/http/response.rb:29:in `read_new'
[snip]
Recently, we saw some internal app fail to authenticate in a new fashion which stress a new failure mode to these request.
Options that I I see:
- We rescue in the
omniauth-shopify-oauth2
as the oauth code path inshopify-app
is merely asking for a token and shouldn't care about why it failed - We push this concern up to the superclass
omniauth-oauth2
to make sure Faraday errors are rescued - We let it bubble up to
shopify-app
and rescue it there
In any event, this should not reach Shopify apps without being first handled.