diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2022-07-27 19:03:35 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2022-07-27 19:03:56 +0000 |
commit | d625f4e9fe78a69be0d481c20cba33b6dd88ef1a (patch) | |
tree | 510ee7d62fa2d6084a5058446cf61d328900325a /spec | |
parent | 9b60052467242bbc071bcb0f74b7437fb3dfc870 (diff) | |
download | gitlab-ce-d625f4e9fe78a69be0d481c20cba33b6dd88ef1a.tar.gz |
Add latest changes from gitlab-org/security/gitlab@15-2-stable-ee
Diffstat (limited to 'spec')
-rw-r--r-- | spec/controllers/invites_controller_spec.rb | 22 | ||||
-rw-r--r-- | spec/features/signed_commits_spec.rb | 9 | ||||
-rw-r--r-- | spec/lib/gitlab/config/loader/yaml_spec.rb | 6 | ||||
-rw-r--r-- | spec/lib/gitlab/gpg/commit_spec.rb | 28 | ||||
-rw-r--r-- | spec/lib/gitlab/legacy_github_import/user_formatter_spec.rb | 19 | ||||
-rw-r--r-- | spec/lib/gitlab/visibility_level_spec.rb | 44 | ||||
-rw-r--r-- | spec/models/commit_spec.rb | 44 | ||||
-rw-r--r-- | spec/models/snippet_spec.rb | 2 | ||||
-rw-r--r-- | spec/models/user_spec.rb | 257 | ||||
-rw-r--r-- | spec/requests/api/invitations_spec.rb | 41 | ||||
-rw-r--r-- | spec/requests/api/oauth_tokens_spec.rb | 34 | ||||
-rw-r--r-- | spec/requests/api/users_spec.rb | 167 | ||||
-rw-r--r-- | spec/services/groups/update_service_spec.rb | 63 | ||||
-rw-r--r-- | spec/services/members/invite_service_spec.rb | 33 | ||||
-rw-r--r-- | spec/services/projects/update_service_spec.rb | 59 |
15 files changed, 736 insertions, 92 deletions
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/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 919335bc9fa..819a5633a78 100644 --- a/spec/lib/gitlab/gpg/commit_spec.rb +++ b/spec/lib/gitlab/gpg/commit_spec.rb @@ -206,12 +206,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(: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: committer_email end end @@ -230,6 +230,30 @@ 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(:committer_email) { GpgHelpers::User2.emails.first } + + let(:user) do + create(:user, email: GpgHelpers::User1.emails.first).tap do |user| + create :email, user: user, email: committer_email + end + 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(:committer_email) { GpgHelpers::User2.emails.first } let(:user_email) { GpgHelpers::User1.emails.first } 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 187be557064..08d770a1beb 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/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/user_spec.rb b/spec/models/user_spec.rb index 6d2ba66d5f4..ae6ebdbc6fd 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -2649,6 +2649,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') @@ -2656,6 +2664,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') @@ -2664,35 +2679,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 @@ -2700,13 +2727,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 @@ -2715,32 +2735,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 @@ -2755,7 +2842,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') } @@ -2813,16 +2903,26 @@ 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 @@ -3082,47 +3182,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 53154aef21e..cb351635081 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' } @@ -92,6 +93,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 68d5fad8ff4..81ca2548995 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -1307,6 +1307,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' } @@ -1663,6 +1738,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 @@ -2227,6 +2350,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/services/groups/update_service_spec.rb b/spec/services/groups/update_service_spec.rb index c0e1691fe26..856dd4a2567 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 7a1512970b4..d25c8996931 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 } } @@ -376,8 +401,8 @@ RSpec.describe Members::InviteService, :aggregate_failures, :clean_gitlab_redis_ expect(existing_member.reset.access_level).to eq ProjectMember::MAINTAINER 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 'allows re-invite to an already invited email' do diff --git a/spec/services/projects/update_service_spec.rb b/spec/services/projects/update_service_spec.rb index f019434a4fe..ca838be0fa8 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) } |