diff options
author | Douwe Maan <douwe@gitlab.com> | 2016-09-20 10:10:45 +0000 |
---|---|---|
committer | Douwe Maan <douwe@gitlab.com> | 2016-09-20 10:10:45 +0000 |
commit | 667d2350911b2c3cca0545897eb67fda4b7d4b80 (patch) | |
tree | 9357cf7a9703fc5dd72a345fcc15255de22d8334 | |
parent | 95b9421ad3b2678da6e0af0131eafd52cdd0b2a5 (diff) | |
parent | 795acf2e4e01f7ddf3a8be73ddc119b4d84a03e3 (diff) | |
download | gitlab-ce-667d2350911b2c3cca0545897eb67fda4b7d4b80.tar.gz |
Merge branch 'lfs-support-for-ssh-enabled' into 'master'
LFS support for ssh enabled
## What does this MR do?
This is follow-up after https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/6043 which is falsely shown as merged due to: https://gitlab.com/gitlab-org/gitlab-ce/issues/22334
## Are there points in the code the reviewer needs to double check?
## Why was this MR needed?
## Screenshots (if relevant)
## Does this MR meet the acceptance criteria?
- [ ] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added
- [ ] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md)
- [ ] API support added
- Tests
- [ ] Added for this feature/bug
- [ ] All builds are passing
- [ ] Conform by the [merge request performance guides](http://docs.gitlab.com/ce/development/merge_request_performance_guidelines.html)
- [ ] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [ ] Branch has no merge conflicts with `master` (if you do - rebase it please)
- [ ] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)
## What are the relevant issue numbers?
See merge request !6413
-rw-r--r-- | CHANGELOG | 1 | ||||
-rw-r--r-- | app/controllers/projects/git_http_client_controller.rb | 8 | ||||
-rw-r--r-- | app/helpers/lfs_helper.rb | 2 | ||||
-rw-r--r-- | doc/workflow/lfs/lfs_administration.md | 4 | ||||
-rw-r--r-- | doc/workflow/lfs/manage_large_binaries_with_git_lfs.md | 8 | ||||
-rw-r--r-- | lib/api/internal.rb | 13 | ||||
-rw-r--r-- | lib/gitlab/auth.rb | 25 | ||||
-rw-r--r-- | lib/gitlab/auth/result.rb | 12 | ||||
-rw-r--r-- | lib/gitlab/lfs_token.rb | 54 | ||||
-rw-r--r-- | spec/lib/gitlab/auth_spec.rb | 18 | ||||
-rw-r--r-- | spec/lib/gitlab/lfs_token_spec.rb | 51 | ||||
-rw-r--r-- | spec/requests/api/internal_spec.rb | 46 | ||||
-rw-r--r-- | spec/requests/lfs_http_spec.rb | 16 |
13 files changed, 250 insertions, 8 deletions
diff --git a/CHANGELOG b/CHANGELOG index 7a19af6b765..ebc3507901c 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -113,6 +113,7 @@ v 8.12.0 (unreleased) - Remove green outline from `New branch unavailable` button on issue page !5858 (winniehell) - Fix repo title alignment (ClemMakesApps) - Change update interval of contacted_at + - Add LFS support to SSH !6043 - Fix branch title trailing space on hover (ClemMakesApps) - Don't include 'Created By' tag line when importing from GitHub if there is a linked GitLab account (EspadaV8) - Award emoji tooltips containing more than 10 usernames are now truncated !4780 (jlogandavison) diff --git a/app/controllers/projects/git_http_client_controller.rb b/app/controllers/projects/git_http_client_controller.rb index d1a2c52d80a..cbfd3cab3dd 100644 --- a/app/controllers/projects/git_http_client_controller.rb +++ b/app/controllers/projects/git_http_client_controller.rb @@ -127,9 +127,11 @@ class Projects::GitHttpClientController < Projects::ApplicationController end def ci? - authentication_result.ci? && - authentication_project && - authentication_project == project + authentication_result.ci?(project) + end + + def lfs_deploy_token? + authentication_result.lfs_deploy_token?(project) end def authentication_has_download_access? diff --git a/app/helpers/lfs_helper.rb b/app/helpers/lfs_helper.rb index 8e827664681..c15ecc8f86e 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_can_download_code? || build_can_download_code? + project.public? || ci? || lfs_deploy_token? || user_can_download_code? || build_can_download_code? end def user_can_download_code? diff --git a/doc/workflow/lfs/lfs_administration.md b/doc/workflow/lfs/lfs_administration.md index 9dc1e9b47e3..b3c73e947f0 100644 --- a/doc/workflow/lfs/lfs_administration.md +++ b/doc/workflow/lfs/lfs_administration.md @@ -45,5 +45,5 @@ In `config/gitlab.yml`: * Currently, storing GitLab Git LFS objects on a non-local storage (like S3 buckets) is not supported * Currently, removing LFS objects from GitLab Git LFS storage is not supported -* LFS authentications via SSH is not supported for the time being -* Only compatible with the GitLFS client versions 1.1.0 or 1.0.2. +* LFS authentications via SSH was added with GitLab 8.12 +* Only compatible with the GitLFS client versions 1.1.0 and up, or 1.0.2. diff --git a/doc/workflow/lfs/manage_large_binaries_with_git_lfs.md b/doc/workflow/lfs/manage_large_binaries_with_git_lfs.md index 9fe065fa680..1a4f213a792 100644 --- a/doc/workflow/lfs/manage_large_binaries_with_git_lfs.md +++ b/doc/workflow/lfs/manage_large_binaries_with_git_lfs.md @@ -35,6 +35,10 @@ Documentation for GitLab instance administrators is under [LFS administration do credentials store is recommended * Git LFS always assumes HTTPS so if you have GitLab server on HTTP you will have to add the URL to Git config manually (see #troubleshooting) + +>**Note**: With 8.12 GitLab added LFS support to SSH. The Git LFS communication + still goes over HTTP, but now the SSH client passes the correct credentials + to the Git LFS client, so no action is required by the user. ## Using Git LFS @@ -132,6 +136,10 @@ git config --add lfs.url "http://gitlab.example.com/group/project.git/info/lfs" ### Credentials are always required when pushing an object +>**Note**: With 8.12 GitLab added LFS support to SSH. The Git LFS communication + still goes over HTTP, but now the SSH client passes the correct credentials + to the Git LFS client, so no action is required by the user. + Given that Git LFS uses HTTP Basic Authentication to authenticate the user pushing the LFS object on every push for every object, user HTTPS credentials are required. diff --git a/lib/api/internal.rb b/lib/api/internal.rb index 1114fd21784..090d04544da 100644 --- a/lib/api/internal.rb +++ b/lib/api/internal.rb @@ -82,6 +82,19 @@ module API response end + post "/lfs_authenticate" do + status 200 + + key = Key.find(params[:key_id]) + token_handler = Gitlab::LfsToken.new(key) + + { + username: token_handler.actor_name, + lfs_token: token_handler.generate, + repository_http_path: project.http_url_to_repo + } + end + get "/merge_request_urls" do ::MergeRequests::GetUrlsService.new(project).execute(params[:changes]) end diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb index 0a0f1c3b17b..7c0f2115d43 100644 --- a/lib/gitlab/auth.rb +++ b/lib/gitlab/auth.rb @@ -11,6 +11,7 @@ module Gitlab build_access_token_check(login, password) || user_with_password_for_git(login, password) || oauth_access_token_check(login, password) || + lfs_token_check(login, password) || personal_access_token_check(login, password) || Gitlab::Auth::Result.new @@ -102,6 +103,30 @@ module Gitlab end end + def lfs_token_check(login, password) + deploy_key_matches = login.match(/\Alfs\+deploy-key-(\d+)\z/) + + actor = + if deploy_key_matches + DeployKey.find(deploy_key_matches[1]) + else + User.by_login(login) + end + + return unless actor + + token_handler = Gitlab::LfsToken.new(actor) + + authentication_abilities = + if token_handler.user? + full_authentication_abilities + else + read_authentication_abilities + end + + Result.new(actor, nil, token_handler.type, authentication_abilities) if Devise.secure_compare(token_handler.value, password) + end + def build_access_token_check(login, password) return unless login == 'gitlab-ci-token' return unless password diff --git a/lib/gitlab/auth/result.rb b/lib/gitlab/auth/result.rb index bf625649cbf..6be7f690676 100644 --- a/lib/gitlab/auth/result.rb +++ b/lib/gitlab/auth/result.rb @@ -1,8 +1,16 @@ module Gitlab module Auth Result = Struct.new(:actor, :project, :type, :authentication_abilities) do - def ci? - type == :ci + def ci?(for_project) + type == :ci && + project && + project == for_project + end + + def lfs_deploy_token?(for_project) + type == :lfs_deploy_token && + actor && + actor.projects.include?(for_project) end def success? diff --git a/lib/gitlab/lfs_token.rb b/lib/gitlab/lfs_token.rb new file mode 100644 index 00000000000..d089a2f9b0b --- /dev/null +++ b/lib/gitlab/lfs_token.rb @@ -0,0 +1,54 @@ +module Gitlab + class LfsToken + attr_accessor :actor + + TOKEN_LENGTH = 50 + EXPIRY_TIME = 1800 + + def initialize(actor) + @actor = + case actor + when DeployKey, User + actor + when Key + actor.user + else + raise 'Bad Actor' + end + end + + def generate + token = Devise.friendly_token(TOKEN_LENGTH) + + Gitlab::Redis.with do |redis| + redis.set(redis_key, token, ex: EXPIRY_TIME) + end + + token + end + + def value + Gitlab::Redis.with do |redis| + redis.get(redis_key) + end + end + + def user? + actor.is_a?(User) + end + + def type + actor.is_a?(User) ? :lfs_token : :lfs_deploy_token + end + + def actor_name + actor.is_a?(User) ? actor.username : "lfs+deploy-key-#{actor.id}" + end + + private + + def redis_key + "gitlab:lfs_token:#{actor.class.name.underscore}_#{actor.id}" if actor + end + end +end diff --git a/spec/lib/gitlab/auth_spec.rb b/spec/lib/gitlab/auth_spec.rb index 8807a68a0a2..745fbc0df45 100644 --- a/spec/lib/gitlab/auth_spec.rb +++ b/spec/lib/gitlab/auth_spec.rb @@ -61,6 +61,24 @@ 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, nil, :gitlab_or_ldap, full_authentication_abilities)) end + it 'recognizes user lfs tokens' do + user = create(:user) + ip = 'ip' + token = Gitlab::LfsToken.new(user).generate + + expect(gl_auth).to receive(:rate_limit!).with(ip, success: true, login: user.username) + expect(gl_auth.find_for_git_client(user.username, token, project: nil, ip: ip)).to eq(Gitlab::Auth::Result.new(user, nil, :lfs_token, full_authentication_abilities)) + end + + it 'recognizes deploy key lfs tokens' do + key = create(:deploy_key) + ip = 'ip' + token = Gitlab::LfsToken.new(key).generate + + expect(gl_auth).to receive(:rate_limit!).with(ip, success: true, login: "lfs+deploy-key-#{key.id}") + expect(gl_auth.find_for_git_client("lfs+deploy-key-#{key.id}", token, project: nil, ip: ip)).to eq(Gitlab::Auth::Result.new(key, nil, :lfs_deploy_token, read_authentication_abilities)) + 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/lib/gitlab/lfs_token_spec.rb b/spec/lib/gitlab/lfs_token_spec.rb new file mode 100644 index 00000000000..9f04f67e0a8 --- /dev/null +++ b/spec/lib/gitlab/lfs_token_spec.rb @@ -0,0 +1,51 @@ +require 'spec_helper' + +describe Gitlab::LfsToken, lib: true do + describe '#generate and #value' do + shared_examples 'an LFS token generator' do + it 'returns a randomly generated token' do + token = handler.generate + + expect(token).not_to be_nil + expect(token).to be_a String + expect(token.length).to eq 50 + end + + it 'returns the correct token based on the key' do + token = handler.generate + + expect(handler.value).to eq(token) + end + end + + context 'when the actor is a user' do + let(:actor) { create(:user) } + let(:handler) { described_class.new(actor) } + + it_behaves_like 'an LFS token generator' + + it 'returns the correct username' do + expect(handler.actor_name).to eq(actor.username) + end + + it 'returns the correct token type' do + expect(handler.type).to eq(:lfs_token) + end + end + + context 'when the actor is a deploy key' do + let(:actor) { create(:deploy_key) } + let(:handler) { described_class.new(actor) } + + it_behaves_like 'an LFS token generator' + + it 'returns the correct username' do + expect(handler.actor_name).to eq("lfs+deploy-key-#{actor.id}") + end + + it 'returns the correct token type' do + expect(handler.type).to eq(:lfs_deploy_token) + end + end + end +end diff --git a/spec/requests/api/internal_spec.rb b/spec/requests/api/internal_spec.rb index 46d1b868782..46e8e6f1169 100644 --- a/spec/requests/api/internal_spec.rb +++ b/spec/requests/api/internal_spec.rb @@ -100,6 +100,43 @@ describe API::API, api: true do end end + describe "POST /internal/lfs_authenticate" do + before do + project.team << [user, :developer] + end + + context 'user key' do + it 'returns the correct information about the key' do + lfs_auth(key.id, project) + + expect(response).to have_http_status(200) + expect(json_response['username']).to eq(user.username) + expect(json_response['lfs_token']).to eq(Gitlab::LfsToken.new(key).value) + + expect(json_response['repository_http_path']).to eq(project.http_url_to_repo) + end + + it 'returns a 404 when the wrong key is provided' do + lfs_auth(nil, project) + + expect(response).to have_http_status(404) + end + end + + context 'deploy key' do + let(:key) { create(:deploy_key) } + + it 'returns the correct information about the key' do + lfs_auth(key.id, project) + + expect(response).to have_http_status(200) + expect(json_response['username']).to eq("lfs+deploy-key-#{key.id}") + expect(json_response['lfs_token']).to eq(Gitlab::LfsToken.new(key).value) + expect(json_response['repository_http_path']).to eq(project.http_url_to_repo) + end + end + end + describe "GET /internal/discover" do it do get(api("/internal/discover"), key_id: key.id, secret_token: secret_token) @@ -389,4 +426,13 @@ describe API::API, api: true do protocol: 'ssh' ) end + + def lfs_auth(key_id, project) + post( + api("/internal/lfs_authenticate"), + key_id: key_id, + secret_token: secret_token, + project: project.path_with_namespace + ) + end end diff --git a/spec/requests/lfs_http_spec.rb b/spec/requests/lfs_http_spec.rb index b58d410b7a3..09e4e265dd1 100644 --- a/spec/requests/lfs_http_spec.rb +++ b/spec/requests/lfs_http_spec.rb @@ -245,6 +245,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 build is authorized as' do let(:authorization) { authorize_ci_project } @@ -1097,6 +1109,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.id}", Gitlab::LfsToken.new(key).generate) + end + def fork_project(project, user, object = nil) allow(RepositoryForkWorker).to receive(:perform_async).and_return(true) Projects::ForkService.new(project, user, {}).execute |