Ensure no exception is raised when Raven tries to get the current user in API context
Signed-off-by: Rémy Coutable <remy@rymai.me>
This commit is contained in:
		
							parent
							
								
									c49d19a5dc
								
							
						
					
					
						commit
						3040b994df
					
				|  | @ -0,0 +1,6 @@ | |||
| --- | ||||
| title: Ensure no exception is raised when Raven tries to get the current user in API | ||||
|   context | ||||
| merge_request: 14580 | ||||
| author: | ||||
| type: fixed | ||||
|  | @ -464,10 +464,12 @@ module API | |||
|       header(*Gitlab::Workhorse.send_artifacts_entry(build, entry)) | ||||
|     end | ||||
| 
 | ||||
|     # The Grape Error Middleware only has access to env but no params. We workaround this by | ||||
|     # defining a method that returns the right value. | ||||
|     # The Grape Error Middleware only has access to `env` but not `params` nor | ||||
|     # `request`. We workaround this by defining methods that returns the right | ||||
|     # values. | ||||
|     def define_params_for_grape_middleware | ||||
|       self.define_singleton_method(:params) { Rack::Request.new(env).params.symbolize_keys } | ||||
|       self.define_singleton_method(:request) { Rack::Request.new(env) } | ||||
|       self.define_singleton_method(:params) { request.params.symbolize_keys } | ||||
|     end | ||||
| 
 | ||||
|     # We could get a Grape or a standard Ruby exception. We should only report anything that | ||||
|  |  | |||
|  | @ -480,6 +480,27 @@ describe API::Helpers do | |||
| 
 | ||||
|       handle_api_exception(exception) | ||||
|     end | ||||
| 
 | ||||
|     context 'with a personal access token given' do | ||||
|       let(:token) { create(:personal_access_token, scopes: ['api'], user: user) } | ||||
| 
 | ||||
|       # Regression test for https://gitlab.com/gitlab-org/gitlab-ce/issues/38571 | ||||
|       it 'does not raise an additional exception because of missing `request`' do | ||||
|         # We need to stub at a lower level than #sentry_enabled? otherwise | ||||
|         # Sentry is not enabled when the request below is made, and the test | ||||
|         # would pass even without the fix | ||||
|         expect(Gitlab::Sentry).to receive(:enabled?).twice.and_return(true) | ||||
|         expect(ProjectsFinder).to receive(:new).and_raise('Runtime Error!') | ||||
| 
 | ||||
|         get api('/projects', personal_access_token: token) | ||||
| 
 | ||||
|         # The 500 status is expected as we're testing a case where an exception | ||||
|         # is raised, but Grape shouldn't raise an additional exception | ||||
|         expect(response).to have_gitlab_http_status(500) | ||||
|         expect(json_response['message']).not_to include("undefined local variable or method `request'") | ||||
|         expect(json_response['message']).to start_with("\nRuntimeError (Runtime Error!):") | ||||
|       end | ||||
|     end | ||||
|   end | ||||
| 
 | ||||
|   describe '.authenticate_non_get!' do | ||||
|  |  | |||
		Loading…
	
		Reference in New Issue