diff options
Diffstat (limited to 'spec')
27 files changed, 1152 insertions, 286 deletions
diff --git a/spec/controllers/admin/users_controller_spec.rb b/spec/controllers/admin/users_controller_spec.rb index 08a1d7c9fa9..9e7f24ff7e3 100644 --- a/spec/controllers/admin/users_controller_spec.rb +++ b/spec/controllers/admin/users_controller_spec.rb @@ -36,7 +36,7 @@ RSpec.describe Admin::UsersController do end end - describe 'DELETE #user with projects', :sidekiq_might_not_need_inline do + describe 'DELETE #destroy', :sidekiq_might_not_need_inline do let(:project) { create(:project, namespace: user.namespace) } let!(:issue) { create(:issue, author: user) } @@ -59,6 +59,41 @@ RSpec.describe Admin::UsersController do expect(User.exists?(user.id)).to be_falsy expect(Issue.exists?(issue.id)).to be_falsy end + + context 'prerequisites for account deletion' do + context 'solo-owned groups' do + let(:group) { create(:group) } + + context 'if the user is the sole owner of at least one group' do + before do + create(:group_member, :owner, group: group, user: user) + end + + context 'soft-delete' do + it 'fails' do + delete :destroy, params: { id: user.username } + + message = s_('AdminUsers|You must transfer ownership or delete the groups owned by this user before you can delete their account') + + expect(flash[:alert]).to eq(message) + expect(response).to have_gitlab_http_status(:see_other) + expect(response).to redirect_to admin_user_path(user) + expect(User.exists?(user.id)).to be_truthy + end + end + + context 'hard-delete' do + it 'succeeds' do + delete :destroy, params: { id: user.username, hard_delete: true } + + expect(response).to redirect_to(admin_users_path) + expect(flash[:notice]).to eq(_('The user is being deleted.')) + expect(User.exists?(user.id)).to be_falsy + end + end + end + end + end end describe 'PUT #activate' do diff --git a/spec/controllers/groups/group_members_controller_spec.rb b/spec/controllers/groups/group_members_controller_spec.rb index 85f1b247ee9..4b9dd3629f1 100644 --- a/spec/controllers/groups/group_members_controller_spec.rb +++ b/spec/controllers/groups/group_members_controller_spec.rb @@ -139,6 +139,45 @@ RSpec.describe Groups::GroupMembersController do expect(group.users).not_to include group_user end end + + context 'access expiry date' do + before do + group.add_owner(user) + end + + subject do + post :create, params: { + group_id: group, + user_ids: group_user.id, + access_level: Gitlab::Access::GUEST, + expires_at: expires_at + } + end + + context 'when set to a date in the past' do + let(:expires_at) { 2.days.ago } + + it 'does not add user to members' do + subject + + expect(flash[:alert]).to include('Expires at cannot be a date in the past') + expect(response).to redirect_to(group_group_members_path(group)) + expect(group.users).not_to include group_user + end + end + + context 'when set to a date in the future' do + let(:expires_at) { 5.days.from_now } + + it 'adds user to members' do + subject + + expect(response).to set_flash.to 'Users were successfully added.' + expect(response).to redirect_to(group_group_members_path(group)) + expect(group.users).to include group_user + end + end + end end describe 'PUT update' do @@ -149,15 +188,49 @@ RSpec.describe Groups::GroupMembersController do sign_in(user) end - Gitlab::Access.options.each do |label, value| - it "can change the access level to #{label}" do - put :update, params: { - group_member: { access_level: value }, - group_id: group, - id: requester - }, xhr: true + context 'access level' do + Gitlab::Access.options.each do |label, value| + it "can change the access level to #{label}" do + put :update, params: { + group_member: { access_level: value }, + group_id: group, + id: requester + }, xhr: true - expect(requester.reload.human_access).to eq(label) + expect(requester.reload.human_access).to eq(label) + end + end + end + + context 'access expiry date' do + subject do + put :update, xhr: true, params: { + group_member: { + expires_at: expires_at + }, + group_id: group, + id: requester + } + end + + context 'when set to a date in the past' do + let(:expires_at) { 2.days.ago } + + it 'does not update the member' do + subject + + expect(requester.reload.expires_at).not_to eq(expires_at.to_date) + end + end + + context 'when set to a date in the future' do + let(:expires_at) { 5.days.from_now } + + it 'updates the member' do + subject + + expect(requester.reload.expires_at).to eq(expires_at.to_date) + end end end end diff --git a/spec/controllers/profiles/emails_controller_spec.rb b/spec/controllers/profiles/emails_controller_spec.rb index 246f8a6cd76..950120ae564 100644 --- a/spec/controllers/profiles/emails_controller_spec.rb +++ b/spec/controllers/profiles/emails_controller_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' RSpec.describe Profiles::EmailsController do - let(:user) { create(:user) } + let_it_be(:user) { create(:user) } before do sign_in(user) @@ -15,37 +15,75 @@ RSpec.describe Profiles::EmailsController do end end - describe '#create' do - context 'when email address is valid' do - let(:email_params) { { email: "add_email@example.com" } } + shared_examples_for 'respects the rate limit' do + context 'after the rate limit is exceeded' do + before do + allowed_threshold = Gitlab::ApplicationRateLimiter.rate_limits[action][:threshold] + + allow(Gitlab::ApplicationRateLimiter) + .to receive(:increment) + .and_return(allowed_threshold + 1) + end + + it 'does not send any email' do + expect { subject }.not_to change { ActionMailer::Base.deliveries.size } + end - it 'sends an email confirmation' do - expect { post(:create, params: { email: email_params }) }.to change { ActionMailer::Base.deliveries.size } + it 'displays an alert' do + subject + + expect(response).to have_gitlab_http_status(:redirect) + expect(flash[:alert]).to eq(_('This action has been performed too many times. Try again later.')) end end + end + + describe '#create' do + let(:email) { 'add_email@example.com' } + let(:params) { { email: { email: email } } } + + subject { post(:create, params: params) } + + it 'sends an email confirmation' do + expect { subject }.to change { ActionMailer::Base.deliveries.size } + end context 'when email address is invalid' do - let(:email_params) { { email: "test.@example.com" } } + let(:email) { 'invalid.@example.com' } it 'does not send an email confirmation' do - expect { post(:create, params: { email: email_params }) }.not_to change { ActionMailer::Base.deliveries.size } + expect { subject }.not_to change { ActionMailer::Base.deliveries.size } end end + + it_behaves_like 'respects the rate limit' do + let(:action) { :profile_add_new_email } + end end describe '#resend_confirmation_instructions' do - let(:email_params) { { email: "add_email@example.com" } } + let_it_be(:email) { create(:email, user: user) } + let(:params) { { id: email.id } } + + subject { put(:resend_confirmation_instructions, params: params) } it 'resends an email confirmation' do - email = user.emails.create(email: 'add_email@example.com') + expect { subject }.to change { ActionMailer::Base.deliveries.size } - expect { put(:resend_confirmation_instructions, params: { id: email }) }.to change { ActionMailer::Base.deliveries.size } - expect(ActionMailer::Base.deliveries.last.to).to eq [email_params[:email]] - expect(ActionMailer::Base.deliveries.last.subject).to match "Confirmation instructions" + expect(ActionMailer::Base.deliveries.last.to).to eq [email.email] + expect(ActionMailer::Base.deliveries.last.subject).to match 'Confirmation instructions' + end + + context 'email does not exist' do + let(:params) { { id: non_existing_record_id } } + + it 'does not send an email confirmation' do + expect { subject }.not_to change { ActionMailer::Base.deliveries.size } + end end - it 'unable to resend an email confirmation' do - expect { put(:resend_confirmation_instructions, params: { id: 1 }) }.not_to change { ActionMailer::Base.deliveries.size } + it_behaves_like 'respects the rate limit' do + let(:action) { :profile_resend_email_confirmation } end end end diff --git a/spec/controllers/projects/project_members_controller_spec.rb b/spec/controllers/projects/project_members_controller_spec.rb index 40a220d57a7..ae05e2d2631 100644 --- a/spec/controllers/projects/project_members_controller_spec.rb +++ b/spec/controllers/projects/project_members_controller_spec.rb @@ -129,6 +129,46 @@ RSpec.describe Projects::ProjectMembersController do expect(response).to redirect_to(project_project_members_path(project)) end end + + context 'access expiry date' do + before do + project.add_maintainer(user) + end + + subject do + post :create, params: { + namespace_id: project.namespace, + project_id: project, + user_ids: project_user.id, + access_level: Gitlab::Access::GUEST, + expires_at: expires_at + } + end + + context 'when set to a date in the past' do + let(:expires_at) { 2.days.ago } + + it 'does not add user to members' do + subject + + expect(flash[:alert]).to include('Expires at cannot be a date in the past') + expect(response).to redirect_to(project_project_members_path(project)) + expect(project.users).not_to include project_user + end + end + + context 'when set to a date in the future' do + let(:expires_at) { 5.days.from_now } + + it 'adds user to members' do + subject + + expect(response).to set_flash.to 'Users were successfully added.' + expect(response).to redirect_to(project_project_members_path(project)) + expect(project.users).to include project_user + end + end + end end describe 'PUT update' do @@ -139,16 +179,53 @@ RSpec.describe Projects::ProjectMembersController do sign_in(user) end - Gitlab::Access.options.each do |label, value| - it "can change the access level to #{label}" do - put :update, params: { - project_member: { access_level: value }, - namespace_id: project.namespace, - project_id: project, - id: requester - }, xhr: true + context 'access level' do + Gitlab::Access.options.each do |label, value| + it "can change the access level to #{label}" do + params = { + project_member: { access_level: value }, + namespace_id: project.namespace, + project_id: project, + id: requester + } + + put :update, params: params, xhr: true + + expect(requester.reload.human_access).to eq(label) + end + end + end + + context 'access expiry date' do + subject do + put :update, xhr: true, params: { + project_member: { + expires_at: expires_at + }, + namespace_id: project.namespace, + project_id: project, + id: requester + } + end - expect(requester.reload.human_access).to eq(label) + context 'when set to a date in the past' do + let(:expires_at) { 2.days.ago } + + it 'does not update the member' do + subject + + expect(requester.reload.expires_at).not_to eq(expires_at.to_date) + end + end + + context 'when set to a date in the future' do + let(:expires_at) { 5.days.from_now } + + it 'updates the member' do + subject + + expect(requester.reload.expires_at).to eq(expires_at.to_date) + end end end end diff --git a/spec/controllers/projects/raw_controller_spec.rb b/spec/controllers/projects/raw_controller_spec.rb index 5f10343eb76..b3921164c81 100644 --- a/spec/controllers/projects/raw_controller_spec.rb +++ b/spec/controllers/projects/raw_controller_spec.rb @@ -33,6 +33,11 @@ RSpec.describe Projects::RawController do it_behaves_like 'project cache control headers' it_behaves_like 'content disposition headers' + it_behaves_like 'uncached response' do + before do + subject + end + end end context 'image header' do diff --git a/spec/controllers/registrations_controller_spec.rb b/spec/controllers/registrations_controller_spec.rb index 66caa58666f..c1afc75a6fd 100644 --- a/spec/controllers/registrations_controller_spec.rb +++ b/spec/controllers/registrations_controller_spec.rb @@ -384,6 +384,24 @@ RSpec.describe RegistrationsController do expect_success end end + + context 'prerequisites for account deletion' do + context 'solo-owned groups' do + let(:group) { create(:group) } + + context 'if the user is the sole owner of at least one group' do + before do + create(:group_member, :owner, group: group, user: user) + end + + it 'fails' do + delete :destroy, params: { password: '12345678' } + + expect_failure(s_('Profiles|You must transfer ownership or delete groups you are an owner of before you can delete your account')) + end + end + end + end end describe '#update_registration' do diff --git a/spec/factories/projects.rb b/spec/factories/projects.rb index e4b53186ea8..216991c56c5 100644 --- a/spec/factories/projects.rb +++ b/spec/factories/projects.rb @@ -31,6 +31,7 @@ FactoryBot.define do pages_access_level do visibility_level == Gitlab::VisibilityLevel::PUBLIC ? ProjectFeature::ENABLED : ProjectFeature::PRIVATE end + metrics_dashboard_access_level { ProjectFeature::PRIVATE } # we can't assign the delegated `#ci_cd_settings` attributes directly, as the # `#ci_cd_settings` relation needs to be created first @@ -54,7 +55,9 @@ FactoryBot.define do issues_access_level: evaluator.issues_access_level, forking_access_level: evaluator.forking_access_level, merge_requests_access_level: merge_requests_access_level, - repository_access_level: evaluator.repository_access_level + repository_access_level: evaluator.repository_access_level, + pages_access_level: evaluator.pages_access_level, + metrics_dashboard_access_level: evaluator.metrics_dashboard_access_level } if ActiveRecord::Migrator.current_version >= PAGES_ACCESS_LEVEL_SCHEMA_VERSION @@ -300,6 +303,9 @@ FactoryBot.define do trait(:pages_enabled) { pages_access_level { ProjectFeature::ENABLED } } trait(:pages_disabled) { pages_access_level { ProjectFeature::DISABLED } } trait(:pages_private) { pages_access_level { ProjectFeature::PRIVATE } } + trait(:metrics_dashboard_enabled) { metrics_dashboard_access_level { ProjectFeature::ENABLED } } + trait(:metrics_dashboard_disabled) { metrics_dashboard_access_level { ProjectFeature::DISABLED } } + trait(:metrics_dashboard_private) { metrics_dashboard_access_level { ProjectFeature::PRIVATE } } trait :auto_devops do association :auto_devops, factory: :project_auto_devops diff --git a/spec/graphql/mutations/issues/set_confidential_spec.rb b/spec/graphql/mutations/issues/set_confidential_spec.rb index 820f9aa5e17..0b2fc0ecb93 100644 --- a/spec/graphql/mutations/issues/set_confidential_spec.rb +++ b/spec/graphql/mutations/issues/set_confidential_spec.rb @@ -3,7 +3,8 @@ require 'spec_helper' RSpec.describe Mutations::Issues::SetConfidential do - let(:issue) { create(:issue) } + let(:project) { create(:project, :private) } + let(:issue) { create(:issue, project: project, assignees: [user]) } let(:user) { create(:user) } subject(:mutation) { described_class.new(object: nil, context: { current_user: user }, field: nil) } @@ -14,7 +15,7 @@ RSpec.describe Mutations::Issues::SetConfidential do let(:confidential) { true } let(:mutated_issue) { subject[:issue] } - subject { mutation.resolve(project_path: issue.project.full_path, iid: issue.iid, confidential: confidential) } + subject { mutation.resolve(project_path: project.full_path, iid: issue.iid, confidential: confidential) } it 'raises an error if the resource is not accessible to the user' do expect { subject }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable) @@ -22,7 +23,7 @@ RSpec.describe Mutations::Issues::SetConfidential do context 'when the user can update the issue' do before do - issue.project.add_developer(user) + project.add_developer(user) end it 'returns the issue as confidential' do @@ -39,5 +40,19 @@ RSpec.describe Mutations::Issues::SetConfidential do end end end + + context 'when guest user is an assignee' do + let(:project) { create(:project, :public) } + + before do + project.add_guest(user) + end + + it 'does not change issue confidentiality' do + expect(mutated_issue).to eq(issue) + expect(mutated_issue.confidential).to be_falsey + expect(subject[:errors]).to be_empty + end + end end end diff --git a/spec/graphql/mutations/merge_requests/set_milestone_spec.rb b/spec/graphql/mutations/merge_requests/set_milestone_spec.rb index 1c0d655ee83..ccb2d9bd132 100644 --- a/spec/graphql/mutations/merge_requests/set_milestone_spec.rb +++ b/spec/graphql/mutations/merge_requests/set_milestone_spec.rb @@ -3,31 +3,29 @@ require 'spec_helper' RSpec.describe Mutations::MergeRequests::SetMilestone do - let(:merge_request) { create(:merge_request) } let(:user) { create(:user) } + let(:project) { create(:project, :private) } + let(:merge_request) { create(:merge_request, source_project: project, target_project: project, assignees: [user]) } + let(:mutation) { described_class.new(object: nil, context: { current_user: user }, field: nil) } + let(:milestone) { create(:milestone, project: project) } - subject(:mutation) { described_class.new(object: nil, context: { current_user: user }, field: nil) } + subject { mutation.resolve(project_path: project.full_path, iid: merge_request.iid, milestone: milestone) } specify { expect(described_class).to require_graphql_authorizations(:update_merge_request) } describe '#resolve' do - let(:milestone) { create(:milestone, project: merge_request.project) } - let(:mutated_merge_request) { subject[:merge_request] } - - subject { mutation.resolve(project_path: merge_request.project.full_path, iid: merge_request.iid, milestone: milestone) } - it 'raises an error if the resource is not accessible to the user' do expect { subject }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable) end context 'when the user can update the merge request' do before do - merge_request.project.add_developer(user) + project.add_developer(user) end it 'returns the merge request with the milestone' do - expect(mutated_merge_request).to eq(merge_request) - expect(mutated_merge_request.milestone).to eq(milestone) + expect(subject[:merge_request]).to eq(merge_request) + expect(subject[:merge_request].milestone).to eq(milestone) expect(subject[:errors]).to be_empty end @@ -43,13 +41,37 @@ RSpec.describe Mutations::MergeRequests::SetMilestone do let(:milestone) { nil } it 'removes the milestone' do - merge_request.update!(milestone: create(:milestone, project: merge_request.project)) + merge_request.update!(milestone: create(:milestone, project: project)) - expect(mutated_merge_request.milestone).to eq(nil) + expect(subject[:merge_request].milestone).to be_nil end it 'does not do anything if the MR already does not have a milestone' do - expect(mutated_merge_request.milestone).to eq(nil) + expect(subject[:merge_request].milestone).to be_nil + end + end + end + + context 'when issue assignee is a guest' do + let(:project) { create(:project, :public) } + + before do + project.add_guest(user) + end + + it 'does not update the milestone' do + expect(subject[:merge_request]).to eq(merge_request) + expect(subject[:merge_request].milestone).to be_nil + expect(subject[:errors]).to be_empty + end + + context 'when passing milestone_id as nil' do + let(:milestone) { nil } + + it 'does not remove the milestone' do + merge_request.update!(milestone: create(:milestone, project: project)) + + expect(subject[:merge_request].milestone).not_to be_nil end end end diff --git a/spec/migrations/insert_project_feature_flags_plan_limits_spec.rb b/spec/migrations/insert_project_feature_flags_plan_limits_spec.rb new file mode 100644 index 00000000000..1ad070de1ea --- /dev/null +++ b/spec/migrations/insert_project_feature_flags_plan_limits_spec.rb @@ -0,0 +1,80 @@ +# frozen_string_literal: true + +require 'spec_helper' +require Rails.root.join( + 'db', + 'migrate', + '20200831222347_insert_project_feature_flags_plan_limits.rb' +) + +RSpec.describe InsertProjectFeatureFlagsPlanLimits do + let(:migration) { described_class.new } + let(:plans) { table(:plans) } + let(:plan_limits) { table(:plan_limits) } + let!(:default_plan) { plans.create!(name: 'default') } + let!(:free_plan) { plans.create!(name: 'free') } + let!(:bronze_plan) { plans.create!(name: 'bronze') } + let!(:silver_plan) { plans.create!(name: 'silver') } + let!(:gold_plan) { plans.create!(name: 'gold') } + let!(:default_plan_limits) do + plan_limits.create!(plan_id: default_plan.id, project_feature_flags: 200) + end + + context 'when on Gitlab.com' do + before do + expect(Gitlab).to receive(:com?).at_most(:twice).and_return(true) + end + + describe '#up' do + it 'updates the project_feature_flags plan limits' do + migration.up + + expect(plan_limits.pluck(:plan_id, :project_feature_flags)).to contain_exactly( + [default_plan.id, 200], + [free_plan.id, 50], + [bronze_plan.id, 100], + [silver_plan.id, 150], + [gold_plan.id, 200] + ) + end + end + + describe '#down' do + it 'removes the project_feature_flags plan limits' do + migration.up + migration.down + + expect(plan_limits.pluck(:plan_id, :project_feature_flags)).to contain_exactly( + [default_plan.id, 200], + [free_plan.id, 0], + [bronze_plan.id, 0], + [silver_plan.id, 0], + [gold_plan.id, 0] + ) + end + end + end + + context 'when on self-hosted' do + before do + expect(Gitlab).to receive(:com?).at_most(:twice).and_return(false) + end + + describe '#up' do + it 'does not change the plan limits' do + migration.up + + expect(plan_limits.pluck(:project_feature_flags)).to contain_exactly(200) + end + end + + describe '#down' do + it 'does not change the plan limits' do + migration.up + migration.down + + expect(plan_limits.pluck(:project_feature_flags)).to contain_exactly(200) + end + end + end +end 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 4323263855e..d1b81d99867 100644 --- a/spec/models/member_spec.rb +++ b/spec/models/member_spec.rb @@ -18,6 +18,13 @@ RSpec.describe Member do it { is_expected.to validate_presence_of(:source) } it { is_expected.to validate_inclusion_of(:access_level).in_array(Gitlab::Access.all_values) } + 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/requests/api/api_spec.rb b/spec/requests/api/api_spec.rb index bd0426601db..7d637757f38 100644 --- a/spec/requests/api/api_spec.rb +++ b/spec/requests/api/api_spec.rb @@ -67,4 +67,29 @@ RSpec.describe API::API do end end end + + describe 'authentication with deploy token' do + context 'admin mode' do + let_it_be(:project) { create(:project, :public) } + let_it_be(:package) { create(:maven_package, project: project, name: project.full_path) } + let_it_be(:maven_metadatum) { package.maven_metadatum } + let_it_be(:package_file) { package.package_files.first } + let_it_be(:deploy_token) { create(:deploy_token) } + let(:headers_with_deploy_token) do + { + Gitlab::Auth::AuthFinders::DEPLOY_TOKEN_HEADER => deploy_token.token + } + end + + it 'does not bypass the session' do + expect(Gitlab::Auth::CurrentUserMode).not_to receive(:bypass_session!) + + get(api("/packages/maven/#{maven_metadatum.path}/#{package_file.file_name}"), + headers: headers_with_deploy_token) + + expect(response).to have_gitlab_http_status(:ok) + expect(response.media_type).to eq('application/octet-stream') + end + end + end end diff --git a/spec/requests/api/files_spec.rb b/spec/requests/api/files_spec.rb index b50f63ed67c..91877776190 100644 --- a/spec/requests/api/files_spec.rb +++ b/spec/requests/api/files_spec.rb @@ -502,16 +502,13 @@ RSpec.describe API::Files do expect(response).to have_gitlab_http_status(:ok) end - it 'sets no-cache headers' do - url = route('.gitignore') + "/raw" - expect(Gitlab::Workhorse).to receive(:send_git_blob) - - get api(url, current_user), params: params + it_behaves_like 'uncached response' do + before do + url = route('.gitignore') + "/raw" + expect(Gitlab::Workhorse).to receive(:send_git_blob) - expect(response.headers["Cache-Control"]).to include("no-store") - expect(response.headers["Cache-Control"]).to include("no-cache") - expect(response.headers["Pragma"]).to eq("no-cache") - expect(response.headers["Expires"]).to eq("Fri, 01 Jan 1990 00:00:00 GMT") + get api(url, current_user), params: params + end end context 'when mandatory params are not given' do diff --git a/spec/requests/api/graphql/mutations/metrics/dashboard/annotations/delete_spec.rb b/spec/requests/api/graphql/mutations/metrics/dashboard/annotations/delete_spec.rb index 2459a6f3828..2d4ed3ff060 100644 --- a/spec/requests/api/graphql/mutations/metrics/dashboard/annotations/delete_spec.rb +++ b/spec/requests/api/graphql/mutations/metrics/dashboard/annotations/delete_spec.rb @@ -9,13 +9,9 @@ RSpec.describe Mutations::Metrics::Dashboard::Annotations::Delete do let_it_be(:project) { create(:project, :private, :repository) } let_it_be(:environment) { create(:environment, project: project) } let_it_be(:annotation) { create(:metrics_dashboard_annotation, environment: environment) } - let(:mutation) do - variables = { - id: GitlabSchema.id_from_object(annotation).to_s - } - graphql_mutation(:delete_annotation, variables) - end + let(:variables) { { id: GitlabSchema.id_from_object(annotation).to_s } } + let(:mutation) { graphql_mutation(:delete_annotation, variables) } def mutation_response graphql_mutation_response(:delete_annotation) @@ -37,15 +33,11 @@ RSpec.describe Mutations::Metrics::Dashboard::Annotations::Delete do end context 'with invalid params' do - let(:mutation) do - variables = { - id: 'invalid_id' - } + let(:variables) { { id: GitlabSchema.id_from_object(project).to_s } } - graphql_mutation(:delete_annotation, variables) + it_behaves_like 'a mutation that returns top-level errors' do + let(:match_errors) { eq(["#{variables[:id]} is not a valid id for #{annotation.class}."]) } end - - it_behaves_like 'a mutation that returns top-level errors', errors: ['invalid_id is not a valid GitLab id.'] end context 'when the delete fails' do diff --git a/spec/requests/api/members_spec.rb b/spec/requests/api/members_spec.rb index 23889912d7a..549853db88c 100644 --- a/spec/requests/api/members_spec.rb +++ b/spec/requests/api/members_spec.rb @@ -244,13 +244,12 @@ RSpec.describe API::Members do it 'creates a new member' do expect do post api("/#{source_type.pluralize}/#{source.id}/members", maintainer), - params: { user_id: stranger.id, access_level: Member::DEVELOPER, expires_at: '2016-08-05' } + params: { user_id: stranger.id, access_level: Member::DEVELOPER } expect(response).to have_gitlab_http_status(:created) end.to change { source.members.count }.by(1) expect(json_response['id']).to eq(stranger.id) expect(json_response['access_level']).to eq(Member::DEVELOPER) - expect(json_response['expires_at']).to eq('2016-08-05') end end @@ -285,6 +284,40 @@ RSpec.describe API::Members do end end + context 'access expiry date' do + subject do + post api("/#{source_type.pluralize}/#{source.id}/members", maintainer), + params: { user_id: stranger.id, access_level: Member::DEVELOPER, expires_at: expires_at } + end + + context 'when set to a date in the past' do + let(:expires_at) { 2.days.ago.to_date } + + it 'does not create a member' do + expect do + subject + end.not_to change { source.members.count } + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']).to eq({ 'expires_at' => ['cannot be a date in the past'] }) + end + end + + context 'when set to a date in the future' do + let(:expires_at) { 2.days.from_now.to_date } + + it 'creates a member' do + expect do + subject + end.to change { source.members.count }.by(1) + + expect(response).to have_gitlab_http_status(:created) + expect(json_response['id']).to eq(stranger.id) + expect(json_response['expires_at']).to eq(expires_at.to_s) + end + end + end + it "returns 409 if member already exists" do post api("/#{source_type.pluralize}/#{source.id}/members", maintainer), params: { user_id: maintainer.id, access_level: Member::MAINTAINER } @@ -369,12 +402,40 @@ RSpec.describe API::Members do context 'when authenticated as a maintainer/owner' do it 'updates the member' do put api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", maintainer), - params: { access_level: Member::MAINTAINER, expires_at: '2016-08-05' } + params: { access_level: Member::MAINTAINER } expect(response).to have_gitlab_http_status(:ok) expect(json_response['id']).to eq(developer.id) expect(json_response['access_level']).to eq(Member::MAINTAINER) - expect(json_response['expires_at']).to eq('2016-08-05') + end + end + + context 'access expiry date' do + subject do + put api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", maintainer), + params: { expires_at: expires_at, access_level: Member::MAINTAINER } + end + + context 'when set to a date in the past' do + let(:expires_at) { 2.days.ago.to_date } + + it 'does not update the member' do + subject + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']).to eq({ 'expires_at' => ['cannot be a date in the past'] }) + end + end + + context 'when set to a date in the future' do + let(:expires_at) { 2.days.from_now.to_date } + + it 'updates the member' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['expires_at']).to eq(expires_at.to_s) + end end end diff --git a/spec/services/issues/create_service_spec.rb b/spec/services/issues/create_service_spec.rb index fdf2326b75e..0a9e2fd1bcb 100644 --- a/spec/services/issues/create_service_spec.rb +++ b/spec/services/issues/create_service_spec.rb @@ -25,6 +25,7 @@ RSpec.describe Issues::CreateService do assignee_ids: [assignee.id], label_ids: labels.map(&:id), milestone_id: milestone.id, + milestone: milestone, due_date: Date.tomorrow } end @@ -59,6 +60,12 @@ RSpec.describe Issues::CreateService do expect(issue.milestone).to be_nil expect(issue.due_date).to be_nil end + + it 'creates confidential issues' do + issue = described_class.new(project, guest, confidential: true).execute + + expect(issue.confidential).to be_truthy + end end it 'creates a pending todo for new assignee' do diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb index 77bd540e22f..b115879a729 100644 --- a/spec/services/issues/update_service_spec.rb +++ b/spec/services/issues/update_service_spec.rb @@ -10,6 +10,7 @@ RSpec.describe Issues::UpdateService, :mailer do let_it_be(:project, reload: true) { create(:project, :repository, group: group) } let_it_be(:label) { create(:label, project: project) } let_it_be(:label2) { create(:label, project: project) } + let_it_be(:milestone) { create(:milestone, project: project) } let(:issue) do create(:issue, title: 'Old title', @@ -52,7 +53,8 @@ RSpec.describe Issues::UpdateService, :mailer do state_event: 'close', label_ids: [label.id], due_date: Date.tomorrow, - discussion_locked: true + discussion_locked: true, + milestone_id: milestone.id } end @@ -69,6 +71,14 @@ RSpec.describe Issues::UpdateService, :mailer do expect(issue.labels).to match_array [label] expect(issue.due_date).to eq Date.tomorrow expect(issue.discussion_locked).to be_truthy + expect(issue.confidential).to be_falsey + expect(issue.milestone).to eq milestone + end + + it 'updates issue milestone when passing `milestone` param' do + update_issue(milestone: milestone) + + expect(issue.milestone).to eq milestone end it 'refreshes the number of open issues when the issue is made confidential', :use_clean_rails_memory_store_caching do @@ -82,6 +92,8 @@ RSpec.describe Issues::UpdateService, :mailer do expect(TodosDestroyer::ConfidentialIssueWorker).to receive(:perform_in).with(Todo::WAIT_FOR_DELETE, issue.id) update_issue(confidential: true) + + expect(issue.confidential).to be_truthy end it 'does not enqueue ConfidentialIssueWorker when an issue is made non confidential' do @@ -91,6 +103,8 @@ RSpec.describe Issues::UpdateService, :mailer do expect(TodosDestroyer::ConfidentialIssueWorker).not_to receive(:perform_in) update_issue(confidential: false) + + expect(issue.confidential).to be_falsey end it 'updates open issue counter for assignees when issue is reassigned' do @@ -157,7 +171,7 @@ RSpec.describe Issues::UpdateService, :mailer do end it 'filters out params that cannot be set without the :admin_issue permission' do - described_class.new(project, guest, opts).execute(issue) + described_class.new(project, guest, opts.merge(confidential: true)).execute(issue) expect(issue).to be_valid expect(issue.title).to eq 'New title' @@ -167,6 +181,7 @@ RSpec.describe Issues::UpdateService, :mailer do expect(issue.milestone).to be_nil expect(issue.due_date).to be_nil expect(issue.discussion_locked).to be_falsey + expect(issue.confidential).to be_falsey end end diff --git a/spec/services/members/update_service_spec.rb b/spec/services/members/update_service_spec.rb index f510916558b..2d4457f3f62 100644 --- a/spec/services/members/update_service_spec.rb +++ b/spec/services/members/update_service_spec.rb @@ -31,17 +31,35 @@ RSpec.describe Members::UpdateService do end context 'when member is downgraded to guest' do - let(:params) do - { access_level: Gitlab::Access::GUEST } + shared_examples 'schedules to delete confidential todos' do + it do + expect(TodosDestroyer::EntityLeaveWorker).to receive(:perform_in).with(Todo::WAIT_FOR_DELETE, member.user_id, member.source_id, source.class.name).once + + updated_member = described_class.new(current_user, params).execute(member, permission: permission) + + expect(updated_member).to be_valid + expect(updated_member.access_level).to eq(Gitlab::Access::GUEST) + end + end + + context 'with Gitlab::Access::GUEST level as a string' do + let(:params) { { access_level: Gitlab::Access::GUEST.to_s } } + + it_behaves_like 'schedules to delete confidential todos' end - it 'schedules to delete confidential todos' do - expect(TodosDestroyer::EntityLeaveWorker).to receive(:perform_in).with(Todo::WAIT_FOR_DELETE, member.user_id, member.source_id, source.class.name).once + context 'with Gitlab::Access::GUEST level as an integer' do + let(:params) { { access_level: Gitlab::Access::GUEST } } + + it_behaves_like 'schedules to delete confidential todos' + end + end - updated_member = described_class.new(current_user, params).execute(member, permission: permission) + context 'when access_level is invalid' do + let(:params) { { access_level: 'invalid' } } - expect(updated_member).to be_valid - expect(updated_member.access_level).to eq(Gitlab::Access::GUEST) + it 'raises an error' do + expect { described_class.new(current_user, params) }.to raise_error(ArgumentError, 'invalid value for Integer(): "invalid"') end end end diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb index c3433c8c9d2..5cbfb92263f 100644 --- a/spec/services/merge_requests/update_service_spec.rb +++ b/spec/services/merge_requests/update_service_spec.rb @@ -6,12 +6,13 @@ RSpec.describe MergeRequests::UpdateService, :mailer do include ProjectForksHelper let(:group) { create(:group, :public) } - let(:project) { create(:project, :repository, group: group) } + let(:project) { create(:project, :private, :repository, group: group) } let(:user) { create(:user) } let(:user2) { create(:user) } let(:user3) { create(:user) } let(:label) { create(:label, project: project) } let(:label2) { create(:label) } + let(:milestone) { create(:milestone, project: project) } let(:merge_request) do create(:merge_request, :simple, title: 'Old title', @@ -60,7 +61,8 @@ RSpec.describe MergeRequests::UpdateService, :mailer do } end - let(:service) { described_class.new(project, user, opts) } + let(:service) { described_class.new(project, current_user, opts) } + let(:current_user) { user } before do allow(service).to receive(:execute_hooks) @@ -83,6 +85,26 @@ RSpec.describe MergeRequests::UpdateService, :mailer do expect(@merge_request.discussion_locked).to be_truthy end + context 'updating milestone' do + RSpec.shared_examples 'updates milestone' do + it 'sets milestone' do + expect(@merge_request.milestone).to eq milestone + end + end + + context 'when milestone_id param' do + let(:opts) { { milestone_id: milestone.id } } + + it_behaves_like 'updates milestone' + end + + context 'when milestone param' do + let(:opts) { { milestone: milestone } } + + it_behaves_like 'updates milestone' + end + end + it 'executes hooks with update action' do expect(service).to have_received(:execute_hooks) .with( @@ -150,6 +172,46 @@ RSpec.describe MergeRequests::UpdateService, :mailer do expect(note.note).to eq 'locked this merge request' end + context 'when current user cannot admin issues in the project' do + let(:guest) { create(:user) } + let(:current_user) { guest } + + before do + project.add_guest(guest) + end + + it 'filters out params that cannot be set without the :admin_merge_request permission' do + expect(@merge_request).to be_valid + expect(@merge_request.title).to eq('New title') + expect(@merge_request.assignees).to match_array([user3]) + expect(@merge_request).to be_opened + expect(@merge_request.labels.count).to eq(0) + expect(@merge_request.target_branch).to eq('target') + expect(@merge_request.discussion_locked).to be_falsey + expect(@merge_request.milestone).to be_nil + end + + context 'updating milestone' do + RSpec.shared_examples 'does not update milestone' do + it 'sets milestone' do + expect(@merge_request.milestone).to be_nil + end + end + + context 'when milestone_id param' do + let(:opts) { { milestone_id: milestone.id } } + + it_behaves_like 'does not update milestone' + end + + context 'when milestone param' do + let(:opts) { { milestone: milestone } } + + it_behaves_like 'does not update milestone' + end + end + end + context 'when not including source branch removal options' do before do opts.delete(:force_remove_source_branch) diff --git a/spec/services/todos/destroy/entity_leave_service_spec.rb b/spec/services/todos/destroy/entity_leave_service_spec.rb index ccafe3bb7a8..921037bd5db 100644 --- a/spec/services/todos/destroy/entity_leave_service_spec.rb +++ b/spec/services/todos/destroy/entity_leave_service_spec.rb @@ -3,77 +3,145 @@ require 'spec_helper' RSpec.describe Todos::Destroy::EntityLeaveService do + let_it_be(:user, reload: true) { create(:user) } + let_it_be(:user2, reload: true) { create(:user) } + let(:group) { create(:group, :private) } - let(:project) { create(:project, group: group) } - let(:user) { create(:user) } - let(:user2) { create(:user) } - let(:issue) { create(:issue, project: project, confidential: true) } - let(:mr) { create(:merge_request, source_project: project) } - - let!(:todo_mr_user) { create(:todo, user: user, target: mr, project: project) } - let!(:todo_issue_user) { create(:todo, user: user, target: issue, project: project) } - let!(:todo_group_user) { create(:todo, user: user, group: group) } - let!(:todo_issue_user2) { create(:todo, user: user2, target: issue, project: project) } - let!(:todo_group_user2) { create(:todo, user: user2, group: group) } + let(:project) { create(:project, :private, group: group) } + let(:issue) { create(:issue, project: project) } + let(:issue_c) { create(:issue, project: project, confidential: true) } + let!(:todo_group_user) { create(:todo, user: user, group: group) } + let!(:todo_group_user2) { create(:todo, user: user2, group: group) } + + let(:mr) { create(:merge_request, source_project: project) } + let!(:todo_mr_user) { create(:todo, user: user, target: mr, project: project) } + let!(:todo_issue_user) { create(:todo, user: user, target: issue, project: project) } + let!(:todo_issue_c_user) { create(:todo, user: user, target: issue_c, project: project) } + let!(:todo_issue_c_user2) { create(:todo, user: user2, target: issue_c, project: project) } + + shared_examples 'using different access permissions' do |access_table| + using RSpec::Parameterized::TableSyntax + + where(:group_access, :project_access, :c_todos, :mr_todos, :method, &access_table) + + with_them do + before do + set_access(project, user, project_access) if project_access + set_access(group, user, group_access) if group_access + end + + it "#{params[:method].to_s.humanize(capitalize: false)}" do + send(method) + end + end + end + + shared_examples 'does not remove any todos' do + it { does_not_remove_any_todos } + end + + shared_examples 'removes only confidential issues todos' do + it { removes_only_confidential_issues_todos } + end + + def does_not_remove_any_todos + expect { subject }.not_to change { Todo.count } + end + + def removes_only_confidential_issues_todos + expect { subject }.to change { Todo.count }.from(6).to(5) + end + + def removes_confidential_issues_and_merge_request_todos + expect { subject }.to change { Todo.count }.from(6).to(4) + expect(user.todos).to match_array([todo_issue_user, todo_group_user]) + end + + def set_access(object, user, access_name) + case access_name + when :developer + object.add_developer(user) + when :reporter + object.add_reporter(user) + when :guest + object.add_guest(user) + end + end describe '#execute' do - context 'when a user leaves a project' do + describe 'updating a Project' do subject { described_class.new(user.id, project.id, 'Project').execute } + # a private project in a private group is valid context 'when project is private' do - it 'removes project todos for the provided user' do - expect { subject }.to change { Todo.count }.from(5).to(3) - - expect(user.todos).to match_array([todo_group_user]) - expect(user2.todos).to match_array([todo_issue_user2, todo_group_user2]) - end + context 'when user is not a member of the project' do + it 'removes project todos for the provided user' do + expect { subject }.to change { Todo.count }.from(6).to(3) - context 'when the user is member of the project' do - before do - project.add_developer(user) - end - - it 'does not remove any todos' do - expect { subject }.not_to change { Todo.count } + expect(user.todos).to match_array([todo_group_user]) + expect(user2.todos).to match_array([todo_issue_c_user2, todo_group_user2]) end end - context 'when the user is a project guest' do - before do - project.add_guest(user) - end + context 'access permissions' do + # rubocop:disable RSpec/LeakyConstantDeclaration + PRIVATE_PROJECT_PRIVATE_GROUP_ACCESS_TABLE = + lambda do |_| + [ + # :group_access, :project_access, :c_todos, :mr_todos, :method + [nil, :reporter, :keep, :keep, :does_not_remove_any_todos], + [nil, :guest, :delete, :delete, :removes_confidential_issues_and_merge_request_todos], + [:reporter, nil, :keep, :keep, :does_not_remove_any_todos], + [:guest, nil, :delete, :delete, :removes_confidential_issues_and_merge_request_todos], + [:guest, :reporter, :keep, :keep, :does_not_remove_any_todos], + [:guest, :guest, :delete, :delete, :removes_confidential_issues_and_merge_request_todos] + ] + end + # rubocop:enable RSpec/LeakyConstantDeclaration - it 'removes only confidential issues todos' do - expect { subject }.to change { Todo.count }.from(5).to(4) - end + it_behaves_like 'using different access permissions', PRIVATE_PROJECT_PRIVATE_GROUP_ACCESS_TABLE end + end - context 'when the user is member of a parent group' do - before do - group.add_developer(user) - end + # a private project in an internal/public group is valid + context 'when project is private in an internal/public group' do + let(:group) { create(:group, :internal) } + + context 'when user is not a member of the project' do + it 'removes project todos for the provided user' do + expect { subject }.to change { Todo.count }.from(6).to(3) - it 'does not remove any todos' do - expect { subject }.not_to change { Todo.count } + expect(user.todos).to match_array([todo_group_user]) + expect(user2.todos).to match_array([todo_issue_c_user2, todo_group_user2]) end end - context 'when the user is guest of a parent group' do - before do - project.add_guest(user) - end + context 'access permissions' do + # rubocop:disable RSpec/LeakyConstantDeclaration + PRIVATE_PROJECT_INTERNAL_GROUP_ACCESS_TABLE = + lambda do |_| + [ + # :group_access, :project_access, :c_todos, :mr_todos, :method + [nil, :reporter, :keep, :keep, :does_not_remove_any_todos], + [nil, :guest, :delete, :delete, :removes_confidential_issues_and_merge_request_todos], + [:reporter, nil, :keep, :keep, :does_not_remove_any_todos], + [:guest, nil, :delete, :delete, :removes_confidential_issues_and_merge_request_todos], + [:guest, :reporter, :keep, :keep, :does_not_remove_any_todos], + [:guest, :guest, :delete, :delete, :removes_confidential_issues_and_merge_request_todos] + ] + end + # rubocop:enable RSpec/LeakyConstantDeclaration - it 'removes only confidential issues todos' do - expect { subject }.to change { Todo.count }.from(5).to(4) - end + it_behaves_like 'using different access permissions', PRIVATE_PROJECT_INTERNAL_GROUP_ACCESS_TABLE end end + # an internal project in an internal/public group is valid context 'when project is not private' do - before do - group.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL) - project.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL) - end + let(:group) { create(:group, :internal) } + let(:project) { create(:project, :internal, group: group) } + let(:issue) { create(:issue, project: project) } + let(:issue_c) { create(:issue, project: project, confidential: true) } it 'enqueues the PrivateFeaturesWorker' do expect(TodosDestroyer::PrivateFeaturesWorker) @@ -84,50 +152,42 @@ RSpec.describe Todos::Destroy::EntityLeaveService do context 'confidential issues' do context 'when a user is not an author of confidential issue' do - it 'removes only confidential issues todos' do - expect { subject }.to change { Todo.count }.from(5).to(4) - end + it_behaves_like 'removes only confidential issues todos' end context 'when a user is an author of confidential issue' do before do - issue.update!(author: user) + issue_c.update!(author: user) end - it 'does not remove any todos' do - expect { subject }.not_to change { Todo.count } - end + it_behaves_like 'does not remove any todos' end context 'when a user is an assignee of confidential issue' do before do - issue.assignees << user + issue_c.assignees << user end - it 'does not remove any todos' do - expect { subject }.not_to change { Todo.count } - end + it_behaves_like 'does not remove any todos' end - context 'when a user is a project guest' do - before do - project.add_guest(user) - end - - it 'removes only confidential issues todos' do - expect { subject }.to change { Todo.count }.from(5).to(4) - end - end - - context 'when a user is a project guest but group developer' do - before do - project.add_guest(user) - group.add_developer(user) - end - - it 'does not remove any todos' do - expect { subject }.not_to change { Todo.count } - end + context 'access permissions' do + # rubocop:disable RSpec/LeakyConstantDeclaration + INTERNAL_PROJECT_INTERNAL_GROUP_ACCESS_TABLE = + lambda do |_| + [ + # :group_access, :project_access, :c_todos, :mr_todos, :method + [nil, :reporter, :keep, :keep, :does_not_remove_any_todos], + [nil, :guest, :delete, :keep, :removes_only_confidential_issues_todos], + [:reporter, nil, :keep, :keep, :does_not_remove_any_todos], + [:guest, nil, :delete, :keep, :removes_only_confidential_issues_todos], + [:guest, :reporter, :keep, :keep, :does_not_remove_any_todos], + [:guest, :guest, :delete, :keep, :removes_only_confidential_issues_todos] + ] + end + # rubocop:enable RSpec/LeakyConstantDeclaration + + it_behaves_like 'using different access permissions', INTERNAL_PROJECT_INTERNAL_GROUP_ACCESS_TABLE end end @@ -138,42 +198,43 @@ RSpec.describe Todos::Destroy::EntityLeaveService do end it 'removes only users issue todos' do - expect { subject }.to change { Todo.count }.from(5).to(4) + expect { subject }.to change { Todo.count }.from(6).to(5) end end end end end - context 'when a user leaves a group' do + describe 'updating a Group' do subject { described_class.new(user.id, group.id, 'Group').execute } context 'when group is private' do - it 'removes group and subproject todos for the user' do - expect { subject }.to change { Todo.count }.from(5).to(2) - - expect(user.todos).to be_empty - expect(user2.todos).to match_array([todo_issue_user2, todo_group_user2]) - end - - context 'when the user is member of the group' do - before do - group.add_developer(user) - end + context 'when a user leaves a group' do + it 'removes group and subproject todos for the user' do + expect { subject }.to change { Todo.count }.from(6).to(2) - it 'does not remove any todos' do - expect { subject }.not_to change { Todo.count } + expect(user.todos).to be_empty + expect(user2.todos).to match_array([todo_issue_c_user2, todo_group_user2]) end end - context 'when the user is member of the group project but not the group' do - before do - project.add_developer(user) - end + context 'access permissions' do + # rubocop:disable RSpec/LeakyConstantDeclaration + PRIVATE_GROUP_PRIVATE_PROJECT_ACCESS_TABLE = + lambda do |_| + [ + # :group_access, :project_access, :c_todos, :mr_todos, :method + [nil, :reporter, :keep, :keep, :does_not_remove_any_todos], + [nil, :guest, :delete, :delete, :removes_confidential_issues_and_merge_request_todos], + [:reporter, nil, :keep, :keep, :does_not_remove_any_todos], + [:guest, nil, :delete, :delete, :removes_confidential_issues_and_merge_request_todos], + [:guest, :reporter, :keep, :keep, :does_not_remove_any_todos], + [:guest, :guest, :delete, :delete, :removes_confidential_issues_and_merge_request_todos] + ] + end + # rubocop:enable RSpec/LeakyConstantDeclaration - it 'does not remove any todos' do - expect { subject }.not_to change { Todo.count } - end + it_behaves_like 'using different access permissions', PRIVATE_GROUP_PRIVATE_PROJECT_ACCESS_TABLE end context 'with nested groups' do @@ -191,12 +252,12 @@ RSpec.describe Todos::Destroy::EntityLeaveService do context 'when the user is not a member of any groups/projects' do it 'removes todos for the user including subprojects todos' do - expect { subject }.to change { Todo.count }.from(11).to(4) + expect { subject }.to change { Todo.count }.from(12).to(4) expect(user.todos).to be_empty expect(user2.todos) .to match_array( - [todo_issue_user2, todo_group_user2, todo_subproject_user2, todo_subpgroup_user2] + [todo_issue_c_user2, todo_group_user2, todo_subproject_user2, todo_subpgroup_user2] ) end end @@ -208,9 +269,7 @@ RSpec.describe Todos::Destroy::EntityLeaveService do parent_group.add_developer(user) end - it 'does not remove any todos' do - expect { subject }.not_to change { Todo.count } - end + it_behaves_like 'does not remove any todos' end context 'when the user is member of a subgroup' do @@ -219,12 +278,12 @@ RSpec.describe Todos::Destroy::EntityLeaveService do end it 'does not remove group and subproject todos' do - expect { subject }.to change { Todo.count }.from(11).to(7) + expect { subject }.to change { Todo.count }.from(12).to(7) expect(user.todos).to match_array([todo_group_user, todo_subgroup_user, todo_subproject_user]) expect(user2.todos) .to match_array( - [todo_issue_user2, todo_group_user2, todo_subproject_user2, todo_subpgroup_user2] + [todo_issue_c_user2, todo_group_user2, todo_subproject_user2, todo_subpgroup_user2] ) end end @@ -235,12 +294,12 @@ RSpec.describe Todos::Destroy::EntityLeaveService do end it 'does not remove subproject and group todos' do - expect { subject }.to change { Todo.count }.from(11).to(7) + expect { subject }.to change { Todo.count }.from(12).to(7) expect(user.todos).to match_array([todo_subgroup_user, todo_group_user, todo_subproject_user]) expect(user2.todos) .to match_array( - [todo_issue_user2, todo_group_user2, todo_subproject_user2, todo_subpgroup_user2] + [todo_issue_c_user2, todo_group_user2, todo_subproject_user2, todo_subpgroup_user2] ) end end @@ -248,10 +307,10 @@ RSpec.describe Todos::Destroy::EntityLeaveService do end context 'when group is not private' do - before do - group.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL) - project.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL) - end + let(:group) { create(:group, :internal) } + let(:project) { create(:project, :internal, group: group) } + let(:issue) { create(:issue, project: project) } + let(:issue_c) { create(:issue, project: project, confidential: true) } it 'enqueues the PrivateFeaturesWorker' do expect(TodosDestroyer::PrivateFeaturesWorker) @@ -260,31 +319,24 @@ RSpec.describe Todos::Destroy::EntityLeaveService do subject end - context 'when user is not member' do - it 'removes only confidential issues todos' do - expect { subject }.to change { Todo.count }.from(5).to(4) - end - end - - context 'when user is a project guest' do - before do - project.add_guest(user) - end - - it 'removes only confidential issues todos' do - expect { subject }.to change { Todo.count }.from(5).to(4) - end - end - - context 'when user is a project guest & group developer' do - before do - project.add_guest(user) - group.add_developer(user) - end + context 'access permissions' do + # rubocop:disable RSpec/LeakyConstantDeclaration + INTERNAL_GROUP_INTERNAL_PROJECT_ACCESS_TABLE = + lambda do |_| + [ + # :group_access, :project_access, :c_todos, :mr_todos, :method + [nil, nil, :delete, :keep, :removes_only_confidential_issues_todos], + [nil, :reporter, :keep, :keep, :does_not_remove_any_todos], + [nil, :guest, :delete, :keep, :removes_only_confidential_issues_todos], + [:reporter, nil, :keep, :keep, :does_not_remove_any_todos], + [:guest, nil, :delete, :keep, :removes_only_confidential_issues_todos], + [:guest, :reporter, :keep, :keep, :does_not_remove_any_todos], + [:guest, :guest, :delete, :keep, :removes_only_confidential_issues_todos] + ] + end + # rubocop:enable RSpec/LeakyConstantDeclaration - it 'does not remove any todos' do - expect { subject }.not_to change { Todo.count } - end + it_behaves_like 'using different access permissions', INTERNAL_GROUP_INTERNAL_PROJECT_ACCESS_TABLE end end end diff --git a/spec/support/shared_examples/cached_response_shared_examples.rb b/spec/support/shared_examples/cached_response_shared_examples.rb new file mode 100644 index 00000000000..34e5f741b4e --- /dev/null +++ b/spec/support/shared_examples/cached_response_shared_examples.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true +# +# Negates lib/gitlab/no_cache_headers.rb +# + +RSpec.shared_examples 'cached response' do + it 'defines a cached header response' do + expect(response.headers["Cache-Control"]).not_to include("no-store", "no-cache") + expect(response.headers["Pragma"]).not_to eq("no-cache") + expect(response.headers["Expires"]).not_to eq("Fri, 01 Jan 1990 00:00:00 GMT") + end +end diff --git a/spec/validators/future_date_validator_spec.rb b/spec/validators/future_date_validator_spec.rb new file mode 100644 index 00000000000..6814ba7c820 --- /dev/null +++ b/spec/validators/future_date_validator_spec.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe FutureDateValidator do + subject do + Class.new do + include ActiveModel::Model + include ActiveModel::Validations + attr_accessor :expires_at + validates :expires_at, future_date: true + end.new + end + + before do + subject.expires_at = date + end + + context 'past date' do + let(:date) { Date.yesterday } + + it { is_expected.not_to be_valid } + end + + context 'current date' do + let(:date) { Date.today } + + it { is_expected.to be_valid } + end + + context 'future date' do + let(:date) { Date.tomorrow } + + it { is_expected.to be_valid } + end +end diff --git a/spec/workers/remove_expired_members_worker_spec.rb b/spec/workers/remove_expired_members_worker_spec.rb index cbdd5a68698..8a34b41834b 100644 --- a/spec/workers/remove_expired_members_worker_spec.rb +++ b/spec/workers/remove_expired_members_worker_spec.rb @@ -7,9 +7,13 @@ RSpec.describe RemoveExpiredMembersWorker do describe '#perform' do context 'project members' do - let!(:expired_project_member) { create(:project_member, expires_at: 1.hour.ago, access_level: GroupMember::DEVELOPER) } - let!(:project_member_expiring_in_future) { create(:project_member, expires_at: 10.days.from_now, access_level: GroupMember::DEVELOPER) } - let!(:non_expiring_project_member) { create(:project_member, expires_at: nil, access_level: GroupMember::DEVELOPER) } + let_it_be(:expired_project_member) { create(:project_member, expires_at: 1.day.from_now, access_level: GroupMember::DEVELOPER) } + let_it_be(:project_member_expiring_in_future) { create(:project_member, expires_at: 10.days.from_now, access_level: GroupMember::DEVELOPER) } + let_it_be(:non_expiring_project_member) { create(:project_member, expires_at: nil, access_level: GroupMember::DEVELOPER) } + + before do + travel_to(3.days.from_now) + end it 'removes expired members' do expect { worker.perform }.to change { Member.count }.by(-1) @@ -28,9 +32,13 @@ RSpec.describe RemoveExpiredMembersWorker do end context 'group members' do - let!(:expired_group_member) { create(:group_member, expires_at: 1.hour.ago, access_level: GroupMember::DEVELOPER) } - let!(:group_member_expiring_in_future) { create(:group_member, expires_at: 10.days.from_now, access_level: GroupMember::DEVELOPER) } - let!(:non_expiring_group_member) { create(:group_member, expires_at: nil, access_level: GroupMember::DEVELOPER) } + let_it_be(:expired_group_member) { create(:group_member, expires_at: 1.day.from_now, access_level: GroupMember::DEVELOPER) } + let_it_be(:group_member_expiring_in_future) { create(:group_member, expires_at: 10.days.from_now, access_level: GroupMember::DEVELOPER) } + let_it_be(:non_expiring_group_member) { create(:group_member, expires_at: nil, access_level: GroupMember::DEVELOPER) } + + before do + travel_to(3.days.from_now) + end it 'removes expired members' do expect { worker.perform }.to change { Member.count }.by(-1) @@ -49,7 +57,11 @@ RSpec.describe RemoveExpiredMembersWorker do end context 'when the last group owner expires' do - let!(:expired_group_owner) { create(:group_member, expires_at: 1.hour.ago, access_level: GroupMember::OWNER) } + let_it_be(:expired_group_owner) { create(:group_member, expires_at: 1.day.from_now, access_level: GroupMember::OWNER) } + + before do + travel_to(3.days.from_now) + end it 'does not delete the owner' do worker.perform diff --git a/spec/workers/remove_unaccepted_member_invites_worker_spec.rb b/spec/workers/remove_unaccepted_member_invites_worker_spec.rb new file mode 100644 index 00000000000..96d7cf535ed --- /dev/null +++ b/spec/workers/remove_unaccepted_member_invites_worker_spec.rb @@ -0,0 +1,76 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe RemoveUnacceptedMemberInvitesWorker do + let(:worker) { described_class.new } + + describe '#perform' do + context 'unaccepted members' do + before do + stub_const("#{described_class}::EXPIRATION_THRESHOLD", 1.day) + end + + it 'removes unaccepted members', :aggregate_failures do + unaccepted_group_invitee = create( + :group_member, invite_token: 't0ken', + invite_email: 'group_invitee@example.com', + user: nil, + created_at: Time.current - 5.days) + unaccepted_project_invitee = create( + :project_member, invite_token: 't0ken', + invite_email: 'project_invitee@example.com', + user: nil, + created_at: Time.current - 5.days) + + expect { worker.perform }.to change { Member.count }.by(-2) + + expect(Member.where(id: unaccepted_project_invitee.id)).not_to exist + expect(Member.where(id: unaccepted_group_invitee.id)).not_to exist + end + end + + context 'invited members still within expiration threshold' do + it 'leaves invited members', :aggregate_failures do + group_invitee = create( + :group_member, invite_token: 't0ken', + invite_email: 'group_invitee@example.com', + user: nil) + project_invitee = create( + :project_member, invite_token: 't0ken', + invite_email: 'project_invitee@example.com', + user: nil) + + expect { worker.perform }.not_to change { Member.count } + + expect(Member.where(id: group_invitee.id)).to exist + expect(Member.where(id: project_invitee.id)).to exist + end + end + + context 'accepted members' do + before do + stub_const("#{described_class}::EXPIRATION_THRESHOLD", 1.day) + end + + it 'leaves accepted members', :aggregate_failures do + user = create(:user) + accepted_group_invitee = create( + :group_member, invite_token: 't0ken', + invite_email: 'group_invitee@example.com', + user: user, + created_at: Time.current - 5.days) + accepted_project_invitee = create( + :project_member, invite_token: nil, + invite_email: 'project_invitee@example.com', + user: user, + created_at: Time.current - 5.days) + + expect { worker.perform }.not_to change { Member.count } + + expect(Member.where(id: accepted_group_invitee.id)).to exist + expect(Member.where(id: accepted_project_invitee.id)).to exist + end + end + end +end |