diff options
-rw-r--r-- | app/controllers/projects/git_http_client_controller.rb | 3 | ||||
-rw-r--r-- | app/helpers/lfs_helper.rb | 6 | ||||
-rw-r--r-- | app/models/deploy_key.rb | 5 | ||||
-rw-r--r-- | app/models/user.rb | 3 | ||||
-rw-r--r-- | db/migrate/20160825173042_add_lfs_token_to_users.rb | 16 | ||||
-rw-r--r-- | db/migrate/20160825182924_add_lfs_token_to_keys.rb | 16 | ||||
-rw-r--r-- | lib/api/entities.rb | 2 | ||||
-rw-r--r-- | lib/api/internal.rb | 9 | ||||
-rw-r--r-- | lib/gitlab/auth.rb | 12 | ||||
-rw-r--r-- | lib/gitlab/lfs_token.rb | 29 | ||||
-rw-r--r-- | spec/lib/gitlab/auth_spec.rb | 8 | ||||
-rw-r--r-- | spec/lib/gitlab/lfs_token_spec.rb | 35 | ||||
-rw-r--r-- | spec/models/concerns/token_authenticatable_spec.rb | 20 | ||||
-rw-r--r-- | spec/requests/api/internal_spec.rb | 6 | ||||
-rw-r--r-- | spec/requests/lfs_http_spec.rb | 2 |
15 files changed, 93 insertions, 79 deletions
diff --git a/app/controllers/projects/git_http_client_controller.rb b/app/controllers/projects/git_http_client_controller.rb index 1e6709dc8eb..4dff1ce6568 100644 --- a/app/controllers/projects/git_http_client_controller.rb +++ b/app/controllers/projects/git_http_client_controller.rb @@ -132,8 +132,7 @@ class Projects::GitHttpClientController < Projects::ApplicationController end def lfs_deploy_key? - key = user - @lfs_deploy_key.present? && (key && key.projects.include?(project)) + @lfs_deploy_key.present? && (user && user.projects.include?(project)) end def verify_workhorse_api! diff --git a/app/helpers/lfs_helper.rb b/app/helpers/lfs_helper.rb index 0c867fc8741..2f5709a7395 100644 --- a/app/helpers/lfs_helper.rb +++ b/app/helpers/lfs_helper.rb @@ -25,7 +25,11 @@ module LfsHelper def lfs_download_access? return false unless project.lfs_enabled? - project.public? || ci? || lfs_deploy_key? || (user && user.can?(:download_code, project)) + return true if project.public? + return true if ci? + return true if lfs_deploy_key? + + (user && user.can?(:download_code, project)) end def lfs_upload_access? diff --git a/app/models/deploy_key.rb b/app/models/deploy_key.rb index de51b63c120..2c525d4cd7a 100644 --- a/app/models/deploy_key.rb +++ b/app/models/deploy_key.rb @@ -1,12 +1,7 @@ class DeployKey < Key - include TokenAuthenticatable - add_authentication_token_field :lfs_token - has_many :deploy_keys_projects, dependent: :destroy has_many :projects, through: :deploy_keys_projects - before_save :ensure_lfs_token - scope :in_projects, ->(projects) { joins(:deploy_keys_projects).where('deploy_keys_projects.project_id in (?)', projects) } scope :are_public, -> { where(public: true) } diff --git a/app/models/user.rb b/app/models/user.rb index 94ae3911859..6996740eebd 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -13,7 +13,6 @@ class User < ActiveRecord::Base DEFAULT_NOTIFICATION_LEVEL = :participating add_authentication_token_field :authentication_token - add_authentication_token_field :lfs_token default_value_for :admin, false default_value_for(:external) { current_application_settings.user_default_external } @@ -118,7 +117,7 @@ class User < ActiveRecord::Base before_validation :set_public_email, if: ->(user) { user.public_email_changed? } after_update :update_emails_with_primary_email, if: ->(user) { user.email_changed? } - before_save :ensure_authentication_token, :ensure_lfs_token + before_save :ensure_authentication_token before_save :ensure_external_user_rights after_save :ensure_namespace_correct after_initialize :set_projects_limit diff --git a/db/migrate/20160825173042_add_lfs_token_to_users.rb b/db/migrate/20160825173042_add_lfs_token_to_users.rb deleted file mode 100644 index f7f210bcd67..00000000000 --- a/db/migrate/20160825173042_add_lfs_token_to_users.rb +++ /dev/null @@ -1,16 +0,0 @@ -# See http://doc.gitlab.com/ce/development/migration_style_guide.html -# for more information on how to write migrations for GitLab. - -class AddLfsTokenToUsers < ActiveRecord::Migration - include Gitlab::Database::MigrationHelpers - - # Set this constant to true if this migration requires downtime. - DOWNTIME = false - - disable_ddl_transaction! - - def change - add_column :users, :lfs_token, :string - add_concurrent_index :users, :lfs_token - end -end diff --git a/db/migrate/20160825182924_add_lfs_token_to_keys.rb b/db/migrate/20160825182924_add_lfs_token_to_keys.rb deleted file mode 100644 index 3ff010ef328..00000000000 --- a/db/migrate/20160825182924_add_lfs_token_to_keys.rb +++ /dev/null @@ -1,16 +0,0 @@ -# See http://doc.gitlab.com/ce/development/migration_style_guide.html -# for more information on how to write migrations for GitLab. - -class AddLfsTokenToKeys < ActiveRecord::Migration - include Gitlab::Database::MigrationHelpers - - # Set this constant to true if this migration requires downtime. - DOWNTIME = false - - disable_ddl_transaction! - - def change - add_column :keys, :lfs_token, :string - add_concurrent_index :keys, :lfs_token - end -end diff --git a/lib/api/entities.rb b/lib/api/entities.rb index b4fcacca896..4f736e4ec2b 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -1,7 +1,7 @@ module API module Entities class UserSafe < Grape::Entity - expose :name, :username, :lfs_token + expose :name, :username end class UserBasic < UserSafe diff --git a/lib/api/internal.rb b/lib/api/internal.rb index 7c0a6eaa652..760f69663ab 100644 --- a/lib/api/internal.rb +++ b/lib/api/internal.rb @@ -88,12 +88,13 @@ module API get "/discover" do key = Key.find(params[:key_id]) user = key.user + if user - user.ensure_lfs_token! - present user, with: Entities::UserSafe + token = Gitlab::LfsToken.new(user).set_token + { name: user.name, username: user.username, lfs_token: token } else - key.ensure_lfs_token! - { username: 'lfs-deploy-key', lfs_token: key.lfs_token } + token = Gitlab::LfsToken.new(key).set_token + { username: "lfs-deploy-key-#{key.id}", lfs_token: token } end end diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb index 5446093de4d..e43f8119658 100644 --- a/lib/gitlab/auth.rb +++ b/lib/gitlab/auth.rb @@ -117,12 +117,14 @@ module Gitlab end def lfs_token_check(login, password) - if login == 'lfs-deploy-key' - key = DeployKey.find_by_lfs_token(password) - Result.new(key, :lfs_deploy_token) if key + if login.include?('lfs-deploy-key') + key = DeployKey.find(login.gsub('lfs-deploy-key-', '')) + token = Gitlab::LfsToken.new(key).get_value + Result.new(key, :lfs_deploy_token) if key && token == password else - user = User.find_by_lfs_token(password) - Result.new(user, :lfs_token) if user && user.username == login + user = User.by_login(login) + token = Gitlab::LfsToken.new(user).get_value + Result.new(user, :lfs_token) if user && token == password end end end diff --git a/lib/gitlab/lfs_token.rb b/lib/gitlab/lfs_token.rb new file mode 100644 index 00000000000..0685eb775ef --- /dev/null +++ b/lib/gitlab/lfs_token.rb @@ -0,0 +1,29 @@ +module Gitlab + class LfsToken + attr_accessor :actor + + def initialize(actor) + @actor = actor + end + + def set_token + token = Devise.friendly_token(50) + Gitlab::Redis.with do |redis| + redis.set(redis_key, token, ex: 3600) + end + token + end + + def get_value + Gitlab::Redis.with do |redis| + redis.get(redis_key) + end + end + + private + + def redis_key + "gitlab:lfs_token:#{actor.class.name.underscore}_#{actor.id}" if actor + end + end +end diff --git a/spec/lib/gitlab/auth_spec.rb b/spec/lib/gitlab/auth_spec.rb index cd00a15be3b..6ce680e3c26 100644 --- a/spec/lib/gitlab/auth_spec.rb +++ b/spec/lib/gitlab/auth_spec.rb @@ -26,17 +26,19 @@ describe Gitlab::Auth, lib: true do it 'recognizes user lfs tokens' do user = create(:user) ip = 'ip' + token = Gitlab::LfsToken.new(user).set_token expect(gl_auth).to receive(:rate_limit!).with(ip, success: true, login: user.username) - expect(gl_auth.find_for_git_client(user.username, user.lfs_token, project: nil, ip: ip)).to eq(Gitlab::Auth::Result.new(user, :lfs_token)) + expect(gl_auth.find_for_git_client(user.username, token, project: nil, ip: ip)).to eq(Gitlab::Auth::Result.new(user, :lfs_token)) end it 'recognizes deploy key lfs tokens' do key = create(:deploy_key) ip = 'ip' + token = Gitlab::LfsToken.new(key).set_token - expect(gl_auth).to receive(:rate_limit!).with(ip, success: true, login: 'lfs-deploy-key') - expect(gl_auth.find_for_git_client('lfs-deploy-key', key.lfs_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 new file mode 100644 index 00000000000..76b348637c7 --- /dev/null +++ b/spec/lib/gitlab/lfs_token_spec.rb @@ -0,0 +1,35 @@ +require 'spec_helper' + +describe Gitlab::LfsToken, lib: true do + describe '#set_token and #get_value' do + shared_examples 'an LFS token generator' do + it 'returns a randomly generated token' do + token = handler.set_token + + expect(token).not_to be_nil + expect(token).to be_a String + expect(token.length).to eq 50 + end + + it 'returns the correct token based on the key' do + token = handler.set_token + + expect(handler.get_value).to eq(token) + end + end + + context 'when the actor is a user' do + let(:actor) { create(:user) } + let(:handler) { described_class.new(actor) } + + it_behaves_like 'an LFS token generator' + end + + context 'when the actor is a deploy key' do + let(:actor) { create(:deploy_key) } + let(:handler) { described_class.new(actor) } + + it_behaves_like 'an LFS token generator' + end + end +end diff --git a/spec/models/concerns/token_authenticatable_spec.rb b/spec/models/concerns/token_authenticatable_spec.rb index 82076600f3b..eb64f3d0c83 100644 --- a/spec/models/concerns/token_authenticatable_spec.rb +++ b/spec/models/concerns/token_authenticatable_spec.rb @@ -18,26 +18,6 @@ describe User, 'TokenAuthenticatable' do subject { create(:user).send(token_field) } it { is_expected.to be_a String } end - - describe 'lfs token' do - let(:token_field) { :lfs_token } - it_behaves_like 'TokenAuthenticatable' - - describe 'ensure it' do - subject { create(:user).send(token_field) } - it { is_expected.to be_a String } - end - end -end - -describe DeployKey, 'TokenAuthenticatable' do - let(:token_field) { :lfs_token } - it_behaves_like 'TokenAuthenticatable' - - describe 'ensures authentication token' do - subject { create(:deploy_key).send(token_field) } - it { is_expected.to be_a String } - end end describe ApplicationSetting, 'TokenAuthenticatable' do diff --git a/spec/requests/api/internal_spec.rb b/spec/requests/api/internal_spec.rb index 95fc5f790e8..59df5af770b 100644 --- a/spec/requests/api/internal_spec.rb +++ b/spec/requests/api/internal_spec.rb @@ -108,7 +108,7 @@ describe API::API, api: true do expect(response).to have_http_status(200) expect(json_response['name']).to eq(user.name) - expect(json_response['lfs_token']).to eq(user.lfs_token) + expect(json_response['lfs_token']).to eq(Gitlab::LfsToken.new(user).get_value) end end @@ -120,8 +120,8 @@ describe API::API, api: true do expect(response).to have_http_status(200) - expect(json_response['username']).to eq('lfs-deploy-key') - expect(json_response['lfs_token']).to eq(key.lfs_token) + expect(json_response['username']).to eq("lfs-deploy-key-#{key.id}") + expect(json_response['lfs_token']).to eq(Gitlab::LfsToken.new(key).get_value) end end end diff --git a/spec/requests/lfs_http_spec.rb b/spec/requests/lfs_http_spec.rb index 58f8515c0e2..d15e72b2570 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.lfs_token) + ActionController::HttpAuthentication::Basic.encode_credentials("lfs-deploy-key-#{key.id}", Gitlab::LfsToken.new(key).set_token) end def fork_project(project, user, object = nil) |