Commit 71a65ca5 authored by Georges Racinet's avatar Georges Racinet 🦑
Browse files

Forwarding actual permissions to Mercurial processes

Closes #334

Normally, the abstraction layers in GitLab mean that the Project
should not be accessible from the low level repository. However,
we need it to pass it down to the `hg` processes. Doing it in
`hg_env_for_write` is the way to make it really robust, since all
write operations basically have to use it.

To mitigate somewhat this breach of abstraction, we make the
attribute writable only, and we give it a more specific name than
just '@project'. Aesthetically, it would be better to store only
a factory for the `UserAccess` instance that we really need, but
that could be done separately.

This required running some of `HgAccess` RSpec tests with empty
repositories, because in them, the project owner does not have enough
rights to create a first commit any more, and there's not even
an admin user around for that.

--HG--
branch : heptapod
parent 4fb9222b604b
Pipeline #9364 waiting for manual action with stage
......@@ -1229,11 +1229,15 @@ def repository_event(event, tags = {})
end
def initialize_raw_repository
Gitlab::VCS::repository_class(container.vcs_type).new(
repo = Gitlab::VCS::repository_class(container.vcs_type).new(
shard,
disk_path + '.git',
repo_type.identifier_for_container(container),
container.full_path)
if mercurial? && repo_type == Gitlab::GlRepository::PROJECT
repo.hg_project_for_perms = project
end
repo
end
end
......
......@@ -13,6 +13,7 @@ class HgGitRepository < Gitlab::Git::Repository
include Gitlab::Popen
attr_accessor :unit_tests_skip_hooks
attr_writer :hg_project_for_perms
MergeError = Class.new(StandardError)
HgError = Class.new(StandardError)
......@@ -53,6 +54,13 @@ def initialize(*args)
@hg_relative_path = relative_path.sub(/\.git$/, '.hg')
@hg_storage_root = Gitlab.config.repositories.storages.fetch(storage).hg_disk_path
@hgpath = File.join(@hg_storage_root, @hg_relative_path)
# `nil` is the language default for instance variables,
# but we want to be explicit in that case: it will mean no check
# in Mercurial process for write operations (besides what's been done
# at the Model level already, and is enough in the Git case).
# It should stay nil only for Wikis and similar.
@hg_project_for_perms = nil
end
# Mercurial path will stay until HGitaly is ready
......@@ -337,6 +345,15 @@ def hg_branchmap_branch(hg_branch, topic)
# Return a suitable environment for write operations (merge etc.)
def hg_env_for_write(user)
hg_perm_user =
if @hg_project_for_perms.nil?
nil
else
access = ::Gitlab::UserAccess.new(user,
project: @hg_project_for_perms)
access.hg_can_publish? ? 'heptapod-publish' : 'heptapod-write'
end
{
# `name` and `email` are as in `committer` (which
# does not have `username`) but the purpose here is
......@@ -357,6 +374,7 @@ def hg_env_for_write(user)
# TODO won't work on hashed storage
'HEPTAPOD_PROJECT_NAMESPACE_FULL_PATH' => @relative_path.rpartition('/')[0],
'HGUSER' => nil,
'REMOTE_USER' => hg_perm_user,
'HGRCPATH' => Gitlab::Mercurial.hgrc_path
}
end
......
......@@ -211,7 +211,7 @@
# Create a Mercurial repository, ensuring it's not empty
#
# Much simpler that :repository, which relies on copying over the
# Much simpler than :repository, which relies on copying over the
# gitlab-test testing repository and its structure.
trait :hg_repository do
after :create do |project, _evaluator|
......
......@@ -948,6 +948,12 @@ def self.run_permission_checks(permissions_matrix)
enforce_terms
end
# The :hg_repository factory trait cannot be used in that case,
# because it creates a public changeset, which requires publication
# permissions that are not granted until the terms are accepted.
# Fortunately, these examples are designed to work on an empty repo.
let(:project) { create(:hg_git_project, :empty_repo) }
shared_examples 'access after accepting terms' do
let(:actions) do
[-> { pull_access_check },
......@@ -971,13 +977,13 @@ def self.run_permission_checks(permissions_matrix)
describe 'as an anonymous user to a public project' do
let(:actor) { nil }
let(:project) { create(:hg_git_project, :public, :hg_repository) }
let(:project) { create(:hg_git_project, :public, :empty_repo) }
it { expect { pull_access_check }.not_to raise_error }
end
describe 'as a guest to a public project' do
let(:project) { create(:hg_git_project, :public, :hg_repository) }
let(:project) { create(:hg_git_project, :public, :empty_repo) }
it_behaves_like 'access after accepting terms' do
let(:actions) { [-> { pull_access_check }] }
......@@ -1011,7 +1017,7 @@ def self.run_permission_checks(permissions_matrix)
end
describe 'as an owner of the project' do
let(:project) { create(:hg_git_project, :hg_repository, namespace: user.namespace) }
let(:project) { create(:hg_git_project, :empty_repo, namespace: user.namespace) }
it_behaves_like 'access after accepting terms'
end
......
......@@ -14,6 +14,7 @@
end
let(:mutable_repository_path) { File.join(TestEnv.repos_path, mutable_repository.relative_path).chomp('.git') + '.hg' }
let(:user) { build(:user) }
let(:project) { create(:hg_git_project, :hg_repository) }
let(:non_empty_repository) do
repo = mutable_repository
......@@ -103,4 +104,59 @@
end.to raise_error(Gitlab::Mercurial::HgGitRepository::HgError)
end
end
describe "publish permission" do
# we don't need to add `user` to the project, because write
# authorization is entirely managed at higher level (Models).
# Should that change, then the test with topic creation would fail
subject { project.repository.raw_repository }
context "when hg_project_for_perms is provided" do
it "is enforced automatically" do
# we're using #multi_action for an easy check, but the point
# is that it happens without direct involvement of the method
expect do
subject.multi_action(user,
branch_name: 'branch/default',
start_branch_name: 'branch/default',
message: "Public commit on default",
author_email: "jdoe2@heptapod.test",
author_name: "John Doe2",
actions: [{ action: :create,
file_path: 'foo',
content: "Cannot publish that\n" }])
end.to raise_error(Gitlab::Mercurial::HgGitRepository::HgError)
end
it "allows creating draft changesets" do
expect do
subject.multi_action(user,
branch_name: 'topic/default/user',
start_branch_name: 'branch/default',
message: "Topical changeset",
author_email: "jdoe2@heptapod.test",
author_name: "John Doe2",
actions: [{ action: :create,
file_path: 'foo',
content: "Can commit that\n" }])
end.not_to raise_error
end
it "lets owner publish changesets" do
expect do
subject.multi_action(project.creator,
branch_name: 'branch/default',
start_branch_name: 'branch/default',
message: "New changeset",
author_email: "jdoe1@heptapod.test",
author_name: "John Doe1",
actions: [{ action: :create,
file_path: 'bar',
content: "Can publish\n" }])
end.not_to raise_error
end
end
end
end
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment