Commit cb85cf1f authored by Patricio Cano's avatar Patricio Cano

Refactor LFS token logic to use a Redis key instead of a DB field, making it a 1 use only token.

parent 372be2d2
......@@ -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!
......
......@@ -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?
......
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) }
......
......@@ -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
......
# 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
# 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
module API
module Entities
class UserSafe < Grape::Entity
expose :name, :username, :lfs_token
expose :name, :username
end
class UserBasic < UserSafe
......
......@@ -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
......
......@@ -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
......
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
......@@ -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
......
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
......@@ -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
......
......@@ -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
......
......@@ -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)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment