From 359f04e8a11e6714b9cc51df6c251f9c24eef264 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Tue, 7 Mar 2017 19:40:19 -0600 Subject: [PATCH] Fix go-get support for projects in nested groups --- lib/gitlab/middleware/go.rb | 66 ++++++++++++++++--- spec/lib/gitlab/middleware/go_spec.rb | 95 ++++++++++++++++++++++++--- 2 files changed, 143 insertions(+), 18 deletions(-) diff --git a/lib/gitlab/middleware/go.rb b/lib/gitlab/middleware/go.rb index 5764ab15652..6023fa1820f 100644 --- a/lib/gitlab/middleware/go.rb +++ b/lib/gitlab/middleware/go.rb @@ -30,21 +30,69 @@ module Gitlab end def go_body(request) - base_url = Gitlab.config.gitlab.url - # Go subpackages may be in the form of namespace/project/path1/path2/../pathN - # We can just ignore the paths and leave the namespace/project - path_info = request.env["PATH_INFO"] - path_info.sub!(/^\//, '') - project_path = path_info.split('/').first(2).join('/') - request_url = URI.join(base_url, project_path) - domain_path = strip_url(request_url.to_s) + project_url = URI.join(Gitlab.config.gitlab.url, project_path(request)) + import_prefix = strip_url(project_url.to_s) - "\n" + "\n" end def strip_url(url) url.gsub(/\Ahttps?:\/\//, '') end + + def project_path(request) + path_info = request.env["PATH_INFO"] + path_info.sub!(/^\//, '') + + # Go subpackages may be in the form of `namespace/project/path1/path2/../pathN`. + # In a traditional project with a single namespace, this would denote repo + # `namespace/project` with subpath `path1/path2/../pathN`, but with nested + # groups, this could also be `namespace/project/path1` with subpath + # `path2/../pathN`, for example. + + # We find all potential project paths out of the path segments + path_segments = path_info.split('/') + simple_project_path = path_segments.first(2).join('/') + + # If the path is at most 2 segments long, it is a simple `namespace/project` path and we're done + return simple_project_path if path_segments.length <= 2 + + project_paths = [] + begin + project_paths << path_segments.join('/') + path_segments.pop + end while path_segments.length >= 2 + + # We see if a project exists with any of these potential paths + project = project_for_paths(project_paths, request) + + if project + # If a project is found and the user has access, we return the full project path + project.full_path + else + # If not, we return the first two components as if it were a simple `namespace/project` path, + # so that we don't reveal the existence of a nested project the user doesn't have access to. + # This means that for an unauthenticated request to `group/subgroup/project/subpackage` + # for a private `group/subgroup/project` with subpackage path `subpackage`, GitLab will respond + # as if the user is looking for project `group/subgroup`, with subpackage path `project/subpackage`. + # Since `go get` doesn't authenticate by default, this means that + # `go get gitlab.com/group/subgroup/project/subpackage` will not work for private projects. + # `go get gitlab.com/group/subgroup/project.git/subpackage` will work, since Go is smart enough + # to figure that out. `import 'gitlab.com/...'` behaves the same as `go get`. + simple_project_path + end + end + + def project_for_paths(paths, request) + project = Project.where_full_path_in(paths).first + return unless Ability.allowed?(current_user(request), :read_project, project) + + project + end + + def current_user(request) + request.env['warden']&.authenticate + end end end end diff --git a/spec/lib/gitlab/middleware/go_spec.rb b/spec/lib/gitlab/middleware/go_spec.rb index fd3769d75b5..c2ab015d5cb 100644 --- a/spec/lib/gitlab/middleware/go_spec.rb +++ b/spec/lib/gitlab/middleware/go_spec.rb @@ -15,16 +15,93 @@ describe Gitlab::Middleware::Go, lib: true do end describe 'when go-get=1' do - it 'returns a document' do - env = { 'rack.input' => '', - 'QUERY_STRING' => 'go-get=1', - 'PATH_INFO' => '/group/project/path' } - resp = middleware.call(env) - expect(resp[0]).to eq(200) - expect(resp[1]['Content-Type']).to eq('text/html') - expected_body = "\n" - expect(resp[2].body).to eq([expected_body]) + let(:current_user) { nil } + + context 'with simple 2-segment project path' do + let!(:project) { create(:project, :private) } + + context 'with subpackages' do + let(:path) { "#{project.full_path}/subpackage" } + + it 'returns the full project path' do + expect_response_with_path(go, project.full_path) + end + end + + context 'without subpackages' do + let(:path) { project.full_path } + + it 'returns the full project path' do + expect_response_with_path(go, project.full_path) + end + end end + + context 'with a nested project path' do + let(:group) { create(:group, :nested) } + let!(:project) { create(:project, :public, namespace: group) } + + shared_examples 'a nested project' do + context 'when the project is public' do + it 'returns the full project path' do + expect_response_with_path(go, project.full_path) + end + end + + context 'when the project is private' do + before do + project.update_attribute(:visibility_level, Project::PRIVATE) + end + + context 'with access to the project' do + let(:current_user) { project.creator } + + before do + project.team.add_master(current_user) + end + + it 'returns the full project path' do + expect_response_with_path(go, project.full_path) + end + end + + context 'without access to the project' do + it 'returns the 2-segment group path' do + expect_response_with_path(go, group.full_path) + end + end + end + end + + context 'with subpackages' do + let(:path) { "#{project.full_path}/subpackage" } + + it_behaves_like 'a nested project' + end + + context 'without subpackages' do + let(:path) { project.full_path } + + it_behaves_like 'a nested project' + end + end + end + + def go + env = { + 'rack.input' => '', + 'QUERY_STRING' => 'go-get=1', + 'PATH_INFO' => "/#{path}", + 'warden' => double(authenticate: current_user) + } + middleware.call(env) + end + + def expect_response_with_path(response, path) + expect(response[0]).to eq(200) + expect(response[1]['Content-Type']).to eq('text/html') + expected_body = "\n" + expect(response[2].body).to eq([expected_body]) end end end