Refactored LFS auth logic to use its own API endpoint.

Author: suprnova32

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?

lib/gitlab_lfs_authentication.rb

@@ -2,17 +2,23 @@
 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/"
  }

lib/gitlab_net.rb

@@ -6,6 +6,7 @@
 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 ApiUnreachableError < StandardError; end
  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 @@ def check_access(cmd, repo, actor, changes, protocol)
  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 @@ def discover(key)
  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 @@ def redis_client
 
  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

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 InvalidRepositoryPathError < StandardError; end
  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 @@ def verify_access
  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 @@ def gcryptsetup?(args)
  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

spec/gitlab_access_spec.rb

@@ -7,7 +7,7 @@
  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 @@
  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

spec/gitlab_lfs_authentication_spec.rb

@@ -5,11 +5,12 @@
  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
 

spec/gitlab_net_spec.rb

@@ -38,7 +38,6 @@
  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 @@
  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 @@
  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
 

spec/gitlab_shell_spec.rb

@@ -36,7 +36,6 @@
 
  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 @@
  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)