Skip to content

Fix 500 error on unauthenticated audit event

What does this MR do and why?

Adds attempts to find the user based on login / otp, to pass onto the UnauthenticatedSecurityEventAuditor class to ensure author is not blank, before falling back to 'unknown' as a placeholder.

Updates UnauthenticatedSecurityEventAuditor to use user_or_login as that's what we're passing, and check if for User model rather than String class.

References

Please include cross links to any resources that are relevant to this MR. This will give reviewers and future readers helpful context to give an efficient review of the changes introduced.

MR acceptance checklist

Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Screenshots or screen recordings

Screenshots are required for UI changes, and strongly recommended for all other merge requests.

Before After

How to set up and validate locally

  1. git checkout master
  2. touch ee/spec/lib/authn/unauthenticated_security_event_auditor_spec.rb
  3. Add this code:

# frozen_string_literal: true

require 'spec_helper'

RSpec.describe Authn::UnauthenticatedSecurityEventAuditor, feature_category: :audit_events do
  let(:user_model) { create(:user) }
  let(:user_string) { 'test_login' }

  describe '#execute' do
    context 'when author is blank (before fix)' do
      subject(:create_auditor) { described_class.new(nil) }

      before do
        allow_next_instance_of(Gitlab::Audit::Auditor) do |instance|
          allow(instance).to receive(:audit_enabled?)
                  .and_return(true)
        end
      end

      it 'raises MissingAttributeError' do
        expect { create_auditor.execute }.to raise_error(
          AuditEvents::BuildService::MissingAttributeError, "author"
        )
      end
    end
  end
end
  1. Run the spec file, and it should pass on master, as we are raising the error.
  2. Remove the changes and checkout this branch
  3. Add the same test into the file again (ee/spec/lib/authn/unauthenticated_security_event_auditor_spec.rb), and run the spec
  4. This time it should fail since it did not raise.
Edited by Andrew Jung

Merge request reports

Loading