Commit 866e1518 authored by Stan Hu's avatar Stan Hu

Fix 500 API errors with invalid access tokens

When the API is called with a revoked or invalid token, the error
handler did not properly return a `Rack::Response`, causing Grape to
throw an error in the middleware. Instead of a 4xx error code, the
client would receive a 500 error.

We fix this by recreating the Rack::Response after the
`Rack::OAuth2::Server::Abstract::Error#finish` call. The `yield` is
intercepted by the superclass, so the API handler never got it.

Relates to https://gitlab.com/gitlab-com/gl-infra/production/-/issues/2363
parent d52619d7
---
title: Fix 500 errors with invalid access tokens
merge_request: 35895
author:
type: fixed
......@@ -153,16 +153,14 @@ module API
{ scope: e.scopes })
end
finished_response = nil
response.finish do |rack_response|
# Grape expects a Rack::Response
# (https://github.com/ruby-grape/grape/commit/c117bff7d22971675f4b34367d3a98bc31c8fc02),
# and we need to retrieve it here:
# https://github.com/nov/rack-oauth2/blob/40c9a99fd80486ccb8de0e4869ae384547c0d703/lib/rack/oauth2/server/abstract/error.rb#L28
finished_response = rack_response
end
finished_response
status, headers, body = response.finish
# Grape expects a Rack::Response
# (https://github.com/ruby-grape/grape/commit/c117bff7d22971675f4b34367d3a98bc31c8fc02),
# so we need to recreate the response again even though
# response.finish already does this.
# (https://github.com/nov/rack-oauth2/blob/40c9a99fd80486ccb8de0e4869ae384547c0d703/lib/rack/oauth2/server/abstract/error.rb#L26).
Rack::Response.new(body, status, headers)
end
end
end
......
......@@ -36,6 +36,14 @@ RSpec.describe API::API do
expect(response).to have_gitlab_http_status(:ok)
end
it 'does not authorize user for revoked token' do
revoked = create(:personal_access_token, :revoked, user: user, scopes: [:read_api])
get api('/groups', personal_access_token: revoked)
expect(response).to have_gitlab_http_status(:unauthorized)
end
it 'does not authorize user for post request' do
params = attributes_for_group_api
......
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