Support Git access for group wikis

This adds all the necessary routes and other behaviour to enable local
Git access to group wiki repositories.

To handle lookups of the wiki container in `Gitlab::RepoPath`, we
introduce a `Routable.find_by_full_path` helper to find routable objects
without having to know their class.

app/controllers/concerns/wiki_actions.rb

@@ -186,6 +186,10 @@ module WikiActions
   end
   # rubocop:enable Gitlab/ModuleWithInstanceVariables
 
+  def git_access
+    render 'shared/wikis/git_access'
+  end
+
   private
 
   def container

app/controllers/projects/wikis_controller.rb

@@ -6,7 +6,4 @@ class Projects::WikisController < Projects::ApplicationController
   alias_method :container, :project
 
   feature_category :wiki
-
-  def git_access
-  end
 end

app/models/concerns/case_sensitivity.rb

@@ -11,12 +11,14 @@ module CaseSensitivity
     def iwhere(params)
       criteria = self
 
-      params.each do |key, value|
+      params.each do |column, value|
+        column = arel_table[column] unless column.is_a?(Arel::Attribute)
+
         criteria = case value
                    when Array
-                     criteria.where(value_in(key, value))
+                     criteria.where(value_in(column, value))
                    else
-                     criteria.where(value_equal(key, value))
+                     criteria.where(value_equal(column, value))
                    end
       end
 
@@ -28,7 +30,7 @@ module CaseSensitivity
     def value_equal(column, value)
       lower_value = lower_value(value)
 
-      lower_column(arel_table[column]).eq(lower_value).to_sql
+      lower_column(column).eq(lower_value).to_sql
     end
 
     def value_in(column, values)
@@ -36,7 +38,7 @@ module CaseSensitivity
         lower_value(value)
       end
 
-      lower_column(arel_table[column]).in(lower_values).to_sql
+      lower_column(column).in(lower_values).to_sql
     end
 
     def lower_value(value)

app/models/concerns/routable.rb

@@ -4,6 +4,36 @@
 # Object must have name and path db fields and respond to parent and parent_changed? methods.
 module Routable
   extend ActiveSupport::Concern
+  include CaseSensitivity
+
+  # Finds a Routable object by its full path, without knowing the class.
+  #
+  # Usage:
+  #
+  #     Routable.find_by_full_path('groupname')             # -> Group
+  #     Routable.find_by_full_path('groupname/projectname') # -> Project
+  #
+  # Returns a single object, or nil.
+  def self.find_by_full_path(path, follow_redirects: false, route_scope: Route, redirect_route_scope: RedirectRoute)
+    return unless path.present?
+
+    # Case sensitive match first (it's cheaper and the usual case)
+    # If we didn't have an exact match, we perform a case insensitive search
+    #
+    # We need to qualify the columns with the table name, to support both direct lookups on
+    # Route/RedirectRoute, and scoped lookups through the Routable classes.
+    route =
+      route_scope.find_by(routes: { path: path }) ||
+      route_scope.iwhere(Route.arel_table[:path] => path).take
+
+    if follow_redirects
+      route ||= redirect_route_scope.iwhere(RedirectRoute.arel_table[:path] => path).take
+    end
+
+    return unless route
+
+    route.is_a?(Routable) ? route : route.source
+  end
 
   included do
     # Remove `inverse_of: source` when upgraded to rails 5.2
@@ -30,15 +60,14 @@ module Routable
     #
     # Returns a single object, or nil.
     def find_by_full_path(path, follow_redirects: false)
-      # Case sensitive match first (it's cheaper and the usual case)
-      # If we didn't have an exact match, we perform a case insensitive search
-      found = includes(:route).find_by(routes: { path: path }) || where_full_path_in([path]).take
-
-      return found if found
-
-      if follow_redirects
-        joins(:redirect_routes).find_by("LOWER(redirect_routes.path) = LOWER(?)", path)
-      end
+      # TODO: Optimize these queries by avoiding joins
+      # https://gitlab.com/gitlab-org/gitlab/-/issues/292252
+      Routable.find_by_full_path(
+        path,
+        follow_redirects: follow_redirects,
+        route_scope: includes(:route).references(:routes),
+        redirect_route_scope: joins(:redirect_routes)
+      )
     end
 
     # Builds a relation to find multiple objects by their full paths.

app/models/redirect_route.rb

 # frozen_string_literal: true
 
 class RedirectRoute < ApplicationRecord
+  include CaseSensitivity
+
   belongs_to :source, polymorphic: true # rubocop:disable Cop/PolymorphicAssociations
 
   validates :source, presence: true

app/models/route.rb

@@ -4,7 +4,7 @@ class Route < ApplicationRecord
   include CaseSensitivity
   include Gitlab::SQL::Pattern
 
-  belongs_to :source, polymorphic: true # rubocop:disable Cop/PolymorphicAssociations
+  belongs_to :source, polymorphic: true, inverse_of: :route # rubocop:disable Cop/PolymorphicAssociations
   validates :source, presence: true
 
   validates :path,

app/views/shared/wikis/_sidebar.html.haml

@@ -4,7 +4,6 @@
       %a.gutter-toggle.float-right.d-block.d-md-none.js-sidebar-wiki-toggle{ href: "#" }
         = sprite_icon('chevron-double-lg-right', css_class: 'gl-icon')
 
-      - if @wiki.container.is_a?(Project)
       - git_access_url = wiki_path(@wiki, action: :git_access)
       = link_to git_access_url, class: active_nav_link?(path: 'wikis#git_access') ? 'active' : '', data: { qa_selector: 'clone_repository_link' } do
         = sprite_icon('download', css_class: 'gl-mr-2')

changelogs/unreleased/207869-group-wikis-git-support.yml

+---
+title: Support Git access for group wikis
+merge_request: 45892
+author:
+type: added

config/routes/git_http.rb

@@ -9,7 +9,7 @@ scope(path: '*repository_path', format: false) do
       end
 
       # NOTE: LFS routes are exposed on all repository types, but we still check for
-      # LFS availability on the repository container in LfsRequest#require_lfs_enabled!
+      # LFS availability on the repository container in LfsRequest#lfs_check_access!
 
       # Git LFS API (metadata)
       scope(path: 'info/lfs/objects', controller: :lfs_api) do

doc/user/group/index.md

@@ -404,7 +404,7 @@ and above.
 
 There are a few limitations compared to project wikis:
 
-- Local Git access is not supported yet.
+- Git LFS is not supported.
 - Group wikis are not included in global search, group exports, backups, and Geo replication.
 - Changes to group wikis don't show up in the group's activity feed.
 - Group wikis [can't be moved](../../api/project_repository_storage_moves.md#limitations) using the project

ee/app/assets/javascripts/pages/groups/wikis/index.js

 import initWikis from '~/pages/shared/wikis';
+import initClonePanel from '~/clone_panel';
 
 initWikis();
+initClonePanel();

ee/spec/controllers/repositories/git_http_controller_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Repositories::GitHttpController do
+  context 'when repository container is a group wiki' do
+    include WikiHelpers
+
+    let_it_be(:group) { create(:group, :wiki_repo) }
+    let_it_be(:user) { create(:user) }
+
+    before_all do
+      group.add_owner(user)
+    end
+
+    before do
+      stub_group_wikis(true)
+    end
+
+    it_behaves_like Repositories::GitHttpController do
+      let(:container) { group.wiki }
+      let(:access_checker_class) { Gitlab::GitAccessWiki }
+    end
+  end
+end

ee/spec/features/groups/wikis_spec.rb

@@ -23,4 +23,5 @@ RSpec.describe 'Group wikis' do
   it_behaves_like 'User views a wiki page'
   it_behaves_like 'User views wiki pages'
   it_behaves_like 'User views wiki sidebar'
+  it_behaves_like 'User views Git access wiki page'
 end

ee/spec/requests/lfs_http_spec.rb

@@ -21,6 +21,27 @@ RSpec.describe 'Git LFS API and storage' do
   let(:sample_oid) { lfs_object.oid }
   let(:sample_size) { lfs_object.size }
 
+  context 'with group wikis' do
+    let_it_be(:group) { create(:group) }
+
+    # LFS is not supported on group wikis, so we override the shared examples
+    # to expect 404 responses instead.
+    [
+      'LFS http 200 response',
+      'LFS http 200 blob response',
+      'LFS http 403 response'
+    ].each do |examples|
+      shared_examples_for(examples) { it_behaves_like 'LFS http 404 response' }
+    end
+
+    it_behaves_like 'LFS http requests' do
+      let(:container) { create(:group_wiki, :empty_repo, group: group) }
+      let(:authorize_guest) { group.add_guest(user) }
+      let(:authorize_download) { group.add_reporter(user) }
+      let(:authorize_upload) { group.add_developer(user) }
+    end
+  end
+
   describe 'when handling lfs batch request' do
     let(:update_lfs_permissions) { }
     let(:update_user_permissions) { }

ee/spec/requests/repositories/git_http_controller_spec.rb

@@ -131,6 +131,24 @@ RSpec.describe Repositories::GitHttpController, type: :request do
         it_behaves_like 'triggers Geo'
       end
 
+      context 'with a group wiki' do
+        include WikiHelpers
+
+        let_it_be(:group) { create(:group, :wiki_repo) }
+        let_it_be(:user) { create(:user) }
+        let(:path) { "#{group.wiki.full_path}.git" }
+
+        before_all do
+          group.add_owner(user)
+        end
+
+        before do
+          stub_group_wikis(true)
+        end
+
+        it_behaves_like 'triggers Geo'
+      end
+
       context 'with a personal snippet' do
         let_it_be(:snippet) { create(:personal_snippet, :repository, author: user) }
         let_it_be(:path) { "snippets/#{snippet.id}.git" }

lib/gitlab/gl_repository.rb

@@ -20,6 +20,7 @@ module Gitlab
       end,
       container_class: ProjectWiki,
       project_resolver: -> (wiki) { wiki.try(:project) },
+      guest_read_ability: :download_wiki_code,
       suffix: :wiki
     ).freeze
     SNIPPET = RepoType.new(

lib/gitlab/repo_path.rb

@@ -4,6 +4,13 @@ module Gitlab
   module RepoPath
     NotFoundError = Class.new(StandardError)
 
+    # Returns an array containing:
+    # - The repository container
+    # - The related project (if available)
+    # - The repository type
+    # - The original container path (if redirected)
+    #
+    # @returns [HasRepository, Project, String, String]
     def self.parse(path)
       repo_path = path.delete_prefix('/').delete_suffix('.git')
       redirected_path = nil
@@ -30,7 +37,15 @@ module Gitlab
       [nil, nil, Gitlab::GlRepository.default_type, nil]
     end
 
+    # Returns an array containing:
+    # - The repository container
+    # - The related project (if available)
+    # - The original container path (if redirected)
+    #
+    # @returns [HasRepository, Project, String]
     def self.find_container(type, full_path)
+      return [nil, nil, nil] if full_path.blank?
+
       if type.snippet?
         snippet, redirected_path = find_snippet(full_path)
 
@@ -47,26 +62,24 @@ module Gitlab
     end
 
     def self.find_project(project_path)
-      return [nil, nil] if project_path.blank?
-
       project = Project.find_by_full_path(project_path, follow_redirects: true)
-      redirected_path = redirected?(project, project_path) ? project_path : nil
+      redirected_path = project_path if redirected?(project, project_path)
 
       [project, redirected_path]
     end
 
-    def self.redirected?(project, project_path)
-      project && project.full_path.casecmp(project_path) != 0
+    def self.redirected?(container, container_path)
+      container && container.full_path.casecmp(container_path) != 0
     end
 
     # Snippet_path can be either:
     # - snippets/1
     # - h5bp/html5-boilerplate/snippets/53
     def self.find_snippet(snippet_path)
-      return [nil, nil] if snippet_path.blank?
-
       snippet_id, project_path = extract_snippet_info(snippet_path)
-      project, redirected_path = find_project(project_path)
+      return [nil, nil] unless snippet_id
+
+      project, redirected_path = find_project(project_path) if project_path
 
       [Snippet.find_by_id_and_project(id: snippet_id, project: project), redirected_path]
     end
@@ -74,19 +87,23 @@ module Gitlab
     # Wiki path can be either:
     # - namespace/project
     # - group/subgroup/project
-    def self.find_wiki(wiki_path)
-      return [nil, nil] if wiki_path.blank?
-
-      project, redirected_path = find_project(wiki_path)
-
-      [project&.wiki, redirected_path]
+    #
+    # And also in EE:
+    # - group
+    # - group/subgroup
+    def self.find_wiki(container_path)
+      container = Routable.find_by_full_path(container_path, follow_redirects: true)
+      redirected_path = container_path if redirected?(container, container_path)
+
+      # In CE, Group#wiki is not available so this will return nil for a group path.
+      [container&.try(:wiki), redirected_path]
     end
 
     def self.extract_snippet_info(snippet_path)
       path_segments = snippet_path.split('/')
       snippet_id = path_segments.pop
-      path_segments.pop # Remove snippets from path
-      project_path = File.join(path_segments)
+      path_segments.pop # Remove 'snippets' from path
+      project_path = File.join(path_segments).presence
 
       [snippet_id, project_path]
     end

spec/controllers/repositories/git_http_controller_spec.rb

@@ -3,8 +3,6 @@
 require 'spec_helper'
 
 RSpec.describe Repositories::GitHttpController do
-  include GitHttpHelpers
-
   let_it_be(:project) { create(:project, :public, :repository) }
   let_it_be(:personal_snippet) { create(:personal_snippet, :public, :repository) }
   let_it_be(:project_snippet) { create(:project_snippet, :public, :repository, project: project) }

spec/models/concerns/case_sensitivity_spec.rb

@@ -4,16 +4,16 @@ require 'spec_helper'
 
 RSpec.describe CaseSensitivity do
   describe '.iwhere' do
-    let(:connection) { ActiveRecord::Base.connection }
-    let(:model) do
+    let_it_be(:connection) { ActiveRecord::Base.connection }
+    let_it_be(:model) do
       Class.new(ActiveRecord::Base) do
         include CaseSensitivity
         self.table_name = 'namespaces'
       end
     end
 
-    let!(:model_1) { model.create!(path: 'mOdEl-1', name: 'mOdEl 1') }
-    let!(:model_2) { model.create!(path: 'mOdEl-2', name: 'mOdEl 2') }
+    let_it_be(:model_1) { model.create!(path: 'mOdEl-1', name: 'mOdEl 1') }
+    let_it_be(:model_2) { model.create!(path: 'mOdEl-2', name: 'mOdEl 2') }
 
     it 'finds a single instance by a single attribute regardless of case' do
       expect(model.iwhere(path: 'MODEL-1')).to contain_exactly(model_1)
@@ -28,6 +28,10 @@ RSpec.describe CaseSensitivity do
         .to contain_exactly(model_1)
     end
 
+    it 'finds instances by custom Arel attributes' do
+      expect(model.iwhere(model.arel_table[:path] => 'MODEL-1')).to contain_exactly(model_1)
+    end
+
     it 'builds a query using LOWER' do
       query = model.iwhere(path: %w(MODEL-1 model-2), name: 'model 1').to_sql
       expected_query = <<~QRY.strip

spec/models/concerns/routable_spec.rb

@@ -2,8 +2,69 @@
 
 require 'spec_helper'
 
+RSpec.shared_examples '.find_by_full_path' do
+  describe '.find_by_full_path', :aggregate_failures do
+    it 'finds records by their full path' do
+      expect(described_class.find_by_full_path(record.full_path)).to eq(record)
+      expect(described_class.find_by_full_path(record.full_path.upcase)).to eq(record)
+    end
+
+    it 'returns nil for unknown paths' do
+      expect(described_class.find_by_full_path('unknown')).to be_nil
+    end
+
+    it 'includes route information when loading a record' do
+      control_count = ActiveRecord::QueryRecorder.new do
+        described_class.find_by_full_path(record.full_path)
+      end.count
+
+      expect do
+        described_class.find_by_full_path(record.full_path).route
+      end.not_to exceed_all_query_limit(control_count)
+    end
+
+    context 'with redirect routes' do
+      let_it_be(:redirect_route) { create(:redirect_route, source: record) }
+
+      context 'without follow_redirects option' do
+        it 'does not find records by their redirected path' do
+          expect(described_class.find_by_full_path(redirect_route.path)).to be_nil
+          expect(described_class.find_by_full_path(redirect_route.path.upcase)).to be_nil
+        end
+      end
+
+      context 'with follow_redirects option set to true' do
+        it 'finds records by their canonical path' do
+          expect(described_class.find_by_full_path(record.full_path, follow_redirects: true)).to eq(record)
+          expect(described_class.find_by_full_path(record.full_path.upcase, follow_redirects: true)).to eq(record)
+        end
+
+        it 'finds records by their redirected path' do
+          expect(described_class.find_by_full_path(redirect_route.path, follow_redirects: true)).to eq(record)
+          expect(described_class.find_by_full_path(redirect_route.path.upcase, follow_redirects: true)).to eq(record)
+        end
+
+        it 'returns nil for unknown paths' do
+          expect(described_class.find_by_full_path('unknown', follow_redirects: true)).to be_nil
+        end
+      end
+    end
+  end
+end
+
+RSpec.describe Routable do
+  it_behaves_like '.find_by_full_path' do
+    let_it_be(:record) { create(:group) }
+  end
+
+  it_behaves_like '.find_by_full_path' do
+    let_it_be(:record) { create(:project) }
+  end
+end
+
 RSpec.describe Group, 'Routable' do
-  let!(:group) { create(:group, name: 'foo') }
+  let_it_be_with_reload(:group) { create(:group, name: 'foo') }
+  let_it_be(:nested_group) { create(:group, parent: group) }
 
   describe 'Validations' do
     it { is_expected.to validate_presence_of(:route) }
@@ -59,61 +120,20 @@ RSpec.describe Group, 'Routable' do
   end
 
   describe '.find_by_full_path' do
-    let!(:nested_group) { create(:group, parent: group) }
-
-    context 'without any redirect routes' do
-      it { expect(described_class.find_by_full_path(group.to_param)).to eq(group) }
-      it { expect(described_class.find_by_full_path(group.to_param.upcase)).to eq(group) }
-      it { expect(described_class.find_by_full_path(nested_group.to_param)).to eq(nested_group) }
-      it { expect(described_class.find_by_full_path('unknown')).to eq(nil) }
-
-      it 'includes route information when loading a record' do
-        path = group.to_param
-        control_count = ActiveRecord::QueryRecorder.new { described_class.find_by_full_path(path) }.count
-
-        expect { described_class.find_by_full_path(path).route }.not_to exceed_all_query_limit(control_count)
-      end
-    end
-
-    context 'with redirect routes' do
-      let!(:group_redirect_route) { group.redirect_routes.create!(path: 'bar') }
-      let!(:nested_group_redirect_route) { nested_group.redirect_routes.create!(path: nested_group.path.sub('foo', 'bar')) }
-
-      context 'without follow_redirects option' do
-        context 'with the given path not matching any route' do
-          it { expect(described_class.find_by_full_path('unknown')).to eq(nil) }
-        end
-
-        context 'with the given path matching the canonical route' do
-          it { expect(described_class.find_by_full_path(group.to_param)).to eq(group) }
-          it { expect(described_class.find_by_full_path(group.to_param.upcase)).to eq(group) }
-          it { expect(described_class.find_by_full_path(nested_group.to_param)).to eq(nested_group) }
-        end
-
-        context 'with the given path matching a redirect route' do
-          it { expect(described_class.find_by_full_path(group_redirect_route.path)).to eq(nil) }
-          it { expect(described_class.find_by_full_path(group_redirect_route.path.upcase)).to eq(nil) }
-          it { expect(described_class.find_by_full_path(nested_group_redirect_route.path)).to eq(nil) }
-        end
+    it_behaves_like '.find_by_full_path' do
+      let_it_be(:record) { group }
     end
 
-      context 'with follow_redirects option set to true' do
-        context 'with the given path not matching any route' do
-          it { expect(described_class.find_by_full_path('unknown', follow_redirects: true)).to eq(nil) }
+    it_behaves_like '.find_by_full_path' do
+      let_it_be(:record) { nested_group }
     end
 
-        context 'with the given path matching the canonical route' do
-          it { expect(described_class.find_by_full_path(group.to_param, follow_redirects: true)).to eq(group) }
-          it { expect(described_class.find_by_full_path(group.to_param.upcase, follow_redirects: true)).to eq(group) }
-          it { expect(described_class.find_by_full_path(nested_group.to_param, follow_redirects: true)).to eq(nested_group) }
-        end
+    it 'does not find projects with a matching path' do
+      project = create(:project)
+      redirect_route = create(:redirect_route, source: project)
 
-        context 'with the given path matching a redirect route' do
-          it { expect(described_class.find_by_full_path(group_redirect_route.path, follow_redirects: true)).to eq(group) }
-          it { expect(described_class.find_by_full_path(group_redirect_route.path.upcase, follow_redirects: true)).to eq(group) }
-          it { expect(described_class.find_by_full_path(nested_group_redirect_route.path, follow_redirects: true)).to eq(nested_group) }
-        end
-      end
+      expect(described_class.find_by_full_path(project.full_path)).to be_nil
+      expect(described_class.find_by_full_path(redirect_route.path, follow_redirects: true)).to be_nil
     end
   end
 
@@ -131,8 +151,6 @@ RSpec.describe Group, 'Routable' do
     end
 
     context 'with valid paths' do
-      let!(:nested_group) { create(:group, parent: group) }
-
       it 'returns the projects matching the paths' do
         result = described_class.where_full_path_in([group.to_param, nested_group.to_param])
 
@@ -148,32 +166,36 @@ RSpec.describe Group, 'Routable' do
   end
 
   describe '#full_path' do
-    let(:group) { create(:group) }
-    let(:nested_group) { create(:group, parent: group) }
-
     it { expect(group.full_path).to eq(group.path) }
     it { expect(nested_group.full_path).to eq("#{group.full_path}/#{nested_group.path}") }
   end
 
   describe '#full_name' do
-    let(:group) { create(:group) }
-    let(:nested_group) { create(:group, parent: group) }
-
     it { expect(group.full_name).to eq(group.name) }
     it { expect(nested_group.full_name).to eq("#{group.name} / #{nested_group.name}") }
   end
 end
 
 RSpec.describe Project, 'Routable' do
-  describe '#full_path' do
-    let(:project) { build_stubbed(:project) }
+  let_it_be(:project) { create(:project) }
 
+  it_behaves_like '.find_by_full_path' do
+    let_it_be(:record) { project }
+  end
+
+  it 'does not find groups with a matching path' do
+    group = create(:group)
+    redirect_route = create(:redirect_route, source: group)
+
+    expect(described_class.find_by_full_path(group.full_path)).to be_nil
+    expect(described_class.find_by_full_path(redirect_route.path, follow_redirects: true)).to be_nil
+  end
+
+  describe '#full_path' do
     it { expect(project.full_path).to eq "#{project.namespace.full_path}/#{project.path}" }
   end
 
   describe '#full_name' do
-    let(:project) { build_stubbed(:project) }
-
     it { expect(project.full_name).to eq "#{project.namespace.human_name} / #{project.name}" }
   end
 end

spec/support/shared_examples/controllers/wiki_actions_shared_examples.rb

@@ -504,6 +504,17 @@ RSpec.shared_examples 'wiki controller actions' do
     end
   end
 
+  describe '#git_access' do
+    render_views
+
+    it 'renders the git access page' do
+      get :git_access, params: routing_params
+
+      expect(response).to render_template('shared/wikis/git_access')
+      expect(response.body).to include(wiki.http_url_to_repo)
+    end
+  end
+
   def redirect_to_wiki(wiki, page)
     redirect_to(controller.wiki_page_path(wiki, page))
   end

spec/views/shared/wikis/_sidebar.html.haml_spec.rb

@@ -16,16 +16,6 @@ RSpec.describe 'shared/wikis/_sidebar.html.haml' do
     expect(rendered).to have_link('Clone repository')
   end
 
-  context 'the wiki is not a project wiki' do
-    it 'does not include the clone repository link' do
-      allow(wiki).to receive(:container).and_return(create(:group))
-
-      render
-
-      expect(rendered).not_to have_link('Clone repository')
-    end
-  end
-
   context 'the sidebar failed to load' do
     before do
       assign(:sidebar_error, Object.new)