diff options
author | Patricio Cano <suprnova32@gmail.com> | 2016-08-30 13:37:09 -0500 |
---|---|---|
committer | Patricio Cano <suprnova32@gmail.com> | 2016-09-06 12:11:17 -0500 |
commit | f53d09e1eb1323be9cd697813a6f47375c091f6a (patch) | |
tree | 42b1950e5a8f0a7d3f97cf37e1c279793fc7c30d | |
parent | c16f7323bad61601df1ebe93475bd84aee532faf (diff) | |
download | gitlab-shell-f53d09e1eb1323be9cd697813a6f47375c091f6a.tar.gz |
Refactored LFS auth logic to use its own API endpoint.
-rw-r--r-- | lib/gitlab_access_status.rb | 7 | ||||
-rw-r--r-- | lib/gitlab_lfs_authentication.rb | 14 | ||||
-rw-r--r-- | lib/gitlab_net.rb | 27 | ||||
-rw-r--r-- | lib/gitlab_shell.rb | 10 | ||||
-rw-r--r-- | spec/gitlab_access_spec.rb | 4 | ||||
-rw-r--r-- | spec/gitlab_lfs_authentication_spec.rb | 5 | ||||
-rw-r--r-- | spec/gitlab_net_spec.rb | 15 | ||||
-rw-r--r-- | spec/gitlab_shell_spec.rb | 3 | ||||
-rw-r--r-- | spec/vcr_cassettes/allowed-pull.yml | 2 | ||||
-rw-r--r-- | spec/vcr_cassettes/discover-ok.yml | 2 | ||||
-rw-r--r-- | spec/vcr_cassettes/lfs-authenticate-ok.yml | 46 |
11 files changed, 107 insertions, 28 deletions
diff --git a/lib/gitlab_access_status.rb b/lib/gitlab_access_status.rb index 1ae0528..7fb88be 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 b05da21..4b36229 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 42ff94c..994f8d5 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 87fa347..d3f9bbe 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 e2a0c77..2781aa9 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 f4fee77..ff715cf 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 cee5b91..3d38231 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 3c0fbf5..a5e75e2 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 <git-upload-pack gitlab-ci.git> " 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 8e01cd3..5a10ec9 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 c2cee40..a86243c 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 0000000..f3e4d79 --- /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 |