Commit f9616f15 authored by Sean McGivern's avatar Sean McGivern

Merge branch '24496-fix-internal-api-project-lookup' into 'master'

Fix POST /internal/allowed to cope with gitlab-shell v4.0.0 project paths

Closes #24496

See merge request !7480
parents d1afb845 1c994dbc
...@@ -15,16 +15,6 @@ class Repository ...@@ -15,16 +15,6 @@ class Repository
Gitlab.config.repositories.storages Gitlab.config.repositories.storages
end end
def self.remove_storage_from_path(repo_path)
storages.find do |_, storage_path|
if repo_path.start_with?(storage_path)
return repo_path.sub(storage_path, '')
end
end
repo_path
end
def initialize(path_with_namespace, project) def initialize(path_with_namespace, project)
@path_with_namespace = path_with_namespace @path_with_namespace = path_with_namespace
@project = project @project = project
......
---
title: Fix POST /internal/allowed to cope with gitlab-shell v4.0.0 project paths
merge_request: 7480
author:
module API
module Helpers
module InternalHelpers
# Project paths may be any of the following:
# * /repository/storage/path/namespace/project
# * /namespace/project
# * namespace/project
#
# In addition, they may have a '.git' extension and multiple namespaces
#
# Transform all these cases to 'namespace/project'
def clean_project_path(project_path, storage_paths = Repository.storages.values)
project_path = project_path.sub(/\.git\z/, '')
storage_paths.each do |storage_path|
storage_path = File.expand_path(storage_path)
if project_path.start_with?(storage_path)
project_path = project_path.sub(storage_path, '')
break
end
end
project_path.sub(/\A\//, '')
end
def project_path
@project_path ||= clean_project_path(params[:project])
end
def wiki?
@wiki ||= project_path.end_with?('.wiki') &&
!Project.find_with_namespace(project_path)
end
def project
@project ||= begin
# Check for *.wiki repositories.
# Strip out the .wiki from the pathname before finding the
# project. This applies the correct project permissions to
# the wiki repository as well.
project_path.chomp!('.wiki') if wiki?
Project.find_with_namespace(project_path)
end
end
def ssh_authentication_abilities
[
:read_project,
:download_code,
:push_code
]
end
end
end
end
...@@ -3,6 +3,8 @@ module API ...@@ -3,6 +3,8 @@ module API
class Internal < Grape::API class Internal < Grape::API
before { authenticate_by_gitlab_shell_token! } before { authenticate_by_gitlab_shell_token! }
helpers ::API::Helpers::InternalHelpers
namespace 'internal' do namespace 'internal' do
# Check if git command is allowed to project # Check if git command is allowed to project
# #
...@@ -14,42 +16,6 @@ module API ...@@ -14,42 +16,6 @@ module API
# ref - branch name # ref - branch name
# forced_push - forced_push # forced_push - forced_push
# protocol - Git access protocol being used, e.g. HTTP or SSH # protocol - Git access protocol being used, e.g. HTTP or SSH
#
helpers do
def project_path
@project_path ||= begin
project_path = params[:project].sub(/\.git\z/, '')
Repository.remove_storage_from_path(project_path)
end
end
def wiki?
@wiki ||= project_path.end_with?('.wiki') &&
!Project.find_with_namespace(project_path)
end
def project
@project ||= begin
# Check for *.wiki repositories.
# Strip out the .wiki from the pathname before finding the
# project. This applies the correct project permissions to
# the wiki repository as well.
project_path.chomp!('.wiki') if wiki?
Project.find_with_namespace(project_path)
end
end
def ssh_authentication_abilities
[
:read_project,
:download_code,
:push_code
]
end
end
post "/allowed" do post "/allowed" do
status 200 status 200
......
...@@ -1556,14 +1556,4 @@ describe Repository, models: true do ...@@ -1556,14 +1556,4 @@ describe Repository, models: true do
end.to raise_error(Repository::CommitError) end.to raise_error(Repository::CommitError)
end end
end end
describe '#remove_storage_from_path' do
let(:storage_path) { project.repository_storage_path }
let(:project_path) { project.path_with_namespace }
let(:full_path) { File.join(storage_path, project_path) }
it { expect(Repository.remove_storage_from_path(full_path)).to eq(project_path) }
it { expect(Repository.remove_storage_from_path(project_path)).to eq(project_path) }
it { expect(Repository.remove_storage_from_path(storage_path)).to eq('') }
end
end end
require 'spec_helper'
describe ::API::Helpers::InternalHelpers do
include ::API::Helpers::InternalHelpers
describe '.clean_project_path' do
project = 'namespace/project'
namespaced = File.join('namespace2', project)
{
File.join(Dir.pwd, project) => project,
File.join(Dir.pwd, namespaced) => namespaced,
project => project,
namespaced => namespaced,
project + '.git' => project,
namespaced + '.git' => namespaced,
"/" + project => project,
"/" + namespaced => namespaced,
}.each do |project_path, expected|
context project_path do
# Relative and absolute storage paths, with and without trailing /
['.', './', Dir.pwd, Dir.pwd + '/'].each do |storage_path|
context "storage path is #{storage_path}" do
subject { clean_project_path(project_path, [storage_path]) }
it { is_expected.to eq(expected) }
end
end
end
end
end
end
...@@ -191,6 +191,26 @@ describe API::API, api: true do ...@@ -191,6 +191,26 @@ describe API::API, api: true do
expect(json_response["status"]).to be_truthy expect(json_response["status"]).to be_truthy
expect(json_response["repository_path"]).to eq(project.repository.path_to_repo) expect(json_response["repository_path"]).to eq(project.repository.path_to_repo)
end end
context 'project as /namespace/project' do
it do
pull(key, project_with_repo_path('/' + project.path_with_namespace))
expect(response).to have_http_status(200)
expect(json_response["status"]).to be_truthy
expect(json_response["repository_path"]).to eq(project.repository.path_to_repo)
end
end
context 'project as namespace/project' do
it do
pull(key, project_with_repo_path(project.path_with_namespace))
expect(response).to have_http_status(200)
expect(json_response["status"]).to be_truthy
expect(json_response["repository_path"]).to eq(project.repository.path_to_repo)
end
end
end end
end end
...@@ -299,7 +319,7 @@ describe API::API, api: true do ...@@ -299,7 +319,7 @@ describe API::API, api: true do
context 'project does not exist' do context 'project does not exist' do
it do it do
pull(key, OpenStruct.new(path_with_namespace: 'gitlab/notexists')) pull(key, project_with_repo_path('gitlab/notexist'))
expect(response).to have_http_status(200) expect(response).to have_http_status(200)
expect(json_response["status"]).to be_falsey expect(json_response["status"]).to be_falsey
...@@ -392,11 +412,17 @@ describe API::API, api: true do ...@@ -392,11 +412,17 @@ describe API::API, api: true do
end end
end end
def project_with_repo_path(path)
double().tap do |fake_project|
allow(fake_project).to receive_message_chain('repository.path_to_repo' => path)
end
end
def pull(key, project, protocol = 'ssh') def pull(key, project, protocol = 'ssh')
post( post(
api("/internal/allowed"), api("/internal/allowed"),
key_id: key.id, key_id: key.id,
project: project.path_with_namespace, project: project.repository.path_to_repo,
action: 'git-upload-pack', action: 'git-upload-pack',
secret_token: secret_token, secret_token: secret_token,
protocol: protocol protocol: protocol
...@@ -408,7 +434,7 @@ describe API::API, api: true do ...@@ -408,7 +434,7 @@ describe API::API, api: true do
api("/internal/allowed"), api("/internal/allowed"),
changes: 'd14d6c0abdd253381df51a723d58691b2ee1ab08 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/master', changes: 'd14d6c0abdd253381df51a723d58691b2ee1ab08 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/master',
key_id: key.id, key_id: key.id,
project: project.path_with_namespace, project: project.repository.path_to_repo,
action: 'git-receive-pack', action: 'git-receive-pack',
secret_token: secret_token, secret_token: secret_token,
protocol: protocol protocol: protocol
...@@ -420,7 +446,7 @@ describe API::API, api: true do ...@@ -420,7 +446,7 @@ describe API::API, api: true do
api("/internal/allowed"), api("/internal/allowed"),
ref: 'master', ref: 'master',
key_id: key.id, key_id: key.id,
project: project.path_with_namespace, project: project.repository.path_to_repo,
action: 'git-upload-archive', action: 'git-upload-archive',
secret_token: secret_token, secret_token: secret_token,
protocol: 'ssh' protocol: 'ssh'
...@@ -432,7 +458,7 @@ describe API::API, api: true do ...@@ -432,7 +458,7 @@ describe API::API, api: true do
api("/internal/lfs_authenticate"), api("/internal/lfs_authenticate"),
key_id: key_id, key_id: key_id,
secret_token: secret_token, secret_token: secret_token,
project: project.path_with_namespace project: project.repository.path_to_repo
) )
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