Commit 7fd0e153 authored by Douwe Maan's avatar Douwe Maan Committed by Ruben Davila

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
parent e53bcf40
...@@ -111,6 +111,7 @@ v 8.12.0 (unreleased) ...@@ -111,6 +111,7 @@ v 8.12.0 (unreleased)
- Remove green outline from `New branch unavailable` button on issue page !5858 (winniehell) - Remove green outline from `New branch unavailable` button on issue page !5858 (winniehell)
- Fix repo title alignment (ClemMakesApps) - Fix repo title alignment (ClemMakesApps)
- Change update interval of contacted_at - Change update interval of contacted_at
- Add LFS support to SSH !6043
- Fix branch title trailing space on hover (ClemMakesApps) - 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) - 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) - Award emoji tooltips containing more than 10 usernames are now truncated !4780 (jlogandavison)
......
...@@ -127,9 +127,11 @@ class Projects::GitHttpClientController < Projects::ApplicationController ...@@ -127,9 +127,11 @@ class Projects::GitHttpClientController < Projects::ApplicationController
end end
def ci? def ci?
authentication_result.ci? && authentication_result.ci?(project)
authentication_project && end
authentication_project == project
def lfs_deploy_token?
authentication_result.lfs_deploy_token?(project)
end end
def authentication_has_download_access? def authentication_has_download_access?
......
...@@ -25,7 +25,7 @@ module LfsHelper ...@@ -25,7 +25,7 @@ module LfsHelper
def lfs_download_access? def lfs_download_access?
return false unless project.lfs_enabled? 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 end
def user_can_download_code? def user_can_download_code?
......
...@@ -45,5 +45,5 @@ In `config/gitlab.yml`: ...@@ -45,5 +45,5 @@ In `config/gitlab.yml`:
* Currently, storing GitLab Git LFS objects on a non-local storage (like S3 buckets) * Currently, storing GitLab Git LFS objects on a non-local storage (like S3 buckets)
is not supported is not supported
* Currently, removing LFS objects from GitLab Git LFS storage 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 * LFS authentications via SSH was added with GitLab 8.12
* Only compatible with the GitLFS client versions 1.1.0 or 1.0.2. * Only compatible with the GitLFS client versions 1.1.0 and up, or 1.0.2.
...@@ -35,6 +35,10 @@ Documentation for GitLab instance administrators is under [LFS administration do ...@@ -35,6 +35,10 @@ Documentation for GitLab instance administrators is under [LFS administration do
credentials store is recommended credentials store is recommended
* Git LFS always assumes HTTPS so if you have GitLab server on HTTP you will have * 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) 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 ## Using Git LFS
...@@ -132,6 +136,10 @@ git config --add lfs.url "http://gitlab.example.com/group/project.git/info/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 ### 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 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. the LFS object on every push for every object, user HTTPS credentials are required.
......
...@@ -82,6 +82,19 @@ module API ...@@ -82,6 +82,19 @@ module API
response response
end 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 get "/merge_request_urls" do
::MergeRequests::GetUrlsService.new(project).execute(params[:changes]) ::MergeRequests::GetUrlsService.new(project).execute(params[:changes])
end end
......
...@@ -11,6 +11,7 @@ module Gitlab ...@@ -11,6 +11,7 @@ module Gitlab
build_access_token_check(login, password) || build_access_token_check(login, password) ||
user_with_password_for_git(login, password) || user_with_password_for_git(login, password) ||
oauth_access_token_check(login, password) || oauth_access_token_check(login, password) ||
lfs_token_check(login, password) ||
personal_access_token_check(login, password) || personal_access_token_check(login, password) ||
Gitlab::Auth::Result.new Gitlab::Auth::Result.new
...@@ -102,6 +103,30 @@ module Gitlab ...@@ -102,6 +103,30 @@ module Gitlab
end end
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) def build_access_token_check(login, password)
return unless login == 'gitlab-ci-token' return unless login == 'gitlab-ci-token'
return unless password return unless password
......
module Gitlab module Gitlab
module Auth module Auth
Result = Struct.new(:actor, :project, :type, :authentication_abilities) do Result = Struct.new(:actor, :project, :type, :authentication_abilities) do
def ci? def ci?(for_project)
type == :ci type == :ci &&
project &&
project == for_project
end
def lfs_deploy_token?(for_project)
type == :lfs_deploy_token &&
actor &&
actor.projects.include?(for_project)
end end
def success? def success?
......
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
...@@ -61,6 +61,24 @@ describe Gitlab::Auth, lib: true do ...@@ -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)) 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 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 it 'recognizes OAuth tokens' do
user = create(:user) user = create(:user)
application = Doorkeeper::Application.create!(name: "MyApp", redirect_uri: "https://app.com", owner: user) application = Doorkeeper::Application.create!(name: "MyApp", redirect_uri: "https://app.com", owner: user)
......
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
...@@ -100,6 +100,43 @@ describe API::API, api: true do ...@@ -100,6 +100,43 @@ describe API::API, api: true do
end end
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 describe "GET /internal/discover" do
it do it do
get(api("/internal/discover"), key_id: key.id, secret_token: secret_token) get(api("/internal/discover"), key_id: key.id, secret_token: secret_token)
...@@ -389,4 +426,13 @@ describe API::API, api: true do ...@@ -389,4 +426,13 @@ describe API::API, api: true do
protocol: 'ssh' protocol: 'ssh'
) )
end 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 end
...@@ -245,6 +245,18 @@ describe 'Git LFS API and storage' do ...@@ -245,6 +245,18 @@ describe 'Git LFS API and storage' do
end end
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 context 'when build is authorized as' do
let(:authorization) { authorize_ci_project } let(:authorization) { authorize_ci_project }
...@@ -1097,6 +1109,10 @@ describe 'Git LFS API and storage' do ...@@ -1097,6 +1109,10 @@ describe 'Git LFS API and storage' do
ActionController::HttpAuthentication::Basic.encode_credentials(user.username, user.password) ActionController::HttpAuthentication::Basic.encode_credentials(user.username, user.password)
end 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) def fork_project(project, user, object = nil)
allow(RepositoryForkWorker).to receive(:perform_async).and_return(true) allow(RepositoryForkWorker).to receive(:perform_async).and_return(true)
Projects::ForkService.new(project, user, {}).execute Projects::ForkService.new(project, user, {}).execute
......
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