diff --git a/changelogs/unreleased/remote_user.yml b/changelogs/unreleased/remote_user.yml new file mode 100644 index 00000000000..75a941fa95f --- /dev/null +++ b/changelogs/unreleased/remote_user.yml @@ -0,0 +1,4 @@ +--- +title: Add username as GL_USERNAME in hooks +merge_request: +author: diff --git a/lib/gitlab/git/hook.rb b/lib/gitlab/git/hook.rb index 208e4bbaf60..8ccb3e64636 100644 --- a/lib/gitlab/git/hook.rb +++ b/lib/gitlab/git/hook.rb @@ -22,22 +22,22 @@ module Gitlab File.exist?(path) end - def trigger(gl_id, oldrev, newrev, ref) + def trigger(gl_id, gl_username, oldrev, newrev, ref) return [true, nil] unless exists? Bundler.with_clean_env do case name when "pre-receive", "post-receive" - call_receive_hook(gl_id, oldrev, newrev, ref) + call_receive_hook(gl_id, gl_username, oldrev, newrev, ref) when "update" - call_update_hook(gl_id, oldrev, newrev, ref) + call_update_hook(gl_id, gl_username, oldrev, newrev, ref) end end end private - def call_receive_hook(gl_id, oldrev, newrev, ref) + def call_receive_hook(gl_id, gl_username, oldrev, newrev, ref) changes = [oldrev, newrev, ref].join(" ") exit_status = false @@ -45,6 +45,7 @@ module Gitlab vars = { 'GL_ID' => gl_id, + 'GL_USERNAME' => gl_username, 'PWD' => repo_path, 'GL_PROTOCOL' => GL_PROTOCOL, 'GL_REPOSITORY' => repository.gl_repository @@ -80,9 +81,14 @@ module Gitlab [exit_status, exit_message] end - def call_update_hook(gl_id, oldrev, newrev, ref) + def call_update_hook(gl_id, gl_username, oldrev, newrev, ref) Dir.chdir(repo_path) do - stdout, stderr, status = Open3.capture3({ 'GL_ID' => gl_id }, path, ref, oldrev, newrev) + stdout, stderr, status = Open3.capture3( + { + 'GL_ID' => gl_id, + 'GL_USERNAME' => gl_username + }, + path, ref, oldrev, newrev) [status.success?, (stderr.presence || stdout).gsub(/\R/, "
").html_safe] end end diff --git a/lib/gitlab/git/hooks_service.rb b/lib/gitlab/git/hooks_service.rb index ea8a87a1290..c327e9b1616 100644 --- a/lib/gitlab/git/hooks_service.rb +++ b/lib/gitlab/git/hooks_service.rb @@ -5,12 +5,13 @@ module Gitlab attr_accessor :oldrev, :newrev, :ref - def execute(committer, repository, oldrev, newrev, ref) - @repository = repository - @gl_id = committer.gl_id - @oldrev = oldrev - @newrev = newrev - @ref = ref + def execute(pusher, repository, oldrev, newrev, ref) + @repository = repository + @gl_id = pusher.gl_id + @gl_username = pusher.name + @oldrev = oldrev + @newrev = newrev + @ref = ref %w(pre-receive update).each do |hook_name| status, message = run_hook(hook_name) @@ -29,7 +30,7 @@ module Gitlab def run_hook(name) hook = Gitlab::Git::Hook.new(name, @repository) - hook.trigger(@gl_id, oldrev, newrev, ref) + hook.trigger(@gl_id, @gl_username, oldrev, newrev, ref) end end end diff --git a/lib/gitlab/git/user.rb b/lib/gitlab/git/user.rb index e142992f324..cb1af5f3b7c 100644 --- a/lib/gitlab/git/user.rb +++ b/lib/gitlab/git/user.rb @@ -1,20 +1,21 @@ module Gitlab module Git class User - attr_reader :name, :email, :gl_id + attr_reader :username, :name, :email, :gl_id def self.from_gitlab(gitlab_user) - new(gitlab_user.name, gitlab_user.email, Gitlab::GlId.gl_id(gitlab_user)) + new(gitlab_user.username, gitlab_user.name, gitlab_user.email, Gitlab::GlId.gl_id(gitlab_user)) end - def initialize(name, email, gl_id) + def initialize(username, name, email, gl_id) + @username = username @name = name @email = email @gl_id = gl_id end def ==(other) - [name, email, gl_id] == [other.name, other.email, other.gl_id] + [username, name, email, gl_id] == [other.username, other.name, other.email, other.gl_id] end end end diff --git a/spec/lib/gitlab/git/hook_spec.rb b/spec/lib/gitlab/git/hook_spec.rb index 0ff4f3bd105..2fe1f5603ce 100644 --- a/spec/lib/gitlab/git/hook_spec.rb +++ b/spec/lib/gitlab/git/hook_spec.rb @@ -14,6 +14,7 @@ describe Gitlab::Git::Hook do let(:repo_path) { repository.path } let(:user) { create(:user) } let(:gl_id) { Gitlab::GlId.gl_id(user) } + let(:gl_username) { user.username } def create_hook(name) FileUtils.mkdir_p(File.join(repo_path, 'hooks')) @@ -42,6 +43,7 @@ describe Gitlab::Git::Hook do let(:env) do { 'GL_ID' => gl_id, + 'GL_USERNAME' => gl_username, 'PWD' => repo_path, 'GL_PROTOCOL' => 'web', 'GL_REPOSITORY' => gl_repository @@ -59,7 +61,7 @@ describe Gitlab::Git::Hook do .with(env, hook_path, chdir: repo_path).and_call_original end - status, errors = hook.trigger(gl_id, blank, blank, ref) + status, errors = hook.trigger(gl_id, gl_username, blank, blank, ref) expect(status).to be true expect(errors).to be_blank end @@ -72,7 +74,7 @@ describe Gitlab::Git::Hook do blank = Gitlab::Git::BLANK_SHA ref = Gitlab::Git::BRANCH_REF_PREFIX + 'new_branch' - status, errors = hook.trigger(gl_id, blank, blank, ref) + status, errors = hook.trigger(gl_id, gl_username, blank, blank, ref) expect(status).to be false expect(errors).to eq("error message from the hook
error message from the hook line 2
") end @@ -86,7 +88,7 @@ describe Gitlab::Git::Hook do blank = Gitlab::Git::BLANK_SHA ref = Gitlab::Git::BRANCH_REF_PREFIX + 'new_branch' - status, errors = hook.trigger(gl_id, blank, blank, ref) + status, errors = hook.trigger(gl_id, gl_username, blank, blank, ref) expect(status).to be true expect(errors).to be_nil end diff --git a/spec/lib/gitlab/git/hooks_service_spec.rb b/spec/lib/gitlab/git/hooks_service_spec.rb index d4d75b66659..51e4e3fdad1 100644 --- a/spec/lib/gitlab/git/hooks_service_spec.rb +++ b/spec/lib/gitlab/git/hooks_service_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Gitlab::Git::HooksService, seed_helper: true do - let(:user) { Gitlab::Git::User.new('Jane Doe', 'janedoe@example.com', 'user-456') } + let(:user) { Gitlab::Git::User.new('janedoe', 'Jane Doe', 'janedoe@example.com', 'user-456') } let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, 'project-123') } let(:service) { described_class.new } diff --git a/spec/lib/gitlab/git/user_spec.rb b/spec/lib/gitlab/git/user_spec.rb index 0ebcecb26c0..ab64b041187 100644 --- a/spec/lib/gitlab/git/user_spec.rb +++ b/spec/lib/gitlab/git/user_spec.rb @@ -1,22 +1,24 @@ require 'spec_helper' describe Gitlab::Git::User do + let(:username) { 'janedo' } let(:name) { 'Jane Doe' } let(:email) { 'janedoe@example.com' } let(:gl_id) { 'user-123' } - subject { described_class.new(name, email, gl_id) } + subject { described_class.new(username, name, email, gl_id) } describe '#==' do - def eq_other(name, email, gl_id) - eq(described_class.new(name, email, gl_id)) + def eq_other(username, name, email, gl_id) + eq(described_class.new(username, name, email, gl_id)) end - it { expect(subject).to eq_other(name, email, gl_id) } + it { expect(subject).to eq_other(username, name, email, gl_id) } - it { expect(subject).not_to eq_other(nil, nil, nil) } - it { expect(subject).not_to eq_other(name + 'x', email, gl_id) } - it { expect(subject).not_to eq_other(name, email + 'x', gl_id) } - it { expect(subject).not_to eq_other(name, email, gl_id + 'x') } + it { expect(subject).not_to eq_other(nil, nil, nil, nil) } + it { expect(subject).not_to eq_other(username + 'x', name, email, gl_id) } + it { expect(subject).not_to eq_other(username, name + 'x', email, gl_id) } + it { expect(subject).not_to eq_other(username, name, email + 'x', gl_id) } + it { expect(subject).not_to eq_other(username, name, email, gl_id + 'x') } end end diff --git a/spec/lib/gitlab/workhorse_spec.rb b/spec/lib/gitlab/workhorse_spec.rb index 5708aa6754f..e56e2df8376 100644 --- a/spec/lib/gitlab/workhorse_spec.rb +++ b/spec/lib/gitlab/workhorse_spec.rb @@ -182,7 +182,12 @@ describe Gitlab::Workhorse do let(:repo_path) { repository.path_to_repo } let(:action) { 'info_refs' } let(:params) do - { GL_ID: "user-#{user.id}", GL_REPOSITORY: "project-#{project.id}", RepoPath: repo_path } + { + GL_ID: "user-#{user.id}", + GL_USERNAME: user.username, + GL_REPOSITORY: "project-#{project.id}", + RepoPath: repo_path + } end subject { described_class.git_http_ok(repository, false, user, action) } @@ -191,7 +196,12 @@ describe Gitlab::Workhorse do context 'when is_wiki' do let(:params) do - { GL_ID: "user-#{user.id}", GL_REPOSITORY: "wiki-#{project.id}", RepoPath: repo_path } + { + GL_ID: "user-#{user.id}", + GL_USERNAME: user.username, + GL_REPOSITORY: "wiki-#{project.id}", + RepoPath: repo_path + } end subject { described_class.git_http_ok(repository, true, user, action) } diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index ab81d39691b..869cf51f6a2 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -1679,11 +1679,11 @@ describe Repository do tag_sha = tag.target expect(pre_receive_hook).to have_received(:trigger) - .with(anything, anything, commit_sha, anything) + .with(anything, anything, anything, commit_sha, anything) expect(update_hook).to have_received(:trigger) - .with(anything, anything, commit_sha, anything) + .with(anything, anything, anything, commit_sha, anything) expect(post_receive_hook).to have_received(:trigger) - .with(anything, anything, tag_sha, anything) + .with(anything, anything, anything, tag_sha, anything) end end end