summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAsh McKenzie <amckenzie@gitlab.com>2019-05-22 08:26:40 +0000
committerAsh McKenzie <amckenzie@gitlab.com>2019-05-22 08:26:40 +0000
commit0c7300cdde8b14f1538f3221beedcd824f0c7c7c (patch)
treede20192d1a6fc92fab9c6c3e12241946f76d7f70
parent1645cec1298c7cabefaa8fbc05f0774074a8f521 (diff)
parent07a353259424cd225368ca4af59883eebf10066d (diff)
downloadgitlab-ce-0c7300cdde8b14f1538f3221beedcd824f0c7c7c.tar.gz
Merge branch '8723-geo-remove-gitlab-lfstoken-legacyredisdevisetoken-implementation-and-usage-geo' into 'master'
Geo: Remove Gitlab::LfsToken::LegacyRedisDeviseToken implementation Closes gitlab-ee#8723 See merge request gitlab-org/gitlab-ce!28546
-rw-r--r--changelogs/unreleased/8723-geo-remove-gitlab-lfstoken-legacyredisdevisetoken-implementation-and-usage-geo.yml5
-rw-r--r--lib/gitlab/lfs_token.rb42
-rw-r--r--spec/lib/gitlab/lfs_token_spec.rb98
3 files changed, 28 insertions, 117 deletions
diff --git a/changelogs/unreleased/8723-geo-remove-gitlab-lfstoken-legacyredisdevisetoken-implementation-and-usage-geo.yml b/changelogs/unreleased/8723-geo-remove-gitlab-lfstoken-legacyredisdevisetoken-implementation-and-usage-geo.yml
new file mode 100644
index 00000000000..173c7d9383e
--- /dev/null
+++ b/changelogs/unreleased/8723-geo-remove-gitlab-lfstoken-legacyredisdevisetoken-implementation-and-usage-geo.yml
@@ -0,0 +1,5 @@
+---
+title: 'Geo: Remove Gitlab::LfsToken::LegacyRedisDeviseToken implementation and usage'
+merge_request: 28546
+author:
+type: changed
diff --git a/lib/gitlab/lfs_token.rb b/lib/gitlab/lfs_token.rb
index 31e6fc9d8c7..124e34562c1 100644
--- a/lib/gitlab/lfs_token.rb
+++ b/lib/gitlab/lfs_token.rb
@@ -35,8 +35,7 @@ module Gitlab
end
def token_valid?(token_to_check)
- HMACToken.new(actor).token_valid?(token_to_check) ||
- LegacyRedisDeviseToken.new(actor).token_valid?(token_to_check)
+ HMACToken.new(actor).token_valid?(token_to_check)
end
def deploy_key_pushable?(project)
@@ -103,44 +102,5 @@ module Gitlab
Settings.attr_encrypted_db_key_base.first(16)
end
end
-
- # 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 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
diff --git a/spec/lib/gitlab/lfs_token_spec.rb b/spec/lib/gitlab/lfs_token_spec.rb
index 8961ecc4be0..701ed1f3a1b 100644
--- a/spec/lib/gitlab/lfs_token_spec.rb
+++ b/spec/lib/gitlab/lfs_token_spec.rb
@@ -77,96 +77,42 @@ describe Gitlab::LfsToken, :clean_gitlab_redis_shared_state do
let(:actor) { create(:user, username: 'test_user_lfs_1') }
let(:lfs_token) { described_class.new(actor) }
- 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
+ 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 'where the token is valid' do
- it 'returns true' do
- expect(lfs_token.token_valid?(lfs_token.token)).to be_truthy
+ 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
- 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
+ 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
+ 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
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
+ expect(lfs_token.token_valid?(lfs_token.token)).to be_truthy
end
end
end