From 71aff7f6a3ab63f1395bfab6ea49f0175fe08167 Mon Sep 17 00:00:00 2001
From: Patricio Cano <suprnova32@gmail.com>
Date: Wed, 7 Sep 2016 11:55:54 -0500
Subject: Use special characters for `lfs+deploy-key` to prevent a someone from
 creating a user with this username, and method name refactoring.

---
 app/controllers/projects/git_http_client_controller.rb | 4 ++--
 lib/gitlab/auth.rb                                     | 2 +-
 lib/gitlab/lfs_token.rb                                | 2 +-
 spec/lib/gitlab/auth_spec.rb                           | 4 ++--
 spec/lib/gitlab/lfs_token_spec.rb                      | 2 +-
 spec/requests/api/internal_spec.rb                     | 2 +-
 spec/requests/lfs_http_spec.rb                         | 2 +-
 7 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/app/controllers/projects/git_http_client_controller.rb b/app/controllers/projects/git_http_client_controller.rb
index b4ec5b3fae1..4be580e759e 100644
--- a/app/controllers/projects/git_http_client_controller.rb
+++ b/app/controllers/projects/git_http_client_controller.rb
@@ -22,7 +22,7 @@ class Projects::GitHttpClientController < Projects::ApplicationController
     if allow_basic_auth? && basic_auth_provided?
       login, password = user_name_and_password(request)
 
-      handle_authentication(login, password)
+      handle_basic_authentication(login, password)
 
       if ci? || user
         return # Allow access
@@ -110,7 +110,7 @@ class Projects::GitHttpClientController < Projects::ApplicationController
     @ci.present?
   end
 
-  def handle_authentication(login, password)
+  def handle_basic_authentication(login, password)
     auth_result = Gitlab::Auth.find_for_git_client(login, password, project: project, ip: request.ip)
 
     case auth_result.type
diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb
index 14e29124aac..f4e6ebb7bc7 100644
--- a/lib/gitlab/auth.rb
+++ b/lib/gitlab/auth.rb
@@ -115,7 +115,7 @@ module Gitlab
 
       def lfs_token_check(login, password)
         actor =
-          if login =~ /\Alfs-deploy-key-\d+\Z/
+          if login =~ /\Alfs\+deploy-key-\d+\Z/
             /\d+\Z/.match(login) do |id|
               DeployKey.find(id[0])
             end
diff --git a/lib/gitlab/lfs_token.rb b/lib/gitlab/lfs_token.rb
index d7db8017475..edf4dffc4c0 100644
--- a/lib/gitlab/lfs_token.rb
+++ b/lib/gitlab/lfs_token.rb
@@ -30,7 +30,7 @@ module Gitlab
     end
 
     def actor_name
-      actor.is_a?(User) ? actor.username : "lfs-deploy-key-#{actor.id}"
+      actor.is_a?(User) ? actor.username : "lfs+deploy-key-#{actor.id}"
     end
 
     private
diff --git a/spec/lib/gitlab/auth_spec.rb b/spec/lib/gitlab/auth_spec.rb
index 4c8e09cd904..56f349f5d92 100644
--- a/spec/lib/gitlab/auth_spec.rb
+++ b/spec/lib/gitlab/auth_spec.rb
@@ -37,8 +37,8 @@ describe Gitlab::Auth, lib: true do
       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, :lfs_deploy_token))
+      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, :lfs_deploy_token))
     end
 
     it 'recognizes OAuth tokens' do
diff --git a/spec/lib/gitlab/lfs_token_spec.rb b/spec/lib/gitlab/lfs_token_spec.rb
index f9812664e3b..184f235c1b2 100644
--- a/spec/lib/gitlab/lfs_token_spec.rb
+++ b/spec/lib/gitlab/lfs_token_spec.rb
@@ -40,7 +40,7 @@ describe Gitlab::LfsToken, lib: true do
       it_behaves_like 'an LFS token generator'
 
       it 'returns the correct username' do
-        expect(handler.actor_name).to eq("lfs-deploy-key-#{actor.id}")
+        expect(handler.actor_name).to eq("lfs+deploy-key-#{actor.id}")
       end
 
       it 'returns the correct token type' do
diff --git a/spec/requests/api/internal_spec.rb b/spec/requests/api/internal_spec.rb
index 1ee390e0a19..2e1e6a11b53 100644
--- a/spec/requests/api/internal_spec.rb
+++ b/spec/requests/api/internal_spec.rb
@@ -124,7 +124,7 @@ describe API::API, api: true  do
         lfs_auth(key, project)
 
         expect(response).to have_http_status(200)
-        expect(json_response['username']).to eq("lfs-deploy-key-#{key.id}")
+        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
diff --git a/spec/requests/lfs_http_spec.rb b/spec/requests/lfs_http_spec.rb
index e61502400ff..54ecb793729 100644
--- a/spec/requests/lfs_http_spec.rb
+++ b/spec/requests/lfs_http_spec.rb
@@ -917,7 +917,7 @@ describe 'Git LFS API and storage' do
   end
 
   def authorize_deploy_key
-    ActionController::HttpAuthentication::Basic.encode_credentials("lfs-deploy-key-#{key.id}", Gitlab::LfsToken.new(key).generate)
+    ActionController::HttpAuthentication::Basic.encode_credentials("lfs+deploy-key-#{key.id}", Gitlab::LfsToken.new(key).generate)
   end
 
   def fork_project(project, user, object = nil)
-- 
cgit v1.2.1