diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2023-01-07 01:28:28 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2023-01-07 01:28:28 +0300 |
commit | e4a92d342784ccbb929e7d2b1faa42d6c2f591a3 (patch) | |
tree | 0e850cae1809a9224f5dcd773933777dbd4c17de /spec | |
parent | 89e372068b3909b0e8cfb03af4da176357a1abbc (diff) |
Add latest changes from gitlab-org/security/gitlab@15-7-stable-ee
Diffstat (limited to 'spec')
-rw-r--r-- | spec/controllers/projects/grafana_api_controller_spec.rb | 82 | ||||
-rw-r--r-- | spec/mailers/devise_mailer_spec.rb | 12 | ||||
-rw-r--r-- | spec/models/user_spec.rb | 28 | ||||
-rw-r--r-- | spec/policies/project_policy_spec.rb | 29 | ||||
-rw-r--r-- | spec/services/users/update_service_spec.rb | 43 |
5 files changed, 186 insertions, 8 deletions
diff --git a/spec/controllers/projects/grafana_api_controller_spec.rb b/spec/controllers/projects/grafana_api_controller_spec.rb index 2e25b0271ce..90ab49f9467 100644 --- a/spec/controllers/projects/grafana_api_controller_spec.rb +++ b/spec/controllers/projects/grafana_api_controller_spec.rb @@ -2,13 +2,20 @@ require 'spec_helper' -RSpec.describe Projects::GrafanaApiController do - let_it_be(:project) { create(:project) } - let_it_be(:user) { create(:user) } +RSpec.describe Projects::GrafanaApiController, feature_category: :metrics do + let_it_be(:project) { create(:project, :public) } + let_it_be(:reporter) { create(:user) } + let_it_be(:guest) { create(:user) } + let(:anonymous) { nil } + let(:user) { reporter } + + before_all do + project.add_reporter(reporter) + project.add_guest(guest) + end before do - project.add_reporter(user) - sign_in(user) + sign_in(user) if user end describe 'GET #proxy' do @@ -41,6 +48,39 @@ RSpec.describe Projects::GrafanaApiController do end end + shared_examples_for 'accessible' do + let(:service_result) { nil } + + it 'returns non erroneous response' do + get :proxy, params: params + + # We don't care about the specific code as long it's not an error. + expect(response).to have_gitlab_http_status(:no_content) + end + end + + shared_examples_for 'not accessible' do + let(:service_result) { nil } + + it 'returns 404 Not found' do + get :proxy, params: params + + expect(response).to have_gitlab_http_status(:not_found) + expect(Grafana::ProxyService).not_to have_received(:new) + end + end + + shared_examples_for 'login required' do + let(:service_result) { nil } + + it 'redirects to login page' do + get :proxy, params: params + + expect(response).to redirect_to(new_user_session_path) + expect(Grafana::ProxyService).not_to have_received(:new) + end + end + context 'with a successful result' do let(:service_result) { { status: :success, body: '{}' } } @@ -96,6 +136,38 @@ RSpec.describe Projects::GrafanaApiController do it_behaves_like 'error response', :bad_request end end + + context 'as guest' do + let(:user) { guest } + + it_behaves_like 'not accessible' + end + + context 'as anonymous' do + let(:user) { anonymous } + + it_behaves_like 'not accessible' + end + + context 'on a private project' do + let_it_be(:project) { create(:project, :private) } + + before_all do + project.add_guest(guest) + end + + context 'as anonymous' do + let(:user) { anonymous } + + it_behaves_like 'login required' + end + + context 'as guest' do + let(:user) { guest } + + it_behaves_like 'accessible' + end + end end describe 'GET #metrics_dashboard' do diff --git a/spec/mailers/devise_mailer_spec.rb b/spec/mailers/devise_mailer_spec.rb index 360eb827927..b8e768b7a5f 100644 --- a/spec/mailers/devise_mailer_spec.rb +++ b/spec/mailers/devise_mailer_spec.rb @@ -11,7 +11,7 @@ RSpec.describe DeviseMailer do subject { described_class.confirmation_instructions(user, 'faketoken', {}) } context "when confirming a new account" do - let(:user) { build(:user, created_at: 1.minute.ago, unconfirmed_email: nil) } + let(:user) { create(:user, created_at: 1.minute.ago) } it "shows the expected text" do expect(subject.body.encoded).to have_text "Welcome" @@ -20,7 +20,13 @@ RSpec.describe DeviseMailer do end context "when confirming the unconfirmed_email" do - let(:user) { build(:user, unconfirmed_email: 'jdoe@example.com') } + subject { described_class.confirmation_instructions(user, user.confirmation_token, { to: user.unconfirmed_email }) } + + let(:user) { create(:user) } + + before do + user.update!(email: 'unconfirmed-email@example.com') + end it "shows the expected text" do expect(subject.body.encoded).not_to have_text "Welcome" @@ -30,7 +36,7 @@ RSpec.describe DeviseMailer do end context "when re-confirming the primary email after a security issue" do - let(:user) { build(:user, created_at: 10.days.ago, unconfirmed_email: nil) } + let(:user) { create(:user, created_at: Devise.confirm_within.ago) } it "shows the expected text" do expect(subject.body.encoded).not_to have_text "Welcome" diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 4a66af4ddf1..4dbcc68af19 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -311,6 +311,34 @@ RSpec.describe User do end end end + + describe 'confirmation instructions for unconfirmed email' do + let(:unconfirmed_email) { 'first-unconfirmed-email@example.com' } + let(:another_unconfirmed_email) { 'another-unconfirmed-email@example.com' } + + context 'when email is changed to another before performing the job that sends confirmation instructions for previous email change request' do + it "mentions the recipient's email in the message body", :aggregate_failures do + same_user = User.find(user.id) + same_user.update!(email: unconfirmed_email) + + user.update!(email: another_unconfirmed_email) + + perform_enqueued_jobs + + confirmation_instructions_for_unconfirmed_email = ActionMailer::Base.deliveries.find do |message| + message.subject == 'Confirmation instructions' && message.to.include?(unconfirmed_email) + end + expect(confirmation_instructions_for_unconfirmed_email.html_part.body.encoded).to match same_user.unconfirmed_email + expect(confirmation_instructions_for_unconfirmed_email.text_part.body.encoded).to match same_user.unconfirmed_email + + confirmation_instructions_for_another_unconfirmed_email = ActionMailer::Base.deliveries.find do |message| + message.subject == 'Confirmation instructions' && message.to.include?(another_unconfirmed_email) + end + expect(confirmation_instructions_for_another_unconfirmed_email.html_part.body.encoded).to match user.unconfirmed_email + expect(confirmation_instructions_for_another_unconfirmed_email.text_part.body.encoded).to match user.unconfirmed_email + end + end + end end describe 'validations' do diff --git a/spec/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb index 9b2d10283f1..e370f536519 100644 --- a/spec/policies/project_policy_spec.rb +++ b/spec/policies/project_policy_spec.rb @@ -668,6 +668,35 @@ RSpec.describe ProjectPolicy do end end + describe 'read_grafana', feature_category: :metrics do + using RSpec::Parameterized::TableSyntax + + let(:policy) { :read_grafana } + + where(:project_visibility, :role, :allowed) do + :public | :anonymous | false + :public | :guest | false + :public | :reporter | true + :internal | :anonymous | false + :internal | :guest | true + :internal | :reporter | true + :private | :anonymous | false + :private | :guest | true + :private | :reporter | true + end + + with_them do + let(:current_user) { public_send(role) } + let(:project) { public_send("#{project_visibility}_project") } + + if params[:allowed] + it { is_expected.to be_allowed(policy) } + else + it { is_expected.not_to be_allowed(policy) } + end + end + end + describe 'update_max_artifacts_size' do context 'when no user' do let(:current_user) { anonymous } diff --git a/spec/services/users/update_service_spec.rb b/spec/services/users/update_service_spec.rb index 411cd7316d8..f4ea757f81a 100644 --- a/spec/services/users/update_service_spec.rb +++ b/spec/services/users/update_service_spec.rb @@ -77,6 +77,34 @@ RSpec.describe Users::UpdateService do subject end + context 'when race condition' do + # See https://gitlab.com/gitlab-org/gitlab/-/issues/382957 + it 'updates email for stale user', :aggregate_failures do + unconfirmed_email = 'unconfirmed-email-user-has-access-to@example.com' + forgery_email = 'forgery@example.com' + + user.update!(email: unconfirmed_email) + + stale_user = User.find(user.id) + + service1 = described_class.new(stale_user, { email: unconfirmed_email }.merge(user: stale_user)) + + service2 = described_class.new(user, { email: forgery_email }.merge(user: user)) + + service2.execute + reloaded_user = User.find(user.id) + expect(reloaded_user.unconfirmed_email).to eq(forgery_email) + expect(stale_user.confirmation_token).not_to eq(user.confirmation_token) + expect(reloaded_user.confirmation_token).to eq(user.confirmation_token) + + service1.execute + reloaded_user = User.find(user.id) + expect(reloaded_user.unconfirmed_email).to eq(unconfirmed_email) + expect(stale_user.confirmation_token).not_to eq(user.confirmation_token) + expect(reloaded_user.confirmation_token).to eq(stale_user.confirmation_token) + end + end + context 'when check_password is true' do def update_user(user, opts) described_class.new(user, opts.merge(user: user)).execute(check_password: true) @@ -139,9 +167,24 @@ RSpec.describe Users::UpdateService do update_user(user, job_title: 'supreme leader of the universe') end.not_to change { user.user_canonical_email } end + + it 'does not reset unconfirmed email' do + unconfirmed_email = 'unconfirmed-email@example.com' + user.update!(email: unconfirmed_email) + + expect do + update_user(user, job_title: 'supreme leader of the universe') + end.not_to change { user.unconfirmed_email } + end end end + it 'does not try to reset unconfirmed email for a new user' do + expect do + update_user(build(:user), job_title: 'supreme leader of the universe') + end.not_to raise_error + end + def update_user(user, opts) described_class.new(user, opts.merge(user: user)).execute end |