summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDouwe Maan <douwe@gitlab.com>2016-09-20 10:10:45 +0000
committerDouwe Maan <douwe@gitlab.com>2016-09-20 10:10:45 +0000
commit667d2350911b2c3cca0545897eb67fda4b7d4b80 (patch)
tree9357cf7a9703fc5dd72a345fcc15255de22d8334
parent95b9421ad3b2678da6e0af0131eafd52cdd0b2a5 (diff)
parent795acf2e4e01f7ddf3a8be73ddc119b4d84a03e3 (diff)
downloadgitlab-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--CHANGELOG1
-rw-r--r--app/controllers/projects/git_http_client_controller.rb8
-rw-r--r--app/helpers/lfs_helper.rb2
-rw-r--r--doc/workflow/lfs/lfs_administration.md4
-rw-r--r--doc/workflow/lfs/manage_large_binaries_with_git_lfs.md8
-rw-r--r--lib/api/internal.rb13
-rw-r--r--lib/gitlab/auth.rb25
-rw-r--r--lib/gitlab/auth/result.rb12
-rw-r--r--lib/gitlab/lfs_token.rb54
-rw-r--r--spec/lib/gitlab/auth_spec.rb18
-rw-r--r--spec/lib/gitlab/lfs_token_spec.rb51
-rw-r--r--spec/requests/api/internal_spec.rb46
-rw-r--r--spec/requests/lfs_http_spec.rb16
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