LfsToken uses JSONWebToken::HMACToken by default
LfsToken::HMACToken#token_valid?() will be examined and if false, look in redis via LfsToken::LegacyRedisDeviseToken#token_valid?().
This commit is contained in:
parent
3bccd2b179
commit
22954f2202
|
@ -199,7 +199,7 @@ module Gitlab
|
|||
end
|
||||
# rubocop: enable CodeReuse/ActiveRecord
|
||||
|
||||
def lfs_token_check(login, password, project)
|
||||
def lfs_token_check(login, encoded_token, project)
|
||||
deploy_key_matches = login.match(/\Alfs\+deploy-key-(\d+)\z/)
|
||||
|
||||
actor =
|
||||
|
@ -222,7 +222,7 @@ module Gitlab
|
|||
read_authentication_abilities
|
||||
end
|
||||
|
||||
if Devise.secure_compare(token_handler.token, password)
|
||||
if token_handler.token_valid?(encoded_token)
|
||||
Gitlab::Auth::Result.new(actor, nil, token_handler.type, authentication_abilities)
|
||||
end
|
||||
end
|
||||
|
|
|
@ -2,10 +2,21 @@
|
|||
|
||||
module Gitlab
|
||||
class LfsToken
|
||||
attr_accessor :actor
|
||||
module LfsTokenHelper
|
||||
def user?
|
||||
actor.is_a?(User)
|
||||
end
|
||||
|
||||
TOKEN_LENGTH = 50
|
||||
EXPIRY_TIME = 1800
|
||||
def actor_name
|
||||
user? ? actor.username : "lfs+deploy-key-#{actor.id}"
|
||||
end
|
||||
end
|
||||
|
||||
include LfsTokenHelper
|
||||
|
||||
DEFAULT_EXPIRE_TIME = 1800
|
||||
|
||||
attr_accessor :actor
|
||||
|
||||
def initialize(actor)
|
||||
@actor =
|
||||
|
@ -19,36 +30,108 @@ module Gitlab
|
|||
end
|
||||
end
|
||||
|
||||
def token
|
||||
Gitlab::Redis::SharedState.with do |redis|
|
||||
token = redis.get(redis_shared_state_key)
|
||||
token ||= Devise.friendly_token(TOKEN_LENGTH)
|
||||
redis.set(redis_shared_state_key, token, ex: EXPIRY_TIME)
|
||||
def token(expire_time: DEFAULT_EXPIRE_TIME)
|
||||
HMACToken.new(actor).token(expire_time)
|
||||
end
|
||||
|
||||
token
|
||||
end
|
||||
def token_valid?(token_to_check)
|
||||
HMACToken.new(actor).token_valid?(token_to_check) ||
|
||||
LegacyRedisDeviseToken.new(actor).token_valid?(token_to_check)
|
||||
end
|
||||
|
||||
def deploy_key_pushable?(project)
|
||||
actor.is_a?(DeployKey) && actor.can_push_to?(project)
|
||||
end
|
||||
|
||||
def user?
|
||||
actor.is_a?(User)
|
||||
end
|
||||
|
||||
def type
|
||||
user? ? :lfs_token : :lfs_deploy_token
|
||||
end
|
||||
|
||||
def actor_name
|
||||
actor.is_a?(User) ? actor.username : "lfs+deploy-key-#{actor.id}"
|
||||
private # rubocop:disable Lint/UselessAccessModifier
|
||||
|
||||
class HMACToken
|
||||
include LfsTokenHelper
|
||||
|
||||
def initialize(actor)
|
||||
@actor = actor
|
||||
end
|
||||
|
||||
def token(expire_time)
|
||||
hmac_token = JSONWebToken::HMACToken.new(secret)
|
||||
hmac_token.expire_time = Time.now + expire_time
|
||||
hmac_token[:data] = { actor: actor_name }
|
||||
hmac_token.encoded
|
||||
end
|
||||
|
||||
def token_valid?(token_to_check)
|
||||
decoded_token = JSONWebToken::HMACToken.decode(token_to_check, secret).first
|
||||
decoded_token.dig('data', 'actor') == actor_name
|
||||
rescue JWT::DecodeError
|
||||
false
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
attr_reader :actor
|
||||
|
||||
def secret
|
||||
salt + key
|
||||
end
|
||||
|
||||
def salt
|
||||
case actor
|
||||
when DeployKey, Key
|
||||
actor.fingerprint.delete(':').first(16)
|
||||
when User
|
||||
# Take the last 16 characters as they're more unique than the first 16
|
||||
actor.id.to_s + actor.encrypted_password.last(16)
|
||||
end
|
||||
end
|
||||
|
||||
def key
|
||||
# Take 16 characters of attr_encrypted_db_key_base, as that's what the
|
||||
# cipher needs exactly
|
||||
Settings.attr_encrypted_db_key_base.first(16)
|
||||
end
|
||||
end
|
||||
|
||||
private
|
||||
# TODO: LegacyRedisDeviseToken and references need to be removed after
|
||||
# next released milestone
|
||||
#
|
||||
class LegacyRedisDeviseToken
|
||||
TOKEN_LENGTH = 50
|
||||
DEFAULT_EXPIRY_TIME = 1800 * 1000 # 30 mins
|
||||
|
||||
def redis_shared_state_key
|
||||
"gitlab:lfs_token:#{actor.class.name.underscore}_#{actor.id}" if actor
|
||||
def initialize(actor)
|
||||
@actor = actor
|
||||
end
|
||||
|
||||
def token_valid?(token_to_check)
|
||||
Devise.secure_compare(stored_token, token_to_check)
|
||||
end
|
||||
|
||||
def stored_token
|
||||
Gitlab::Redis::SharedState.with { |redis| redis.get(state_key) }
|
||||
end
|
||||
|
||||
# This method exists purely to facilitate legacy testing to ensure the
|
||||
# same redis key is used.
|
||||
#
|
||||
def store_new_token(expiry_time_in_ms = DEFAULT_EXPIRY_TIME)
|
||||
Gitlab::Redis::SharedState.with do |redis|
|
||||
new_token = Devise.friendly_token(TOKEN_LENGTH)
|
||||
redis.set(state_key, new_token, px: expiry_time_in_ms)
|
||||
new_token
|
||||
end
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
attr_reader :actor
|
||||
|
||||
def state_key
|
||||
"gitlab:lfs_token:#{actor.class.name.underscore}_#{actor.id}"
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -1,50 +1,187 @@
|
|||
# frozen_string_literal: true
|
||||
|
||||
require 'spec_helper'
|
||||
|
||||
describe Gitlab::LfsToken do
|
||||
describe Gitlab::LfsToken, :clean_gitlab_redis_shared_state do
|
||||
describe '#token' do
|
||||
shared_examples 'an LFS token generator' do
|
||||
it 'returns a randomly generated token' do
|
||||
token = handler.token
|
||||
it 'returns a computed token' do
|
||||
expect(Settings).to receive(:attr_encrypted_db_key_base).and_return('fbnbv6hdjweo53qka7kza8v8swxc413c05pb51qgtfte0bygh1p2e508468hfsn5ntmjcyiz7h1d92ashpet5pkdyejg7g8or3yryhuso4h8o5c73h429d9c3r6bjnet').twice
|
||||
|
||||
token = lfs_token.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.token
|
||||
|
||||
expect(handler.token).to eq(token)
|
||||
expect(described_class.new(actor).token_valid?(token)).to be_truthy
|
||||
end
|
||||
end
|
||||
|
||||
context 'when the actor is a user' do
|
||||
let(:actor) { create(:user) }
|
||||
let(:handler) { described_class.new(actor) }
|
||||
let(:actor) { create(:user, username: 'test_user_lfs_1') }
|
||||
let(:lfs_token) { described_class.new(actor) }
|
||||
|
||||
before do
|
||||
allow(actor).to receive(:encrypted_password).and_return('$2a$04$ETfzVS5spE9Hexn9wh6NUenCHG1LyZ2YdciOYxieV1WLSa8DHqOFO')
|
||||
end
|
||||
|
||||
it_behaves_like 'an LFS token generator'
|
||||
|
||||
it 'returns the correct username' do
|
||||
expect(handler.actor_name).to eq(actor.username)
|
||||
expect(lfs_token.actor_name).to eq(actor.username)
|
||||
end
|
||||
|
||||
it 'returns the correct token type' do
|
||||
expect(handler.type).to eq(:lfs_token)
|
||||
expect(lfs_token.type).to eq(:lfs_token)
|
||||
end
|
||||
end
|
||||
|
||||
context 'when the actor is a key' do
|
||||
let(:user) { create(:user, username: 'test_user_lfs_2') }
|
||||
let(:actor) { create(:key, user: user) }
|
||||
let(:lfs_token) { described_class.new(actor) }
|
||||
|
||||
before do
|
||||
allow(user).to receive(:encrypted_password).and_return('$2a$04$C1GAMKsOKouEbhKy2JQoe./3LwOfQAZc.hC8zW2u/wt8xgukvnlV.')
|
||||
end
|
||||
|
||||
it_behaves_like 'an LFS token generator'
|
||||
|
||||
it 'returns the correct username' do
|
||||
expect(lfs_token.actor_name).to eq(user.username)
|
||||
end
|
||||
|
||||
it 'returns the correct token type' do
|
||||
expect(lfs_token.type).to eq(:lfs_token)
|
||||
end
|
||||
end
|
||||
|
||||
context 'when the actor is a deploy key' do
|
||||
let(:actor_id) { 1 }
|
||||
let(:actor) { create(:deploy_key) }
|
||||
let(:handler) { described_class.new(actor) }
|
||||
let(:project) { create(:project) }
|
||||
let(:lfs_token) { described_class.new(actor) }
|
||||
|
||||
before do
|
||||
allow(actor).to receive(:id).and_return(actor_id)
|
||||
end
|
||||
|
||||
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(lfs_token.actor_name).to eq("lfs+deploy-key-#{actor_id}")
|
||||
end
|
||||
|
||||
it 'returns the correct token type' do
|
||||
expect(handler.type).to eq(:lfs_deploy_token)
|
||||
expect(lfs_token.type).to eq(:lfs_deploy_token)
|
||||
end
|
||||
end
|
||||
|
||||
context 'when the actor is invalid' do
|
||||
it 'raises an exception' do
|
||||
expect { described_class.new('invalid') }.to raise_error('Bad Actor')
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe '#token_valid?' do
|
||||
let(:actor) { create(:user, username: 'test_user_lfs_1') }
|
||||
let(:lfs_token) { described_class.new(actor) }
|
||||
|
||||
before do
|
||||
allow(actor).to receive(:encrypted_password).and_return('$2a$04$ETfzVS5spE9Hexn9wh6NUenCHG1LyZ2YdciOYxieV1WLSa8DHqOFO')
|
||||
end
|
||||
|
||||
context 'for an HMAC token' do
|
||||
before do
|
||||
# We're not interested in testing LegacyRedisDeviseToken here
|
||||
allow(Gitlab::LfsToken::LegacyRedisDeviseToken).to receive_message_chain(:new, :token_valid?).and_return(false)
|
||||
end
|
||||
|
||||
context 'where the token is invalid' do
|
||||
context "because it's junk" do
|
||||
it 'returns false' do
|
||||
expect(lfs_token.token_valid?('junk')).to be_falsey
|
||||
end
|
||||
end
|
||||
|
||||
context "because it's been fiddled with" do
|
||||
it 'returns false' do
|
||||
fiddled_token = lfs_token.token.tap { |token| token[0] = 'E' }
|
||||
expect(lfs_token.token_valid?(fiddled_token)).to be_falsey
|
||||
end
|
||||
end
|
||||
|
||||
context "because it was generated with a different secret" do
|
||||
it 'returns false' do
|
||||
different_actor = create(:user, username: 'test_user_lfs_2')
|
||||
different_secret_token = described_class.new(different_actor).token
|
||||
expect(lfs_token.token_valid?(different_secret_token)).to be_falsey
|
||||
end
|
||||
end
|
||||
|
||||
context "because it's expired" do
|
||||
it 'returns false' do
|
||||
expired_token = lfs_token.token
|
||||
# Needs to be at least 1860 seconds, because the default expiry is
|
||||
# 1800 seconds with an additional 60 second leeway.
|
||||
Timecop.freeze(Time.now + 1865) do
|
||||
expect(lfs_token.token_valid?(expired_token)).to be_falsey
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
context 'where the token is valid' do
|
||||
it 'returns true' do
|
||||
expect(lfs_token.token_valid?(lfs_token.token)).to be_truthy
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
context 'for a LegacyRedisDevise token' do
|
||||
before do
|
||||
# We're not interested in testing HMACToken here
|
||||
allow_any_instance_of(Gitlab::LfsToken::HMACToken).to receive(:token_valid?).and_return(false)
|
||||
end
|
||||
|
||||
context 'where the token is invalid' do
|
||||
context "because it's junk" do
|
||||
it 'returns false' do
|
||||
expect(lfs_token.token_valid?('junk')).to be_falsey
|
||||
end
|
||||
end
|
||||
|
||||
context "because it's been fiddled with" do
|
||||
it 'returns false' do
|
||||
generated_token = Gitlab::LfsToken::LegacyRedisDeviseToken.new(actor).store_new_token
|
||||
fiddled_token = generated_token.tap { |token| token[0] = 'E' }
|
||||
expect(lfs_token.token_valid?(fiddled_token)).to be_falsey
|
||||
end
|
||||
end
|
||||
|
||||
context "because it was generated with a different secret" do
|
||||
it 'returns false' do
|
||||
different_actor = create(:user, username: 'test_user_lfs_2')
|
||||
different_secret_token = described_class.new(different_actor).token
|
||||
expect(lfs_token.token_valid?(different_secret_token)).to be_falsey
|
||||
end
|
||||
end
|
||||
|
||||
context "because it's expired" do
|
||||
it 'returns false' do
|
||||
generated_token = Gitlab::LfsToken::LegacyRedisDeviseToken.new(actor).store_new_token(1)
|
||||
# We need a real sleep here because we need to wait for redis to expire the key.
|
||||
sleep(0.01)
|
||||
expect(lfs_token.token_valid?(generated_token)).to be_falsey
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
context 'where the token is valid' do
|
||||
it 'returns true' do
|
||||
generated_token = Gitlab::LfsToken::LegacyRedisDeviseToken.new(actor).store_new_token
|
||||
expect(lfs_token.token_valid?(generated_token)).to be_truthy
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -156,9 +156,8 @@ describe API::Internal do
|
|||
|
||||
expect(response).to have_gitlab_http_status(200)
|
||||
expect(json_response['username']).to eq(user.username)
|
||||
expect(json_response['lfs_token']).to eq(Gitlab::LfsToken.new(key).token)
|
||||
|
||||
expect(json_response['repository_http_path']).to eq(project.http_url_to_repo)
|
||||
expect(Gitlab::LfsToken.new(key).token_valid?(json_response['lfs_token'])).to be_truthy
|
||||
end
|
||||
|
||||
it 'returns the correct information about the user' do
|
||||
|
@ -166,9 +165,8 @@ describe API::Internal do
|
|||
|
||||
expect(response).to have_gitlab_http_status(200)
|
||||
expect(json_response['username']).to eq(user.username)
|
||||
expect(json_response['lfs_token']).to eq(Gitlab::LfsToken.new(user).token)
|
||||
|
||||
expect(json_response['repository_http_path']).to eq(project.http_url_to_repo)
|
||||
expect(Gitlab::LfsToken.new(user).token_valid?(json_response['lfs_token'])).to be_truthy
|
||||
end
|
||||
|
||||
it 'returns a 404 when no key or user is provided' do
|
||||
|
@ -198,8 +196,8 @@ describe API::Internal do
|
|||
|
||||
expect(response).to have_gitlab_http_status(200)
|
||||
expect(json_response['username']).to eq("lfs+deploy-key-#{key.id}")
|
||||
expect(json_response['lfs_token']).to eq(Gitlab::LfsToken.new(key).token)
|
||||
expect(json_response['repository_http_path']).to eq(project.http_url_to_repo)
|
||||
expect(Gitlab::LfsToken.new(key).token_valid?(json_response['lfs_token'])).to be_truthy
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
Loading…
Reference in New Issue