Merge branch 'dm-nested-groups-go-get' into 'master'

Fix go-get support for projects in nested groups See merge request !9793

Merge branch 'dm-nested-groups-go-get' into 'master'
Fix go-get support for projects in nested groups See merge request !9793
e53fa960 · Douwe Maan · ff9267a7 · 359f04e8 · e53fa960 · e53fa960
Commit e53fa960 authored Mar 08, 2017 by Douwe Maan
Show whitespace changes
Inline Side-by-side

Showing with 143 additions and 18 deletions

lib/gitlab/middleware/go.rb lib/gitlab/middleware/go.rb +57 -9

spec/lib/gitlab/middleware/go_spec.rb spec/lib/gitlab/middleware/go_spec.rb +86 -9

No files found.
--- 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)

-        "<!DOCTYPE html><html><head><meta content='#{domain_path} git #{request_url}.git' name='go-import'></head></html>\n"
+        "<!DOCTYPE html><html><head><meta content='#{import_prefix} git #{project_url}.git' name='go-import'></head></html>\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
--- 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' => '',
+      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' => '/group/project/path' }
-        resp = middleware.call(env)
-        expect(resp[0]).to eq(200)
-        expect(resp[1]['Content-Type']).to eq('text/html')
-        expected_body = "<!DOCTYPE html><html><head><meta content='#{Gitlab.config.gitlab.host}/group/project git http://#{Gitlab.config.gitlab.host}/group/project.git' name='go-import'></head></html>\n"
-        expect(resp[2].body).to eq([expected_body])
+        '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 = "<!DOCTYPE html><html><head><meta content='#{Gitlab.config.gitlab.host}/#{path} git http://#{Gitlab.config.gitlab.host}/#{path}.git' name='go-import'></head></html>\n"
+      expect(response[2].body).to eq([expected_body])
    end
  end
 end