Merge branch 'sh-fix-api-error-handling' into 'master'

Fix 500 API errors with invalid access tokens

See merge request gitlab-org/gitlab!35895

changelogs/unreleased/sh-fix-api-error-handling.yml

+---
+title: Fix 500 errors with invalid access tokens
+merge_request: 35895
+author:
+type: fixed

lib/api/api_guard.rb

@@ -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

spec/requests/api/api_spec.rb

@@ -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