diff options
author | GitLab Release Tools Bot <delivery-team+release-tools@gitlab.com> | 2020-10-01 20:11:57 +0300 |
---|---|---|
committer | GitLab Release Tools Bot <delivery-team+release-tools@gitlab.com> | 2020-10-01 20:11:57 +0300 |
commit | d5cc1255b64367c04f208150dbafa0be7b50dc45 (patch) | |
tree | de52cd1ea513079768947ba3a96c5d10545b3006 /spec/models | |
parent | c90be62bdefdb6bb67c73a9c4a6d164c9f78a28d (diff) | |
parent | b08b36dccc3857641fe4e170b11c30cfb47c49e1 (diff) |
Merge remote-tracking branch 'dev/13-4-stable' into 13-4-stable
Diffstat (limited to 'spec/models')
-rw-r--r-- | spec/models/active_session_spec.rb | 218 | ||||
-rw-r--r-- | spec/models/concerns/expirable_spec.rb | 10 | ||||
-rw-r--r-- | spec/models/member_spec.rb | 7 | ||||
-rw-r--r-- | spec/models/members/project_member_spec.rb | 5 | ||||
-rw-r--r-- | spec/models/operations/feature_flag_spec.rb | 4 |
5 files changed, 190 insertions, 54 deletions
diff --git a/spec/models/active_session_spec.rb b/spec/models/active_session_spec.rb index de39c8c7c5c..f0bae3f29c0 100644 --- a/spec/models/active_session_spec.rb +++ b/spec/models/active_session_spec.rb @@ -23,7 +23,7 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_shared_state do describe '#current?' do it 'returns true if the active session matches the current session' do - active_session = ActiveSession.new(session_id: rack_session) + active_session = ActiveSession.new(session_private_id: rack_session.private_id) expect(active_session.current?(session)).to be true end @@ -45,7 +45,7 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_shared_state do describe '#public_id' do it 'returns an encrypted, url-encoded session id' do original_session_id = Rack::Session::SessionId.new("!*'();:@&\n=+$,/?%abcd#123[4567]8") - active_session = ActiveSession.new(session_id: original_session_id) + active_session = ActiveSession.new(session_id: original_session_id.public_id) encrypted_id = active_session.public_id derived_session_id = Gitlab::CryptoHelper.aes256_gcm_decrypt(encrypted_id) @@ -106,8 +106,8 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_shared_state do redis.sadd( "session:lookup:user:gitlab:#{user.id}", %w[ - 6919a6f1bb119dd7396fadc38fd18d0d - 59822c7d9fcdfa03725eff41782ad97d + 2::418729c72310bbf349a032f0bb6e3fce9f5a69df8f000d8ae0ac5d159d8f21ae + 2::d2ee6f70d6ef0e8701efa3f6b281cbe8e6bf3d109ef052a8b5ce88bfc7e71c26 ] ) end @@ -135,7 +135,7 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_shared_state do redis.set("session:gitlab:#{rack_session.private_id}", Marshal.dump({ _csrf_token: 'abcd' })) end - expect(ActiveSession.sessions_from_ids([rack_session])).to eq [{ _csrf_token: 'abcd' }] + expect(ActiveSession.sessions_from_ids([rack_session.private_id])).to eq [{ _csrf_token: 'abcd' }] end it 'avoids a redis lookup for an empty array' do @@ -150,12 +150,11 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_shared_state do redis = double(:redis) expect(Gitlab::Redis::SharedState).to receive(:with).and_yield(redis) - sessions = %w[session-a session-b session-c session-d] + sessions = %w[session-a session-b] mget_responses = sessions.map { |session| [Marshal.dump(session)]} - expect(redis).to receive(:mget).exactly(4).times.and_return(*mget_responses) + expect(redis).to receive(:mget).twice.times.and_return(*mget_responses) - session_ids = [1, 2].map { |id| Rack::Session::SessionId.new(id.to_s) } - expect(ActiveSession.sessions_from_ids(session_ids).map(&:to_s)).to eql(sessions) + expect(ActiveSession.sessions_from_ids([1, 2])).to eql(sessions) end end @@ -165,7 +164,7 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_shared_state do Gitlab::Redis::SharedState.with do |redis| expect(redis.scan_each.to_a).to include( - "session:user:gitlab:#{user.id}:6919a6f1bb119dd7396fadc38fd18d0d", + "session:user:gitlab:#{user.id}:2::418729c72310bbf349a032f0bb6e3fce9f5a69df8f000d8ae0ac5d159d8f21ae", "session:lookup:user:gitlab:#{user.id}" ) end @@ -208,13 +207,41 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_shared_state do end end end + + context 'ActiveSession stored by deprecated rack_session_public_key' do + let(:active_session) { ActiveSession.new(session_id: rack_session.public_id) } + let(:deprecated_active_session_lookup_key) { rack_session.public_id } + + before do + Gitlab::Redis::SharedState.with do |redis| + redis.set("session:user:gitlab:#{user.id}:#{deprecated_active_session_lookup_key}", + '') + redis.sadd(described_class.lookup_key_name(user.id), + deprecated_active_session_lookup_key) + end + end + + it 'removes deprecated key and stores only new one' do + expected_session_keys = ["session:user:gitlab:#{user.id}:#{rack_session.private_id}", + "session:lookup:user:gitlab:#{user.id}"] + + ActiveSession.set(user, request) + + Gitlab::Redis::SharedState.with do |redis| + actual_session_keys = redis.scan_each(match: 'session:*').to_a + expect(actual_session_keys).to(match_array(expected_session_keys)) + + expect(redis.smembers("session:lookup:user:gitlab:#{user.id}")).to eq [rack_session.private_id] + end + end + end end - describe '.destroy' do + describe '.destroy_with_rack_session_id' do it 'gracefully handles a nil session ID' do expect(described_class).not_to receive(:destroy_sessions) - ActiveSession.destroy(user, nil) + ActiveSession.destroy_with_rack_session_id(user, nil) end it 'removes the entry associated with the currently killed user session' do @@ -224,7 +251,7 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_shared_state do redis.set("session:user:gitlab:9999:5c8611e4f9c69645ad1a1492f4131358", '') end - ActiveSession.destroy(user, request.session.id) + ActiveSession.destroy_with_rack_session_id(user, request.session.id) Gitlab::Redis::SharedState.with do |redis| expect(redis.scan_each(match: "session:user:gitlab:*")).to match_array [ @@ -240,7 +267,7 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_shared_state do redis.sadd("session:lookup:user:gitlab:#{user.id}", '6919a6f1bb119dd7396fadc38fd18d0d') end - ActiveSession.destroy(user, request.session.id) + ActiveSession.destroy_with_rack_session_id(user, request.session.id) Gitlab::Redis::SharedState.with do |redis| expect(redis.scan_each(match: "session:lookup:user:gitlab:#{user.id}").to_a).to be_empty @@ -249,12 +276,12 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_shared_state do it 'removes the devise session' do Gitlab::Redis::SharedState.with do |redis| - redis.set("session:user:gitlab:#{user.id}:#{rack_session.public_id}", '') + redis.set("session:user:gitlab:#{user.id}:#{rack_session.private_id}", '') # Emulate redis-rack: https://github.com/redis-store/redis-rack/blob/c75f7f1a6016ee224e2615017fbfee964f23a837/lib/rack/session/redis.rb#L88 redis.set("session:gitlab:#{rack_session.private_id}", '') end - ActiveSession.destroy(user, request.session.id) + ActiveSession.destroy_with_rack_session_id(user, request.session.id) Gitlab::Redis::SharedState.with do |redis| expect(redis.scan_each(match: "session:gitlab:*").to_a).to be_empty @@ -262,37 +289,83 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_shared_state do end end - describe '.destroy_with_public_id' do - it 'receives a user and public id and destroys the associated session' do - ActiveSession.set(user, request) - session = ActiveSession.list(user).first + describe '.destroy_with_deprecated_encryption' do + shared_examples 'removes all session data' do + before do + Gitlab::Redis::SharedState.with do |redis| + redis.set("session:user:gitlab:#{user.id}:#{active_session_lookup_key}", '') + # Emulate redis-rack: https://github.com/redis-store/redis-rack/blob/c75f7f1a6016ee224e2615017fbfee964f23a837/lib/rack/session/redis.rb#L88 + redis.set("session:gitlab:#{rack_session.private_id}", '') + + redis.set(described_class.key_name(user.id, active_session_lookup_key), + Marshal.dump(active_session)) + redis.sadd(described_class.lookup_key_name(user.id), + active_session_lookup_key) + end + end + + it 'removes the devise session' do + subject + + Gitlab::Redis::SharedState.with do |redis| + expect(redis.scan_each(match: "session:gitlab:*").to_a).to be_empty + end + end - ActiveSession.destroy_with_public_id(user, session.public_id) + it 'removes the lookup entry' do + subject - total_sessions = ActiveSession.list(user).count - expect(total_sessions).to eq 0 + Gitlab::Redis::SharedState.with do |redis| + expect(redis.scan_each(match: "session:lookup:user:gitlab:#{user.id}").to_a).to be_empty + end + end + + it 'removes the ActiveSession' do + subject + + Gitlab::Redis::SharedState.with do |redis| + expect(redis.scan_each(match: "session:user:gitlab:*").to_a).to be_empty + end + end end - it 'handles invalid input for public id' do - expect do - ActiveSession.destroy_with_public_id(user, nil) - end.not_to raise_error + context 'destroy called with Rack::Session::SessionId#private_id' do + subject { ActiveSession.destroy_with_deprecated_encryption(user, rack_session.private_id) } + + it 'calls .destroy_sessions' do + expect(ActiveSession).to( + receive(:destroy_sessions) + .with(anything, user, [rack_session.private_id])) + + subject + end - expect do - ActiveSession.destroy_with_public_id(user, "") - end.not_to raise_error + context 'ActiveSession with session_private_id' do + let(:active_session) { ActiveSession.new(session_private_id: rack_session.private_id) } + let(:active_session_lookup_key) { rack_session.private_id } - expect do - ActiveSession.destroy_with_public_id(user, "aaaaaaaa") - end.not_to raise_error + include_examples 'removes all session data' + end end - it 'does not attempt to destroy session when given invalid input for public id' do - expect(ActiveSession).not_to receive(:destroy) + context 'destroy called with ActiveSession#public_id (deprecated)' do + let(:active_session) { ActiveSession.new(session_id: rack_session.public_id) } + let(:encrypted_active_session_id) { active_session.public_id } + let(:active_session_lookup_key) { rack_session.public_id } + + subject { ActiveSession.destroy_with_deprecated_encryption(user, encrypted_active_session_id) } + + it 'calls .destroy_sessions' do + expect(ActiveSession).to( + receive(:destroy_sessions) + .with(anything, user, [active_session.public_id, rack_session.public_id, rack_session.private_id])) + + subject + end - ActiveSession.destroy_with_public_id(user, nil) - ActiveSession.destroy_with_public_id(user, "") - ActiveSession.destroy_with_public_id(user, "aaaaaaaa") + context 'ActiveSession with session_id (deprecated)' do + include_examples 'removes all session data' + end end end @@ -308,29 +381,43 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_shared_state do before do Gitlab::Redis::SharedState.with do |redis| - redis.set(described_class.key_name(user.id, current_session_id), - Marshal.dump(ActiveSession.new(session_id: Rack::Session::SessionId.new(current_session_id)))) - redis.set(described_class.key_name(user.id, '59822c7d9fcdfa03725eff41782ad97d'), - Marshal.dump(ActiveSession.new(session_id: Rack::Session::SessionId.new('59822c7d9fcdfa03725eff41782ad97d')))) - redis.set(described_class.key_name(9999, '5c8611e4f9c69645ad1a1492f4131358'), - Marshal.dump(ActiveSession.new(session_id: Rack::Session::SessionId.new('5c8611e4f9c69645ad1a1492f4131358')))) - redis.sadd(described_class.lookup_key_name(user.id), '59822c7d9fcdfa03725eff41782ad97d') - redis.sadd(described_class.lookup_key_name(user.id), current_session_id) - redis.sadd(described_class.lookup_key_name(9999), '5c8611e4f9c69645ad1a1492f4131358') + # setup for current user + [current_session_id, '59822c7d9fcdfa03725eff41782ad97d'].each do |session_public_id| + session_private_id = Rack::Session::SessionId.new(session_public_id).private_id + active_session = ActiveSession.new(session_private_id: session_private_id) + redis.set(described_class.key_name(user.id, session_private_id), + Marshal.dump(active_session)) + redis.sadd(described_class.lookup_key_name(user.id), + session_private_id) + end + + # setup for unrelated user + unrelated_user_id = 9999 + session_private_id = Rack::Session::SessionId.new('5c8611e4f9c69645ad1a1492f4131358').private_id + active_session = ActiveSession.new(session_private_id: session_private_id) + + redis.set(described_class.key_name(unrelated_user_id, session_private_id), + Marshal.dump(active_session)) + redis.sadd(described_class.lookup_key_name(unrelated_user_id), + session_private_id) end end it 'removes the entry associated with the all user sessions but current' do - expect { ActiveSession.destroy_all_but_current(user, request.session) }.to change { ActiveSession.session_ids_for_user(user.id).size }.from(2).to(1) + expect { ActiveSession.destroy_all_but_current(user, request.session) } + .to(change { ActiveSession.session_ids_for_user(user.id).size }.from(2).to(1)) expect(ActiveSession.session_ids_for_user(9999).size).to eq(1) end it 'removes the lookup entry of deleted sessions' do + session_private_id = Rack::Session::SessionId.new(current_session_id).private_id ActiveSession.destroy_all_but_current(user, request.session) Gitlab::Redis::SharedState.with do |redis| - expect(redis.smembers(described_class.lookup_key_name(user.id))).to eq [current_session_id] + expect( + redis.smembers(described_class.lookup_key_name(user.id)) + ).to eq([session_private_id]) end end @@ -464,5 +551,38 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_shared_state do end end end + + context 'cleaning up old sessions stored by Rack::Session::SessionId#private_id' do + let(:max_number_of_sessions_plus_one) { ActiveSession::ALLOWED_NUMBER_OF_ACTIVE_SESSIONS + 1 } + let(:max_number_of_sessions_plus_two) { ActiveSession::ALLOWED_NUMBER_OF_ACTIVE_SESSIONS + 2 } + + before do + Gitlab::Redis::SharedState.with do |redis| + (1..max_number_of_sessions_plus_two).each do |number| + redis.set( + "session:user:gitlab:#{user.id}:#{number}", + Marshal.dump(ActiveSession.new(session_private_id: number.to_s, updated_at: number.days.ago)) + ) + redis.sadd( + "session:lookup:user:gitlab:#{user.id}", + "#{number}" + ) + end + end + end + + it 'removes obsolete active sessions entries' do + ActiveSession.cleanup(user) + + Gitlab::Redis::SharedState.with do |redis| + sessions = redis.scan_each(match: "session:user:gitlab:#{user.id}:*").to_a + + expect(sessions.count).to eq(ActiveSession::ALLOWED_NUMBER_OF_ACTIVE_SESSIONS) + expect(sessions).not_to( + include("session:user:gitlab:#{user.id}:#{max_number_of_sessions_plus_one}", + "session:user:gitlab:#{user.id}:#{max_number_of_sessions_plus_two}")) + end + end + end end end diff --git a/spec/models/concerns/expirable_spec.rb b/spec/models/concerns/expirable_spec.rb index b20d759fc3f..5eb6530881e 100644 --- a/spec/models/concerns/expirable_spec.rb +++ b/spec/models/concerns/expirable_spec.rb @@ -4,9 +4,13 @@ require 'spec_helper' RSpec.describe Expirable do describe 'ProjectMember' do - let(:no_expire) { create(:project_member) } - let(:expire_later) { create(:project_member, expires_at: Time.current + 6.days) } - let(:expired) { create(:project_member, expires_at: Time.current - 6.days) } + let_it_be(:no_expire) { create(:project_member) } + let_it_be(:expire_later) { create(:project_member, expires_at: 8.days.from_now) } + let_it_be(:expired) { create(:project_member, expires_at: 1.day.from_now) } + + before do + travel_to(3.days.from_now) + end describe '.expired' do it { expect(ProjectMember.expired).to match_array([expired]) } diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb index 39807747cc0..90950d93db4 100644 --- a/spec/models/member_spec.rb +++ b/spec/models/member_spec.rb @@ -17,6 +17,13 @@ RSpec.describe Member do it { is_expected.to validate_presence_of(:user) } it { is_expected.to validate_presence_of(:source) } + context 'expires_at' do + it { is_expected.not_to allow_value(Date.yesterday).for(:expires_at) } + it { is_expected.to allow_value(Date.tomorrow).for(:expires_at) } + it { is_expected.to allow_value(Date.today).for(:expires_at) } + it { is_expected.to allow_value(nil).for(:expires_at) } + end + it_behaves_like 'an object with email-formated attributes', :invite_email do subject { build(:project_member) } end diff --git a/spec/models/members/project_member_spec.rb b/spec/models/members/project_member_spec.rb index f25f8933184..388d04c8012 100644 --- a/spec/models/members/project_member_spec.rb +++ b/spec/models/members/project_member_spec.rb @@ -44,8 +44,9 @@ RSpec.describe ProjectMember do let(:maintainer) { create(:project_member, project: project) } it "creates an expired event when left due to expiry" do - expired = create(:project_member, project: project, expires_at: Time.current - 6.days) - expired.destroy + expired = create(:project_member, project: project, expires_at: 1.day.from_now) + travel_to(2.days.from_now) { expired.destroy } + expect(Event.recent.first).to be_expired_action end diff --git a/spec/models/operations/feature_flag_spec.rb b/spec/models/operations/feature_flag_spec.rb index 83d6c6b95a3..db432e73355 100644 --- a/spec/models/operations/feature_flag_spec.rb +++ b/spec/models/operations/feature_flag_spec.rb @@ -7,6 +7,10 @@ RSpec.describe Operations::FeatureFlag do subject { create(:operations_feature_flag) } + it_behaves_like 'includes Limitable concern' do + subject { build(:operations_feature_flag, project: create(:project)) } + end + describe 'associations' do it { is_expected.to belong_to(:project) } it { is_expected.to have_many(:scopes) } |