Merge branch 'bvl-fix-git-http-user-logging' into 'master'

Load user before logging git http-requests

Closes #223027

See merge request gitlab-org/gitlab!34923

app/controllers/repositories/git_http_client_controller.rb

@@ -18,8 +18,7 @@ module Repositories
     skip_around_action :set_session_storage
     skip_before_action :verify_authenticity_token
 
-    before_action :parse_repo_path
-    before_action :authenticate_user
+    prepend_before_action :authenticate_user, :parse_repo_path
 
     private
 

changelogs/unreleased/bvl-fix-git-http-user-logging.yml

+---
+title: Load user before logging git http-requests
+merge_request: 34923
+author:
+type: fixed

spec/controllers/repositories/git_http_controller_spec.rb

@@ -60,10 +60,21 @@ RSpec.describe Repositories::GitHttpController do
 
           get :info_refs, params: params
         end
+
+        include_context 'parsed logs' do
+          it 'adds user info to the logs' do
+            get :info_refs, params: params
+
+            expect(log_data).to include('username' => user.username,
+                                        'user_id' => user.id,
+                                        'meta.user' => user.username)
+          end
+        end
       end
 
       context 'with exceptions' do
         before do
+          allow(controller).to receive(:authenticate_user).and_return(true)
           allow(controller).to receive(:verify_workhorse_api!).and_return(true)
         end
 

spec/initializers/lograge_spec.rb

@@ -99,6 +99,8 @@ describe 'lograge', type: :request do
   end
 
   context 'with a log subscriber' do
+    include_context 'parsed logs'
+
     let(:subscriber) { Lograge::LogSubscribers::ActionController.new }
 
     let(:event) do
@@ -119,16 +121,6 @@ describe 'lograge', type: :request do
       )
     end
 
-    let(:log_output) { StringIO.new }
-    let(:logger) do
-      Logger.new(log_output).tap { |logger| logger.formatter = ->(_, _, _, msg) { msg } }
-    end
-    let(:log_data) { Gitlab::Json.parse(log_output.string) }
-
-    before do
-      Lograge.logger = logger
-    end
-
     describe 'with an exception' do
       let(:exception) { RuntimeError.new('bad request') }
       let(:backtrace) { caller }

spec/requests/jwt_controller_spec.rb

@@ -3,19 +3,14 @@
 require 'spec_helper'
 
 describe JwtController do
+  include_context 'parsed logs'
+
   let(:service) { double(execute: {}) }
   let(:service_class) { double(new: service) }
   let(:service_name) { 'test' }
   let(:parameters) { { service: service_name } }
-  let(:log_output) { StringIO.new }
-  let(:logger) do
-    Logger.new(log_output).tap { |logger| logger.formatter = ->(_, _, _, msg) { msg } }
-  end
-  let(:log_data) { Gitlab::Json.parse(log_output.string) }
 
   before do
-    Lograge.logger = logger
-
     stub_const('JwtController::SERVICES', service_name => service_class)
   end
 

spec/support/shared_contexts/controllers/logging_shared_context.rb

+# frozen_string_literal: true
+
+# This context replaces the logger and exposes the `log_data` variable for
+# inspection
+RSpec.shared_context 'parsed logs' do
+  let(:logger) do
+    Logger.new(log_output).tap { |logger| logger.formatter = ->(_, _, _, msg) { msg } }
+  end
+
+  let(:log_output) { StringIO.new }
+  let(:log_data) { Gitlab::Json.parse(log_output.string) }
+
+  around do |example|
+    initial_logger = Lograge.logger
+    Lograge.logger = logger
+
+    example.run
+
+    Lograge.logger = initial_logger
+  end
+end