From e40e3fdc8271d1becf7952c7e30546c5abecb79b Mon Sep 17 00:00:00 2001
From: Patricio Cano <suprnova32@gmail.com>
Date: Thu, 25 Aug 2016 17:26:20 -0500
Subject: [PATCH] Added LFS support to SSH - Required on the GitLab Rails side
is mostly authentication and API related.
---
.../projects/git_http_client_controller.rb | 42 +++++++++++++------
app/helpers/lfs_helper.rb | 2 +-
app/models/deploy_key.rb | 5 +++
app/models/user.rb | 3 +-
.../20160825173042_add_lfs_token_to_users.rb | 16 +++++++
.../20160825182924_add_lfs_token_to_keys.rb | 16 +++++++
lib/api/entities.rb | 2 +-
lib/api/internal.rb | 13 +++++-
lib/gitlab/auth.rb | 13 +++++-
spec/lib/gitlab/auth_spec.rb | 16 +++++++
.../concerns/token_authenticatable_spec.rb | 20 +++++++++
spec/requests/api/internal_spec.rb | 26 ++++++++++--
spec/requests/lfs_http_spec.rb | 16 +++++++
13 files changed, 169 insertions(+), 21 deletions(-)
create mode 100644 db/migrate/20160825173042_add_lfs_token_to_users.rb
create mode 100644 db/migrate/20160825182924_add_lfs_token_to_keys.rb
diff --git a/app/controllers/projects/git_http_client_controller.rb b/app/controllers/projects/git_http_client_controller.rb
index f5ce63fdfed..1e6709dc8eb 100644
--- a/app/controllers/projects/git_http_client_controller.rb
+++ b/app/controllers/projects/git_http_client_controller.rb
@@ -4,6 +4,8 @@ class Projects::GitHttpClientController < Projects::ApplicationController
include ActionController::HttpAuthentication::Basic
include KerberosSpnegoHelper
+ class MissingPersonalTokenError < StandardError; end
+
attr_reader :user
# Git clients will not know what authenticity token to send along
@@ -21,18 +23,8 @@ class Projects::GitHttpClientController < Projects::ApplicationController
if allow_basic_auth? && basic_auth_provided?
login, password = user_name_and_password(request)
- auth_result = Gitlab::Auth.find_for_git_client(login, password, project: project, ip: request.ip)
-
- if auth_result.type == :ci && download_request?
- @ci = true
- elsif auth_result.type == :oauth && !download_request?
- # Not allowed
- elsif auth_result.type == :missing_personal_token
- render_missing_personal_token
- return # Render above denied access, nothing left to do
- else
- @user = auth_result.user
- end
+
+ handle_authentication(login, password)
if ci? || user
return # Allow access
@@ -48,6 +40,10 @@ class Projects::GitHttpClientController < Projects::ApplicationController
send_challenges
render plain: "HTTP Basic: Access denied\n", status: 401
+
+ rescue MissingPersonalTokenError
+ render_missing_personal_token
+ return
end
def basic_auth_provided?
@@ -118,6 +114,28 @@ class Projects::GitHttpClientController < Projects::ApplicationController
@ci.present?
end
+ def handle_authentication(login, password)
+ auth_result = Gitlab::Auth.find_for_git_client(login, password, project: project, ip: request.ip)
+
+ if auth_result.type == :ci && download_request?
+ @ci = true
+ elsif auth_result.type == :oauth && !download_request?
+ # Not allowed
+ elsif auth_result.type == :missing_personal_token
+ raise MissingPersonalTokenError
+ elsif auth_result.type == :lfs_deploy_token && download_request?
+ @lfs_deploy_key = true
+ @user = auth_result.user
+ else
+ @user = auth_result.user
+ end
+ end
+
+ def lfs_deploy_key?
+ key = user
+ @lfs_deploy_key.present? && (key && key.projects.include?(project))
+ end
+
def verify_workhorse_api!
Gitlab::Workhorse.verify_api_request!(request.headers)
end
diff --git a/app/helpers/lfs_helper.rb b/app/helpers/lfs_helper.rb
index 5d82abfca79..0c867fc8741 100644
--- a/app/helpers/lfs_helper.rb
+++ b/app/helpers/lfs_helper.rb
@@ -25,7 +25,7 @@ module LfsHelper
def lfs_download_access?
return false unless project.lfs_enabled?
- project.public? || ci? || (user && user.can?(:download_code, project))
+ project.public? || ci? || lfs_deploy_key? || (user && user.can?(:download_code, project))
end
def lfs_upload_access?
diff --git a/app/models/deploy_key.rb b/app/models/deploy_key.rb
index 2c525d4cd7a..de51b63c120 100644
--- a/app/models/deploy_key.rb
+++ b/app/models/deploy_key.rb
@@ -1,7 +1,12 @@
class DeployKey < Key
+ include TokenAuthenticatable
+ add_authentication_token_field :lfs_token
+
has_many :deploy_keys_projects, dependent: :destroy
has_many :projects, through: :deploy_keys_projects
+ before_save :ensure_lfs_token
+
scope :in_projects, ->(projects) { joins(:deploy_keys_projects).where('deploy_keys_projects.project_id in (?)', projects) }
scope :are_public, -> { where(public: true) }
diff --git a/app/models/user.rb b/app/models/user.rb
index 6996740eebd..94ae3911859 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -13,6 +13,7 @@ class User < ActiveRecord::Base
DEFAULT_NOTIFICATION_LEVEL = :participating
add_authentication_token_field :authentication_token
+ add_authentication_token_field :lfs_token
default_value_for :admin, false
default_value_for(:external) { current_application_settings.user_default_external }
@@ -117,7 +118,7 @@ class User < ActiveRecord::Base
before_validation :set_public_email, if: ->(user) { user.public_email_changed? }
after_update :update_emails_with_primary_email, if: ->(user) { user.email_changed? }
- before_save :ensure_authentication_token
+ before_save :ensure_authentication_token, :ensure_lfs_token
before_save :ensure_external_user_rights
after_save :ensure_namespace_correct
after_initialize :set_projects_limit
diff --git a/db/migrate/20160825173042_add_lfs_token_to_users.rb b/db/migrate/20160825173042_add_lfs_token_to_users.rb
new file mode 100644
index 00000000000..f7f210bcd67
--- /dev/null
+++ b/db/migrate/20160825173042_add_lfs_token_to_users.rb
@@ -0,0 +1,16 @@
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class AddLfsTokenToUsers < ActiveRecord::Migration
+ include Gitlab::Database::MigrationHelpers
+
+ # Set this constant to true if this migration requires downtime.
+ DOWNTIME = false
+
+ disable_ddl_transaction!
+
+ def change
+ add_column :users, :lfs_token, :string
+ add_concurrent_index :users, :lfs_token
+ end
+end
diff --git a/db/migrate/20160825182924_add_lfs_token_to_keys.rb b/db/migrate/20160825182924_add_lfs_token_to_keys.rb
new file mode 100644
index 00000000000..3ff010ef328
--- /dev/null
+++ b/db/migrate/20160825182924_add_lfs_token_to_keys.rb
@@ -0,0 +1,16 @@
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class AddLfsTokenToKeys < ActiveRecord::Migration
+ include Gitlab::Database::MigrationHelpers
+
+ # Set this constant to true if this migration requires downtime.
+ DOWNTIME = false
+
+ disable_ddl_transaction!
+
+ def change
+ add_column :keys, :lfs_token, :string
+ add_concurrent_index :keys, :lfs_token
+ end
+end
diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index 4f736e4ec2b..b4fcacca896 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -1,7 +1,7 @@
module API
module Entities
class UserSafe < Grape::Entity
- expose :name, :username
+ expose :name, :username, :lfs_token
end
class UserBasic < UserSafe
diff --git a/lib/api/internal.rb b/lib/api/internal.rb
index 6e6efece7c4..7c0a6eaa652 100644
--- a/lib/api/internal.rb
+++ b/lib/api/internal.rb
@@ -69,6 +69,10 @@ module API
else
project.repository.path_to_repo
end
+
+ # Return HTTP full path, so that gitlab-shell has this information
+ # ready for git-lfs-authenticate
+ response[:repository_http_path] = project.http_url_to_repo
end
response
@@ -83,7 +87,14 @@ module API
#
get "/discover" do
key = Key.find(params[:key_id])
- present key.user, with: Entities::UserSafe
+ user = key.user
+ if user
+ user.ensure_lfs_token!
+ present user, with: Entities::UserSafe
+ else
+ key.ensure_lfs_token!
+ { username: 'lfs-deploy-key', lfs_token: key.lfs_token }
+ end
end
get "/check" do
diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb
index 91f0270818a..5446093de4d 100644
--- a/lib/gitlab/auth.rb
+++ b/lib/gitlab/auth.rb
@@ -79,12 +79,13 @@ module Gitlab
result =
user_with_password_for_git(login, password) ||
oauth_access_token_check(login, password) ||
+ lfs_token_check(login, password) ||
personal_access_token_check(login, password)
if result
result.type = nil unless result.user
- if result.user && result.user.two_factor_enabled? && result.type == :gitlab_or_ldap
+ if result.user && result.type == :gitlab_or_ldap && result.user.two_factor_enabled?
result.type = :missing_personal_token
end
end
@@ -114,6 +115,16 @@ module Gitlab
Result.new(user, :personal_token) if user == validation
end
end
+
+ def lfs_token_check(login, password)
+ if login == 'lfs-deploy-key'
+ key = DeployKey.find_by_lfs_token(password)
+ Result.new(key, :lfs_deploy_token) if key
+ else
+ user = User.find_by_lfs_token(password)
+ Result.new(user, :lfs_token) if user && user.username == login
+ end
+ end
end
end
end
diff --git a/spec/lib/gitlab/auth_spec.rb b/spec/lib/gitlab/auth_spec.rb
index 7c23e02d05a..cd00a15be3b 100644
--- a/spec/lib/gitlab/auth_spec.rb
+++ b/spec/lib/gitlab/auth_spec.rb
@@ -23,6 +23,22 @@ describe Gitlab::Auth, lib: true do
expect(gl_auth.find_for_git_client(user.username, 'password', project: nil, ip: ip)).to eq(Gitlab::Auth::Result.new(user, :gitlab_or_ldap))
end
+ it 'recognizes user lfs tokens' do
+ user = create(:user)
+ ip = 'ip'
+
+ expect(gl_auth).to receive(:rate_limit!).with(ip, success: true, login: user.username)
+ expect(gl_auth.find_for_git_client(user.username, user.lfs_token, project: nil, ip: ip)).to eq(Gitlab::Auth::Result.new(user, :lfs_token))
+ end
+
+ it 'recognizes deploy key lfs tokens' do
+ key = create(:deploy_key)
+ ip = 'ip'
+
+ expect(gl_auth).to receive(:rate_limit!).with(ip, success: true, login: 'lfs-deploy-key')
+ expect(gl_auth.find_for_git_client('lfs-deploy-key', key.lfs_token, project: nil, ip: ip)).to eq(Gitlab::Auth::Result.new(key, :lfs_deploy_token))
+ end
+
it 'recognizes OAuth tokens' do
user = create(:user)
application = Doorkeeper::Application.create!(name: "MyApp", redirect_uri: "https://app.com", owner: user)
diff --git a/spec/models/concerns/token_authenticatable_spec.rb b/spec/models/concerns/token_authenticatable_spec.rb
index eb64f3d0c83..82076600f3b 100644
--- a/spec/models/concerns/token_authenticatable_spec.rb
+++ b/spec/models/concerns/token_authenticatable_spec.rb
@@ -18,6 +18,26 @@ describe User, 'TokenAuthenticatable' do
subject { create(:user).send(token_field) }
it { is_expected.to be_a String }
end
+
+ describe 'lfs token' do
+ let(:token_field) { :lfs_token }
+ it_behaves_like 'TokenAuthenticatable'
+
+ describe 'ensure it' do
+ subject { create(:user).send(token_field) }
+ it { is_expected.to be_a String }
+ end
+ end
+end
+
+describe DeployKey, 'TokenAuthenticatable' do
+ let(:token_field) { :lfs_token }
+ it_behaves_like 'TokenAuthenticatable'
+
+ describe 'ensures authentication token' do
+ subject { create(:deploy_key).send(token_field) }
+ it { is_expected.to be_a String }
+ end
end
describe ApplicationSetting, 'TokenAuthenticatable' do
diff --git a/spec/requests/api/internal_spec.rb b/spec/requests/api/internal_spec.rb
index 46d1b868782..95fc5f790e8 100644
--- a/spec/requests/api/internal_spec.rb
+++ b/spec/requests/api/internal_spec.rb
@@ -101,12 +101,28 @@ describe API::API, api: true do
end
describe "GET /internal/discover" do
- it do
- get(api("/internal/discover"), key_id: key.id, secret_token: secret_token)
+ context 'user key' do
+ it 'returns the correct information about the key' do
+ get(api("/internal/discover"), key_id: key.id, secret_token: secret_token)
- expect(response).to have_http_status(200)
+ expect(response).to have_http_status(200)
+
+ expect(json_response['name']).to eq(user.name)
+ expect(json_response['lfs_token']).to eq(user.lfs_token)
+ end
+ end
- expect(json_response['name']).to eq(user.name)
+ context 'deploy key' do
+ let(:key) { create(:deploy_key) }
+
+ it 'returns the correct information about the key' do
+ get(api("/internal/discover"), key_id: key.id, secret_token: secret_token)
+
+ expect(response).to have_http_status(200)
+
+ expect(json_response['username']).to eq('lfs-deploy-key')
+ expect(json_response['lfs_token']).to eq(key.lfs_token)
+ end
end
end
@@ -143,6 +159,7 @@ describe API::API, api: true do
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)
+ expect(json_response["repository_http_path"]).to eq(project.http_url_to_repo)
end
end
@@ -153,6 +170,7 @@ describe API::API, api: true do
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)
+ expect(json_response["repository_http_path"]).to eq(project.http_url_to_repo)
end
end
end
diff --git a/spec/requests/lfs_http_spec.rb b/spec/requests/lfs_http_spec.rb
index 6e551bb65fa..58f8515c0e2 100644
--- a/spec/requests/lfs_http_spec.rb
+++ b/spec/requests/lfs_http_spec.rb
@@ -244,6 +244,18 @@ describe 'Git LFS API and storage' do
end
end
+ context 'when deploy key is authorized' do
+ let(:key) { create(:deploy_key) }
+ let(:authorization) { authorize_deploy_key }
+
+ let(:update_permissions) do
+ project.deploy_keys << key
+ project.lfs_objects << lfs_object
+ end
+
+ it_behaves_like 'responds with a file'
+ end
+
context 'when CI is authorized' do
let(:authorization) { authorize_ci_project }
@@ -904,6 +916,10 @@ describe 'Git LFS API and storage' do
ActionController::HttpAuthentication::Basic.encode_credentials(user.username, user.password)
end
+ def authorize_deploy_key
+ ActionController::HttpAuthentication::Basic.encode_credentials('lfs-deploy-key', key.lfs_token)
+ end
+
def fork_project(project, user, object = nil)
allow(RepositoryForkWorker).to receive(:perform_async).and_return(true)
Projects::ForkService.new(project, user, {}).execute
--
GitLab