Refactored LFS auth logic to use its own API endpoint.

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