From dbf374e10ad859a02ef69af53031e245913b6e65 Mon Sep 17 00:00:00 2001 From: Patricio Cano Date: Thu, 25 Aug 2016 17:34:34 -0500 Subject: [PATCH 1/6] Added LFS support to SSH - Required changes to GitLab Shell include the actual handling of the `git-lfs-authenticate` command and the retrieval of the correct credentials. --- CHANGELOG | 1 + lib/gitlab_access_status.rb | 7 ++++--- lib/gitlab_net.rb | 2 +- lib/gitlab_shell.rb | 23 ++++++++++++++++++++++- spec/gitlab_access_spec.rb | 4 ++-- spec/gitlab_net_spec.rb | 3 +++ spec/gitlab_shell_spec.rb | 16 +++++++++++++++- spec/vcr_cassettes/allowed-pull.yml | 2 +- spec/vcr_cassettes/discover-ok.yml | 2 +- 9 files changed, 50 insertions(+), 10 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index bce87832..bd2a2a4a 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,5 +1,6 @@ v3.5.0 - Add option to recover 2FA via SSH + - Added full support for `git-lfs-authenticate` to properly handle LFS requests and pass them on to Workhorse v3.4.0 - Redis Sentinel support diff --git a/lib/gitlab_access_status.rb b/lib/gitlab_access_status.rb index 7fb88bee..1ae05284 100644 --- a/lib/gitlab_access_status.rb +++ b/lib/gitlab_access_status.rb @@ -1,17 +1,18 @@ require 'json' class GitAccessStatus - attr_reader :message, :repository_path + attr_reader :message, :repository_path, :repository_http_path - def initialize(status, message, repository_path) + def initialize(status, message, repository_path, repository_http_path) @status = status @message = message @repository_path = repository_path + @repository_http_path = repository_http_path end def self.create_from_json(json) values = JSON.parse(json) - self.new(values["status"], values["message"], values["repository_path"]) + self.new(values["status"], values["message"], values["repository_path"], values["repository_http_path"]) end def allowed? diff --git a/lib/gitlab_net.rb b/lib/gitlab_net.rb index 47bae957..42ff94c8 100644 --- a/lib/gitlab_net.rb +++ b/lib/gitlab_net.rb @@ -39,7 +39,7 @@ class GitlabNet if resp.code == '200' GitAccessStatus.create_from_json(resp.body) else - GitAccessStatus.new(false, 'API is not accessible', nil) + GitAccessStatus.new(false, 'API is not accessible', nil, nil) end end diff --git a/lib/gitlab_shell.rb b/lib/gitlab_shell.rb index 1fdb9e5f..ab98a6e5 100644 --- a/lib/gitlab_shell.rb +++ b/lib/gitlab_shell.rb @@ -1,4 +1,6 @@ require 'shellwords' +require 'base64' +require 'json' require_relative 'gitlab_net' @@ -11,7 +13,7 @@ class GitlabShell API_COMMANDS = %w(2fa_recovery_codes) GL_PROTOCOL = 'ssh'.freeze - attr_accessor :key_id, :repo_name, :command + attr_accessor :key_id, :repo_name, :command, :git_access, :repository_http_path attr_reader :repo_path def initialize(key_id) @@ -94,6 +96,7 @@ class GitlabShell raise AccessDeniedError, status.message unless status.allowed? self.repo_path = status.repository_path + @repository_http_path = status.repository_http_path end def process_cmd(args) @@ -117,6 +120,11 @@ class GitlabShell $logger.info "gitlab-shell: executing git-annex command <#{parsed_args.join(' ')}> for #{log_username}." exec_cmd(*parsed_args) + + elsif @command == 'git-lfs-authenticate' + $logger.info "gitlab-shell: Processing LFS authentication for #{log_username}." + lfs_authenticate + else $logger.info "gitlab-shell: executing git command <#{@command} #{repo_path}> for #{log_username}." exec_cmd(@command, repo_path) @@ -184,6 +192,19 @@ class GitlabShell non_dashed[0, 2] == %w{git-annex-shell gcryptsetup} end + def lfs_authenticate + return unless user + + authorization = { + header: { + Authorization: "Basic #{Base64.strict_encode64("#{user['username']}:#{user['lfs_token']}")}" + }, + href: "#{repository_http_path}/info/lfs/" + } + + puts JSON.generate(authorization) + end + private def continue?(question) diff --git a/spec/gitlab_access_spec.rb b/spec/gitlab_access_spec.rb index 2781aa99..e2a0c77c 100644 --- a/spec/gitlab_access_spec.rb +++ b/spec/gitlab_access_spec.rb @@ -7,7 +7,7 @@ describe GitlabAccess do let(:repo_path) { File.join(repository_path, repo_name) + ".git" } let(:api) do double(GitlabNet).tap do |api| - api.stub(check_access: GitAccessStatus.new(true, 'ok', '/home/git/repositories')) + api.stub(check_access: GitAccessStatus.new(true, 'ok', '/home/git/repositories', 'http://gitlab.dev/repo')) end end subject do @@ -39,7 +39,7 @@ describe GitlabAccess do context "access is denied" do before do - api.stub(check_access: GitAccessStatus.new(false, 'denied', nil)) + api.stub(check_access: GitAccessStatus.new(false, 'denied', nil, nil)) end it "returns false" do diff --git a/spec/gitlab_net_spec.rb b/spec/gitlab_net_spec.rb index bcd0d798..cee5b911 100644 --- a/spec/gitlab_net_spec.rb +++ b/spec/gitlab_net_spec.rb @@ -38,6 +38,8 @@ describe GitlabNet, vcr: true do VCR.use_cassette("discover-ok") do user = gitlab_net.discover('key-126') user['name'].should == 'Dmitriy Zaporozhets' + user['lfs_token'].should == 'wsnys8Zm8Jn7zyhHTAAK' + user['username'].should == 'dzaporozhets' end end @@ -130,6 +132,7 @@ describe GitlabNet, vcr: true do VCR.use_cassette("allowed-pull") do access = gitlab_net.check_access('git-receive-pack', 'gitlab/gitlabhq.git', 'key-126', changes, 'ssh') access.allowed?.should be_true + access.repository_http_path.should == 'http://gitlab.dev/gitlab/gitlabhq.git' end end diff --git a/spec/gitlab_shell_spec.rb b/spec/gitlab_shell_spec.rb index ea116524..378b9642 100644 --- a/spec/gitlab_shell_spec.rb +++ b/spec/gitlab_shell_spec.rb @@ -36,6 +36,7 @@ describe GitlabShell do let(:repo_name) { 'gitlab-ci.git' } let(:repo_path) { File.join(tmp_repos_path, repo_name) } + let(:repo_http_path) { 'http://gitlab.dev/dzaporozhets/gitlab.git' } before do GitlabConfig.any_instance.stub(audit_usernames: false) @@ -112,6 +113,19 @@ describe GitlabShell do its(:repo_name) { should == 'dzaporozhets/gitlab.git' } its(:command) { should == 'git-annex-shell' } end + + describe 'git-lfs' do + let(:repo_name) { 'dzaporozhets/gitlab.git' } + let(:ssh_args) { %W(git-lfs-authenticate dzaporozhets/gitlab.git download) } + + before do + subject.send :parse_cmd, ssh_args + end + + its(:repo_name) { should == 'dzaporozhets/gitlab.git' } + its(:command) { should == 'git-lfs-authenticate' } + its(:git_access) { should == 'git-upload-pack' } + end end describe :exec do @@ -306,7 +320,7 @@ describe GitlabShell do end it "should disallow access and log the attempt if check_access returns false status" do - api.stub(check_access: GitAccessStatus.new(false, 'denied', nil)) + api.stub(check_access: GitAccessStatus.new(false, 'denied', nil, nil)) message = "gitlab-shell: Access denied for git command " message << "by user with key #{key_id}." $logger.should_receive(:warn).with(message) diff --git a/spec/vcr_cassettes/allowed-pull.yml b/spec/vcr_cassettes/allowed-pull.yml index 5a10ec9b..8e01cd37 100644 --- a/spec/vcr_cassettes/allowed-pull.yml +++ b/spec/vcr_cassettes/allowed-pull.yml @@ -42,7 +42,7 @@ http_interactions: - '0.089741' body: encoding: UTF-8 - string: '{"status": "true"}' + string: '{"status": "true","repository_http_path": "http://gitlab.dev/gitlab/gitlabhq.git"}' http_version: recorded_at: Wed, 03 Sep 2014 11:27:36 GMT recorded_with: VCR 2.4.0 diff --git a/spec/vcr_cassettes/discover-ok.yml b/spec/vcr_cassettes/discover-ok.yml index a86243cf..c2cee401 100644 --- a/spec/vcr_cassettes/discover-ok.yml +++ b/spec/vcr_cassettes/discover-ok.yml @@ -40,7 +40,7 @@ http_interactions: - '0.016934' body: encoding: UTF-8 - string: '{"name":"Dmitriy Zaporozhets","username":"dzaporozhets"}' + string: '{"name":"Dmitriy Zaporozhets","username":"dzaporozhets","lfs_token":"wsnys8Zm8Jn7zyhHTAAK"}' http_version: recorded_at: Wed, 03 Sep 2014 11:27:35 GMT recorded_with: VCR 2.4.0 -- GitLab From a64f174d83caddfd3fe31698b5b2fd7701b5a573 Mon Sep 17 00:00:00 2001 From: Patricio Cano Date: Mon, 29 Aug 2016 14:41:39 -0500 Subject: [PATCH 2/6] Refactored JSON header generation to its own class and added tests for it --- lib/gitlab_lfs_authentication.rb | 22 ++++++++++++++++++++++ lib/gitlab_shell.rb | 12 ++---------- spec/gitlab_lfs_authentication_spec.rb | 21 +++++++++++++++++++++ 3 files changed, 45 insertions(+), 10 deletions(-) create mode 100644 lib/gitlab_lfs_authentication.rb create mode 100644 spec/gitlab_lfs_authentication_spec.rb diff --git a/lib/gitlab_lfs_authentication.rb b/lib/gitlab_lfs_authentication.rb new file mode 100644 index 00000000..b05da21c --- /dev/null +++ b/lib/gitlab_lfs_authentication.rb @@ -0,0 +1,22 @@ +require 'base64' +require 'json' + +class GitlabLfsAuthentication + attr_accessor :user, :repository_http_path + + def initialize(user, repository_http_path) + @user = user + @repository_http_path = repository_http_path + end + + def authenticate! + authorization = { + header: { + Authorization: "Basic #{Base64.strict_encode64("#{user['username']}:#{user['lfs_token']}")}" + }, + href: "#{repository_http_path}/info/lfs/" + } + + JSON.generate(authorization) + end +end diff --git a/lib/gitlab_shell.rb b/lib/gitlab_shell.rb index ab98a6e5..87fa3479 100644 --- a/lib/gitlab_shell.rb +++ b/lib/gitlab_shell.rb @@ -1,8 +1,7 @@ require 'shellwords' -require 'base64' -require 'json' require_relative 'gitlab_net' +require_relative 'gitlab_lfs_authentication' class GitlabShell class AccessDeniedError < StandardError; end @@ -195,14 +194,7 @@ class GitlabShell def lfs_authenticate return unless user - authorization = { - header: { - Authorization: "Basic #{Base64.strict_encode64("#{user['username']}:#{user['lfs_token']}")}" - }, - href: "#{repository_http_path}/info/lfs/" - } - - puts JSON.generate(authorization) + puts GitlabLfsAuthentication.new(user, repository_http_path).authenticate! end private diff --git a/spec/gitlab_lfs_authentication_spec.rb b/spec/gitlab_lfs_authentication_spec.rb new file mode 100644 index 00000000..f4fee77a --- /dev/null +++ b/spec/gitlab_lfs_authentication_spec.rb @@ -0,0 +1,21 @@ +require 'spec_helper' +require 'gitlab_lfs_authentication' + +describe GitlabLfsAuthentication do + let(:user) { { 'username' => 'dzaporozhets', 'lfs_token' => 'wsnys8Zm8Jn7zyhHTAAK' } } + + subject do + GitlabLfsAuthentication.new(user, 'http://gitlab.dev/repo') + end + + describe '#initialize' do + it { subject.user.should == user } + it { subject.repository_http_path.should == 'http://gitlab.dev/repo' } + end + + describe '#authenticate!' do + result = "{\"header\":{\"Authorization\":\"Basic ZHphcG9yb3poZXRzOndzbnlzOFptOEpuN3p5aEhUQUFL\"},\"href\":\"http://gitlab.dev/repo/info/lfs/\"}" + + it { subject.authenticate!.should == result } + end +end -- GitLab From c16f7323bad61601df1ebe93475bd84aee532faf Mon Sep 17 00:00:00 2001 From: Patricio Cano Date: Mon, 29 Aug 2016 20:28:18 -0500 Subject: [PATCH 3/6] Added test for old Git LFS clients that submit an extra :oid argument to `git-lfs-authenticate` --- spec/gitlab_shell_spec.rb | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/spec/gitlab_shell_spec.rb b/spec/gitlab_shell_spec.rb index 378b9642..3c0fbf5b 100644 --- a/spec/gitlab_shell_spec.rb +++ b/spec/gitlab_shell_spec.rb @@ -126,6 +126,19 @@ describe GitlabShell do its(:command) { should == 'git-lfs-authenticate' } its(:git_access) { should == 'git-upload-pack' } end + + describe 'git-lfs old clients' do + let(:repo_name) { 'dzaporozhets/gitlab.git' } + let(:ssh_args) { %W(git-lfs-authenticate dzaporozhets/gitlab.git download long_oid) } + + before do + subject.send :parse_cmd, ssh_args + end + + its(:repo_name) { should == 'dzaporozhets/gitlab.git' } + its(:git_cmd) { should == 'git-lfs-authenticate' } + its(:git_access) { should == 'git-upload-pack' } + end end describe :exec do -- GitLab From f53d09e1eb1323be9cd697813a6f47375c091f6a Mon Sep 17 00:00:00 2001 From: Patricio Cano Date: Tue, 30 Aug 2016 13:37:09 -0500 Subject: [PATCH 4/6] Refactored LFS auth logic to use its own API endpoint. --- lib/gitlab_access_status.rb | 7 ++-- lib/gitlab_lfs_authentication.rb | 14 +++++-- lib/gitlab_net.rb | 27 ++++++++++--- lib/gitlab_shell.rb | 10 ++--- spec/gitlab_access_spec.rb | 4 +- spec/gitlab_lfs_authentication_spec.rb | 5 ++- spec/gitlab_net_spec.rb | 15 ++++++- spec/gitlab_shell_spec.rb | 3 +- spec/vcr_cassettes/allowed-pull.yml | 2 +- spec/vcr_cassettes/discover-ok.yml | 2 +- spec/vcr_cassettes/lfs-authenticate-ok.yml | 46 ++++++++++++++++++++++ 11 files changed, 107 insertions(+), 28 deletions(-) create mode 100644 spec/vcr_cassettes/lfs-authenticate-ok.yml diff --git a/lib/gitlab_access_status.rb b/lib/gitlab_access_status.rb index 1ae05284..7fb88bee 100644 --- a/lib/gitlab_access_status.rb +++ b/lib/gitlab_access_status.rb @@ -1,18 +1,17 @@ require 'json' class GitAccessStatus - attr_reader :message, :repository_path, :repository_http_path + attr_reader :message, :repository_path - def initialize(status, message, repository_path, repository_http_path) + def initialize(status, message, repository_path) @status = status @message = message @repository_path = repository_path - @repository_http_path = repository_http_path end def self.create_from_json(json) values = JSON.parse(json) - self.new(values["status"], values["message"], values["repository_path"], values["repository_http_path"]) + self.new(values["status"], values["message"], values["repository_path"]) end def allowed? diff --git a/lib/gitlab_lfs_authentication.rb b/lib/gitlab_lfs_authentication.rb index b05da21c..4b362290 100644 --- a/lib/gitlab_lfs_authentication.rb +++ b/lib/gitlab_lfs_authentication.rb @@ -2,17 +2,23 @@ require 'base64' require 'json' class GitlabLfsAuthentication - attr_accessor :user, :repository_http_path + attr_accessor :username, :lfs_token, :repository_http_path - def initialize(user, repository_http_path) - @user = user + def initialize(username, lfs_token, repository_http_path) + @username = username + @lfs_token = lfs_token @repository_http_path = repository_http_path end + def self.build_from_json(json) + values = JSON.parse(json) + self.new(values['username'], values['lfs_token'], values['repository_http_path']) + end + def authenticate! authorization = { header: { - Authorization: "Basic #{Base64.strict_encode64("#{user['username']}:#{user['lfs_token']}")}" + Authorization: "Basic #{Base64.strict_encode64("#{username}:#{lfs_token}")}" }, href: "#{repository_http_path}/info/lfs/" } diff --git a/lib/gitlab_net.rb b/lib/gitlab_net.rb index 42ff94c8..994f8d51 100644 --- a/lib/gitlab_net.rb +++ b/lib/gitlab_net.rb @@ -6,6 +6,7 @@ require_relative 'gitlab_config' require_relative 'gitlab_logger' require_relative 'gitlab_access' require_relative 'gitlab_redis' +require_relative 'gitlab_lfs_authentication' require_relative 'httpunix' class GitlabNet @@ -15,15 +16,12 @@ class GitlabNet READ_TIMEOUT = 300 def check_access(cmd, repo, actor, changes, protocol) - project_name = repo.gsub("'", "") - project_name = project_name.gsub(/\.git\Z/, "") - project_name = project_name.gsub(/\A\//, "") changes = changes.join("\n") unless changes.kind_of?(String) params = { action: cmd, changes: changes, - project: project_name, + project: project_name(repo), protocol: protocol } @@ -39,7 +37,7 @@ class GitlabNet if resp.code == '200' GitAccessStatus.create_from_json(resp.body) else - GitAccessStatus.new(false, 'API is not accessible', nil, nil) + GitAccessStatus.new(false, 'API is not accessible', nil) end end @@ -49,6 +47,19 @@ class GitlabNet JSON.parse(resp.body) rescue nil end + def lfs_authenticate(key, repo) + params = { + project: project_name(repo), + key_id: key.gsub('key-', '') + } + + resp = post("#{host}/lfs_authenticate", params) + + if resp.code == '200' + GitlabLfsAuthentication.build_from_json(resp.body) + end + end + def broadcast_message resp = get("#{host}/broadcast_message") JSON.parse(resp.body) rescue {} @@ -107,6 +118,12 @@ class GitlabNet protected + def project_name(repo) + project_name = repo.gsub("'", "") + project_name = project_name.gsub(/\.git\Z/, "") + project_name.gsub(/\A\//, "") + end + def config @config ||= GitlabConfig.new end diff --git a/lib/gitlab_shell.rb b/lib/gitlab_shell.rb index 87fa3479..d3f9bbec 100644 --- a/lib/gitlab_shell.rb +++ b/lib/gitlab_shell.rb @@ -1,7 +1,6 @@ require 'shellwords' require_relative 'gitlab_net' -require_relative 'gitlab_lfs_authentication' class GitlabShell class AccessDeniedError < StandardError; end @@ -12,7 +11,7 @@ class GitlabShell API_COMMANDS = %w(2fa_recovery_codes) GL_PROTOCOL = 'ssh'.freeze - attr_accessor :key_id, :repo_name, :command, :git_access, :repository_http_path + attr_accessor :key_id, :repo_name, :command, :git_access attr_reader :repo_path def initialize(key_id) @@ -95,7 +94,6 @@ class GitlabShell raise AccessDeniedError, status.message unless status.allowed? self.repo_path = status.repository_path - @repository_http_path = status.repository_http_path end def process_cmd(args) @@ -192,9 +190,11 @@ class GitlabShell end def lfs_authenticate - return unless user + lfs_access = api.lfs_authenticate(@key_id, @repo_name) - puts GitlabLfsAuthentication.new(user, repository_http_path).authenticate! + return unless lfs_access + + puts lfs_access.authenticate! end private diff --git a/spec/gitlab_access_spec.rb b/spec/gitlab_access_spec.rb index e2a0c77c..2781aa99 100644 --- a/spec/gitlab_access_spec.rb +++ b/spec/gitlab_access_spec.rb @@ -7,7 +7,7 @@ describe GitlabAccess do let(:repo_path) { File.join(repository_path, repo_name) + ".git" } let(:api) do double(GitlabNet).tap do |api| - api.stub(check_access: GitAccessStatus.new(true, 'ok', '/home/git/repositories', 'http://gitlab.dev/repo')) + api.stub(check_access: GitAccessStatus.new(true, 'ok', '/home/git/repositories')) end end subject do @@ -39,7 +39,7 @@ describe GitlabAccess do context "access is denied" do before do - api.stub(check_access: GitAccessStatus.new(false, 'denied', nil, nil)) + api.stub(check_access: GitAccessStatus.new(false, 'denied', nil)) end it "returns false" do diff --git a/spec/gitlab_lfs_authentication_spec.rb b/spec/gitlab_lfs_authentication_spec.rb index f4fee77a..ff715cff 100644 --- a/spec/gitlab_lfs_authentication_spec.rb +++ b/spec/gitlab_lfs_authentication_spec.rb @@ -5,11 +5,12 @@ describe GitlabLfsAuthentication do let(:user) { { 'username' => 'dzaporozhets', 'lfs_token' => 'wsnys8Zm8Jn7zyhHTAAK' } } subject do - GitlabLfsAuthentication.new(user, 'http://gitlab.dev/repo') + GitlabLfsAuthentication.new('dzaporozhets', 'wsnys8Zm8Jn7zyhHTAAK', 'http://gitlab.dev/repo') end describe '#initialize' do - it { subject.user.should == user } + it { subject.username.should == 'dzaporozhets' } + it { subject.lfs_token.should == 'wsnys8Zm8Jn7zyhHTAAK' } it { subject.repository_http_path.should == 'http://gitlab.dev/repo' } end diff --git a/spec/gitlab_net_spec.rb b/spec/gitlab_net_spec.rb index cee5b911..3d382318 100644 --- a/spec/gitlab_net_spec.rb +++ b/spec/gitlab_net_spec.rb @@ -38,7 +38,6 @@ describe GitlabNet, vcr: true do VCR.use_cassette("discover-ok") do user = gitlab_net.discover('key-126') user['name'].should == 'Dmitriy Zaporozhets' - user['lfs_token'].should == 'wsnys8Zm8Jn7zyhHTAAK' user['username'].should == 'dzaporozhets' end end @@ -58,6 +57,19 @@ describe GitlabNet, vcr: true do end end + describe '#lfs_authenticate' do + context 'lfs authentication succeeded' do + it 'should return the correct data' do + VCR.use_cassette('lfs-authenticate-ok') do + lfs_access = gitlab_net.lfs_authenticate('key-126', 'gitlab/gitlabhq.git') + lfs_access.username.should == 'dzaporozhets' + lfs_access.lfs_token.should == 'wsnys8Zm8Jn7zyhHTAAK' + lfs_access.repository_http_path.should == 'http://gitlab.dev/gitlab/gitlabhq.git' + end + end + end + end + describe :broadcast_message do context "broadcast message exists" do it 'should return message' do @@ -132,7 +144,6 @@ describe GitlabNet, vcr: true do VCR.use_cassette("allowed-pull") do access = gitlab_net.check_access('git-receive-pack', 'gitlab/gitlabhq.git', 'key-126', changes, 'ssh') access.allowed?.should be_true - access.repository_http_path.should == 'http://gitlab.dev/gitlab/gitlabhq.git' end end diff --git a/spec/gitlab_shell_spec.rb b/spec/gitlab_shell_spec.rb index 3c0fbf5b..a5e75e25 100644 --- a/spec/gitlab_shell_spec.rb +++ b/spec/gitlab_shell_spec.rb @@ -36,7 +36,6 @@ describe GitlabShell do let(:repo_name) { 'gitlab-ci.git' } let(:repo_path) { File.join(tmp_repos_path, repo_name) } - let(:repo_http_path) { 'http://gitlab.dev/dzaporozhets/gitlab.git' } before do GitlabConfig.any_instance.stub(audit_usernames: false) @@ -333,7 +332,7 @@ describe GitlabShell do end it "should disallow access and log the attempt if check_access returns false status" do - api.stub(check_access: GitAccessStatus.new(false, 'denied', nil, nil)) + api.stub(check_access: GitAccessStatus.new(false, 'denied', nil)) message = "gitlab-shell: Access denied for git command " message << "by user with key #{key_id}." $logger.should_receive(:warn).with(message) diff --git a/spec/vcr_cassettes/allowed-pull.yml b/spec/vcr_cassettes/allowed-pull.yml index 8e01cd37..5a10ec9b 100644 --- a/spec/vcr_cassettes/allowed-pull.yml +++ b/spec/vcr_cassettes/allowed-pull.yml @@ -42,7 +42,7 @@ http_interactions: - '0.089741' body: encoding: UTF-8 - string: '{"status": "true","repository_http_path": "http://gitlab.dev/gitlab/gitlabhq.git"}' + string: '{"status": "true"}' http_version: recorded_at: Wed, 03 Sep 2014 11:27:36 GMT recorded_with: VCR 2.4.0 diff --git a/spec/vcr_cassettes/discover-ok.yml b/spec/vcr_cassettes/discover-ok.yml index c2cee401..a86243cf 100644 --- a/spec/vcr_cassettes/discover-ok.yml +++ b/spec/vcr_cassettes/discover-ok.yml @@ -40,7 +40,7 @@ http_interactions: - '0.016934' body: encoding: UTF-8 - string: '{"name":"Dmitriy Zaporozhets","username":"dzaporozhets","lfs_token":"wsnys8Zm8Jn7zyhHTAAK"}' + string: '{"name":"Dmitriy Zaporozhets","username":"dzaporozhets"}' http_version: recorded_at: Wed, 03 Sep 2014 11:27:35 GMT recorded_with: VCR 2.4.0 diff --git a/spec/vcr_cassettes/lfs-authenticate-ok.yml b/spec/vcr_cassettes/lfs-authenticate-ok.yml new file mode 100644 index 00000000..f3e4d79b --- /dev/null +++ b/spec/vcr_cassettes/lfs-authenticate-ok.yml @@ -0,0 +1,46 @@ +--- +http_interactions: +- request: + method: post + uri: https://dev.gitlab.org/api/v3/internal/lfs_authenticate + body: + encoding: US-ASCII + string: project=gitlab%2Fgitlabhq&key_id=126&secret_token=a123 + headers: + Accept-Encoding: + - gzip;q=1.0,deflate;q=0.6,identity;q=0.3 + Accept: + - "*/*" + User-Agent: + - Ruby + response: + status: + code: 200 + message: OK + headers: + Server: + - nginx/1.1.19 + Date: + - Wed, 03 Sep 2014 11:27:35 GMT + Content-Type: + - application/json + Content-Length: + - '56' + Connection: + - keep-alive + Status: + - 200 OK + Etag: + - '"1d75c1cf3d4bfa4d2b7bb6a0bcfd7f55"' + Cache-Control: + - max-age=0, private, must-revalidate + X-Request-Id: + - ef4513ae-0424-4941-8be0-b5a3a7b4bf12 + X-Runtime: + - '0.016934' + body: + encoding: UTF-8 + string: '{"username":"dzaporozhets","lfs_token":"wsnys8Zm8Jn7zyhHTAAK","repository_http_path":"http://gitlab.dev/gitlab/gitlabhq.git"}' + http_version: + recorded_at: Wed, 03 Sep 2014 11:27:35 GMT +recorded_with: VCR 2.4.0 -- GitLab From a070ae4a320f8da830862e1dd11eea8b1281ab8c Mon Sep 17 00:00:00 2001 From: Patricio Cano Date: Tue, 6 Sep 2016 16:13:04 -0500 Subject: [PATCH 5/6] Style fixes and better tests. --- lib/gitlab_lfs_authentication.rb | 4 ++-- lib/gitlab_shell.rb | 2 +- spec/gitlab_lfs_authentication_spec.rb | 27 ++++++++++++++++++++------ spec/gitlab_shell_spec.rb | 2 +- 4 files changed, 25 insertions(+), 10 deletions(-) diff --git a/lib/gitlab_lfs_authentication.rb b/lib/gitlab_lfs_authentication.rb index 4b362290..196cdd74 100644 --- a/lib/gitlab_lfs_authentication.rb +++ b/lib/gitlab_lfs_authentication.rb @@ -11,11 +11,11 @@ class GitlabLfsAuthentication end def self.build_from_json(json) - values = JSON.parse(json) + values = JSON.parse(json) rescue nil self.new(values['username'], values['lfs_token'], values['repository_http_path']) end - def authenticate! + def authentication_payload authorization = { header: { Authorization: "Basic #{Base64.strict_encode64("#{username}:#{lfs_token}")}" diff --git a/lib/gitlab_shell.rb b/lib/gitlab_shell.rb index d3f9bbec..971b22ff 100644 --- a/lib/gitlab_shell.rb +++ b/lib/gitlab_shell.rb @@ -194,7 +194,7 @@ class GitlabShell return unless lfs_access - puts lfs_access.authenticate! + puts lfs_access.authentication_payload end private diff --git a/spec/gitlab_lfs_authentication_spec.rb b/spec/gitlab_lfs_authentication_spec.rb index ff715cff..9e93a07d 100644 --- a/spec/gitlab_lfs_authentication_spec.rb +++ b/spec/gitlab_lfs_authentication_spec.rb @@ -1,22 +1,37 @@ require 'spec_helper' require 'gitlab_lfs_authentication' +require 'json' describe GitlabLfsAuthentication do - let(:user) { { 'username' => 'dzaporozhets', 'lfs_token' => 'wsnys8Zm8Jn7zyhHTAAK' } } - subject do - GitlabLfsAuthentication.new('dzaporozhets', 'wsnys8Zm8Jn7zyhHTAAK', 'http://gitlab.dev/repo') + GitlabLfsAuthentication.build_from_json( + JSON.generate( + { + username: 'dzaporozhets', + lfs_token: 'wsnys8Zm8Jn7zyhHTAAK', + repository_http_path: 'http://gitlab.dev/repo' + } + ) + ) end - describe '#initialize' do + describe '#build_from_json' do it { subject.username.should == 'dzaporozhets' } it { subject.lfs_token.should == 'wsnys8Zm8Jn7zyhHTAAK' } it { subject.repository_http_path.should == 'http://gitlab.dev/repo' } end - describe '#authenticate!' do + describe '#authentication_payload' do result = "{\"header\":{\"Authorization\":\"Basic ZHphcG9yb3poZXRzOndzbnlzOFptOEpuN3p5aEhUQUFL\"},\"href\":\"http://gitlab.dev/repo/info/lfs/\"}" - it { subject.authenticate!.should == result } + it { subject.authentication_payload.should eq(result) } + + it 'should be a proper JSON' do + payload = subject.authentication_payload + json_payload = JSON.parse(payload) + + json_payload['header']['Authorization'].should eq('Basic ZHphcG9yb3poZXRzOndzbnlzOFptOEpuN3p5aEhUQUFL') + json_payload['href'].should eq('http://gitlab.dev/repo/info/lfs/') + end end end diff --git a/spec/gitlab_shell_spec.rb b/spec/gitlab_shell_spec.rb index a5e75e25..96cae405 100644 --- a/spec/gitlab_shell_spec.rb +++ b/spec/gitlab_shell_spec.rb @@ -135,7 +135,7 @@ describe GitlabShell do end its(:repo_name) { should == 'dzaporozhets/gitlab.git' } - its(:git_cmd) { should == 'git-lfs-authenticate' } + its(:command) { should == 'git-lfs-authenticate' } its(:git_access) { should == 'git-upload-pack' } end end -- GitLab From 3c9ef9eba3a188cb7d742c4be5c75fca6ea9de80 Mon Sep 17 00:00:00 2001 From: Patricio Cano Date: Thu, 8 Sep 2016 13:08:05 -0500 Subject: [PATCH 6/6] Properly rescue from JSON parse. --- lib/gitlab_lfs_authentication.rb | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/gitlab_lfs_authentication.rb b/lib/gitlab_lfs_authentication.rb index 196cdd74..96d06d8f 100644 --- a/lib/gitlab_lfs_authentication.rb +++ b/lib/gitlab_lfs_authentication.rb @@ -11,8 +11,12 @@ class GitlabLfsAuthentication end def self.build_from_json(json) - values = JSON.parse(json) rescue nil - self.new(values['username'], values['lfs_token'], values['repository_http_path']) + begin + values = JSON.parse(json) + self.new(values['username'], values['lfs_token'], values['repository_http_path']) + rescue + nil + end end def authentication_payload -- GitLab