diff options
Diffstat (limited to 'spec')
36 files changed, 1219 insertions, 221 deletions
diff --git a/spec/controllers/autocomplete_controller_spec.rb b/spec/controllers/autocomplete_controller_spec.rb index 0a809e80fcd..1df685e3e5a 100644 --- a/spec/controllers/autocomplete_controller_spec.rb +++ b/spec/controllers/autocomplete_controller_spec.rb @@ -378,61 +378,72 @@ RSpec.describe AutocompleteController do end context 'GET deploy_keys_with_owners' do - let!(:deploy_key) { create(:deploy_key, user: user) } - let!(:deploy_keys_project) { create(:deploy_keys_project, :write_access, project: project, deploy_key: deploy_key) } + let_it_be(:public_project) { create(:project, :public) } + let_it_be(:user) { create(:user) } + let_it_be(:deploy_key) { create(:deploy_key, user: user) } + let_it_be(:deploy_keys_project) do + create(:deploy_keys_project, :write_access, project: public_project, deploy_key: deploy_key) + end context 'unauthorized user' do it 'returns a not found response' do - get(:deploy_keys_with_owners, params: { project_id: project.id }) + get(:deploy_keys_with_owners, params: { project_id: public_project.id }) expect(response).to have_gitlab_http_status(:redirect) end end - context 'when the user who can read the project is logged in' do + context 'when the user is logged in' do before do sign_in(user) end - context 'and they cannot read the project' do + context 'with a non-existing project' do it 'returns a not found response' do - allow(Ability).to receive(:allowed?).and_call_original - allow(Ability).to receive(:allowed?).with(user, :read_project, project).and_return(false) - - get(:deploy_keys_with_owners, params: { project_id: project.id }) + get(:deploy_keys_with_owners, params: { project_id: 9999 }) expect(response).to have_gitlab_http_status(:not_found) end end - it 'renders the deploy key in a json payload, with its owner' do - get(:deploy_keys_with_owners, params: { project_id: project.id }) + context 'with an existing project' do + context 'when user cannot admin project' do + it 'returns a forbidden response' do + get(:deploy_keys_with_owners, params: { project_id: public_project.id }) - expect(json_response.count).to eq(1) - expect(json_response.first['title']).to eq(deploy_key.title) - expect(json_response.first['owner']['id']).to eq(deploy_key.user.id) - end + expect(response).to have_gitlab_http_status(:forbidden) + end + end - context 'with an unknown project' do - it 'returns a not found response' do - get(:deploy_keys_with_owners, params: { project_id: 9999 }) + context 'when user can admin project' do + before do + public_project.add_maintainer(user) + end - expect(response).to have_gitlab_http_status(:not_found) - end - end + context 'and user can read owner of key' do + it 'renders the deploy keys in a json payload, with owner' do + get(:deploy_keys_with_owners, params: { project_id: public_project.id }) - context 'and the user cannot read the owner of the key' do - before do - allow(Ability).to receive(:allowed?).and_call_original - allow(Ability).to receive(:allowed?).with(user, :read_user, deploy_key.user).and_return(false) - end + expect(json_response.count).to eq(1) + expect(json_response.first['title']).to eq(deploy_key.title) + expect(json_response.first['owner']['id']).to eq(deploy_key.user.id) + end + end + + context 'and user cannot read owner of key' do + before do + allow(Ability).to receive(:allowed?).and_call_original + allow(Ability).to receive(:allowed?).with(user, :read_user, deploy_key.user).and_return(false) + end - it 'returns a payload without owner' do - get(:deploy_keys_with_owners, params: { project_id: project.id }) + it 'returns a payload without owner' do + get(:deploy_keys_with_owners, params: { project_id: public_project.id }) - expect(json_response.count).to eq(1) - expect(json_response.first['title']).to eq(deploy_key.title) - expect(json_response.first['owner']).to be_nil + expect(json_response.count).to eq(1) + expect(json_response.first['title']).to eq(deploy_key.title) + expect(json_response.first['owner']).to be_nil + end + end end end end diff --git a/spec/controllers/invites_controller_spec.rb b/spec/controllers/invites_controller_spec.rb index c5e693e3489..b3b7753df61 100644 --- a/spec/controllers/invites_controller_spec.rb +++ b/spec/controllers/invites_controller_spec.rb @@ -143,14 +143,28 @@ RSpec.describe InvitesController do context 'when user exists with the invited email as secondary email' do before do - secondary_email = create(:email, user: user, email: 'foo@example.com') member.update!(invite_email: secondary_email.email) end - it 'is redirected to a new session with invite email param' do - request + context 'when secondary email is confirmed' do + let(:secondary_email) { create(:email, :confirmed, user: user, email: 'foo@example.com') } - expect(response).to redirect_to(new_user_session_path(invite_email: member.invite_email)) + it 'is redirected to a new session with invite email param' do + request + + expect(response).to redirect_to(new_user_session_path(invite_email: member.invite_email)) + end + end + + context 'when secondary email is unconfirmed' do + let(:secondary_email) { create(:email, user: user, email: 'foo@example.com') } + + it 'is redirected to a new registration with invite email param and flash message', :aggregate_failures do + request + + expect(response).to redirect_to(new_user_registration_path(invite_email: member.invite_email)) + expect(flash[:notice]).to eq 'To accept this invitation, create an account or sign in.' + end end end diff --git a/spec/features/signed_commits_spec.rb b/spec/features/signed_commits_spec.rb index 610a80eb12c..dbf35567803 100644 --- a/spec/features/signed_commits_spec.rb +++ b/spec/features/signed_commits_spec.rb @@ -61,7 +61,7 @@ RSpec.describe 'GPG signed commits' do let(:user_2) do create(:user, email: GpgHelpers::User2.emails.first, username: 'bette.cartwright', name: 'Bette Cartwright').tap do |user| # secondary, unverified email - create :email, user: user, email: GpgHelpers::User2.emails.last + create :email, user: user, email: 'mail@koffeinfrei.org' end end @@ -83,10 +83,11 @@ RSpec.describe 'GPG signed commits' do end end - it 'unverified signature: user email does not match the committer email, but is the same user' do + it 'unverified signature: gpg key email does not match the committer_email but is the same user when the committer_email belongs to the user as a confirmed secondary email' do user_2_key + user_2.emails.find_by(email: 'mail@koffeinfrei.org').confirm - visit project_commit_path(project, GpgHelpers::DIFFERING_EMAIL_SHA) + visit project_commit_path(project, GpgHelpers::SIGNED_COMMIT_SHA) wait_for_all_requests page.find('.gpg-status-box', text: 'Unverified').click @@ -99,7 +100,7 @@ RSpec.describe 'GPG signed commits' do end end - it 'unverified signature: user email does not match the committer email' do + it 'unverified signature: gpg key email does not match the committer_email when the committer_email belongs to the user as a unconfirmed secondary email' do user_2_key visit project_commit_path(project, GpgHelpers::SIGNED_COMMIT_SHA) diff --git a/spec/finders/issues_finder_spec.rb b/spec/finders/issues_finder_spec.rb index 3f5a55410d2..be779129971 100644 --- a/spec/finders/issues_finder_spec.rb +++ b/spec/finders/issues_finder_spec.rb @@ -893,42 +893,65 @@ RSpec.describe IssuesFinder do end end - context 'filtering by crm contact' do - let_it_be(:contact1) { create(:contact, group: group) } - let_it_be(:contact2) { create(:contact, group: group) } + context 'crm filtering' do + let_it_be(:root_group) { create(:group) } + let_it_be(:group) { create(:group, parent: root_group) } + let_it_be(:project_crm) { create(:project, :public, group: group) } + let_it_be(:organization) { create(:organization, group: root_group) } + let_it_be(:contact1) { create(:contact, group: root_group, organization: organization) } + let_it_be(:contact2) { create(:contact, group: root_group, organization: organization) } - let_it_be(:contact1_issue1) { create(:issue, project: project1) } - let_it_be(:contact1_issue2) { create(:issue, project: project1) } - let_it_be(:contact2_issue1) { create(:issue, project: project1) } + let_it_be(:contact1_item1) { create(:issue, project: project_crm) } + let_it_be(:contact1_item2) { create(:issue, project: project_crm) } + let_it_be(:contact2_item1) { create(:issue, project: project_crm) } + let_it_be(:item_no_contact) { create(:issue, project: project_crm) } - let(:params) { { crm_contact_id: contact1.id } } + let_it_be(:all_project_issues) do + [contact1_item1, contact1_item2, contact2_item1, item_no_contact] + end - it 'returns for that contact' do - create(:issue_customer_relations_contact, issue: contact1_issue1, contact: contact1) - create(:issue_customer_relations_contact, issue: contact1_issue2, contact: contact1) - create(:issue_customer_relations_contact, issue: contact2_issue1, contact: contact2) + before do + create(:crm_settings, group: root_group, enabled: true) - expect(issues).to contain_exactly(contact1_issue1, contact1_issue2) + create(:issue_customer_relations_contact, issue: contact1_item1, contact: contact1) + create(:issue_customer_relations_contact, issue: contact1_item2, contact: contact1) + create(:issue_customer_relations_contact, issue: contact2_item1, contact: contact2) end - end - context 'filtering by crm organization' do - let_it_be(:organization) { create(:organization, group: group) } - let_it_be(:contact1) { create(:contact, group: group, organization: organization) } - let_it_be(:contact2) { create(:contact, group: group, organization: organization) } + context 'filtering by crm contact' do + let(:params) { { project_id: project_crm.id, crm_contact_id: contact1.id } } - let_it_be(:contact1_issue1) { create(:issue, project: project1) } - let_it_be(:contact1_issue2) { create(:issue, project: project1) } - let_it_be(:contact2_issue1) { create(:issue, project: project1) } + context 'when the user can read crm contacts' do + it 'returns for that contact' do + root_group.add_reporter(user) - let(:params) { { crm_organization_id: organization.id } } + expect(issues).to contain_exactly(contact1_item1, contact1_item2) + end + end - it 'returns for that contact' do - create(:issue_customer_relations_contact, issue: contact1_issue1, contact: contact1) - create(:issue_customer_relations_contact, issue: contact1_issue2, contact: contact1) - create(:issue_customer_relations_contact, issue: contact2_issue1, contact: contact2) + context 'when the user can not read crm contacts' do + it 'does not filter by contact' do + expect(issues).to match_array(all_project_issues) + end + end + end - expect(issues).to contain_exactly(contact1_issue1, contact1_issue2, contact2_issue1) + context 'filtering by crm organization' do + let(:params) { { project_id: project_crm.id, crm_organization_id: organization.id } } + + context 'when the user can read crm organization' do + it 'returns for that organization' do + root_group.add_reporter(user) + + expect(issues).to contain_exactly(contact1_item1, contact1_item2, contact2_item1) + end + end + + context 'when the user can not read crm organization' do + it 'does not filter by organization' do + expect(issues).to match_array(all_project_issues) + end + end end end diff --git a/spec/graphql/resolvers/issues_resolver_spec.rb b/spec/graphql/resolvers/issues_resolver_spec.rb index e6ec9d8c895..609484639cb 100644 --- a/spec/graphql/resolvers/issues_resolver_spec.rb +++ b/spec/graphql/resolvers/issues_resolver_spec.rb @@ -30,6 +30,9 @@ RSpec.describe Resolvers::IssuesResolver do before_all do project.add_developer(current_user) project.add_reporter(reporter) + + create(:crm_settings, group: group, enabled: true) + create(:label_link, label: label1, target: issue1) create(:label_link, label: label1, target: issue2) create(:label_link, label: label2, target: issue2) @@ -399,6 +402,8 @@ RSpec.describe Resolvers::IssuesResolver do let_it_be(:crm_issue3) { create(:issue, project: project) } before_all do + group.add_developer(current_user) + create(:issue_customer_relations_contact, issue: crm_issue1, contact: contact1) create(:issue_customer_relations_contact, issue: crm_issue2, contact: contact2) create(:issue_customer_relations_contact, issue: crm_issue3, contact: contact3) @@ -631,13 +636,13 @@ RSpec.describe Resolvers::IssuesResolver do end it 'finds a specific issue with iid', :request_store do - result = batch_sync(max_queries: 7) { resolve_issues(iid: issue1.iid).to_a } + result = batch_sync(max_queries: 8) { resolve_issues(iid: issue1.iid).to_a } expect(result).to contain_exactly(issue1) end it 'batches queries that only include IIDs', :request_store do - result = batch_sync(max_queries: 7) do + result = batch_sync(max_queries: 8) do [issue1, issue2] .map { |issue| resolve_issues(iid: issue.iid.to_s) } .flat_map(&:to_a) @@ -647,7 +652,7 @@ RSpec.describe Resolvers::IssuesResolver do end it 'finds a specific issue with iids', :request_store do - result = batch_sync(max_queries: 7) do + result = batch_sync(max_queries: 8) do resolve_issues(iids: [issue1.iid]).to_a end diff --git a/spec/lib/gitlab/config/loader/yaml_spec.rb b/spec/lib/gitlab/config/loader/yaml_spec.rb index 66ea931a42c..c7f84cd583c 100644 --- a/spec/lib/gitlab/config/loader/yaml_spec.rb +++ b/spec/lib/gitlab/config/loader/yaml_spec.rb @@ -120,12 +120,6 @@ RSpec.describe Gitlab::Config::Loader::Yaml do it 'returns false' do expect(loader).not_to be_valid end - - it 'returns true if "ci_yaml_limit_size" feature flag is disabled' do - stub_feature_flags(ci_yaml_limit_size: false) - - expect(loader).to be_valid - end end describe '#load!' do diff --git a/spec/lib/gitlab/gpg/commit_spec.rb b/spec/lib/gitlab/gpg/commit_spec.rb index 9c399e78d80..b6b4ea29888 100644 --- a/spec/lib/gitlab/gpg/commit_spec.rb +++ b/spec/lib/gitlab/gpg/commit_spec.rb @@ -274,12 +274,12 @@ RSpec.describe Gitlab::Gpg::Commit do it_behaves_like 'returns the cached signature on second call' end - context 'user email does not match the committer email, but is the same user' do + context 'gpg key email does not match the committer_email but is the same user when the committer_email belongs to the user as a confirmed secondary email' do let!(:commit) { create :commit, project: project, sha: commit_sha, committer_email: GpgHelpers::User2.emails.first } let(:user) do create(:user, email: GpgHelpers::User1.emails.first).tap do |user| - create :email, user: user, email: GpgHelpers::User2.emails.first + create :email, :confirmed, user: user, email: GpgHelpers::User2.emails.first end end @@ -313,6 +313,45 @@ RSpec.describe Gitlab::Gpg::Commit do it_behaves_like 'returns the cached signature on second call' end + context 'gpg key email does not match the committer_email when the committer_email belongs to the user as a unconfirmed secondary email' do + let!(:commit) { create :commit, project: project, sha: commit_sha, committer_email: GpgHelpers::User2.emails.first } + + let(:user) do + create(:user, email: GpgHelpers::User1.emails.first).tap do |user| + create :email, user: user, email: GpgHelpers::User2.emails.first + end + end + + let!(:gpg_key) do + create :gpg_key, key: GpgHelpers::User1.public_key, user: user + end + + before do + allow(Gitlab::Git::Commit).to receive(:extract_signature_lazily) + .with(Gitlab::Git::Repository, commit_sha) + .and_return( + [ + GpgHelpers::User1.signed_commit_signature, + GpgHelpers::User1.signed_commit_base_data + ] + ) + end + + it 'returns an invalid signature' do + expect(described_class.new(commit).signature).to have_attributes( + commit_sha: commit_sha, + project: project, + gpg_key: gpg_key, + gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid, + gpg_key_user_name: GpgHelpers::User1.names.first, + gpg_key_user_email: GpgHelpers::User1.emails.first, + verification_status: 'other_user' + ) + end + + it_behaves_like 'returns the cached signature on second call' + end + context 'user email does not match the committer email' do let!(:commit) { create :commit, project: project, sha: commit_sha, committer_email: GpgHelpers::User2.emails.first } diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index 1546b6a26c8..a342d1f6c1b 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -565,7 +565,6 @@ project: - remove_source_branch_after_merge - deleting_user - upstream_projects -- downstream_projects - upstream_project_subscriptions - downstream_project_subscriptions - service_desk_setting diff --git a/spec/lib/gitlab/legacy_github_import/user_formatter_spec.rb b/spec/lib/gitlab/legacy_github_import/user_formatter_spec.rb index 34659030020..ab3ffddc042 100644 --- a/spec/lib/gitlab/legacy_github_import/user_formatter_spec.rb +++ b/spec/lib/gitlab/legacy_github_import/user_formatter_spec.rb @@ -20,18 +20,31 @@ RSpec.describe Gitlab::LegacyGithubImport::UserFormatter do expect(user.gitlab_id).to eq gl_user.id end - it 'returns GitLab user id when user primary email matches GitHub email' do + it 'returns GitLab user id when user confirmed primary email matches GitHub email' do gl_user = create(:user, email: octocat.email) expect(user.gitlab_id).to eq gl_user.id end - it 'returns GitLab user id when any of user linked emails matches GitHub email' do + it 'returns GitLab user id when user unconfirmed primary email matches GitHub email' do + gl_user = create(:user, :unconfirmed, email: octocat.email) + + expect(user.gitlab_id).to eq gl_user.id + end + + it 'returns GitLab user id when user confirmed secondary email matches GitHub email' do gl_user = create(:user, email: 'johndoe@example.com') - create(:email, user: gl_user, email: octocat.email) + create(:email, :confirmed, user: gl_user, email: octocat.email) expect(user.gitlab_id).to eq gl_user.id end + + it 'returns nil when user unconfirmed secondary email matches GitHub email' do + gl_user = create(:user, email: 'johndoe@example.com') + create(:email, user: gl_user, email: octocat.email) + + expect(user.gitlab_id).to be_nil + end end it 'returns nil when GitHub user is not a GitLab user' do diff --git a/spec/lib/gitlab/visibility_level_spec.rb b/spec/lib/gitlab/visibility_level_spec.rb index 0d34d22cbbe..9555c0afba2 100644 --- a/spec/lib/gitlab/visibility_level_spec.rb +++ b/spec/lib/gitlab/visibility_level_spec.rb @@ -4,20 +4,52 @@ require 'spec_helper' RSpec.describe Gitlab::VisibilityLevel do describe '.level_value' do - it 'converts "public" to integer value' do - expect(described_class.level_value('public')).to eq(Gitlab::VisibilityLevel::PUBLIC) + where(:string_value, :integer_value) do + [ + ['private', described_class::PRIVATE], + ['internal', described_class::INTERNAL], + ['public', described_class::PUBLIC] + ] end - it 'converts string integer to integer value' do - expect(described_class.level_value('20')).to eq(20) + with_them do + it "converts '#{params[:string_value]}' to integer value #{params[:integer_value]}" do + expect(described_class.level_value(string_value)).to eq(integer_value) + end + + it "converts string integer '#{params[:integer_value]}' to integer value #{params[:integer_value]}" do + expect(described_class.level_value(integer_value.to_s)).to eq(integer_value) + end + + it 'defaults to PRIVATE when string integer value is not valid' do + expect(described_class.level_value(integer_value.to_s + 'r')).to eq(described_class::PRIVATE) + expect(described_class.level_value(integer_value.to_s + ' ')).to eq(described_class::PRIVATE) + expect(described_class.level_value('r' + integer_value.to_s)).to eq(described_class::PRIVATE) + end + + it 'defaults to PRIVATE when string value is not valid' do + expect(described_class.level_value(string_value.capitalize)).to eq(described_class::PRIVATE) + expect(described_class.level_value(string_value + ' ')).to eq(described_class::PRIVATE) + expect(described_class.level_value(string_value + 'r')).to eq(described_class::PRIVATE) + end end it 'defaults to PRIVATE when string value is not valid' do - expect(described_class.level_value('invalid')).to eq(Gitlab::VisibilityLevel::PRIVATE) + expect(described_class.level_value('invalid')).to eq(described_class::PRIVATE) end it 'defaults to PRIVATE when integer value is not valid' do - expect(described_class.level_value(100)).to eq(Gitlab::VisibilityLevel::PRIVATE) + expect(described_class.level_value(100)).to eq(described_class::PRIVATE) + end + + context 'when `fallback_value` is set to `nil`' do + it 'returns `nil` when string value is not valid' do + expect(described_class.level_value('invalid', fallback_value: nil)).to be_nil + end + + it 'returns `nil` when integer value is not valid' do + expect(described_class.level_value(100, fallback_value: nil)).to be_nil + end end end diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index 7c67b9a3d63..fe54b1574a2 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -226,27 +226,45 @@ RSpec.describe Commit do end describe '#committer' do - context 'with a confirmed e-mail' do - it 'returns the user' do - user = create(:user, email: commit.committer_email) + context "when committer_email is the user's primary email" do + context 'when the user email is confirmed' do + let!(:user) { create(:user, email: commit.committer_email) } - expect(commit.committer).to eq(user) + it 'returns the user' do + expect(commit.committer).to eq(user) + expect(commit.committer(confirmed: false)).to eq(user) + end end - end - context 'with an unconfirmed e-mail' do - let(:user) { create(:user) } + context 'when the user email is unconfirmed' do + let!(:user) { create(:user, :unconfirmed, email: commit.committer_email) } - before do - create(:email, user: user, email: commit.committer_email) + it 'returns the user according to confirmed argument' do + expect(commit.committer).to be_nil + expect(commit.committer(confirmed: false)).to eq(user) + end end + end - it 'returns no user' do - expect(commit.committer).to be_nil + context "when committer_email is the user's secondary email" do + let!(:user) { create(:user) } + + context 'when the user email is confirmed' do + let!(:email) { create(:email, :confirmed, user: user, email: commit.committer_email) } + + it 'returns the user' do + expect(commit.committer).to eq(user) + expect(commit.committer(confirmed: false)).to eq(user) + end end - it 'returns the user' do - expect(commit.committer(confirmed: false)).to eq(user) + context 'when the user email is unconfirmed' do + let!(:email) { create(:email, user: user, email: commit.committer_email) } + + it 'does not return the user' do + expect(commit.committer).to be_nil + expect(commit.committer(confirmed: false)).to be_nil + end end end end diff --git a/spec/models/error_tracking/project_error_tracking_setting_spec.rb b/spec/models/error_tracking/project_error_tracking_setting_spec.rb index 15b6b45eaba..0685144dea6 100644 --- a/spec/models/error_tracking/project_error_tracking_setting_spec.rb +++ b/spec/models/error_tracking/project_error_tracking_setting_spec.rb @@ -121,6 +121,38 @@ RSpec.describe ErrorTracking::ProjectErrorTrackingSetting do end end end + + describe 'before_validation :reset_token' do + context 'when a token was previously set' do + subject { create(:project_error_tracking_setting, project: project) } + + it 'resets token if url changed' do + subject.api_url = 'http://sentry.com/api/0/projects/org-slug/proj-slug/' + + expect(subject).not_to be_valid + expect(subject.token).to be_nil + end + + it "does not reset token if new url is set together with the same token" do + subject.api_url = 'http://sentrytest.com/api/0/projects/org-slug/proj-slug/' + current_token = subject.token + subject.token = current_token + + expect(subject).to be_valid + expect(subject.token).to eq(current_token) + expect(subject.api_url).to eq('http://sentrytest.com/api/0/projects/org-slug/proj-slug/') + end + + it 'does not reset token if new url is set together with a new token' do + subject.api_url = 'http://sentrytest.com/api/0/projects/org-slug/proj-slug/' + subject.token = 'token' + + expect(subject).to be_valid + expect(subject.token).to eq('token') + expect(subject.api_url).to eq('http://sentrytest.com/api/0/projects/org-slug/proj-slug/') + end + end + end end describe '.extract_sentry_external_url' do diff --git a/spec/models/grafana_integration_spec.rb b/spec/models/grafana_integration_spec.rb index bb822187e0c..73ec2856c05 100644 --- a/spec/models/grafana_integration_spec.rb +++ b/spec/models/grafana_integration_spec.rb @@ -86,4 +86,38 @@ RSpec.describe GrafanaIntegration do end end end + + describe 'Callbacks' do + describe 'before_validation :reset_token' do + context 'when a token was previously set' do + subject(:grafana_integration) { create(:grafana_integration) } + + it 'resets token if url changed' do + grafana_integration.grafana_url = 'http://gitlab1.com' + + expect(grafana_integration).not_to be_valid + expect(grafana_integration.send(:token)).to be_nil + end + + it "does not reset token if new url is set together with the same token" do + grafana_integration.grafana_url = 'http://gitlab_edited.com' + current_token = grafana_integration.send(:token) + grafana_integration.token = current_token + + expect(grafana_integration).to be_valid + expect(grafana_integration.send(:token)).to eq(current_token) + expect(grafana_integration.grafana_url).to eq('http://gitlab_edited.com') + end + + it 'does not reset token if new url is set together with a new token' do + grafana_integration.grafana_url = 'http://gitlab_edited.com' + grafana_integration.token = 'token' + + expect(grafana_integration).to be_valid + expect(grafana_integration.send(:token)).to eq('token') + expect(grafana_integration.grafana_url).to eq('http://gitlab_edited.com') + end + end + end + end end diff --git a/spec/models/hooks/web_hook_log_spec.rb b/spec/models/hooks/web_hook_log_spec.rb index 9cfbb14e087..25df569c461 100644 --- a/spec/models/hooks/web_hook_log_spec.rb +++ b/spec/models/hooks/web_hook_log_spec.rb @@ -30,15 +30,12 @@ RSpec.describe WebHookLog do end describe '#save' do - let(:web_hook_log) { build(:web_hook_log, url: url) } - let(:url) { 'http://example.com' } - - subject { web_hook_log.save! } + context 'with basic auth credentials' do + let(:web_hook_log) { build(:web_hook_log, url: 'http://test:123@example.com') } - it { is_expected.to eq(true) } + subject { web_hook_log.save! } - context 'with basic auth credentials' do - let(:url) { 'http://test:123@example.com'} + it { is_expected.to eq(true) } it 'obfuscates the basic auth credentials' do subject @@ -46,6 +43,30 @@ RSpec.describe WebHookLog do expect(web_hook_log.url).to eq('http://*****:*****@example.com') end end + + context 'with author email' do + let(:author) { create(:user) } + let(:web_hook_log) { create(:web_hook_log, request_data: data) } + let(:data) do + { + commit: { + author: { + name: author.name, + email: author.email + } + } + }.deep_stringify_keys + end + + it "redacts author's email" do + expect(web_hook_log.request_data['commit']).to match a_hash_including( + 'author' => { + 'name' => author.name, + 'email' => _('[REDACTED]') + } + ) + end + end end describe '#success?' do diff --git a/spec/models/integrations/campfire_spec.rb b/spec/models/integrations/campfire_spec.rb index 0044e6fae21..d249c8470ca 100644 --- a/spec/models/integrations/campfire_spec.rb +++ b/spec/models/integrations/campfire_spec.rb @@ -5,7 +5,17 @@ require 'spec_helper' RSpec.describe Integrations::Campfire do include StubRequests + it_behaves_like Integrations::ResetSecretFields do + let(:integration) { described_class.new } + end + describe 'Validations' do + it { is_expected.to validate_numericality_of(:room).is_greater_than(0).only_integer } + it { is_expected.to validate_length_of(:subdomain).is_at_most(63) } + it { is_expected.to allow_value("foo").for(:subdomain) } + it { is_expected.not_to allow_value("foo.bar").for(:subdomain) } + it { is_expected.not_to allow_value("foo.bar/#").for(:subdomain) } + context 'when integration is active' do before do subject.active = true diff --git a/spec/models/integrations/drone_ci_spec.rb b/spec/models/integrations/drone_ci_spec.rb index 78d55c49e7b..5ae4af1a665 100644 --- a/spec/models/integrations/drone_ci_spec.rb +++ b/spec/models/integrations/drone_ci_spec.rb @@ -7,6 +7,10 @@ RSpec.describe Integrations::DroneCi, :use_clean_rails_memory_store_caching do subject(:integration) { described_class.new } + it_behaves_like Integrations::ResetSecretFields do + let(:integration) { subject } + end + describe 'validations' do context 'active' do before do diff --git a/spec/models/integrations/jira_spec.rb b/spec/models/integrations/jira_spec.rb index 061c770a61a..84abcf69fc1 100644 --- a/spec/models/integrations/jira_spec.rb +++ b/spec/models/integrations/jira_spec.rb @@ -12,6 +12,7 @@ RSpec.describe Integrations::Jira do let(:api_url) { 'http://api-jira.example.com' } let(:username) { 'jira-username' } let(:password) { 'jira-password' } + let(:project_key) { nil } let(:transition_id) { 'test27' } let(:server_info_results) { { 'deploymentType' => 'Cloud' } } let(:jira_integration) do @@ -19,7 +20,8 @@ RSpec.describe Integrations::Jira do project: project, url: url, username: username, - password: password + password: password, + project_key: project_key ) end @@ -478,6 +480,22 @@ RSpec.describe Integrations::Jira do expect(WebMock).to have_requested(:get, issue_url) end end + + context 'with restricted restrict_project_key option' do + subject(:find_issue) { jira_integration.find_issue(issue_key, restrict_project_key: true) } + + it { is_expected.to eq(nil) } + + context 'and project_key matches' do + let(:project_key) { 'JIRA' } + + it 'calls the Jira API to get the issue' do + find_issue + + expect(WebMock).to have_requested(:get, issue_url) + end + end + end end describe '#close_issue' do diff --git a/spec/models/integrations/packagist_spec.rb b/spec/models/integrations/packagist_spec.rb index dce96890522..d1976e73e2e 100644 --- a/spec/models/integrations/packagist_spec.rb +++ b/spec/models/integrations/packagist_spec.rb @@ -29,6 +29,10 @@ RSpec.describe Integrations::Packagist do let(:hook_url) { "#{packagist_server}/api/update-package?username=#{packagist_username}&apiToken=#{packagist_token}" } end + it_behaves_like Integrations::ResetSecretFields do + let(:integration) { described_class.new(packagist_params) } + end + describe '#execute' do let(:user) { create(:user) } let(:project) { create(:project, :repository) } diff --git a/spec/models/integrations/zentao_spec.rb b/spec/models/integrations/zentao_spec.rb index 2b0532c7930..4ef977ba3d2 100644 --- a/spec/models/integrations/zentao_spec.rb +++ b/spec/models/integrations/zentao_spec.rb @@ -9,6 +9,31 @@ RSpec.describe Integrations::Zentao do let(:zentao_product_xid) { '3' } let(:zentao_integration) { create(:zentao_integration) } + it_behaves_like Integrations::ResetSecretFields do + let(:integration) { zentao_integration } + end + + describe 'set_default_data' do + let(:project) { create(:project, :repository) } + + context 'when gitlab.yml was initialized' do + it 'is prepopulated with the settings' do + settings = { + 'zentao' => { + 'url' => 'http://zentao.sample/projects/project_a', + 'api_url' => 'http://zentao.sample/api' + } + } + allow(Gitlab.config).to receive(:issues_tracker).and_return(settings) + + integration = project.create_zentao_integration(active: true) + + expect(integration.url).to eq('http://zentao.sample/projects/project_a') + expect(integration.api_url).to eq('http://zentao.sample/api') + end + end + end + describe '#create' do let(:project) { create(:project, :repository) } let(:params) do diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index ed5b3d4e0be..990189a10a1 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -5623,6 +5623,7 @@ RSpec.describe Project, factory_default: :keep do let(:import_state) { create(:import_state, project: project) } it 'runs the correct hooks' do + expect(project.repository).to receive(:remove_prohibited_branches) expect(project.repository).to receive(:expire_content_cache) expect(project.wiki.repository).to receive(:expire_content_cache) expect(import_state).to receive(:finish) diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 215f83adf5d..e24adc20143 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -3375,4 +3375,62 @@ RSpec.describe Repository do end end end + + describe '#remove_prohibited_branches' do + let(:branch_name) { '37fd3601be4c25497a39fa2e6a206e09e759d597' } + + before do + allow(repository.raw_repository).to receive(:branch_names).and_return([branch_name]) + end + + context 'when prohibited branch exists' do + it 'deletes prohibited branch' do + expect(repository.raw_repository).to receive(:delete_branch).with(branch_name) + + repository.remove_prohibited_branches + end + end + + shared_examples 'does not delete branch' do + it 'returns without removing the branch' do + expect(repository.raw_repository).not_to receive(:delete_branch) + + repository.remove_prohibited_branches + end + end + + context 'when branch name is 40-characters long but not hexadecimal' do + let(:branch_name) { '37fd3601be4c25497a39fa2e6a206e09e759d59s' } + + include_examples 'does not delete branch' + end + + context 'when branch name is hexadecimal' do + context 'when branch name is less than 40-characters long' do + let(:branch_name) { '37fd3601be4c25497a39fa2e6a206e09e759d' } + + include_examples 'does not delete branch' + end + + context 'when branch name is more than 40-characters long' do + let(:branch_name) { '37fd3601be4c25497a39fa2e6a206e09e759dfdfd' } + + include_examples 'does not delete branch' + end + end + + context 'when prohibited branch does not exist' do + let(:branch_name) { 'main' } + + include_examples 'does not delete branch' + end + + context 'when raw repository does not exist' do + before do + allow(repository).to receive(:exists?).and_return(false) + end + + include_examples 'does not delete branch' + end + end end diff --git a/spec/models/snippet_spec.rb b/spec/models/snippet_spec.rb index 70afafce132..a54edc8510e 100644 --- a/spec/models/snippet_spec.rb +++ b/spec/models/snippet_spec.rb @@ -36,8 +36,6 @@ RSpec.describe Snippet do it { is_expected.to validate_presence_of(:content) } - it { is_expected.to validate_inclusion_of(:visibility_level).in_array(Gitlab::VisibilityLevel.values) } - it do allow(Gitlab::CurrentSettings).to receive(:snippet_size_limit).and_return(1) diff --git a/spec/models/todo_spec.rb b/spec/models/todo_spec.rb index 651e2cf273f..7a312a13259 100644 --- a/spec/models/todo_spec.rb +++ b/spec/models/todo_spec.rb @@ -475,4 +475,14 @@ RSpec.describe Todo do it { is_expected.to contain_exactly(user1.id, user2.id) } end + + describe '.for_internal_notes' do + it 'returns todos created from internal notes' do + internal_note = create(:note, confidential: true ) + todo = create(:todo, note: internal_note) + create(:todo) + + expect(described_class.for_internal_notes).to contain_exactly(todo) + end + end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 6c887777dac..c9067833b63 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -2634,6 +2634,14 @@ RSpec.describe User do expect(described_class.find_by_any_email(private_email, confirmed: true)).to eq(user) end + it 'finds user through private commit email when user is unconfirmed' do + user = create(:user, :unconfirmed) + private_email = user.private_commit_email + + expect(described_class.find_by_any_email(private_email)).to eq(user) + expect(described_class.find_by_any_email(private_email, confirmed: true)).to eq(user) + end + it 'finds by primary email' do user = create(:user, email: 'foo@example.com') @@ -2641,6 +2649,13 @@ RSpec.describe User do expect(described_class.find_by_any_email(user.email, confirmed: true)).to eq user end + it 'finds by primary email when user is unconfirmed according to confirmed argument' do + user = create(:user, :unconfirmed, email: 'foo@example.com') + + expect(described_class.find_by_any_email(user.email)).to eq user + expect(described_class.find_by_any_email(user.email, confirmed: true)).to be_nil + end + it 'finds by uppercased email' do user = create(:user, email: 'foo@example.com') @@ -2649,35 +2664,47 @@ RSpec.describe User do end context 'finds by secondary email' do - let(:user) { email.user } + context 'when primary email is confirmed' do + let(:user) { email.user } - context 'primary email confirmed' do - context 'secondary email confirmed' do + context 'when secondary email is confirmed' do let!(:email) { create(:email, :confirmed, email: 'foo@example.com') } - it 'finds user respecting the confirmed flag' do + it 'finds user' do expect(described_class.find_by_any_email(email.email)).to eq user expect(described_class.find_by_any_email(email.email, confirmed: true)).to eq user end end - context 'secondary email not confirmed' do + context 'when secondary email is unconfirmed' do let!(:email) { create(:email, email: 'foo@example.com') } - it 'finds user respecting the confirmed flag' do - expect(described_class.find_by_any_email(email.email)).to eq user + it 'does not find user' do + expect(described_class.find_by_any_email(email.email)).to be_nil expect(described_class.find_by_any_email(email.email, confirmed: true)).to be_nil end end end - context 'primary email not confirmed' do + context 'when primary email is unconfirmed' do let(:user) { create(:user, :unconfirmed) } - let!(:email) { create(:email, :confirmed, user: user, email: 'foo@example.com') } - it 'finds user respecting the confirmed flag' do - expect(described_class.find_by_any_email(email.email)).to eq user - expect(described_class.find_by_any_email(email.email, confirmed: true)).to be_nil + context 'when secondary email is confirmed' do + let!(:email) { create(:email, :confirmed, user: user, email: 'foo@example.com') } + + it 'finds user according to confirmed argument' do + expect(described_class.find_by_any_email(email.email)).to eq user + expect(described_class.find_by_any_email(email.email, confirmed: true)).to be_nil + end + end + + context 'when secondary email is unconfirmed' do + let!(:email) { create(:email, user: user, email: 'foo@example.com') } + + it 'does not find user' do + expect(described_class.find_by_any_email(email.email)).to be_nil + expect(described_class.find_by_any_email(email.email, confirmed: true)).to be_nil + end end end end @@ -2685,13 +2712,6 @@ RSpec.describe User do it 'returns nil when nothing found' do expect(described_class.find_by_any_email('')).to be_nil end - - it 'returns nil when user is not confirmed' do - user = create(:user, :unconfirmed, email: 'foo@example.com') - - expect(described_class.find_by_any_email(user.email, confirmed: false)).to eq(user) - expect(described_class.find_by_any_email(user.email, confirmed: true)).to be_nil - end end describe '.by_any_email' do @@ -2700,32 +2720,99 @@ RSpec.describe User do .to be_a_kind_of(ActiveRecord::Relation) end - it 'returns a relation of users' do + it 'returns empty relation of users when nothing found' do + expect(described_class.by_any_email('')).to be_empty + end + + it 'returns a relation of users for confirmed primary emails' do user = create(:user) - expect(described_class.by_any_email(user.email)).to eq([user]) + expect(described_class.by_any_email(user.email)).to match_array([user]) + expect(described_class.by_any_email(user.email, confirmed: true)).to match_array([user]) end - it 'returns a relation of users for confirmed users' do - user = create(:user) + it 'returns a relation of users for unconfirmed primary emails according to confirmed argument' do + user = create(:user, :unconfirmed) - expect(described_class.by_any_email(user.email, confirmed: true)).to eq([user]) + expect(described_class.by_any_email(user.email)).to match_array([user]) + expect(described_class.by_any_email(user.email, confirmed: true)).to be_empty end - it 'finds user through a private commit email' do + it 'finds users through private commit emails' do user = create(:user) private_email = user.private_commit_email - expect(described_class.by_any_email(private_email)).to eq([user]) - expect(described_class.by_any_email(private_email, confirmed: true)).to eq([user]) + expect(described_class.by_any_email(private_email)).to match_array([user]) + expect(described_class.by_any_email(private_email, confirmed: true)).to match_array([user]) + end + + it 'finds unconfirmed users through private commit emails' do + user = create(:user, :unconfirmed) + private_email = user.private_commit_email + + expect(described_class.by_any_email(private_email)).to match_array([user]) + expect(described_class.by_any_email(private_email, confirmed: true)).to match_array([user]) end it 'finds user through a private commit email in an array' do user = create(:user) private_email = user.private_commit_email - expect(described_class.by_any_email([private_email])).to eq([user]) - expect(described_class.by_any_email([private_email], confirmed: true)).to eq([user]) + expect(described_class.by_any_email([private_email])).to match_array([user]) + expect(described_class.by_any_email([private_email], confirmed: true)).to match_array([user]) + end + + it 'finds by uppercased email' do + user = create(:user, email: 'foo@example.com') + + expect(described_class.by_any_email(user.email.upcase)).to match_array([user]) + expect(described_class.by_any_email(user.email.upcase, confirmed: true)).to match_array([user]) + end + + context 'finds by secondary email' do + context 'when primary email is confirmed' do + let(:user) { email.user } + + context 'when secondary email is confirmed' do + let!(:email) { create(:email, :confirmed, email: 'foo@example.com') } + + it 'finds user' do + expect(described_class.by_any_email(email.email)).to match_array([user]) + expect(described_class.by_any_email(email.email, confirmed: true)).to match_array([user]) + end + end + + context 'when secondary email is unconfirmed' do + let!(:email) { create(:email, email: 'foo@example.com') } + + it 'does not find user' do + expect(described_class.by_any_email(email.email)).to be_empty + expect(described_class.by_any_email(email.email, confirmed: true)).to be_empty + end + end + end + + context 'when primary email is unconfirmed' do + let(:user) { create(:user, :unconfirmed) } + + context 'when secondary email is confirmed' do + let!(:email) { create(:email, :confirmed, user: user, email: 'foo@example.com') } + + it 'finds user according to confirmed argument' do + expect(described_class.by_any_email(email.email)).to match_array([user]) + expect(described_class.by_any_email(email.email, confirmed: true)).to be_empty + end + end + + context 'when secondary email is unconfirmed' do + let!(:email) { create(:email, user: user, email: 'foo@example.com') } + + it 'does not find user' do + expect(described_class.by_any_email(email.email)).to be_empty + expect(described_class.by_any_email(email.email, confirmed: true)).to be_empty + end + end + end end end @@ -2739,7 +2826,10 @@ RSpec.describe User do let_it_be(:user2) { create(:user, name: 'user name', username: 'username', email: 'someemail@example.com') } let_it_be(:user3) { create(:user, name: 'us', username: 'se', email: 'foo@example.com') } - let_it_be(:email) { create(:email, user: user, email: 'alias@example.com') } + let_it_be(:unconfirmed_user) { create(:user, :unconfirmed, name: 'not verified', username: 'notverified') } + + let_it_be(:unconfirmed_secondary_email) { create(:email, user: user, email: 'alias@example.com') } + let_it_be(:confirmed_secondary_email) { create(:email, :confirmed, user: user, email: 'alias2@example.com') } describe 'name user and email relative ordering' do let_it_be(:named_alexander) { create(:user, name: 'Alexander Person', username: 'abcd', email: 'abcd@example.com') } @@ -2797,16 +2887,25 @@ RSpec.describe User do it 'does not return users with a matching private email' do expect(described_class.search(user.email)).to be_empty - expect(described_class.search(email.email)).to be_empty + expect(described_class.search(unconfirmed_secondary_email.email)).to be_empty + expect(described_class.search(confirmed_secondary_email.email)).to be_empty end context 'with private emails search' do - it 'returns users with matching private email' do + it 'returns users with matching private primary email' do expect(described_class.search(user.email, with_private_emails: true)).to match_array([user]) end - it 'returns users with matching private secondary email' do - expect(described_class.search(email.email, with_private_emails: true)).to match_array([user]) + it 'returns users with matching private unconfirmed primary email' do + expect(described_class.search(unconfirmed_user.email, with_private_emails: true)).to match_array([unconfirmed_user]) + end + + it 'returns users with matching private confirmed secondary email' do + expect(described_class.search(confirmed_secondary_email.email, with_private_emails: true)).to match_array([user]) + end + + it 'does not return users with matching private unconfirmed secondary email' do + expect(described_class.search(unconfirmed_secondary_email.email, with_private_emails: true)).to be_empty end end end @@ -3049,47 +3148,108 @@ RSpec.describe User do describe '#accept_pending_invitations!' do let(:user) { create(:user, email: 'user@email.com') } + + let(:confirmed_secondary_email) { create(:email, :confirmed, email: 'confirmedsecondary@example.com', user: user) } + let(:unconfirmed_secondary_email) { create(:email, email: 'unconfirmedsecondary@example.com', user: user) } + let!(:project_member_invite) { create(:project_member, :invited, invite_email: user.email) } let!(:group_member_invite) { create(:group_member, :invited, invite_email: user.email) } + let!(:external_project_member_invite) { create(:project_member, :invited, invite_email: 'external@email.com') } let!(:external_group_member_invite) { create(:group_member, :invited, invite_email: 'external@email.com') } + let!(:project_member_invite_via_confirmed_secondary_email) { create(:project_member, :invited, invite_email: confirmed_secondary_email.email) } + let!(:group_member_invite_via_confirmed_secondary_email) { create(:group_member, :invited, invite_email: confirmed_secondary_email.email) } + + let!(:project_member_invite_via_unconfirmed_secondary_email) { create(:project_member, :invited, invite_email: unconfirmed_secondary_email.email) } + let!(:group_member_invite_via_unconfirmed_secondary_email) { create(:group_member, :invited, invite_email: unconfirmed_secondary_email.email) } + it 'accepts all the user members pending invitations and returns the accepted_members' do accepted_members = user.accept_pending_invitations! - expect(accepted_members).to match_array([project_member_invite, group_member_invite]) + expect(accepted_members).to match_array( + [ + project_member_invite, + group_member_invite, + project_member_invite_via_confirmed_secondary_email, + group_member_invite_via_confirmed_secondary_email + ] + ) + expect(group_member_invite.reload).not_to be_invite expect(project_member_invite.reload).not_to be_invite + expect(external_project_member_invite.reload).to be_invite expect(external_group_member_invite.reload).to be_invite + + expect(project_member_invite_via_confirmed_secondary_email.reload).not_to be_invite + expect(group_member_invite_via_confirmed_secondary_email.reload).not_to be_invite + + expect(project_member_invite_via_unconfirmed_secondary_email.reload).to be_invite + expect(group_member_invite_via_unconfirmed_secondary_email.reload).to be_invite end end describe '#all_emails' do let(:user) { create(:user) } - let!(:email_confirmed) { create :email, user: user, confirmed_at: Time.current } - let!(:email_unconfirmed) { create :email, user: user } + let!(:unconfirmed_secondary_email) { create(:email, user: user) } + let!(:confirmed_secondary_email) { create(:email, :confirmed, user: user) } + + it 'returns all emails' do + expect(user.all_emails).to contain_exactly( + user.email, + user.private_commit_email, + confirmed_secondary_email.email + ) + end + + context 'when the primary email is confirmed' do + it 'includes the primary email' do + expect(user.all_emails).to include(user.email) + end + end + + context 'when the primary email is unconfirmed' do + let!(:user) { create(:user, :unconfirmed) } + + it 'includes the primary email' do + expect(user.all_emails).to include(user.email) + end + end + + context 'when the primary email is temp email for oauth' do + let!(:user) { create(:omniauth_user, :unconfirmed, email: 'temp-email-for-oauth-user@gitlab.localhost') } + + it 'does not include the primary email' do + expect(user.all_emails).not_to include(user.email) + end + end context 'when `include_private_email` is true' do - it 'returns all emails' do - expect(user.reload.all_emails).to contain_exactly( - user.email, - user.private_commit_email, - email_unconfirmed.email, - email_confirmed.email - ) + it 'includes the private commit email' do + expect(user.all_emails).to include(user.private_commit_email) end end context 'when `include_private_email` is false' do it 'does not include the private commit email' do - expect(user.reload.all_emails(include_private_email: false)).to contain_exactly( - user.email, - email_unconfirmed.email, - email_confirmed.email + expect(user.all_emails(include_private_email: false)).not_to include( + user.private_commit_email ) end end + + context 'when the secondary email is confirmed' do + it 'includes the secondary email' do + expect(user.all_emails).to include(confirmed_secondary_email.email) + end + end + + context 'when the secondary email is unconfirmed' do + it 'does not include the secondary email' do + expect(user.all_emails).not_to include(unconfirmed_secondary_email.email) + end + end end describe '#verified_emails' do diff --git a/spec/requests/api/invitations_spec.rb b/spec/requests/api/invitations_spec.rb index d093894720e..2ca6d52033d 100644 --- a/spec/requests/api/invitations_spec.rb +++ b/spec/requests/api/invitations_spec.rb @@ -7,6 +7,7 @@ RSpec.describe API::Invitations do let_it_be(:developer) { create(:user) } let_it_be(:access_requester) { create(:user) } let_it_be(:stranger) { create(:user) } + let_it_be(:unconfirmed_stranger) { create(:user, :unconfirmed) } let(:email) { 'email1@example.com' } let(:email2) { 'email2@example.com' } @@ -78,6 +79,46 @@ RSpec.describe API::Invitations do end.to change { source.members.invite.count }.by(1) end + it 'adds a new member by confirmed primary email' do + expect do + post invitations_url(source, maintainer), + params: { email: stranger.email, access_level: Member::DEVELOPER } + + expect(response).to have_gitlab_http_status(:created) + end.to change { source.members.non_invite.count }.by(1) + end + + it 'adds a new member by unconfirmed primary email' do + expect do + post invitations_url(source, maintainer), + params: { email: unconfirmed_stranger.email, access_level: Member::DEVELOPER } + + expect(response).to have_gitlab_http_status(:created) + end.to change { source.members.non_invite.count }.by(1) + end + + it 'adds a new member by confirmed secondary email' do + secondary_email = create(:email, :confirmed, email: 'secondary@example.com', user: stranger) + + expect do + post invitations_url(source, maintainer), + params: { email: secondary_email.email, access_level: Member::DEVELOPER } + + expect(response).to have_gitlab_http_status(:created) + end.to change { source.members.non_invite.count }.by(1) + end + + it 'adds a new member as an invite for unconfirmed secondary email' do + secondary_email = create(:email, email: 'secondary@example.com', user: stranger) + + expect do + post invitations_url(source, maintainer), + params: { email: secondary_email.email, access_level: Member::DEVELOPER } + + expect(response).to have_gitlab_http_status(:created) + end.to change { source.members.invite.count }.by(1).and change { source.members.non_invite.count }.by(0) + end + it 'adds a new member by user_id' do expect do post invitations_url(source, maintainer), diff --git a/spec/requests/api/oauth_tokens_spec.rb b/spec/requests/api/oauth_tokens_spec.rb index edadfbc3d0c..f07dcfcccd6 100644 --- a/spec/requests/api/oauth_tokens_spec.rb +++ b/spec/requests/api/oauth_tokens_spec.rb @@ -25,6 +25,40 @@ RSpec.describe 'OAuth tokens' do end end + context 'when 2FA enforced' do + let_it_be(:user) { create(:user, otp_grace_period_started_at: 1.day.ago) } + + before do + stub_application_setting(require_two_factor_authentication: true) + end + + context 'when grace period expired' do + before do + stub_application_setting(two_factor_grace_period: 0) + end + + it 'does not create an access token' do + request_oauth_token(user, client_basic_auth_header(client)) + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['error']).to eq('invalid_grant') + end + end + + context 'when grace period is not expired' do + before do + stub_application_setting(two_factor_grace_period: 72) + end + + it 'creates an access token' do + request_oauth_token(user, client_basic_auth_header(client)) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['access_token']).not_to be_nil + end + end + end + context 'when user does not have 2FA enabled' do context 'when no client credentials provided' do it 'creates an access token' do diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index 2c5a734a0e1..e0635bc8506 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -1198,6 +1198,81 @@ RSpec.describe API::Users do end end + context 'when user with a primary email exists' do + context 'when the primary email is confirmed' do + let!(:confirmed_user) { create(:user, email: 'foo@example.com') } + + it 'returns 409 conflict error' do + expect do + post api('/users', admin), + params: { + name: 'foo', + email: confirmed_user.email, + password: 'password', + username: 'TEST' + } + end.to change { User.count }.by(0) + expect(response).to have_gitlab_http_status(:conflict) + expect(json_response['message']).to eq('Email has already been taken') + end + end + + context 'when the primary email is unconfirmed' do + let!(:unconfirmed_user) { create(:user, :unconfirmed, email: 'foo@example.com') } + + it 'returns 409 conflict error' do + expect do + post api('/users', admin), + params: { + name: 'foo', + email: unconfirmed_user.email, + password: 'password', + username: 'TEST' + } + end.to change { User.count }.by(0) + expect(response).to have_gitlab_http_status(:conflict) + expect(json_response['message']).to eq('Email has already been taken') + end + end + end + + context 'when user with a secondary email exists' do + context 'when the secondary email is confirmed' do + let!(:email) { create(:email, :confirmed, email: 'foo@example.com') } + + it 'returns 409 conflict error' do + expect do + post api('/users', admin), + params: { + name: 'foo', + email: email.email, + password: 'password', + username: 'TEST' + } + end.to change { User.count }.by(0) + expect(response).to have_gitlab_http_status(:conflict) + expect(json_response['message']).to eq('Email has already been taken') + end + end + + context 'when the secondary email is unconfirmed' do + let!(:email) { create(:email, email: 'foo@example.com') } + + it 'does not create user' do + expect do + post api('/users', admin), + params: { + name: 'foo', + email: email.email, + password: 'password', + username: 'TEST' + } + end.to change { User.count }.by(0) + expect(response).to have_gitlab_http_status(:bad_request) + end + end + end + context "scopes" do let(:user) { admin } let(:path) { '/users' } @@ -1554,6 +1629,54 @@ RSpec.describe API::Users do expect(@user.reload.username).to eq(@user.username) end end + + context 'when user with a primary email exists' do + context 'when the primary email is confirmed' do + let!(:confirmed_user) { create(:user, email: 'foo@example.com') } + + it 'returns 409 conflict error' do + put api("/users/#{user.id}", admin), params: { email: confirmed_user.email } + + expect(response).to have_gitlab_http_status(:conflict) + expect(user.reload.email).not_to eq(confirmed_user.email) + end + end + + context 'when the primary email is unconfirmed' do + let!(:unconfirmed_user) { create(:user, :unconfirmed, email: 'foo@example.com') } + + it 'returns 409 conflict error' do + put api("/users/#{user.id}", admin), params: { email: unconfirmed_user.email } + + expect(response).to have_gitlab_http_status(:conflict) + expect(user.reload.email).not_to eq(unconfirmed_user.email) + end + end + end + + context 'when user with a secondary email exists' do + context 'when the secondary email is confirmed' do + let!(:email) { create(:email, :confirmed, email: 'foo@example.com') } + + it 'returns 409 conflict error' do + put api("/users/#{user.id}", admin), params: { email: email.email } + + expect(response).to have_gitlab_http_status(:conflict) + expect(user.reload.email).not_to eq(email.email) + end + end + + context 'when the secondary email is unconfirmed' do + let!(:email) { create(:email, email: 'foo@example.com') } + + it 'does not update email' do + put api("/users/#{user.id}", admin), params: { email: email.email } + + expect(response).to have_gitlab_http_status(:bad_request) + expect(user.reload.email).not_to eq(email.email) + end + end + end end describe "PUT /user/:id/credit_card_validation" do @@ -2013,6 +2136,50 @@ RSpec.describe API::Users do expect(json_response['confirmed_at']).not_to be_nil end + + context 'when user with a primary email exists' do + context 'when the primary email is confirmed' do + let!(:confirmed_user) { create(:user, email: 'foo@example.com') } + + it 'returns 400 error' do + post api("/users/#{user.id}/emails", admin), params: { email: confirmed_user.email } + + expect(response).to have_gitlab_http_status(:bad_request) + end + end + + context 'when the primary email is unconfirmed' do + let!(:unconfirmed_user) { create(:user, :unconfirmed, email: 'foo@example.com') } + + it 'returns 400 error' do + post api("/users/#{user.id}/emails", admin), params: { email: unconfirmed_user.email } + + expect(response).to have_gitlab_http_status(:bad_request) + end + end + end + + context 'when user with a secondary email exists' do + context 'when the secondary email is confirmed' do + let!(:email) { create(:email, :confirmed, email: 'foo@example.com') } + + it 'returns 400 error' do + post api("/users/#{user.id}/emails", admin), params: { email: email.email } + + expect(response).to have_gitlab_http_status(:bad_request) + end + end + + context 'when the secondary email is unconfirmed' do + let!(:email) { create(:email, email: 'foo@example.com') } + + it 'returns 400 error' do + post api("/users/#{user.id}/emails", admin), params: { email: email.email } + + expect(response).to have_gitlab_http_status(:bad_request) + end + end + end end describe 'GET /user/:id/emails' do diff --git a/spec/serializers/build_details_entity_spec.rb b/spec/serializers/build_details_entity_spec.rb index dd8238456aa..916798c669c 100644 --- a/spec/serializers/build_details_entity_spec.rb +++ b/spec/serializers/build_details_entity_spec.rb @@ -170,6 +170,24 @@ RSpec.describe BuildDetailsEntity do expect(message).to include('could not retrieve the needed artifacts.') end end + + context 'when dependency contains invalid dependency names' do + invalid_name = 'XSS<a href=# data-disable-with="<img src=x onerror=alert(document.domain)>">' + let!(:test1) { create(:ci_build, :success, :expired, pipeline: pipeline, name: invalid_name, stage_idx: 0) } + let!(:build) { create(:ci_build, :pending, pipeline: pipeline, stage_idx: 1, options: { dependencies: [invalid_name] }) } + + before do + build.pipeline.unlocked! + build.drop!(:missing_dependency_failure) + end + + it { is_expected.to include(failure_reason: 'missing_dependency_failure') } + + it 'escapes the invalid dependency names' do + escaped_name = html_escape(invalid_name) + expect(message).to include(escaped_name) + end + end end context 'when a build has environment with latest deployment' do diff --git a/spec/services/grafana/proxy_service_spec.rb b/spec/services/grafana/proxy_service_spec.rb index 7ddc31d45d9..99120de3593 100644 --- a/spec/services/grafana/proxy_service_spec.rb +++ b/spec/services/grafana/proxy_service_spec.rb @@ -50,12 +50,8 @@ RSpec.describe Grafana::ProxyService do describe '#execute' do subject(:result) { service.execute } - context 'when grafana integration is not configured' do - before do - allow(project).to receive(:grafana_integration).and_return(nil) - end - - it 'returns error' do + shared_examples 'missing proxy support' do + it 'returns API not supported error' do expect(result).to eq( status: :error, message: 'Proxy support for this API is not available currently' @@ -63,6 +59,40 @@ RSpec.describe Grafana::ProxyService do end end + context 'with unsupported proxy path' do + where(:proxy_path) do + %w[ + /api/vl/query_range + api/vl/query_range/ + api/vl/labels + api/v2/query_range + ../../../org/users + ] + end + + with_them do + include_examples 'missing proxy support' + end + end + + context 'with unsupported datasource_id' do + where(:datasource_id) do + ['', '-1', '1str', 'str1', '../../1', '1/../..', "1\n1"] + end + + with_them do + include_examples 'missing proxy support' + end + end + + context 'when grafana integration is not configured' do + before do + allow(project).to receive(:grafana_integration).and_return(nil) + end + + include_examples 'missing proxy support' + end + context 'with caching', :use_clean_rails_memory_store_caching do context 'when value not present in cache' do it 'returns nil' do diff --git a/spec/services/groups/destroy_service_spec.rb b/spec/services/groups/destroy_service_spec.rb index 628943e40ff..161a0907870 100644 --- a/spec/services/groups/destroy_service_spec.rb +++ b/spec/services/groups/destroy_service_spec.rb @@ -35,6 +35,20 @@ RSpec.describe Groups::DestroyService do it { expect(NotificationSetting.unscoped.all).not_to include(notification_setting) } end + context 'bot tokens', :sidekiq_might_not_need_inline do + it 'removes group bot', :aggregate_failures do + bot = create(:user, :project_bot) + group.add_developer(bot) + token = create(:personal_access_token, user: bot) + + destroy_group(group, user, async) + + expect(PersonalAccessToken.find_by(id: token.id)).to be_nil + expect(User.find_by(id: bot.id)).to be_nil + expect(User.find_by(id: user.id)).not_to be_nil + end + end + context 'mattermost team', :sidekiq_might_not_need_inline do let!(:chat_team) { create(:chat_team, namespace: group) } diff --git a/spec/services/groups/update_service_spec.rb b/spec/services/groups/update_service_spec.rb index 46c5e2a9818..c9f247be04c 100644 --- a/spec/services/groups/update_service_spec.rb +++ b/spec/services/groups/update_service_spec.rb @@ -242,6 +242,69 @@ RSpec.describe Groups::UpdateService do end end + context 'when user is not group owner' do + context 'when group is private' do + before do + private_group.add_maintainer(user) + end + + it 'does not update the group to public' do + result = described_class.new(private_group, user, visibility_level: Gitlab::VisibilityLevel::PUBLIC).execute + + expect(result).to eq(false) + expect(private_group.errors.count).to eq(1) + expect(private_group).to be_private + end + + it 'does not update the group to public with tricky value' do + result = described_class.new(private_group, user, visibility_level: Gitlab::VisibilityLevel::PUBLIC.to_s + 'r').execute + + expect(result).to eq(false) + expect(private_group.errors.count).to eq(1) + expect(private_group).to be_private + end + end + + context 'when group is public' do + before do + public_group.add_maintainer(user) + end + + it 'does not update the group to private' do + result = described_class.new(public_group, user, visibility_level: Gitlab::VisibilityLevel::PRIVATE).execute + + expect(result).to eq(false) + expect(public_group.errors.count).to eq(1) + expect(public_group).to be_public + end + + it 'does not update the group to private with invalid string value' do + result = described_class.new(public_group, user, visibility_level: 'invalid').execute + + expect(result).to eq(false) + expect(public_group.errors.count).to eq(1) + expect(public_group).to be_public + end + + it 'does not update the group to private with valid string value' do + result = described_class.new(public_group, user, visibility_level: 'private').execute + + expect(result).to eq(false) + expect(public_group.errors.count).to eq(1) + expect(public_group).to be_public + end + + # See https://gitlab.com/gitlab-org/gitlab/-/issues/359910 + it 'does not update the group to private because of Active Record typecasting' do + result = described_class.new(public_group, user, visibility_level: 'public').execute + + expect(result).to eq(true) + expect(public_group.errors.count).to eq(0) + expect(public_group).to be_public + end + end + end + context 'when updating #emails_disabled' do let(:service) { described_class.new(internal_group, user, emails_disabled: true) } diff --git a/spec/services/members/invite_service_spec.rb b/spec/services/members/invite_service_spec.rb index 8213e8baae0..2e9809bdd56 100644 --- a/spec/services/members/invite_service_spec.rb +++ b/spec/services/members/invite_service_spec.rb @@ -30,8 +30,8 @@ RSpec.describe Members::InviteService, :aggregate_failures, :clean_gitlab_redis_ end end - context 'when email belongs to an existing user as a secondary email' do - let(:secondary_email) { create(:email, email: 'secondary@example.com', user: project_user) } + context 'when email belongs to an existing user as a confirmed secondary email' do + let(:secondary_email) { create(:email, :confirmed, email: 'secondary@example.com', user: project_user) } let(:params) { { email: secondary_email.email } } it 'adds an existing user to members', :aggregate_failures do @@ -42,6 +42,18 @@ RSpec.describe Members::InviteService, :aggregate_failures, :clean_gitlab_redis_ end end + context 'when email belongs to an existing user as an unconfirmed secondary email' do + let(:unconfirmed_secondary_email) { create(:email, email: 'secondary@example.com', user: project_user) } + let(:params) { { email: unconfirmed_secondary_email.email } } + + it 'does not link the email with any user and successfully creates a member as an invite for that email' do + expect_to_create_members(count: 1) + expect(result[:status]).to eq(:success) + expect(project.users).not_to include project_user + expect(project.members.last).to be_invite + end + end + context 'when invites are passed as array' do context 'with emails' do let(:params) { { email: %w[email@example.org email2@example.org] } } @@ -291,6 +303,19 @@ RSpec.describe Members::InviteService, :aggregate_failures, :clean_gitlab_redis_ end end + context 'with unconfirmed primary email' do + let_it_be(:unconfirmed_user) { create(:user, :unconfirmed) } + + let(:params) { { email: unconfirmed_user.email } } + + it 'adds an existing user to members' do + expect_to_create_members(count: 1) + expect(result[:status]).to eq(:success) + expect(project.users).to include unconfirmed_user + expect(project.members.last).not_to be_invite + end + end + context 'with user_id' do let(:params) { { user_id: project_user.id } } @@ -374,8 +399,8 @@ RSpec.describe Members::InviteService, :aggregate_failures, :clean_gitlab_redis_ expect(result[:message][existing_member.user.email]).to eq("User already exists in source") end - context 'when email belongs to an existing user as a secondary email' do - let(:secondary_email) { create(:email, email: 'secondary@example.com', user: existing_member.user) } + context 'when email belongs to an existing user as a confirmed secondary email' do + let(:secondary_email) { create(:email, :confirmed, email: 'secondary@example.com', user: existing_member.user) } let(:params) { { email: "#{secondary_email.email}" } } it 'returns an error for the already invited email' do diff --git a/spec/services/projects/operations/update_service_spec.rb b/spec/services/projects/operations/update_service_spec.rb index 3ee867ba6f2..57d0e824a83 100644 --- a/spec/services/projects/operations/update_service_spec.rb +++ b/spec/services/projects/operations/update_service_spec.rb @@ -306,6 +306,11 @@ RSpec.describe Projects::Operations::UpdateService do let(:params) do { error_tracking_setting_attributes: { + api_host: 'https://sentrytest.gitlab.com/', + project: { + slug: 'sentry-project', + organization_slug: 'sentry-org' + }, enabled: false, token: '*' * 8 } @@ -313,7 +318,7 @@ RSpec.describe Projects::Operations::UpdateService do end before do - create(:project_error_tracking_setting, project: project, token: 'token') + create(:project_error_tracking_setting, project: project, token: 'token', api_url: 'https://sentrytest.gitlab.com/api/0/projects/sentry-org/sentry-project/') end it 'does not update token' do diff --git a/spec/services/projects/update_service_spec.rb b/spec/services/projects/update_service_spec.rb index 7b5bf1db030..7e2c7c89276 100644 --- a/spec/services/projects/update_service_spec.rb +++ b/spec/services/projects/update_service_spec.rb @@ -120,6 +120,65 @@ RSpec.describe Projects::UpdateService do end end + context 'when user is not project owner' do + let_it_be(:maintainer) { create(:user) } + + before do + project.add_maintainer(maintainer) + end + + context 'when project is private' do + it 'does not update the project to public' do + result = update_project(project, maintainer, visibility_level: Gitlab::VisibilityLevel::PUBLIC) + + expect(result).to eq({ status: :error, message: 'New visibility level not allowed!' }) + expect(project).to be_private + end + + it 'does not update the project to public with tricky value' do + result = update_project(project, maintainer, visibility_level: Gitlab::VisibilityLevel::PUBLIC.to_s + 'r') + + expect(result).to eq({ status: :error, message: 'New visibility level not allowed!' }) + expect(project).to be_private + end + end + + context 'when project is public' do + before do + project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC) + end + + it 'does not update the project to private' do + result = update_project(project, maintainer, visibility_level: Gitlab::VisibilityLevel::PRIVATE) + + expect(result).to eq({ status: :error, message: 'New visibility level not allowed!' }) + expect(project).to be_public + end + + it 'does not update the project to private with invalid string value' do + result = update_project(project, maintainer, visibility_level: 'invalid') + + expect(result).to eq({ status: :error, message: 'New visibility level not allowed!' }) + expect(project).to be_public + end + + it 'does not update the project to private with valid string value' do + result = update_project(project, maintainer, visibility_level: 'private') + + expect(result).to eq({ status: :error, message: 'New visibility level not allowed!' }) + expect(project).to be_public + end + + # See https://gitlab.com/gitlab-org/gitlab/-/issues/359910 + it 'does not update the project to private because of Active Record typecasting' do + result = update_project(project, maintainer, visibility_level: 'public') + + expect(result).to eq({ status: :success }) + expect(project).to be_public + end + end + end + context 'when updating shared runners' do context 'can enable shared runners' do let(:group) { create(:group, shared_runners_enabled: true) } diff --git a/spec/services/todos/destroy/entity_leave_service_spec.rb b/spec/services/todos/destroy/entity_leave_service_spec.rb index 03fa2482bbf..225e7933d79 100644 --- a/spec/services/todos/destroy/entity_leave_service_spec.rb +++ b/spec/services/todos/destroy/entity_leave_service_spec.rb @@ -3,21 +3,24 @@ 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, :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_it_be(:user, reload: true) { create(:user) } + let_it_be(:user2, reload: true) { create(:user) } + let_it_be_with_refind(:group) { create(:group, :private) } + let_it_be(: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) } + let(:internal_note) { create(:note, noteable: issue, project: project, confidential: true ) } + let!(:todo_for_internal_note) do + create(:todo, user: user, target: issue, project: project, note: internal_note) + end shared_examples 'using different access permissions' do before do @@ -34,20 +37,28 @@ RSpec.describe Todos::Destroy::EntityLeaveService do it { does_not_remove_any_todos } end - shared_examples 'removes only confidential issues todos' do - it { removes_only_confidential_issues_todos } + shared_examples 'removes confidential issues and internal notes todos' do + it { removes_confidential_issues_and_internal_notes_todos } + end + + shared_examples 'removes only internal notes todos' do + it { removes_only_internal_notes_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) + def removes_only_internal_notes_todos + expect { subject }.to change { Todo.count }.from(7).to(6) + end + + def removes_confidential_issues_and_internal_notes_todos + expect { subject }.to change { Todo.count }.from(7).to(5) end - def removes_confidential_issues_and_merge_request_todos - expect { subject }.to change { Todo.count }.from(6).to(4) + def removes_confidential_issues_and_internal_notes_and_merge_request_todos + expect { subject }.to change { Todo.count }.from(7).to(4) expect(user.todos).to match_array([todo_issue_user, todo_group_user]) end @@ -70,7 +81,7 @@ RSpec.describe Todos::Destroy::EntityLeaveService do context 'when project is private' 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) + expect { subject }.to change { Todo.count }.from(7).to(3) expect(user.todos).to match_array([todo_group_user]) expect(user2.todos).to match_array([todo_issue_c_user2, todo_group_user2]) @@ -81,11 +92,11 @@ RSpec.describe Todos::Destroy::EntityLeaveService do where(:group_access, :project_access, :method_name) do [ [nil, :reporter, :does_not_remove_any_todos], - [nil, :guest, :removes_confidential_issues_and_merge_request_todos], + [nil, :guest, :removes_confidential_issues_and_internal_notes_and_merge_request_todos], [:reporter, nil, :does_not_remove_any_todos], - [:guest, nil, :removes_confidential_issues_and_merge_request_todos], + [:guest, nil, :removes_confidential_issues_and_internal_notes_and_merge_request_todos], [:guest, :reporter, :does_not_remove_any_todos], - [:guest, :guest, :removes_confidential_issues_and_merge_request_todos] + [:guest, :guest, :removes_confidential_issues_and_internal_notes_and_merge_request_todos] ] end @@ -97,11 +108,12 @@ RSpec.describe Todos::Destroy::EntityLeaveService do # 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) } + let_it_be(:group) { create(:group, :internal) } + let_it_be(:project) { create(:project, :private, group: group) } 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 { subject }.to change { Todo.count }.from(7).to(3) expect(user.todos).to match_array([todo_group_user]) expect(user2.todos).to match_array([todo_issue_c_user2, todo_group_user2]) @@ -112,11 +124,11 @@ RSpec.describe Todos::Destroy::EntityLeaveService do where(:group_access, :project_access, :method_name) do [ [nil, :reporter, :does_not_remove_any_todos], - [nil, :guest, :removes_confidential_issues_and_merge_request_todos], + [nil, :guest, :removes_confidential_issues_and_internal_notes_and_merge_request_todos], [:reporter, nil, :does_not_remove_any_todos], - [:guest, nil, :removes_confidential_issues_and_merge_request_todos], + [:guest, nil, :removes_confidential_issues_and_internal_notes_and_merge_request_todos], [:guest, :reporter, :does_not_remove_any_todos], - [:guest, :guest, :removes_confidential_issues_and_merge_request_todos] + [:guest, :guest, :removes_confidential_issues_and_internal_notes_and_merge_request_todos] ] end @@ -142,7 +154,7 @@ RSpec.describe Todos::Destroy::EntityLeaveService do context 'confidential issues' do context 'when a user is not an author of confidential issue' do - it_behaves_like 'removes only confidential issues todos' + it_behaves_like 'removes confidential issues and internal notes todos' end context 'when a user is an author of confidential issue' do @@ -150,7 +162,7 @@ RSpec.describe Todos::Destroy::EntityLeaveService do issue_c.update!(author: user) end - it_behaves_like 'does not remove any todos' + it_behaves_like 'removes only internal notes todos' end context 'when a user is an assignee of confidential issue' do @@ -158,18 +170,18 @@ RSpec.describe Todos::Destroy::EntityLeaveService do issue_c.assignees << user end - it_behaves_like 'does not remove any todos' + it_behaves_like 'removes only internal notes todos' end context 'access permissions' do where(:group_access, :project_access, :method_name) do [ [nil, :reporter, :does_not_remove_any_todos], - [nil, :guest, :removes_only_confidential_issues_todos], + [nil, :guest, :removes_confidential_issues_and_internal_notes_todos], [:reporter, nil, :does_not_remove_any_todos], - [:guest, nil, :removes_only_confidential_issues_todos], + [:guest, nil, :removes_confidential_issues_and_internal_notes_todos], [:guest, :reporter, :does_not_remove_any_todos], - [:guest, :guest, :removes_only_confidential_issues_todos] + [:guest, :guest, :removes_confidential_issues_and_internal_notes_todos] ] end @@ -186,7 +198,7 @@ RSpec.describe Todos::Destroy::EntityLeaveService do end it 'removes only users issue todos' do - expect { subject }.to change { Todo.count }.from(6).to(5) + expect { subject }.to change { Todo.count }.from(7).to(5) end end end @@ -199,7 +211,7 @@ RSpec.describe Todos::Destroy::EntityLeaveService do context 'when group is private' do 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) + expect { subject }.to change { Todo.count }.from(7).to(2) expect(user.todos).to be_empty expect(user2.todos).to match_array([todo_issue_c_user2, todo_group_user2]) @@ -210,11 +222,11 @@ RSpec.describe Todos::Destroy::EntityLeaveService do where(:group_access, :project_access, :method_name) do [ [nil, :reporter, :does_not_remove_any_todos], - [nil, :guest, :removes_confidential_issues_and_merge_request_todos], + [nil, :guest, :removes_confidential_issues_and_internal_notes_and_merge_request_todos], [:reporter, nil, :does_not_remove_any_todos], - [:guest, nil, :removes_confidential_issues_and_merge_request_todos], + [:guest, nil, :removes_confidential_issues_and_internal_notes_and_merge_request_todos], [:guest, :reporter, :does_not_remove_any_todos], - [:guest, :guest, :removes_confidential_issues_and_merge_request_todos] + [:guest, :guest, :removes_confidential_issues_and_internal_notes_and_merge_request_todos] ] end @@ -224,12 +236,12 @@ RSpec.describe Todos::Destroy::EntityLeaveService do end context 'with nested groups' do - let(:parent_group) { create(:group, :public) } - let(:parent_subgroup) { create(:group)} - let(:subgroup) { create(:group, :private, parent: group) } - let(:subgroup2) { create(:group, :private, parent: group) } - let(:subproject) { create(:project, group: subgroup) } - let(:subproject2) { create(:project, group: subgroup2) } + let_it_be_with_refind(:parent_group) { create(:group, :public) } + let_it_be_with_refind(:parent_subgroup) { create(:group) } + let_it_be(:subgroup) { create(:group, :private, parent: group) } + let_it_be(:subgroup2) { create(:group, :private, parent: group) } + let_it_be(:subproject) { create(:project, group: subgroup) } + let_it_be(:subproject2) { create(:project, group: subgroup2) } let!(:todo_subproject_user) { create(:todo, user: user, project: subproject) } let!(:todo_subproject2_user) { create(:todo, user: user, project: subproject2) } @@ -238,6 +250,10 @@ RSpec.describe Todos::Destroy::EntityLeaveService do let!(:todo_subproject_user2) { create(:todo, user: user2, project: subproject) } let!(:todo_subpgroup_user2) { create(:todo, user: user2, group: subgroup) } let!(:todo_parent_group_user) { create(:todo, user: user, group: parent_group) } + let(:subproject_internal_note) { create(:note, noteable: issue, project: project, confidential: true ) } + let!(:todo_for_internal_subproject_note) do + create(:todo, user: user, target: issue, project: project, note: subproject_internal_note) + end before do group.update!(parent: parent_group) @@ -245,7 +261,7 @@ 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(13).to(5) + expect { subject }.to change { Todo.count }.from(15).to(5) expect(user.todos).to eq([todo_parent_group_user]) expect(user2.todos) @@ -269,7 +285,7 @@ RSpec.describe Todos::Destroy::EntityLeaveService do end it 'does not remove group and subproject todos' do - expect { subject }.to change { Todo.count }.from(13).to(8) + expect { subject }.to change { Todo.count }.from(15).to(8) expect(user.todos) .to match_array( @@ -288,7 +304,7 @@ RSpec.describe Todos::Destroy::EntityLeaveService do end it 'does not remove subproject and group todos' do - expect { subject }.to change { Todo.count }.from(13).to(8) + expect { subject }.to change { Todo.count }.from(15).to(8) expect(user.todos) .to match_array( @@ -319,13 +335,13 @@ RSpec.describe Todos::Destroy::EntityLeaveService do context 'access permissions' do where(:group_access, :project_access, :method_name) do [ - [nil, nil, :removes_only_confidential_issues_todos], + [nil, nil, :removes_confidential_issues_and_internal_notes_todos], [nil, :reporter, :does_not_remove_any_todos], - [nil, :guest, :removes_only_confidential_issues_todos], + [nil, :guest, :removes_confidential_issues_and_internal_notes_todos], [:reporter, nil, :does_not_remove_any_todos], - [:guest, nil, :removes_only_confidential_issues_todos], + [:guest, nil, :removes_confidential_issues_and_internal_notes_todos], [:guest, :reporter, :does_not_remove_any_todos], - [:guest, :guest, :removes_only_confidential_issues_todos] + [:guest, :guest, :removes_confidential_issues_and_internal_notes_todos] ] end diff --git a/spec/support/shared_contexts/features/integrations/integrations_shared_context.rb b/spec/support/shared_contexts/features/integrations/integrations_shared_context.rb index 3ea6658c0c1..d0f7853eb58 100644 --- a/spec/support/shared_contexts/features/integrations/integrations_shared_context.rb +++ b/spec/support/shared_contexts/features/integrations/integrations_shared_context.rb @@ -36,6 +36,8 @@ Integration.available_integration_names.each do |integration| hash.merge!(k => 'foo@bar.com') elsif integration == 'slack' || integration == 'mattermost' && k == :labels_to_be_notified_behavior hash.merge!(k => "match_any") + elsif integration == 'campfire' && k = :room + hash.merge!(k => '1234') else hash.merge!(k => "someword") end |