diff options
Diffstat (limited to 'spec')
47 files changed, 1125 insertions, 203 deletions
diff --git a/spec/controllers/admin/users_controller_spec.rb b/spec/controllers/admin/users_controller_spec.rb index e346c329720..0ee773f291c 100644 --- a/spec/controllers/admin/users_controller_spec.rb +++ b/spec/controllers/admin/users_controller_spec.rb @@ -42,7 +42,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) } @@ -65,6 +65,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 08552cc28fa..950120ae564 100644 --- a/spec/controllers/profiles/emails_controller_spec.rb +++ b/spec/controllers/profiles/emails_controller_spec.rb @@ -15,6 +15,29 @@ RSpec.describe Profiles::EmailsController do end end + 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 '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 } } } @@ -32,6 +55,10 @@ RSpec.describe Profiles::EmailsController do 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 @@ -54,5 +81,9 @@ RSpec.describe Profiles::EmailsController do expect { subject }.not_to change { ActionMailer::Base.deliveries.size } end end + + 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 f80e18df22e..60957dc72e6 100644 --- a/spec/controllers/registrations_controller_spec.rb +++ b/spec/controllers/registrations_controller_spec.rb @@ -459,6 +459,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 '#welcome' do diff --git a/spec/factories/projects.rb b/spec/factories/projects.rb index 3b579f970cb..091c9d5a245 100644 --- a/spec/factories/projects.rb +++ b/spec/factories/projects.rb @@ -29,6 +29,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 @@ -53,7 +54,8 @@ FactoryBot.define do forking_access_level: evaluator.forking_access_level, merge_requests_access_level: merge_requests_access_level, repository_access_level: evaluator.repository_access_level, - pages_access_level: evaluator.pages_access_level + pages_access_level: evaluator.pages_access_level, + metrics_dashboard_access_level: evaluator.metrics_dashboard_access_level } project.build_project_feature(hash) @@ -315,6 +317,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/features/admin/clusters/eks_spec.rb b/spec/features/admin/clusters/eks_spec.rb index ef49aebc7c5..ad7122bf182 100644 --- a/spec/features/admin/clusters/eks_spec.rb +++ b/spec/features/admin/clusters/eks_spec.rb @@ -13,7 +13,7 @@ RSpec.describe 'Instance-level AWS EKS Cluster', :js do before do visit admin_clusters_path - click_link 'Add Kubernetes cluster' + click_link 'Integrate with a cluster certificate' end context 'when user creates a cluster on AWS EKS' do diff --git a/spec/features/groups/clusters/eks_spec.rb b/spec/features/groups/clusters/eks_spec.rb index 5a62741250a..c361c502cbb 100644 --- a/spec/features/groups/clusters/eks_spec.rb +++ b/spec/features/groups/clusters/eks_spec.rb @@ -19,7 +19,7 @@ RSpec.describe 'Group AWS EKS Cluster', :js do before do visit group_clusters_path(group) - click_link 'Add Kubernetes cluster' + click_link 'Integrate with a cluster certificate' end context 'when user creates a cluster on AWS EKS' do diff --git a/spec/features/groups/clusters/user_spec.rb b/spec/features/groups/clusters/user_spec.rb index 0a1d2284831..97f8864aab2 100644 --- a/spec/features/groups/clusters/user_spec.rb +++ b/spec/features/groups/clusters/user_spec.rb @@ -25,7 +25,7 @@ RSpec.describe 'User Cluster', :js do before do visit group_clusters_path(group) - click_link 'Add Kubernetes cluster' + click_link 'Integrate with a cluster certificate' click_link 'Connect existing cluster' end @@ -129,7 +129,7 @@ RSpec.describe 'User Cluster', :js do it 'user sees creation form with the successful message' do expect(page).to have_content('Kubernetes cluster integration was successfully removed.') - expect(page).to have_link('Add Kubernetes cluster') + expect(page).to have_link('Integrate with a cluster certificate') end end end diff --git a/spec/features/issuables/markdown_references/internal_references_spec.rb b/spec/features/issuables/markdown_references/internal_references_spec.rb index aceaea8d2ed..07d4271eed7 100644 --- a/spec/features/issuables/markdown_references/internal_references_spec.rb +++ b/spec/features/issuables/markdown_references/internal_references_spec.rb @@ -25,7 +25,7 @@ RSpec.describe "Internal references", :js do add_note("##{public_project_issue.to_reference(private_project)}") end - context "when user doesn't have access to private project" do + context "when user doesn't have access to private project", quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/257832' do before do sign_in(public_project_user) @@ -52,7 +52,7 @@ RSpec.describe "Internal references", :js do visit(project_issue_path(public_project, public_project_issue)) end - it "doesn't show any references" do + it "doesn't show any references", quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/257832' do page.within(".issue-details") do expect(page).not_to have_content("#merge-requests .merge-requests-title") end @@ -94,7 +94,7 @@ RSpec.describe "Internal references", :js do add_note("##{public_project_merge_request.to_reference(private_project)}") end - context "when user doesn't have access to private project" do + context "when user doesn't have access to private project", quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/257832' do before do sign_in(public_project_user) @@ -121,7 +121,7 @@ RSpec.describe "Internal references", :js do visit(project_merge_request_path(public_project, public_project_merge_request)) end - it "doesn't show any references" do + it "doesn't show any references", quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/257832' do page.within(".merge-request-details") do expect(page).not_to have_content("#merge-requests .merge-requests-title") end diff --git a/spec/features/projects/clusters/eks_spec.rb b/spec/features/projects/clusters/eks_spec.rb index c5feef6c6f3..9f3f331cfab 100644 --- a/spec/features/projects/clusters/eks_spec.rb +++ b/spec/features/projects/clusters/eks_spec.rb @@ -19,7 +19,7 @@ RSpec.describe 'AWS EKS Cluster', :js do before do visit project_clusters_path(project) - click_link 'Add Kubernetes cluster' + click_link 'Integrate with a cluster certificate' end context 'when user creates a cluster on AWS EKS' do diff --git a/spec/features/projects/clusters/gcp_spec.rb b/spec/features/projects/clusters/gcp_spec.rb index b86a74469f7..a0519d88532 100644 --- a/spec/features/projects/clusters/gcp_spec.rb +++ b/spec/features/projects/clusters/gcp_spec.rb @@ -33,7 +33,7 @@ RSpec.describe 'Gcp Cluster', :js, :do_not_mock_admin_mode do before do visit project_clusters_path(project) - click_link 'Add Kubernetes cluster' + click_link 'Integrate with a cluster certificate' click_link 'Create new cluster' click_link 'Google GKE' end @@ -143,7 +143,7 @@ RSpec.describe 'Gcp Cluster', :js, :do_not_mock_admin_mode do before do visit project_clusters_path(project) - click_link 'Add Kubernetes cluster' + click_link 'Connect cluster with certificate' click_link 'Connect existing cluster' end @@ -162,7 +162,7 @@ RSpec.describe 'Gcp Cluster', :js, :do_not_mock_admin_mode do it 'user sees creation form with the successful message' do expect(page).to have_content('Kubernetes cluster integration was successfully removed.') - expect(page).to have_link('Add Kubernetes cluster') + expect(page).to have_link('Integrate with a cluster certificate') end end end @@ -178,7 +178,7 @@ RSpec.describe 'Gcp Cluster', :js, :do_not_mock_admin_mode do end it 'user sees offer on cluster create page' do - click_link 'Add Kubernetes cluster' + click_link 'Integrate with a cluster certificate' expect(page).to have_css('.gcp-signup-offer') end @@ -195,7 +195,7 @@ RSpec.describe 'Gcp Cluster', :js, :do_not_mock_admin_mode do find('.gcp-signup-offer .js-close').click wait_for_requests - click_link 'Add Kubernetes cluster' + click_link 'Integrate with a cluster certificate' expect(page).not_to have_css('.gcp-signup-offer') end diff --git a/spec/features/projects/clusters/user_spec.rb b/spec/features/projects/clusters/user_spec.rb index 97d2f204036..748eba558aa 100644 --- a/spec/features/projects/clusters/user_spec.rb +++ b/spec/features/projects/clusters/user_spec.rb @@ -25,7 +25,7 @@ RSpec.describe 'User Cluster', :js do before do visit project_clusters_path(project) - click_link 'Add Kubernetes cluster' + click_link 'Integrate with a cluster certificate' click_link 'Connect existing cluster' end @@ -116,7 +116,7 @@ RSpec.describe 'User Cluster', :js do it 'user sees creation form with the successful message' do expect(page).to have_content('Kubernetes cluster integration was successfully removed.') - expect(page).to have_link('Add Kubernetes cluster') + expect(page).to have_link('Integrate with a cluster certificate') end end end diff --git a/spec/features/projects/clusters_spec.rb b/spec/features/projects/clusters_spec.rb index 058a5c3e607..6c6e65005f6 100644 --- a/spec/features/projects/clusters_spec.rb +++ b/spec/features/projects/clusters_spec.rb @@ -19,7 +19,7 @@ RSpec.describe 'Clusters', :js do end it 'sees empty state' do - expect(page).to have_link('Add Kubernetes cluster') + expect(page).to have_link('Integrate with a cluster certificate') expect(page).to have_selector('.empty-state') end end @@ -41,7 +41,7 @@ RSpec.describe 'Clusters', :js do context 'when user filled form with environment scope' do before do - click_link 'Add Kubernetes cluster' + click_link 'Connect cluster with certificate' click_link 'Connect existing cluster' fill_in 'cluster_name', with: 'staging-cluster' fill_in 'cluster_environment_scope', with: 'staging/*' @@ -70,7 +70,7 @@ RSpec.describe 'Clusters', :js do context 'when user updates duplicated environment scope' do before do - click_link 'Add Kubernetes cluster' + click_link 'Connect cluster with certificate' click_link 'Connect existing cluster' fill_in 'cluster_name', with: 'staging-cluster' fill_in 'cluster_environment_scope', with: '*' @@ -116,7 +116,7 @@ RSpec.describe 'Clusters', :js do context 'when user filled form with environment scope' do before do - click_link 'Add Kubernetes cluster' + click_link 'Connect cluster with certificate' click_link 'Create new cluster' click_link 'Google GKE' @@ -161,7 +161,7 @@ RSpec.describe 'Clusters', :js do context 'when user updates duplicated environment scope' do before do - click_link 'Add Kubernetes cluster' + click_link 'Connect cluster with certificate' click_link 'Create new cluster' click_link 'Google GKE' @@ -214,7 +214,7 @@ RSpec.describe 'Clusters', :js do before do visit project_clusters_path(project) - click_link 'Add Kubernetes cluster' + click_link 'Integrate with a cluster certificate' click_link 'Create new cluster' end diff --git a/spec/features/projects/releases/user_creates_release_spec.rb b/spec/features/projects/releases/user_creates_release_spec.rb index dd0d7338a26..a440a4211e3 100644 --- a/spec/features/projects/releases/user_creates_release_spec.rb +++ b/spec/features/projects/releases/user_creates_release_spec.rb @@ -18,7 +18,7 @@ RSpec.describe 'User creates release', :js do project.add_developer(user) - gitlab_sign_in(user) + sign_in(user) visit new_page_url diff --git a/spec/features/projects/releases/user_views_edit_release_spec.rb b/spec/features/projects/releases/user_views_edit_release_spec.rb index 4ed1be6db6b..976840ee4e7 100644 --- a/spec/features/projects/releases/user_views_edit_release_spec.rb +++ b/spec/features/projects/releases/user_views_edit_release_spec.rb @@ -13,7 +13,7 @@ RSpec.describe 'User edits Release', :js do project.add_developer(user) - gitlab_sign_in(user) + sign_in(user) visit edit_project_release_path(project, release) diff --git a/spec/features/projects/releases/user_views_release_spec.rb b/spec/features/projects/releases/user_views_release_spec.rb index c82588746a8..4410f345e56 100644 --- a/spec/features/projects/releases/user_views_release_spec.rb +++ b/spec/features/projects/releases/user_views_release_spec.rb @@ -4,17 +4,25 @@ require 'spec_helper' RSpec.describe 'User views Release', :js do let(:project) { create(:project, :repository) } - let(:release) { create(:release, project: project, name: 'The first release' ) } let(:user) { create(:user) } + let(:release) do + create(:release, + project: project, + name: 'The first release', + description: '**Lorem** _ipsum_ dolor sit [amet](https://example.com)') + end + before do project.add_developer(user) - gitlab_sign_in(user) + sign_in(user) visit project_release_path(project, release) end + it_behaves_like 'page meta description', 'Lorem ipsum dolor sit amet' + it 'renders the breadcrumbs' do within('.breadcrumbs') do expect(page).to have_content("#{project.creator.name} #{project.name} Releases #{release.name}") @@ -31,7 +39,7 @@ RSpec.describe 'User views Release', :js do expect(page).to have_content(release.name) expect(page).to have_content(release.tag) expect(page).to have_content(release.commit.short_id) - expect(page).to have_content(release.description) + expect(page).to have_content('Lorem ipsum dolor sit amet') end end end diff --git a/spec/features/projects/releases/user_views_releases_spec.rb b/spec/features/projects/releases/user_views_releases_spec.rb index 993d3371904..3bf472e82ec 100644 --- a/spec/features/projects/releases/user_views_releases_spec.rb +++ b/spec/features/projects/releases/user_views_releases_spec.rb @@ -16,7 +16,7 @@ RSpec.describe 'User views releases', :js do shared_examples 'releases page' do context('when the user is a maintainer') do before do - gitlab_sign_in(maintainer) + sign_in(maintainer) end it 'sees the release' do @@ -110,7 +110,7 @@ RSpec.describe 'User views releases', :js do context('when the user is a guest') do before do - gitlab_sign_in(guest) + sign_in(guest) end it 'renders release info except for Git-related data' do 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/helpers/clusters_helper_spec.rb b/spec/helpers/clusters_helper_spec.rb index f0945106ce3..6b08b6515cf 100644 --- a/spec/helpers/clusters_helper_spec.rb +++ b/spec/helpers/clusters_helper_spec.rb @@ -59,6 +59,24 @@ RSpec.describe ClustersHelper do end end + describe '#js_cluster_agents_list_data' do + let_it_be(:project) { build(:project, :repository) } + + subject { helper.js_cluster_agents_list_data(project) } + + it 'displays project default branch' do + expect(subject[:default_branch_name]).to eq(project.default_branch) + end + + it 'displays image path' do + expect(subject[:empty_state_image]).to match(%r(/illustrations/logos/clusters_empty|svg)) + end + + it 'displays project path' do + expect(subject[:project_path]).to eq(project.full_path) + end + end + describe '#js_clusters_list_data' do subject { helper.js_clusters_list_data('/path') } diff --git a/spec/lib/gitlab/database/postgres_index_spec.rb b/spec/lib/gitlab/database/postgres_index_spec.rb index 2a9598736ed..1da67a5a6c0 100644 --- a/spec/lib/gitlab/database/postgres_index_spec.rb +++ b/spec/lib/gitlab/database/postgres_index_spec.rb @@ -30,7 +30,7 @@ RSpec.describe Gitlab::Database::PostgresIndex do end end - describe '#regular' do + describe '.regular' do it 'only non-unique indexes' do expect(described_class.regular).to all(have_attributes(unique: false)) end @@ -44,7 +44,17 @@ RSpec.describe Gitlab::Database::PostgresIndex do end end - describe '#random_few' do + describe '.not_match' do + it 'excludes indexes matching the given regex' do + expect(described_class.not_match('^bar_k').map(&:name)).to all(match(/^(?!bar_k).*/)) + end + + it 'matches indexes without this prefix regex' do + expect(described_class.not_match('^bar_k')).not_to be_empty + end + end + + describe '.random_few' do it 'limits to two records by default' do expect(described_class.random_few(2).size).to eq(2) end diff --git a/spec/lib/gitlab/database/reindexing/concurrent_reindex_spec.rb b/spec/lib/gitlab/database/reindexing/concurrent_reindex_spec.rb index 49087e7ff2c..a80bf8176d2 100644 --- a/spec/lib/gitlab/database/reindexing/concurrent_reindex_spec.rb +++ b/spec/lib/gitlab/database/reindexing/concurrent_reindex_spec.rb @@ -58,6 +58,28 @@ RSpec.describe Gitlab::Database::Reindexing::ConcurrentReindex, '#perform' do end end + context 'when the index is a lingering temporary index from a previous reindexing run' do + context 'with the temporary index prefix' do + let(:index_name) { 'tmp_reindex_something' } + + it 'raises an error' do + expect do + subject.perform + end.to raise_error(described_class::ReindexError, /left-over temporary index/) + end + end + + context 'with the replaced index prefix' do + let(:index_name) { 'old_reindex_something' } + + it 'raises an error' do + expect do + subject.perform + end.to raise_error(described_class::ReindexError, /left-over temporary index/) + end + end + end + context 'replacing the original index with a rebuilt copy' do let(:replacement_name) { 'tmp_reindex_42' } let(:replaced_name) { 'old_reindex_42' } diff --git a/spec/lib/gitlab/database/reindexing_spec.rb b/spec/lib/gitlab/database/reindexing_spec.rb index 32580ae9e3a..26954a9a32f 100644 --- a/spec/lib/gitlab/database/reindexing_spec.rb +++ b/spec/lib/gitlab/database/reindexing_spec.rb @@ -52,4 +52,15 @@ RSpec.describe Gitlab::Database::Reindexing do it_behaves_like 'reindexing' end end + + describe '.candidate_indexes' do + subject { described_class.candidate_indexes } + + it 'retrieves regular indexes that are no left-overs from previous runs' do + result = double + expect(Gitlab::Database::PostgresIndex).to receive_message_chain('regular.not_match.not_match').with(no_args).with('^tmp_reindex_').with('^old_reindex_').and_return(result) + + expect(subject).to eq(result) + 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 fb2c3be21bf..118b1492cd6 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 58323bbe26a..b4e941f2856 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) } 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 a388c813640..f77f127ddc8 100644 --- a/spec/requests/api/files_spec.rb +++ b/spec/requests/api/files_spec.rb @@ -532,16 +532,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 7357f3e1e35..9a612c841a2 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 de52087340c..55b2447fc68 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/requests/projects/metrics_dashboard_spec.rb b/spec/requests/projects/metrics_dashboard_spec.rb index f0e0e6a02ee..0a4100f2bf5 100644 --- a/spec/requests/projects/metrics_dashboard_spec.rb +++ b/spec/requests/projects/metrics_dashboard_spec.rb @@ -39,7 +39,9 @@ RSpec.describe 'Projects::MetricsDashboardController' do context 'with anonymous user and public dashboard visibility' do let(:anonymous_user) { create(:user) } - let(:project) { create(:project, :public) } + let(:project) do + create(:project, :public, :metrics_dashboard_enabled) + end before do project.update!(metrics_dashboard_access_level: 'enabled') diff --git a/spec/services/issues/create_service_spec.rb b/spec/services/issues/create_service_spec.rb index e09a7faece5..eeac7fb9923 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 @@ -102,6 +103,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 f0092c35fda..b3e8fba4e9a 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', @@ -53,7 +54,8 @@ RSpec.describe Issues::UpdateService, :mailer do label_ids: [label.id], due_date: Date.tomorrow, discussion_locked: true, - severity: 'low' + severity: 'low', + milestone_id: milestone.id } end @@ -70,6 +72,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 context 'when issue type is not incident' do @@ -128,6 +138,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 @@ -137,6 +149,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 context 'issue in incident type' do @@ -297,7 +311,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' @@ -307,6 +321,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 65f591c9f22..84ca77b4ad5 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', @@ -61,7 +62,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) @@ -85,6 +87,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( @@ -182,6 +204,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 30083357d23..921037bd5db 100644 --- a/spec/services/todos/destroy/entity_leave_service_spec.rb +++ b/spec/services/todos/destroy/entity_leave_service_spec.rb @@ -3,14 +3,15 @@ require 'spec_helper' RSpec.describe Todos::Destroy::EntityLeaveService do - let_it_be(:group, reload: true) { create(:group, :private) } - let_it_be(:project, reload: true) { create(:project, group: group) } let_it_be(:user, reload: true) { create(:user) } let_it_be(:user2, reload: true) { create(:user) } - let_it_be(:issue) { create(:issue, project: project) } - let_it_be(:issue_c) { create(:issue, project: project, confidential: true) } - let_it_be(:todo_group_user) { create(:todo, user: user, group: group) } - let_it_be(:todo_group_user2) { create(:todo, user: user2, group: group) } + + let(:group) { create(:group, :private) } + 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) } @@ -43,10 +44,6 @@ RSpec.describe Todos::Destroy::EntityLeaveService do it { removes_only_confidential_issues_todos } end - shared_examples 'removes confidential issues and merge request todos' do - it { removes_confidential_issues_and_merge_request_todos } - end - def does_not_remove_any_todos expect { subject }.not_to change { Todo.count } end @@ -75,8 +72,9 @@ RSpec.describe Todos::Destroy::EntityLeaveService 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 - context 'when a user leaves a project' do + 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) @@ -87,27 +85,63 @@ RSpec.describe Todos::Destroy::EntityLeaveService do context 'access permissions' do # rubocop:disable RSpec/LeakyConstantDeclaration - PROJECT_PRIVATE_ACCESS_TABLE = + 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, :keep, :removes_only_confidential_issues_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_behaves_like 'using different access permissions', PRIVATE_PROJECT_PRIVATE_GROUP_ACCESS_TABLE + end + 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) + + 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 '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, :keep, :removes_only_confidential_issues_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_behaves_like 'using different access permissions', PROJECT_PRIVATE_ACCESS_TABLE + 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 - let_it_be(:group, reload: true) { create(:group, :internal) } - let_it_be(:project, reload: true) { create(:project, :internal, group: group) } - let_it_be(:issue, reload: true) { create(:issue, project: project) } - let_it_be(:issue_c, reload: true) { create(:issue, project: project, confidential: true) } + 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) @@ -139,7 +173,7 @@ RSpec.describe Todos::Destroy::EntityLeaveService do context 'access permissions' do # rubocop:disable RSpec/LeakyConstantDeclaration - PROJECT_NOT_PRIVATE_ACCESS_TABLE = + INTERNAL_PROJECT_INTERNAL_GROUP_ACCESS_TABLE = lambda do |_| [ # :group_access, :project_access, :c_todos, :mr_todos, :method @@ -147,12 +181,13 @@ RSpec.describe Todos::Destroy::EntityLeaveService do [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], - [:reporter, :guest, :keep, :keep, :does_not_remove_any_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', PROJECT_NOT_PRIVATE_ACCESS_TABLE + it_behaves_like 'using different access permissions', INTERNAL_PROJECT_INTERNAL_GROUP_ACCESS_TABLE end end @@ -185,17 +220,21 @@ RSpec.describe Todos::Destroy::EntityLeaveService do context 'access permissions' do # rubocop:disable RSpec/LeakyConstantDeclaration - GROUP_PRIVATE_ACCESS_TABLE = + 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], - [:reporter, nil, :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_behaves_like 'using different access permissions', GROUP_PRIVATE_ACCESS_TABLE + it_behaves_like 'using different access permissions', PRIVATE_GROUP_PRIVATE_PROJECT_ACCESS_TABLE end context 'with nested groups' do @@ -268,10 +307,10 @@ RSpec.describe Todos::Destroy::EntityLeaveService do end context 'when group is not private' do - let_it_be(:group, reload: true) { create(:group, :internal) } - let_it_be(:project, reload: true) { create(:project, :internal, group: group) } - let_it_be(:issue, reload: true) { create(:issue, project: project) } - let_it_be(:issue_c, reload: true) { create(:issue, project: project, confidential: true) } + 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) @@ -282,18 +321,22 @@ RSpec.describe Todos::Destroy::EntityLeaveService do context 'access permissions' do # rubocop:disable RSpec/LeakyConstantDeclaration - GROUP_NOT_PRIVATE_ACCESS_TABLE = + 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, :guest, :keep, :keep, :does_not_remove_any_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', GROUP_NOT_PRIVATE_ACCESS_TABLE + 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/support/shared_examples/services/merge_request_shared_examples.rb b/spec/support/shared_examples/services/merge_request_shared_examples.rb index a7032640217..2bd06ac3e9c 100644 --- a/spec/support/shared_examples/services/merge_request_shared_examples.rb +++ b/spec/support/shared_examples/services/merge_request_shared_examples.rb @@ -13,11 +13,10 @@ RSpec.shared_examples 'reviewer_ids filter' do end context 'with reviewer_ids' do - let(:reviewer_ids_param) { { reviewer_ids: [reviewer1.id, reviewer2.id, reviewer3.id] } } + let(:reviewer_ids_param) { { reviewer_ids: [reviewer1.id, reviewer2.id] } } let(:reviewer1) { create(:user) } let(:reviewer2) { create(:user) } - let(:reviewer3) { create(:user) } context 'when the current user can admin the merge_request' do context 'when merge_request_reviewer feature is enabled' do @@ -25,14 +24,13 @@ RSpec.shared_examples 'reviewer_ids filter' do stub_feature_flags(merge_request_reviewer: true) end - context 'with reviewers who can read the merge_request' do + context 'with a reviewer who can read the merge_request' do before do project.add_developer(reviewer1) - project.add_developer(reviewer2) end it 'contains reviewers who can read the merge_request' do - expect(execute.reviewers).to contain_exactly(reviewer1, reviewer2) + expect(execute.reviewers).to contain_exactly(reviewer1) end end end diff --git a/spec/tasks/gitlab/db_rake_spec.rb b/spec/tasks/gitlab/db_rake_spec.rb index 183e1ff0831..d379a6873cf 100644 --- a/spec/tasks/gitlab/db_rake_spec.rb +++ b/spec/tasks/gitlab/db_rake_spec.rb @@ -170,7 +170,7 @@ RSpec.describe 'gitlab:db namespace rake task' do context 'when no index_name is given' do it 'rebuilds a random number of large indexes' do - expect(Gitlab::Database::PostgresIndex).to receive_message_chain('regular.random_few').and_return(indexes) + expect(Gitlab::Database::Reindexing).to receive_message_chain('candidate_indexes.random_few').and_return(indexes) expect(Gitlab::Database::Reindexing).to receive(:perform).with(indexes) run_rake_task('gitlab:db:reindex') 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 |