From e40e3fdc8271d1becf7952c7e30546c5abecb79b Mon Sep 17 00:00:00 2001 From: Patricio Cano 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