Commit abe3a8cf authored by Markus Koller's avatar Markus Koller Committed by Nick Thomas

Generalize repository routing

Previously we used both a `:namespace_id` and a `:repository_id` in the
repository routes, which originally made sense for project repositories,
but was confusing with personal snippets where `:namespace_id` was just
the `/snippets` prefix and not an actual namespace.

For group wiki repositories we also need to support toplevel groups,
where we only have a namespace path.

So we combine `:namespace_id` and `:repository_id` into a single
`:repository_path` route argument, and refactor the related controllers
and internal APIs accordingly.

The repository path is mainly used in `Gitlab::RepoPath` to determine
the container for the repository. In the `GitAccess` classes it's only
used in `GitAccessProject` when auto-creating project repositories,
where we need to know in which namespace to create the new project.
parent 4fc145c1
......@@ -87,8 +87,12 @@ module Repositories
@project
end
def repository_path
@repository_path ||= params[:repository_path]
end
def parse_repo_path
@container, @project, @repo_type, @redirected_path = Gitlab::RepoPath.parse("#{params[:namespace_id]}/#{params[:repository_id]}")
@container, @project, @repo_type, @redirected_path = Gitlab::RepoPath.parse(repository_path)
end
def render_missing_personal_access_token
......
......@@ -90,7 +90,6 @@ module Repositories
def access
@access ||= access_klass.new(access_actor, container, 'http',
authentication_abilities: authentication_abilities,
namespace_path: params[:namespace_id],
repository_path: repository_path,
redirected_path: redirected_path,
auth_result_type: auth_result_type)
......@@ -113,10 +112,6 @@ module Repositories
@access_klass ||= repo_type.access_checker_class
end
def repository_path
@repository_path ||= params[:repository_id].sub(/\.git$/, '')
end
def log_user_activity
Users::ActivityService.new(user).execute
end
......
concern :gitactionable do
scope(controller: :git_http) do
get '/info/refs', action: :info_refs
post '/git-upload-pack', action: :git_upload_pack
post '/git-receive-pack', action: :git_receive_pack
end
end
concern :lfsable do
# Git LFS API (metadata)
scope(path: 'info/lfs/objects', controller: :lfs_api) do
post :batch
post '/', action: :deprecated
get '/*oid', action: :deprecated
end
scope(path: 'info/lfs') do
resources :lfs_locks, controller: :lfs_locks_api, path: 'locks' do
post :unlock, on: :member
post :verify, on: :collection
end
end
# GitLab LFS object storage
scope(path: 'gitlab-lfs/objects/*oid', controller: :lfs_storage, constraints: { oid: /[a-f0-9]{64}/ }) do
get '/', action: :download
scope constraints: { size: /[0-9]+/ } do
put '/*size/authorize', action: :upload_authorize
put '/*size', action: :upload_finalize
end
end
end
# Git route for personal and project snippets
scope(path: ':namespace_id/:repository_id',
format: nil,
constraints: { namespace_id: Gitlab::PathRegex.personal_and_project_snippets_path_regex, repository_id: /\d+\.git/ },
module: :repositories) do
concerns :gitactionable
end
scope(path: '*namespace_id/:repository_id',
format: nil,
constraints: { namespace_id: Gitlab::PathRegex.full_namespace_route_regex }) do
scope(constraints: { repository_id: Gitlab::PathRegex.project_git_route_regex }) do
scope(path: '*repository_path', format: false) do
constraints(repository_path: Gitlab::PathRegex.repository_git_route_regex) do
scope(module: :repositories) do
concerns :gitactionable
concerns :lfsable
# Git HTTP API
scope(controller: :git_http) do
get '/info/refs', action: :info_refs
post '/git-upload-pack', action: :git_upload_pack
post '/git-receive-pack', action: :git_receive_pack
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!
# Git LFS API (metadata)
scope(path: 'info/lfs/objects', controller: :lfs_api) do
post :batch
post '/', action: :deprecated
get '/*oid', action: :deprecated
end
scope(path: 'info/lfs') do
resources :lfs_locks, controller: :lfs_locks_api, path: 'locks' do
post :unlock, on: :member
post :verify, on: :collection
end
end
# GitLab LFS object storage
scope(path: 'gitlab-lfs/objects/*oid', controller: :lfs_storage, constraints: { oid: /[a-f0-9]{64}/ }) do
get '/', action: :download
constraints(size: /[0-9]+/) do
put '/*size/authorize', action: :upload_authorize
put '/*size', action: :upload_finalize
end
end
end
end
# Redirect /group/project.wiki.git to the project wiki
scope(format: true, constraints: { repository_id: Gitlab::PathRegex.project_wiki_git_route_regex, format: :git }) do
constraints(repository_path: Gitlab::PathRegex.repository_wiki_git_route_regex) do
wiki_redirect = redirect do |params, request|
project_id = params[:repository_id].delete_suffix('.wiki')
path = [params[:namespace_id], project_id, 'wikis'].join('/')
container_path = params[:repository_path].delete_suffix('.wiki.git')
path = File.join(container_path, '-', 'wikis')
path << "?#{request.query_string}" unless request.query_string.blank?
path
end
......@@ -63,22 +50,14 @@ scope(path: '*namespace_id/:repository_id',
end
# Redirect /group/project/info/refs to /group/project.git/info/refs
scope(constraints: { repository_id: Gitlab::PathRegex.project_route_regex }) do
# Allow /info/refs, /info/refs?service=git-upload-pack, and
# /info/refs?service=git-receive-pack, but nothing else.
#
git_http_handshake = lambda do |request|
::Constraints::ProjectUrlConstrainer.new.matches?(request, existence_check: false) &&
(request.query_string.blank? ||
request.query_string.match(/\Aservice=git-(upload|receive)-pack\z/))
end
# This allows cloning a repository without the trailing `.git`
constraints(repository_path: Gitlab::PathRegex.repository_route_regex) do
ref_redirect = redirect do |params, request|
path = "#{params[:namespace_id]}/#{params[:repository_id]}.git/info/refs"
path = "#{params[:repository_path]}.git/info/refs"
path << "?#{request.query_string}" unless request.query_string.blank?
path
end
get '/info/refs', constraints: git_http_handshake, to: ref_redirect
get '/info/refs', constraints: ::Constraints::RepositoryRedirectUrlConstrainer.new, to: ref_redirect
end
end
......@@ -75,11 +75,7 @@ module EE
end
def jwt_scope_valid?
decoded_authorization[:scope] == repository_full_path
end
def repository_full_path
File.join(params[:namespace_id], repository_path)
decoded_authorization[:scope] == repository_path.delete_suffix('.git')
end
def decoded_authorization
......
......@@ -11,8 +11,7 @@ RSpec.describe Gitlab::GitAccess do
let(:actor) { user }
let(:project) { create(:project, :repository) }
let(:repository) { project.repository }
let(:namespace_path) { nil }
let(:project_path) { nil }
let(:repository_path) { "#{project.full_path}.git" }
let(:protocol) { 'web' }
let(:authentication_abilities) { %i[read_project download_code push_code] }
let(:redirected_path) { nil }
......@@ -408,8 +407,6 @@ RSpec.describe Gitlab::GitAccess do
end
describe '#check_push_access!' do
let(:project_path) { project.path }
let(:namespace_path) { project&.namespace&.path }
let(:protocol) { 'ssh' }
let(:unprotected_branch) { 'unprotected_branch' }
......@@ -771,8 +768,7 @@ RSpec.describe Gitlab::GitAccess do
project,
protocol,
authentication_abilities: authentication_abilities,
namespace_path: namespace_path,
repository_path: project_path,
repository_path: repository_path,
redirected_path: redirected_path
)
end
......
......@@ -97,16 +97,23 @@ RSpec.describe Repositories::GitHttpController, type: :request do
it_behaves_like 'triggers Geo'
end
context 'with personal snippet' do
context 'with a project wiki' do
let_it_be(:wiki) { create(:project_wiki, :empty_repo, project: project) }
let_it_be(:path) { "#{wiki.full_path}.git" }
it_behaves_like 'triggers Geo'
end
context 'with a personal snippet' do
let_it_be(:snippet) { create(:personal_snippet, :repository, author: user) }
let(:path) { "snippets/#{snippet.id}.git" }
let_it_be(:path) { "snippets/#{snippet.id}.git" }
it_behaves_like 'triggers Geo'
end
context 'with project snippet' do
context 'with a project snippet' do
let_it_be(:snippet) { create(:project_snippet, :repository, author: user, project: project) }
let(:path) { "#{project.full_path}/snippets/#{snippet.id}.git" }
let_it_be(:path) { "#{project.full_path}/snippets/#{snippet.id}.git" }
it_behaves_like 'triggers Geo'
end
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe 'EE git_http routing' do
describe 'Geo routing' do
it_behaves_like 'git repository routes' do
let(:path) { '/-/push_from_secondary/node/gitlab-org/gitlab-test.git' }
let(:container_path) { '/gitlab-org/gitlab-test' }
let(:params) { { geo_node_id: 'node', repository_path: 'gitlab-org/gitlab-test.git' } }
end
end
end
......@@ -31,8 +31,7 @@ module API
def access_checker_for(actor, protocol)
access_checker_klass.new(actor.key_or_user, container, protocol,
authentication_abilities: ssh_authentication_abilities,
namespace_path: namespace_path,
repository_path: project_path,
repository_path: repository_path,
redirected_path: redirected_path)
end
......@@ -71,18 +70,22 @@ module API
false
end
def project_path
project&.path || project_path_match[:project_path]
end
def namespace_path
project&.namespace&.full_path || project_path_match[:namespace_path]
end
private
def project_path_match
@project_path_match ||= params[:project].match(Gitlab::PathRegex.full_project_git_path_regex) || {}
def repository_path
if container
"#{container.full_path}.git"
elsif params[:project]
# When the project doesn't exist, we still need to pass on the path
# to support auto-creation in `GitAccessProject`.
#
# For consistency with the Git HTTP controllers, we normalize the path
# to remove a leading slash and ensure a trailing `.git`.
#
# NOTE: For GitLab Shell, `params[:project]` is the full repository path
# from the SSH command, with an optional trailing `.git`.
"#{params[:project].delete_prefix('/').delete_suffix('.git')}.git"
end
end
# rubocop:disable Gitlab/ModuleWithInstanceVariables
......@@ -96,7 +99,7 @@ module API
end
# rubocop:enable Gitlab/ModuleWithInstanceVariables
# Project id to pass between components that don't share/don't have
# Repository id to pass between components that don't share/don't have
# access to the same filesystem mounts
def gl_repository
repo_type.identifier_for_container(container)
......@@ -106,8 +109,9 @@ module API
repository.full_path
end
# Return the repository depending on whether we want the wiki or the
# regular repository
# Return the repository for the detected type and container
#
# @returns [Repository]
def repository
@repository ||= repo_type.repository_for(container)
end
......
......@@ -4,7 +4,7 @@ module Constraints
class ProjectUrlConstrainer
def matches?(request, existence_check: true)
namespace_path = request.params[:namespace_id]
project_path = request.params[:project_id] || request.params[:id] || request.params[:repository_id]
project_path = request.params[:project_id] || request.params[:id]
full_path = [namespace_path, project_path].join('/')
return false unless ProjectPathValidator.valid_path?(full_path)
......
# frozen_string_literal: true
module Constraints
class RepositoryRedirectUrlConstrainer
def matches?(request)
path = request.params[:repository_path].delete_suffix('.git')
query = request.query_string
git_request?(query) && container_path?(path)
end
# Allow /info/refs, /info/refs?service=git-upload-pack, and
# /info/refs?service=git-receive-pack, but nothing else.
def git_request?(query)
query.blank? ||
query == 'service=git-upload-pack' ||
query == 'service=git-receive-pack'
end
# Check if the path matches any known repository containers.
# These also cover wikis, since a `.wiki` suffix is valid in project/group paths too.
def container_path?(path)
NamespacePathValidator.valid_path?(path) ||
ProjectPathValidator.valid_path?(path) ||
path =~ Gitlab::PathRegex.full_snippets_repository_path_regex
end
end
end
......@@ -43,7 +43,7 @@ module Gitlab
ALL_COMMANDS = DOWNLOAD_COMMANDS + PUSH_COMMANDS
attr_reader :actor, :protocol, :authentication_abilities,
:namespace_path, :redirected_path, :auth_result_type,
:repository_path, :redirected_path, :auth_result_type,
:cmd, :changes
attr_accessor :container
......@@ -57,21 +57,16 @@ module Gitlab
raise ArgumentError, "No error message defined for #{key}"
end
def initialize(actor, container, protocol, authentication_abilities:, namespace_path: nil, repository_path: nil, redirected_path: nil, auth_result_type: nil)
def initialize(actor, container, protocol, authentication_abilities:, repository_path: nil, redirected_path: nil, auth_result_type: nil)
@actor = actor
@container = container
@protocol = protocol
@authentication_abilities = Array(authentication_abilities)
@namespace_path = namespace_path
@repository_path = repository_path
@redirected_path = redirected_path
@auth_result_type = auth_result_type
end
def repository_path
@repository_path ||= project&.path
end
def check(cmd, changes)
@changes = changes
@cmd = cmd
......
......@@ -35,7 +35,19 @@ module Gitlab
end
def namespace
@namespace ||= Namespace.find_by_full_path(namespace_path)
strong_memoize(:namespace) { Namespace.find_by_full_path(namespace_path) }
end
def namespace_path
strong_memoize(:namespace_path) { repository_path_match[:namespace_path] }
end
def project_path
strong_memoize(:project_path) { repository_path_match[:project_path] }
end
def repository_path_match
strong_memoize(:repository_path_match) { repository_path.match(Gitlab::PathRegex.full_project_git_path_regex) || {} }
end
def ensure_project_on_push!
......@@ -44,7 +56,7 @@ module Gitlab
return unless user&.can?(:create_projects, namespace)
project_params = {
path: repository_path,
path: project_path,
namespace_id: namespace.id,
visibility_level: Gitlab::VisibilityLevel::PRIVATE
}
......
......@@ -180,12 +180,16 @@ module Gitlab
end
end
def project_git_route_regex
@project_git_route_regex ||= /#{project_route_regex}\.git/.freeze
def repository_route_regex
@repository_route_regex ||= /#{full_namespace_route_regex}|#{personal_snippet_repository_path_regex}/.freeze
end
def project_wiki_git_route_regex
@project_wiki_git_route_regex ||= /#{PATH_REGEX_STR}\.wiki/.freeze
def repository_git_route_regex
@repository_git_route_regex ||= /#{repository_route_regex}\.git/.freeze
end
def repository_wiki_git_route_regex
@repository_wiki_git_route_regex ||= /#{full_namespace_route_regex}\.wiki\.git/.freeze
end
def full_namespace_path_regex
......@@ -250,10 +254,6 @@ module Gitlab
%r{\A(#{personal_snippet_repository_path_regex}|#{project_snippet_repository_path_regex})\z}
end
def personal_and_project_snippets_path_regex
%r{#{personal_snippet_path_regex}|#{project_snippet_path_regex}}
end
def container_image_regex
@container_image_regex ||= %r{([\w\.-]+\/){0,1}[\w\.-]+}.freeze
end
......
......@@ -5,7 +5,7 @@ module Gitlab
NotFoundError = Class.new(StandardError)
def self.parse(path)
repo_path = path.sub(/\.git\z/, '').sub(%r{\A/}, '')
repo_path = path.delete_prefix('/').delete_suffix('.git')
redirected_path = nil
# Detect the repo type based on the path, the first one tried is the project
......
......@@ -9,28 +9,20 @@ RSpec.describe Repositories::GitHttpController do
let_it_be(:personal_snippet) { create(:personal_snippet, :public, :repository) }
let_it_be(:project_snippet) { create(:project_snippet, :public, :repository, project: project) }
let(:namespace_id) { project.namespace.to_param }
let(:repository_id) { project.path + '.git' }
let(:container_params) do
{
namespace_id: namespace_id,
repository_id: repository_id
}
end
let(:params) { container_params }
shared_examples Repositories::GitHttpController do
let(:repository_path) { "#{container.full_path}.git" }
let(:params) { { repository_path: repository_path } }
describe 'HEAD #info_refs' do
it 'returns 403' do
head :info_refs, params: params
describe 'HEAD #info_refs' do
it 'returns 403' do
head :info_refs, params: params
expect(response).to have_gitlab_http_status(:forbidden)
expect(response).to have_gitlab_http_status(:forbidden)
end
end
end
shared_examples 'info_refs behavior' do
describe 'GET #info_refs' do
let(:params) { container_params.merge(service: 'git-upload-pack') }
let(:params) { super().merge(service: 'git-upload-pack') }
it 'returns 401 for unauthenticated requests to public repositories when http protocol is disabled' do
stub_application_setting(enabled_git_access_protocol: 'ssh')
......@@ -43,6 +35,26 @@ RSpec.describe Repositories::GitHttpController do
expect(response).to have_gitlab_http_status(:unauthorized)
end
it 'calls the right access checker class with the right object' do
allow(controller).to receive(:verify_workhorse_api!).and_return(true)
access_double = double
options = {
authentication_abilities: [:download_code],
repository_path: repository_path,
redirected_path: nil,
auth_result_type: :none
}
expect(access_checker_class).to receive(:new)
.with(nil, container, 'http', hash_including(options))
.and_return(access_double)
allow(access_double).to receive(:check).and_return(false)
get :info_refs, params: params
end
context 'with authorized user' do
before do
request.headers.merge! auth_env(user.username, user.password, nil)
......@@ -97,125 +109,89 @@ RSpec.describe Repositories::GitHttpController do
end
end
end
end
shared_examples 'git_upload_pack behavior' do |expected|
describe 'POST #git_upload_pack' do
before do
allow(controller).to receive(:authenticate_user).and_return(true)
allow(controller).to receive(:verify_workhorse_api!).and_return(true)
allow(controller).to receive(:access_check).and_return(nil)
end
def send_request
it 'returns 200' do
post :git_upload_pack, params: params
expect(response).to have_gitlab_http_status(:ok)
end
end
end
context 'when repository container is a project' do
it_behaves_like Repositories::GitHttpController do
let(:container) { project }
let(:user) { project.owner }
let(:access_checker_class) { Gitlab::GitAccess }
context 'on a read-only instance' do
describe 'POST #git_upload_pack' do
before do
allow(Gitlab::Database).to receive(:read_only?).and_return(true)
allow(controller).to receive(:verify_workhorse_api!).and_return(true)
end
it 'does not update project statistics' do
expect(ProjectDailyStatisticsWorker).not_to receive(:perform_async)
send_request
def send_request
post :git_upload_pack, params: params
end
end
if expected
context 'when project_statistics_sync feature flag is disabled' do
context 'on a read-only instance' do
before do
stub_feature_flags(project_statistics_sync: false)
allow(Gitlab::Database).to receive(:read_only?).and_return(true)
end
it 'updates project statistics async' do
expect(ProjectDailyStatisticsWorker).to receive(:perform_async)
it 'does not update project statistics' do
expect(ProjectDailyStatisticsWorker).not_to receive(:perform_async)
send_request
end
end
it 'updates project statistics sync' do
expect { send_request }.to change {
Projects::DailyStatisticsFinder.new(project).total_fetch_count
}.from(0).to(1)
end
else
context 'when project_statistics_sync feature flag is disabled' do
before do
stub_feature_flags(project_statistics_sync: false)
end
it 'does not update project statistics' do
expect(ProjectDailyStatisticsWorker).not_to receive(:perform_async)
it 'updates project statistics async for projects' do
expect(ProjectDailyStatisticsWorker).to receive(:perform_async)
send_request
end
end
it 'does not update project statistics' do
expect { send_request }.not_to change {
Projects::DailyStatisticsFinder.new(project).total_fetch_count
}.from(0)
it 'updates project statistics sync for projects' do
expect { send_request }.to change {
Projects::DailyStatisticsFinder.new(container).total_fetch_count
}.from(0).to(1)
end
end
end
end
shared_examples 'access checker class' do
let(:params) { container_params.merge(service: 'git-upload-pack') }
it 'calls the right access class checker with the right object' do
allow(controller).to receive(:verify_workhorse_api!).and_return(true)
access_double = double
expect(expected_class).to receive(:new).with(anything, expected_object, 'http', anything).and_return(access_double)
allow(access_double).to receive(:check).and_return(false)
get :info_refs, params: params
end
end
context 'when repository container is a project' do
it_behaves_like 'info_refs behavior' do
context 'when repository container is a project wiki' do
it_behaves_like Repositories::GitHttpController do
let(:container) { create(:project_wiki, :empty_repo, project: project) }
let(:user) { project.owner }
end
it_behaves_like 'git_upload_pack behavior', true
it_behaves_like 'access checker class' do
let(:expected_class) { Gitlab::GitAccess }
let(:expected_object) { project }
let(:access_checker_class) { Gitlab::GitAccessWiki }
end
end
context 'when repository container is a personal snippet' do
let(:namespace_id) { 'snippets' }
let(:repository_id) { personal_snippet.to_param + '.git' }
it_behaves_like 'info_refs behavior' do
it_behaves_like Repositories::GitHttpController do
let(:container) { personal_snippet }
let(:user) { personal_snippet.author }
end
it_behaves_like 'git_upload_pack behavior', false
it_behaves_like 'access checker class' do
let(:expected_class) { Gitlab::GitAccessSnippet }
let(:expected_object) { personal_snippet }
let(:access_checker_class) { Gitlab::GitAccessSnippet }
end
end
context 'when repository container is a project snippet' do
let(:namespace_id) { project.full_path + '/snippets' }
let(:repository_id) { project_snippet.to_param + '.git' }
it_behaves_like 'info_refs behavior' do
it_behaves_like Repositories::GitHttpController do
let(:container) { project_snippet }
let(:user) { project_snippet.author }
end
it_behaves_like 'git_upload_pack behavior', false
it_behaves_like 'access checker class' do
let(:expected_class) { Gitlab::GitAccessSnippet }
let(:expected_object) { project_snippet }
let(:access_checker_class) { Gitlab::GitAccessSnippet }
end
end
end
......@@ -23,8 +23,7 @@ RSpec.describe Repositories::LfsStorageController do
let(:params) do
{
namespace_id: project.namespace.path,
repository_id: "#{project.path}.git",
repository_path: "#{project.full_path}.git",
oid: '6b9765d3888aaec789e8c309eb05b05c3a87895d6ad70d2264bd7270fff665ac',
size: '6725030'
}
......
......@@ -13,5 +13,6 @@ FactoryBot.define do
sequence(:past_time) { |n| 4.hours.ago + (2 * n).seconds }
sequence(:iid)
sequence(:sha) { |n| Digest::SHA1.hexdigest("commit-like-#{n}") }
sequence(:oid) { |n| Digest::SHA2.hexdigest("oid-like-#{n}") }
sequence(:variable) { |n| "var#{n}" }
end
......@@ -9,6 +9,7 @@ RSpec.describe Gitlab::GitAccessProject do
let(:actor) { user }
let(:project_path) { project.path }
let(:namespace_path) { project&.namespace&.path }
let(:repository_path) { "#{namespace_path}/#{project_path}.git" }
let(:protocol) { 'ssh' }
let(:authentication_abilities) { %i[read_project download_code push_code] }
let(:changes) { Gitlab::GitAccess::ANY }
......@@ -17,7 +18,7 @@ RSpec.describe Gitlab::GitAccessProject do
let(:access) do
described_class.new(actor, container, protocol,
authentication_abilities: authentication_abilities,
repository_path: project_path, namespace_path: namespace_path)
repository_path: repository_path)
end
describe '#check_namespace!' do
......@@ -103,6 +104,20 @@ RSpec.describe Gitlab::GitAccessProject do
end
end
context 'when namespace is blank' do
let(:repository_path) { 'project.git' }
it_behaves_like 'no project is created' do
let(:raise_specific_error) { raise_namespace_not_found }
end
end
context 'when namespace does not exist' do
let(:namespace_path) { 'unknown' }
it_behaves_like 'no project is created'
end
context 'when user cannot create project in namespace' do
let(:user2) { create(:user) }
let(:namespace_path) { user2.namespace.path }
......
......@@ -10,8 +10,7 @@ RSpec.describe Gitlab::GitAccess do
let(:actor) { user }
let(:project) { create(:project, :repository) }
let(:project_path) { project&.path }
let(:namespace_path) { project&.namespace&.path }
let(:repository_path) { "#{project.full_path}.git" }
let(:protocol) { 'ssh' }
let(:authentication_abilities) { %i[read_project download_code push_code] }
let(:redirected_path) { nil }
......@@ -210,10 +209,9 @@ RSpec.describe Gitlab::GitAccess do
end
end
context 'when the project is nil' do
context 'when the project does not exist' do
let(:project) { nil }
let(:project_path) { "new-project" }
let(:namespace_path) { user.namespace.path }
let(:repository_path) { "#{user.namespace.path}/new-project.git" }
it 'blocks push and pull with "not found"' do
aggregate_failures do
......@@ -452,9 +450,8 @@ RSpec.describe Gitlab::GitAccess do
context 'when project is public' do
let(:public_project) { create(:project, :public, :repository) }
let(:project_path) { public_project.path }
let(:namespace_path) { public_project.namespace.path }
let(:access) { access_class.new(nil, public_project, 'web', authentication_abilities: [:download_code], repository_path: project_path, namespace_path: namespace_path) }
let(:repository_path) { "#{public_project.full_path}.git" }
let(:access) { access_class.new(nil, public_project, 'web', authentication_abilities: [:download_code], repository_path: repository_path) }
context 'when repository is enabled' do
it 'give access to download code' do
......@@ -1169,7 +1166,7 @@ RSpec.describe Gitlab::GitAccess do
def access
access_class.new(actor, project, protocol,
authentication_abilities: authentication_abilities,
namespace_path: namespace_path, repository_path: project_path,
repository_path: repository_path,
redirected_path: redirected_path, auth_result_type: auth_result_type)
end
......
......@@ -433,37 +433,85 @@ RSpec.describe Gitlab::PathRegex do
it { is_expected.not_to match('gitlab.git') }
end
shared_examples 'invalid snippet routes' do
it { is_expected.not_to match('gitlab-org/gitlab/snippets/1.git') }
it { is_expected.not_to match('snippets/1.git') }
it { is_expected.not_to match('gitlab-org/gitlab/snippets/') }
it { is_expected.not_to match('/gitlab-org/gitlab/snippets/1') }
it { is_expected.not_to match('gitlab-org/gitlab/snippets/foo') }
it { is_expected.not_to match('root/snippets/1') }
it { is_expected.not_to match('/snippets/1') }
it { is_expected.not_to match('snippets/') }
it { is_expected.not_to match('snippets/foo') }
end
context 'repository routes' do
# Paths that match a known container
let_it_be(:container_paths) do
[
'gitlab-org',
'gitlab-org/gitlab-test',
'gitlab-org/gitlab-test/snippets/1',
'gitlab-org/gitlab-test/snippets/foo', # ambiguous, we allow creating a sub-group called 'snippets'
'snippets/1'
]
end
# Paths that never match a container
let_it_be(:invalid_paths) do
[
'gitlab/',
'/gitlab',
'gitlab/foo/',
'?gitlab',
'git lab',
'/snippets/1',
'snippets/foo',
'gitlab-org/gitlab/snippets/'
]
end
let_it_be(:git_paths) { container_paths.map { |path| path + '.git' } }
let_it_be(:snippet_paths) { container_paths.grep(%r{snippets/\d}) }
let_it_be(:wiki_git_paths) { (container_paths - snippet_paths).map { |path| path + '.wiki.git' } }
let_it_be(:invalid_git_paths) { invalid_paths.map { |path| path + '.git' } }
def expect_route_match(paths)
paths.each { |path| is_expected.to match(path) }
end
def expect_no_route_match(paths)
paths.each { |path| is_expected.not_to match(path) }
end
describe '.repository_route_regex' do
subject { %r{\A#{described_class.repository_route_regex}\z} }
it 'matches the expected paths' do
expect_route_match(container_paths)
expect_no_route_match(invalid_paths + git_paths)
end
end
describe '.full_snippets_repository_path_regex' do
subject { described_class.full_snippets_repository_path_regex }
describe '.repository_git_route_regex' do
subject { %r{\A#{described_class.repository_git_route_regex}\z} }
it { is_expected.to match('gitlab-org/gitlab/snippets/1') }
it { is_expected.to match('snippets/1') }
it 'matches the expected paths' do
expect_route_match(git_paths + wiki_git_paths)
expect_no_route_match(container_paths + invalid_paths + invalid_git_paths)
end
end
it_behaves_like 'invalid snippet routes'
end
describe '.repository_wiki_git_route_regex' do
subject { %r{\A#{described_class.repository_wiki_git_route_regex}\z} }
describe '.personal_and_project_snippets_path_regex' do
subject { %r{\A#{described_class.personal_and_project_snippets_path_regex}\z} }
it 'matches the expected paths' do
expect_route_match(wiki_git_paths)
expect_no_route_match(git_paths + invalid_git_paths)
end
it { is_expected.to match('gitlab-org/gitlab/snippets') }
it { is_expected.to match('snippets') }
it { is_expected.not_to match('snippets/1.wiki.git') }
end
it { is_expected.not_to match('gitlab-org/gitlab/snippets/1') }
it { is_expected.not_to match('snippets/1') }
describe '.full_snippets_repository_path_regex' do
subject { described_class.full_snippets_repository_path_regex }
it_behaves_like 'invalid snippet routes'
it 'matches the expected paths' do
expect_route_match(snippet_paths)
expect_no_route_match(container_paths - snippet_paths + git_paths + invalid_paths)
end
it { is_expected.not_to match('root/snippets/1') }
it { is_expected.not_to match('gitlab-org/gitlab-test/snippets/foo') }
end
end
describe '.container_image_regex' do
......
......@@ -722,8 +722,7 @@ RSpec.describe API::Internal::Base do
'ssh',
{
authentication_abilities: [:read_project, :download_code, :push_code],
namespace_path: project.namespace.path,
repository_path: project.path,
repository_path: "#{project.full_path}.git",
redirected_path: nil
}
).and_return(access_checker)
......
......@@ -3,22 +3,60 @@
require 'spec_helper'
RSpec.describe 'git_http routing' do
include RSpec::Rails::RequestExampleGroup
describe 'code repositories' do
it_behaves_like 'git repository routes' do
let(:path) { '/gitlab-org/gitlab-test.git' }
end
end
describe 'wiki repositories' do
context 'in project' do
let(:path) { '/gitlab-org/gitlab-test.wiki.git' }
it_behaves_like 'git repository routes'
describe 'redirects', type: :request do
let(:web_path) { '/gitlab-org/gitlab-test/-/wikis' }
it 'redirects namespace/project.wiki.git to the project wiki' do
expect(get(path)).to redirect_to(web_path)
end
describe 'wiki.git routing', 'routing' do
let(:wiki_path) { '/gitlab/gitlabhq/wikis' }
it 'preserves query parameters' do
expect(get("#{path}?foo=bar&baz=qux")).to redirect_to("#{web_path}?foo=bar&baz=qux")
end
it 'redirects namespace/project.wiki.git to the project wiki' do
expect(get('/gitlab/gitlabhq.wiki.git')).to redirect_to(wiki_path)
it 'only redirects when the format is .git' do
expect(get(path.delete_suffix('.git'))).not_to redirect_to(web_path)
expect(get(path.delete_suffix('.git') + '.json')).not_to redirect_to(web_path)
end
end
end
it 'preserves query parameters' do
expect(get('/gitlab/gitlabhq.wiki.git?foo=bar&baz=qux')).to redirect_to("#{wiki_path}?foo=bar&baz=qux")
context 'in toplevel group' do
it_behaves_like 'git repository routes' do
let(:path) { '/gitlab-org.wiki.git' }
end
end
context 'in child group' do
it_behaves_like 'git repository routes' do
let(:path) { '/gitlab-org/child.wiki.git' }
end
end
end
describe 'snippet repositories' do
context 'personal snippet' do
it_behaves_like 'git repository routes' do
let(:path) { '/snippets/123.git' }
end
end
it 'only redirects when the format is .git' do
expect(get('/gitlab/gitlabhq.wiki')).not_to redirect_to(wiki_path)
expect(get('/gitlab/gitlabhq.wiki.json')).not_to redirect_to(wiki_path)
context 'project snippet' do
it_behaves_like 'git repository routes' do
let(:path) { '/gitlab-org/gitlab-test/snippets/123.git' }
end
end
end
end
......@@ -54,10 +54,6 @@ RSpec.describe "Mounted Apps", "routing" do
it "to API" do
expect(get("/api/issues")).to be_routable
end
it "to Grack" do
expect(get("/gitlab/gitlabhq.git")).to be_routable
end
end
# snippets GET /snippets(.:format) snippets#index
......
......@@ -5,45 +5,45 @@ require_relative 'workhorse_helpers'
module GitHttpHelpers
include WorkhorseHelpers
def clone_get(project, **options)
get "/#{project}/info/refs", params: { service: 'git-upload-pack' }, headers: auth_env(*options.values_at(:user, :password, :spnego_request_token))
def clone_get(repository_path, **options)
get "/#{repository_path}/info/refs", params: { service: 'git-upload-pack' }, headers: auth_env(*options.values_at(:user, :password, :spnego_request_token))
end
def clone_post(project, **options)
post "/#{project}/git-upload-pack", headers: auth_env(*options.values_at(:user, :password, :spnego_request_token))
def clone_post(repository_path, **options)
post "/#{repository_path}/git-upload-pack", headers: auth_env(*options.values_at(:user, :password, :spnego_request_token))
end
def push_get(project, **options)
get "/#{project}/info/refs", params: { service: 'git-receive-pack' }, headers: auth_env(*options.values_at(:user, :password, :spnego_request_token))
def push_get(repository_path, **options)
get "/#{repository_path}/info/refs", params: { service: 'git-receive-pack' }, headers: auth_env(*options.values_at(:user, :password, :spnego_request_token))
end
def push_post(project, **options)
post "/#{project}/git-receive-pack", headers: auth_env(*options.values_at(:user, :password, :spnego_request_token))
def push_post(repository_path, **options)
post "/#{repository_path}/git-receive-pack", headers: auth_env(*options.values_at(:user, :password, :spnego_request_token))
end
def download(project, user: nil, password: nil, spnego_request_token: nil)
def download(repository_path, user: nil, password: nil, spnego_request_token: nil)
args = { user: user, password: password, spnego_request_token: spnego_request_token }
clone_get(project, **args)
clone_get(repository_path, **args)
yield response
clone_post(project, **args)
clone_post(repository_path, **args)
yield response
end
def upload(project, user: nil, password: nil, spnego_request_token: nil)
def upload(repository_path, user: nil, password: nil, spnego_request_token: nil)
args = { user: user, password: password, spnego_request_token: spnego_request_token }
push_get(project, **args)
push_get(repository_path, **args)
yield response
push_post(project, **args)
push_post(repository_path, **args)
yield response
end
def download_or_upload(project, **args, &block)
download(project, **args, &block)
upload(project, **args, &block)
def download_or_upload(repository_path, **args, &block)
download(repository_path, **args, &block)
upload(repository_path, **args, &block)
end
def auth_env(user, password, spnego_request_token)
......
# frozen_string_literal: true
RSpec.shared_examples 'git repository routes' do
let(:params) { { repository_path: path.delete_prefix('/') } }
let(:container_path) { path.delete_suffix('.git') }
it 'routes Git endpoints' do
expect(get("#{path}/info/refs")).to route_to('repositories/git_http#info_refs', **params)
expect(post("#{path}/git-upload-pack")).to route_to('repositories/git_http#git_upload_pack', **params)
expect(post("#{path}/git-receive-pack")).to route_to('repositories/git_http#git_receive_pack', **params)
end
context 'requests without .git format' do
it 'redirects requests to /info/refs', type: :request do
expect(get("#{container_path}/info/refs")).to redirect_to("#{container_path}.git/info/refs")
expect(get("#{container_path}/info/refs?service=git-upload-pack")).to redirect_to("#{container_path}.git/info/refs?service=git-upload-pack")
expect(get("#{container_path}/info/refs?service=git-receive-pack")).to redirect_to("#{container_path}.git/info/refs?service=git-receive-pack")
end
it 'does not redirect other requests' do
expect(post("#{container_path}/git-upload-pack")).not_to be_routable
end
end
it 'routes LFS endpoints' do
oid = generate(:oid)
expect(post("#{path}/info/lfs/objects/batch")).to route_to('repositories/lfs_api#batch', **params)
expect(post("#{path}/info/lfs/objects")).to route_to('repositories/lfs_api#deprecated', **params)
expect(get("#{path}/info/lfs/objects/#{oid}")).to route_to('repositories/lfs_api#deprecated', oid: oid, **params)
expect(post("#{path}/info/lfs/locks/123/unlock")).to route_to('repositories/lfs_locks_api#unlock', id: '123', **params)
expect(post("#{path}/info/lfs/locks/verify")).to route_to('repositories/lfs_locks_api#verify', **params)
expect(get("#{path}/gitlab-lfs/objects/#{oid}")).to route_to('repositories/lfs_storage#download', oid: oid, **params)
expect(put("#{path}/gitlab-lfs/objects/#{oid}/456/authorize")).to route_to('repositories/lfs_storage#upload_authorize', oid: oid, size: '456', **params)
expect(put("#{path}/gitlab-lfs/objects/#{oid}/456")).to route_to('repositories/lfs_storage#upload_finalize', oid: oid, size: '456', **params)
expect(put("#{path}/gitlab-lfs/objects/foo")).not_to be_routable
expect(put("#{path}/gitlab-lfs/objects/#{oid}/foo")).not_to be_routable
expect(put("#{path}/gitlab-lfs/objects/#{oid}/foo/authorize")).not_to be_routable
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