diff options
Diffstat (limited to 'spec/controllers')
47 files changed, 1656 insertions, 436 deletions
diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index 84bbbac39b0..0b3833e6515 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -641,24 +641,32 @@ describe ApplicationController do end end - it 'does not set a custom header' do + it 'sets a custom header' do get :index, format: :json - expect(response.headers['X-GitLab-Custom-Error']).to be_nil + expect(response.headers['X-GitLab-Custom-Error']).to eq '1' end - end - context 'given a json response for an html request' do - controller do - def index - render json: {}, status: :unprocessable_entity + context 'for html request' do + it 'sets a custom header' do + get :index + + expect(response.headers['X-GitLab-Custom-Error']).to eq '1' end end - it 'does not set a custom header' do - get :index + context 'for 200 response' do + controller do + def index + render json: {}, status: :ok + end + end - expect(response.headers['X-GitLab-Custom-Error']).to be_nil + it 'does not set a custom header' do + get :index, format: :json + + expect(response.headers['X-GitLab-Custom-Error']).to be_nil + end end end end diff --git a/spec/controllers/concerns/confirm_email_warning_spec.rb b/spec/controllers/concerns/confirm_email_warning_spec.rb new file mode 100644 index 00000000000..0c598a360af --- /dev/null +++ b/spec/controllers/concerns/confirm_email_warning_spec.rb @@ -0,0 +1,98 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe ConfirmEmailWarning do + before do + stub_feature_flags(soft_email_confirmation: true) + allow(User).to receive(:allow_unconfirmed_access_for).and_return 2.days + end + + controller(ApplicationController) do + # `described_class` is not available in this context + include ConfirmEmailWarning # rubocop:disable RSpec/DescribedClass + + def index + head :ok + end + end + + RSpec::Matchers.define :set_confirm_warning_for do |email| + match do |response| + expect(response).to set_flash.now[:warning].to include("Please check your email (#{email}) to verify that you own this address.") + end + end + + describe 'confirm email flash warning' do + context 'when not signed in' do + let(:user) { create(:user, confirmed_at: nil) } + + before do + get :index + end + + it { is_expected.not_to set_confirm_warning_for(user.email) } + end + + context 'when signed in' do + before do + sign_in(user) + end + + context 'with a confirmed user' do + let(:user) { create(:user) } + + before do + get :index + end + + it { is_expected.not_to set_confirm_warning_for(user.email) } + end + + context 'with an unconfirmed user' do + let(:user) { create(:user, confirmed_at: nil) } + + context 'when executing a peek request' do + before do + request.path = '/-/peek' + get :index + end + + it { is_expected.not_to set_confirm_warning_for(user.email) } + end + + context 'when executing a json request' do + before do + get :index, format: :json + end + + it { is_expected.not_to set_confirm_warning_for(user.email) } + end + + context 'when executing a post request' do + before do + post :index + end + + it { is_expected.not_to set_confirm_warning_for(user.email) } + end + + context 'when executing a get request' do + before do + get :index + end + + context 'with an unconfirmed email address present' do + let(:user) { create(:user, confirmed_at: nil, unconfirmed_email: 'unconfirmed@gitlab.com') } + + it { is_expected.to set_confirm_warning_for(user.unconfirmed_email) } + end + + context 'without an unconfirmed email address present' do + it { is_expected.to set_confirm_warning_for(user.email) } + end + end + end + end + end +end diff --git a/spec/controllers/concerns/issuable_collections_spec.rb b/spec/controllers/concerns/issuable_collections_spec.rb index f210537aad5..7bdf5c49425 100644 --- a/spec/controllers/concerns/issuable_collections_spec.rb +++ b/spec/controllers/concerns/issuable_collections_spec.rb @@ -24,78 +24,6 @@ describe IssuableCollections do controller end - describe '#set_sort_order_from_user_preference' do - describe 'when sort param given' do - let(:params) { { sort: 'updated_desc' } } - - context 'when issuable_sorting_field is defined' do - before do - controller.class.define_method(:issuable_sorting_field) { :issues_sort} - end - - it 'sets user_preference with the right value' do - controller.send(:set_sort_order_from_user_preference) - - expect(user.user_preference.reload.issues_sort).to eq('updated_desc') - end - end - - context 'when no issuable_sorting_field is defined on the controller' do - it 'does not touch user_preference' do - allow(user).to receive(:user_preference) - - controller.send(:set_sort_order_from_user_preference) - - expect(user).not_to have_received(:user_preference) - end - end - end - - context 'when a user sorting preference exists' do - let(:params) { {} } - - before do - controller.class.define_method(:issuable_sorting_field) { :issues_sort } - end - - it 'returns the set preference' do - user.user_preference.update(issues_sort: 'updated_asc') - - sort_preference = controller.send(:set_sort_order_from_user_preference) - - expect(sort_preference).to eq('updated_asc') - end - end - end - - describe '#set_set_order_from_cookie' do - describe 'when sort param given' do - let(:cookies) { {} } - let(:params) { { sort: 'downvotes_asc' } } - - it 'sets the cookie with the right values and flags' do - allow(controller).to receive(:cookies).and_return(cookies) - - controller.send(:set_sort_order_from_cookie) - - expect(cookies['issue_sort']).to eq({ value: 'popularity', secure: false, httponly: false }) - end - end - - describe 'when cookie exists' do - let(:cookies) { { 'issue_sort' => 'id_asc' } } - let(:params) { {} } - - it 'sets the cookie with the right values and flags' do - allow(controller).to receive(:cookies).and_return(cookies) - - controller.send(:set_sort_order_from_cookie) - - expect(cookies['issue_sort']).to eq({ value: 'created_asc', secure: false, httponly: false }) - end - end - end - describe '#page_count_for_relation' do let(:params) { { state: 'opened' } } diff --git a/spec/controllers/concerns/sorting_preference_spec.rb b/spec/controllers/concerns/sorting_preference_spec.rb new file mode 100644 index 00000000000..a36124c6776 --- /dev/null +++ b/spec/controllers/concerns/sorting_preference_spec.rb @@ -0,0 +1,93 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe SortingPreference do + let(:user) { create(:user) } + + let(:controller_class) do + Class.new do + def self.helper_method(name); end + + include SortingPreference + include SortingHelper + end + end + + let(:controller) { controller_class.new } + + before do + allow(controller).to receive(:params).and_return(ActionController::Parameters.new(params)) + allow(controller).to receive(:current_user).and_return(user) + allow(controller).to receive(:legacy_sort_cookie_name).and_return('issuable_sort') + allow(controller).to receive(:sorting_field).and_return(:issues_sort) + end + + describe '#set_sort_order_from_user_preference' do + subject { controller.send(:set_sort_order_from_user_preference) } + + context 'when sort param given' do + let(:params) { { sort: 'updated_desc' } } + + context 'when sorting_field is defined' do + it 'sets user_preference with the right value' do + is_expected.to eq('updated_desc') + end + end + + context 'when no sorting_field is defined on the controller' do + before do + allow(controller).to receive(:sorting_field).and_return(nil) + end + + it 'does not touch user_preference' do + expect(user).not_to receive(:user_preference) + + subject + end + end + end + + context 'when a user sorting preference exists' do + let(:params) { {} } + + before do + user.user_preference.update!(issues_sort: 'updated_asc') + end + + it 'returns the set preference' do + is_expected.to eq('updated_asc') + end + end + end + + describe '#set_set_order_from_cookie' do + subject { controller.send(:set_sort_order_from_cookie) } + + before do + allow(controller).to receive(:cookies).and_return(cookies) + end + + context 'when sort param given' do + let(:cookies) { {} } + let(:params) { { sort: 'downvotes_asc' } } + + it 'sets the cookie with the right values and flags' do + subject + + expect(cookies['issue_sort']).to eq(value: 'popularity', secure: false, httponly: false) + end + end + + context 'when cookie exists' do + let(:cookies) { { 'issue_sort' => 'id_asc' } } + let(:params) { {} } + + it 'sets the cookie with the right values and flags' do + subject + + expect(cookies['issue_sort']).to eq(value: 'created_asc', secure: false, httponly: false) + end + end + end +end diff --git a/spec/controllers/dashboard/projects_controller_spec.rb b/spec/controllers/dashboard/projects_controller_spec.rb index 6591901a9dc..8b95c9f2496 100644 --- a/spec/controllers/dashboard/projects_controller_spec.rb +++ b/spec/controllers/dashboard/projects_controller_spec.rb @@ -40,6 +40,14 @@ describe Dashboard::ProjectsController do expect(assigns(:projects)).to eq([project, project2]) end + + context 'project sorting' do + let(:project) { create(:project) } + + it_behaves_like 'set sort order from user preference' do + let(:sorting_param) { 'created_asc' } + end + end end end diff --git a/spec/controllers/explore/projects_controller_spec.rb b/spec/controllers/explore/projects_controller_spec.rb index 463586ee422..6752d2b8ebd 100644 --- a/spec/controllers/explore/projects_controller_spec.rb +++ b/spec/controllers/explore/projects_controller_spec.rb @@ -3,56 +3,91 @@ require 'spec_helper' describe Explore::ProjectsController do - describe 'GET #index.json' do - render_views + shared_examples 'explore projects' do + describe 'GET #index.json' do + render_views - before do - get :index, format: :json + before do + get :index, format: :json + end + + it { is_expected.to respond_with(:success) } end - it { is_expected.to respond_with(:success) } - end + describe 'GET #trending.json' do + render_views - describe 'GET #trending.json' do - render_views + before do + get :trending, format: :json + end - before do - get :trending, format: :json + it { is_expected.to respond_with(:success) } + end + + describe 'GET #starred.json' do + render_views + + before do + get :starred, format: :json + end + + it { is_expected.to respond_with(:success) } end - it { is_expected.to respond_with(:success) } + describe 'GET #trending' do + context 'sorting by update date' do + let(:project1) { create(:project, :public, updated_at: 3.days.ago) } + let(:project2) { create(:project, :public, updated_at: 1.day.ago) } + + before do + create(:trending_project, project: project1) + create(:trending_project, project: project2) + end + + it 'sorts by last updated' do + get :trending, params: { sort: 'updated_desc' } + + expect(assigns(:projects)).to eq [project2, project1] + end + + it 'sorts by oldest updated' do + get :trending, params: { sort: 'updated_asc' } + + expect(assigns(:projects)).to eq [project1, project2] + end + end + end end - describe 'GET #starred.json' do - render_views + context 'when user is signed in' do + let(:user) { create(:user) } before do - get :starred, format: :json + sign_in(user) end - it { is_expected.to respond_with(:success) } - end + include_examples 'explore projects' - describe 'GET #trending' do - context 'sorting by update date' do - let(:project1) { create(:project, :public, updated_at: 3.days.ago) } - let(:project2) { create(:project, :public, updated_at: 1.day.ago) } + context 'user preference sorting' do + let(:project) { create(:project) } - before do - create(:trending_project, project: project1) - create(:trending_project, project: project2) + it_behaves_like 'set sort order from user preference' do + let(:sorting_param) { 'created_asc' } end + end + end - it 'sorts by last updated' do - get :trending, params: { sort: 'updated_desc' } + context 'when user is not signed in' do + include_examples 'explore projects' - expect(assigns(:projects)).to eq [project2, project1] - end + context 'user preference sorting' do + let(:project) { create(:project) } + let(:sorting_param) { 'created_asc' } - it 'sorts by oldest updated' do - get :trending, params: { sort: 'updated_asc' } + it 'does not set sort order from user preference' do + expect_any_instance_of(UserPreference).not_to receive(:update) - expect(assigns(:projects)).to eq [project1, project2] + get :index, params: { sort: sorting_param } end end end diff --git a/spec/controllers/groups/group_members_controller_spec.rb b/spec/controllers/groups/group_members_controller_spec.rb index 413598ddde0..0c3dd971582 100644 --- a/spec/controllers/groups/group_members_controller_spec.rb +++ b/spec/controllers/groups/group_members_controller_spec.rb @@ -16,6 +16,39 @@ describe Groups::GroupMembersController do expect(response).to have_gitlab_http_status(200) expect(response).to render_template(:index) end + + context 'user with owner access' do + let!(:invited) { create_list(:group_member, 3, :invited, group: group) } + + before do + group.add_owner(user) + sign_in(user) + end + + it 'assigns invited members' do + get :index, params: { group_id: group } + + expect(assigns(:invited_members).map(&:invite_email)).to match_array(invited.map(&:invite_email)) + end + + it 'restricts search to one email' do + get :index, params: { group_id: group, search_invited: invited.first.invite_email } + + expect(assigns(:invited_members).map(&:invite_email)).to match_array(invited.first.invite_email) + end + + it 'paginates invited list' do + stub_const('Groups::GroupMembersController::MEMBER_PER_PAGE_LIMIT', 2) + + get :index, params: { group_id: group, invited_members_page: 1 } + + expect(assigns(:invited_members).count).to eq(2) + + get :index, params: { group_id: group, invited_members_page: 2 } + + expect(assigns(:invited_members).count).to eq(1) + end + end end describe 'POST create' do @@ -139,7 +172,7 @@ describe Groups::GroupMembersController do it '[JS] removes user from members' do delete :destroy, params: { group_id: group, id: member }, xhr: true - expect(response).to be_success + expect(response).to be_successful expect(group.members).not_to include member end end diff --git a/spec/controllers/groups/milestones_controller_spec.rb b/spec/controllers/groups/milestones_controller_spec.rb index bf164aeed38..41927907fd1 100644 --- a/spec/controllers/groups/milestones_controller_spec.rb +++ b/spec/controllers/groups/milestones_controller_spec.rb @@ -186,7 +186,7 @@ describe Groups::MilestonesController do it "removes milestone" do delete :destroy, params: { group_id: group.to_param, id: milestone.iid }, format: :js - expect(response).to be_success + expect(response).to be_successful expect { Milestone.find(milestone.id) }.to raise_exception(ActiveRecord::RecordNotFound) end end diff --git a/spec/controllers/groups/runners_controller_spec.rb b/spec/controllers/groups/runners_controller_spec.rb index 91f9e2c7832..14b0cf959b3 100644 --- a/spec/controllers/groups/runners_controller_spec.rb +++ b/spec/controllers/groups/runners_controller_spec.rb @@ -3,73 +3,202 @@ require 'spec_helper' describe Groups::RunnersController do - let(:user) { create(:user) } - let(:group) { create(:group) } + let(:user) { create(:user) } + let(:group) { create(:group) } let(:runner) { create(:ci_runner, :group, groups: [group]) } - - let(:params) do - { - group_id: group, - id: runner - } - end + let(:params) { { group_id: group, id: runner } } before do sign_in(user) - group.add_maintainer(user) + end + + describe '#show' do + context 'when user is owner' do + before do + group.add_owner(user) + end + + it 'renders show with 200 status code' do + get :show, params: { group_id: group, id: runner } + + expect(response).to have_gitlab_http_status(200) + expect(response).to render_template(:show) + end + end + + context 'when user is not owner' do + before do + group.add_maintainer(user) + end + + it 'renders a 404' do + get :show, params: { group_id: group, id: runner } + + expect(response).to have_gitlab_http_status(404) + end + end + end + + describe '#edit' do + context 'when user is owner' do + before do + group.add_owner(user) + end + + it 'renders show with 200 status code' do + get :edit, params: { group_id: group, id: runner } + + expect(response).to have_gitlab_http_status(200) + expect(response).to render_template(:edit) + end + end + + context 'when user is not owner' do + before do + group.add_maintainer(user) + end + + it 'renders a 404' do + get :edit, params: { group_id: group, id: runner } + + expect(response).to have_gitlab_http_status(404) + end + end end describe '#update' do - it 'updates the runner and ticks the queue' do - new_desc = runner.description.swapcase + context 'when user is an owner' do + before do + group.add_owner(user) + end - expect do - post :update, params: params.merge(runner: { description: new_desc } ) - end.to change { runner.ensure_runner_queue_value } + it 'updates the runner, ticks the queue, and redirects' do + new_desc = runner.description.swapcase - runner.reload + expect do + post :update, params: params.merge(runner: { description: new_desc } ) + end.to change { runner.ensure_runner_queue_value } - expect(response).to have_gitlab_http_status(302) - expect(runner.description).to eq(new_desc) + expect(response).to have_gitlab_http_status(302) + expect(runner.reload.description).to eq(new_desc) + end + end + + context 'when user is not an owner' do + before do + group.add_maintainer(user) + end + + it 'rejects the update and responds 404' do + old_desc = runner.description + + expect do + post :update, params: params.merge(runner: { description: old_desc.swapcase } ) + end.not_to change { runner.ensure_runner_queue_value } + + expect(response).to have_gitlab_http_status(404) + expect(runner.reload.description).to eq(old_desc) + end end end describe '#destroy' do - it 'destroys the runner' do - delete :destroy, params: params + context 'when user is an owner' do + before do + group.add_owner(user) + end + + it 'destroys the runner and redirects' do + delete :destroy, params: params + + expect(response).to have_gitlab_http_status(302) + expect(Ci::Runner.find_by(id: runner.id)).to be_nil + end + end + + context 'when user is not an owner' do + before do + group.add_maintainer(user) + end + + it 'responds 404 and does not destroy the runner' do + delete :destroy, params: params - expect(response).to have_gitlab_http_status(302) - expect(Ci::Runner.find_by(id: runner.id)).to be_nil + expect(response).to have_gitlab_http_status(404) + expect(Ci::Runner.find_by(id: runner.id)).to be_present + end end end describe '#resume' do - it 'marks the runner as active and ticks the queue' do - runner.update(active: false) + context 'when user is an owner' do + before do + group.add_owner(user) + end - expect do - post :resume, params: params - end.to change { runner.ensure_runner_queue_value } + it 'marks the runner as active, ticks the queue, and redirects' do + runner.update(active: false) - runner.reload + expect do + post :resume, params: params + end.to change { runner.ensure_runner_queue_value } - expect(response).to have_gitlab_http_status(302) - expect(runner.active).to eq(true) + expect(response).to have_gitlab_http_status(302) + expect(runner.reload.active).to eq(true) + end + end + + context 'when user is not an owner' do + before do + group.add_maintainer(user) + end + + it 'responds 404 and does not activate the runner' do + runner.update(active: false) + + expect do + post :resume, params: params + end.not_to change { runner.ensure_runner_queue_value } + + expect(response).to have_gitlab_http_status(404) + expect(runner.reload.active).to eq(false) + end end end describe '#pause' do - it 'marks the runner as inactive and ticks the queue' do - runner.update(active: true) + context 'when user is an owner' do + before do + group.add_owner(user) + end + + it 'marks the runner as inactive, ticks the queue, and redirects' do + runner.update(active: true) + + expect do + post :pause, params: params + end.to change { runner.ensure_runner_queue_value } + + expect(response).to have_gitlab_http_status(302) + expect(runner.reload.active).to eq(false) + end + end + + context 'when user is not an owner' do + before do + group.add_maintainer(user) + end - expect do - post :pause, params: params - end.to change { runner.ensure_runner_queue_value } + it 'responds 404 and does not update the runner or queue' do + runner.update(active: true) - runner.reload + expect do + post :pause, params: params + end.not_to change { runner.ensure_runner_queue_value } - expect(response).to have_gitlab_http_status(302) - expect(runner.active).to eq(false) + expect(response).to have_gitlab_http_status(404) + expect(runner.reload.active).to eq(true) + end end end end diff --git a/spec/controllers/health_check_controller_spec.rb b/spec/controllers/health_check_controller_spec.rb index 92f005faf4a..b48b7dc86e0 100644 --- a/spec/controllers/health_check_controller_spec.rb +++ b/spec/controllers/health_check_controller_spec.rb @@ -33,14 +33,14 @@ describe HealthCheckController do get :index - expect(response).to be_success + expect(response).to be_successful expect(response.content_type).to eq 'text/plain' end it 'supports passing the token in query params' do get :index, params: { token: token } - expect(response).to be_success + expect(response).to be_successful expect(response.content_type).to eq 'text/plain' end end @@ -54,14 +54,14 @@ describe HealthCheckController do it 'supports successful plaintext response' do get :index - expect(response).to be_success + expect(response).to be_successful expect(response.content_type).to eq 'text/plain' end it 'supports successful json response' do get :index, format: :json - expect(response).to be_success + expect(response).to be_successful expect(response.content_type).to eq 'application/json' expect(json_response['healthy']).to be true end @@ -69,7 +69,7 @@ describe HealthCheckController do it 'supports successful xml response' do get :index, format: :xml - expect(response).to be_success + expect(response).to be_successful expect(response.content_type).to eq 'application/xml' expect(xml_response['healthy']).to be true end @@ -77,7 +77,7 @@ describe HealthCheckController do it 'supports successful responses for specific checks' do get :index, params: { checks: 'email' }, format: :json - expect(response).to be_success + expect(response).to be_successful expect(response.content_type).to eq 'application/json' expect(json_response['healthy']).to be true end diff --git a/spec/controllers/help_controller_spec.rb b/spec/controllers/help_controller_spec.rb index 43c910da7a5..03b6e85b653 100644 --- a/spec/controllers/help_controller_spec.rb +++ b/spec/controllers/help_controller_spec.rb @@ -114,7 +114,7 @@ describe HelpController do path: 'user/project/img/labels_default_v12_1' }, format: :png - expect(response).to be_success + expect(response).to be_successful expect(response.content_type).to eq 'image/png' expect(response.headers['Content-Disposition']).to match(/^inline;/) end diff --git a/spec/controllers/import/bitbucket_server_controller_spec.rb b/spec/controllers/import/bitbucket_server_controller_spec.rb index b89d7317b9c..e1aeab46fca 100644 --- a/spec/controllers/import/bitbucket_server_controller_spec.rb +++ b/spec/controllers/import/bitbucket_server_controller_spec.rb @@ -134,6 +134,8 @@ describe Import::BitbucketServerController do describe 'GET status' do render_views + let(:repos) { instance_double(BitbucketServer::Collection) } + before do allow(controller).to receive(:bitbucket_client).and_return(client) @@ -145,7 +147,6 @@ describe Import::BitbucketServerController do it 'assigns repository categories' do created_project = create(:project, :import_finished, import_type: 'bitbucket_server', creator_id: user.id, import_source: @created_repo.browse_url) - repos = instance_double(BitbucketServer::Collection) expect(repos).to receive(:partition).and_return([[@repo, @created_repo], [@invalid_repo]]) expect(repos).to receive(:current_page).and_return(1) @@ -159,6 +160,17 @@ describe Import::BitbucketServerController do expect(assigns(:repos)).to eq([@repo]) expect(assigns(:incompatible_repos)).to eq([@invalid_repo]) end + + context 'when filtering' do + let(:filter) { 'test' } + + it 'passes filter param to bitbucket client' do + expect(repos).to receive(:partition).and_return([[@repo, @created_repo], [@invalid_repo]]) + expect(client).to receive(:repos).with(filter: filter, limit: 25, page_offset: 0).and_return(repos) + + get :status, params: { filter: filter }, as: :json + end + end end describe 'GET jobs' do diff --git a/spec/controllers/metrics_controller_spec.rb b/spec/controllers/metrics_controller_spec.rb index 84027119491..7fb3578cd0a 100644 --- a/spec/controllers/metrics_controller_spec.rb +++ b/spec/controllers/metrics_controller_spec.rb @@ -5,12 +5,19 @@ require 'spec_helper' describe MetricsController do include StubENV - let(:metrics_multiproc_dir) { Dir.mktmpdir } + let(:metrics_multiproc_dir) { @metrics_multiproc_dir } let(:whitelisted_ip) { '127.0.0.1' } let(:whitelisted_ip_range) { '10.0.0.0/24' } let(:ip_in_whitelisted_range) { '10.0.0.1' } let(:not_whitelisted_ip) { '10.0.1.1' } + around do |example| + Dir.mktmpdir do |path| + @metrics_multiproc_dir = path + example.run + end + end + before do stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false') allow(Prometheus::Client.configuration).to receive(:multiprocess_files_dir).and_return(metrics_multiproc_dir) diff --git a/spec/controllers/oauth/applications_controller_spec.rb b/spec/controllers/oauth/applications_controller_spec.rb index 228c97d591d..df836c2c3e3 100644 --- a/spec/controllers/oauth/applications_controller_spec.rb +++ b/spec/controllers/oauth/applications_controller_spec.rb @@ -17,7 +17,7 @@ describe Oauth::ApplicationsController do expect(response).to have_gitlab_http_status(200) end - it 'shows list of applications' do + it 'redirects back to profile page if OAuth applications are disabled' do disable_user_oauth get :index diff --git a/spec/controllers/profiles/keys_controller_spec.rb b/spec/controllers/profiles/keys_controller_spec.rb index 753eb432c5e..3bed117deb0 100644 --- a/spec/controllers/profiles/keys_controller_spec.rb +++ b/spec/controllers/profiles/keys_controller_spec.rb @@ -10,7 +10,7 @@ describe Profiles::KeysController do it "does not generally work" do get :get_keys, params: { username: 'not-existent' } - expect(response).not_to be_success + expect(response).not_to be_successful end end @@ -18,7 +18,7 @@ describe Profiles::KeysController do it "does generally work" do get :get_keys, params: { username: user.username } - expect(response).to be_success + expect(response).to be_successful end it "renders all keys separated with a new line" do @@ -41,7 +41,7 @@ describe Profiles::KeysController do it "does generally work" do get :get_keys, params: { username: user.username } - expect(response).to be_success + expect(response).to be_successful end it "renders all non deploy keys separated with a new line" do diff --git a/spec/controllers/projects/ci/lints_controller_spec.rb b/spec/controllers/projects/ci/lints_controller_spec.rb index 96e82b7086c..14128fb5b0e 100644 --- a/spec/controllers/projects/ci/lints_controller_spec.rb +++ b/spec/controllers/projects/ci/lints_controller_spec.rb @@ -20,9 +20,7 @@ describe Projects::Ci::LintsController do get :show, params: { namespace_id: project.namespace, project_id: project } end - it 'is success' do - expect(response).to be_success - end + it { expect(response).to be_successful } it 'renders show page' do expect(response).to render_template :show @@ -78,9 +76,7 @@ describe Projects::Ci::LintsController do post :create, params: { namespace_id: project.namespace, project_id: project, content: content } end - it 'is success' do - expect(response).to be_success - end + it { expect(response).to be_successful } it 'render show page' do expect(response).to render_template :show diff --git a/spec/controllers/projects/commit_controller_spec.rb b/spec/controllers/projects/commit_controller_spec.rb index 58a1d96d010..afd5cb15e0f 100644 --- a/spec/controllers/projects/commit_controller_spec.rb +++ b/spec/controllers/projects/commit_controller_spec.rb @@ -45,14 +45,14 @@ describe Projects::CommitController do it 'handles binary files' do go(id: TestEnv::BRANCH_SHA['binary-encoding'], format: 'html') - expect(response).to be_success + expect(response).to be_successful end shared_examples "export as" do |format| it "does generally work" do go(id: commit.id, format: format) - expect(response).to be_success + expect(response).to be_successful end it "generates it" do @@ -110,7 +110,7 @@ describe Projects::CommitController do id: commit.id }) - expect(response).to be_success + expect(response).to be_successful end end @@ -177,7 +177,7 @@ describe Projects::CommitController do id: commit.id }) - expect(response).not_to be_success + expect(response).not_to be_successful expect(response).to have_gitlab_http_status(404) end end @@ -234,7 +234,7 @@ describe Projects::CommitController do id: master_pickable_commit.id }) - expect(response).not_to be_success + expect(response).not_to be_successful expect(response).to have_gitlab_http_status(404) end end diff --git a/spec/controllers/projects/commits_controller_spec.rb b/spec/controllers/projects/commits_controller_spec.rb index 9db1ac2a46c..9c4d6fdcb2a 100644 --- a/spec/controllers/projects/commits_controller_spec.rb +++ b/spec/controllers/projects/commits_controller_spec.rb @@ -79,7 +79,7 @@ describe Projects::CommitsController do end it "renders as atom" do - expect(response).to be_success + expect(response).to be_successful expect(response.content_type).to eq('application/atom+xml') end @@ -104,7 +104,7 @@ describe Projects::CommitsController do end it "renders as HTML" do - expect(response).to be_success + expect(response).to be_successful expect(response.content_type).to eq('text/html') end end diff --git a/spec/controllers/projects/compare_controller_spec.rb b/spec/controllers/projects/compare_controller_spec.rb index 48a92a772dc..9afc46c4be9 100644 --- a/spec/controllers/projects/compare_controller_spec.rb +++ b/spec/controllers/projects/compare_controller_spec.rb @@ -19,7 +19,7 @@ describe Projects::CompareController do end it 'returns successfully' do - expect(response).to be_success + expect(response).to be_successful end end @@ -49,7 +49,7 @@ describe Projects::CompareController do it 'shows some diffs with ignore whitespace change option' do show_request - expect(response).to be_success + expect(response).to be_successful diff_file = assigns(:diffs).diff_files.first expect(diff_file).not_to be_nil expect(assigns(:commits).length).to be >= 1 @@ -67,7 +67,7 @@ describe Projects::CompareController do it 'sets the diffs and commits ivars' do show_request - expect(response).to be_success + expect(response).to be_successful expect(assigns(:diffs).diff_files.first).not_to be_nil expect(assigns(:commits).length).to be >= 1 end @@ -81,7 +81,7 @@ describe Projects::CompareController do it 'sets empty diff and commit ivars' do show_request - expect(response).to be_success + expect(response).to be_successful expect(assigns(:diffs)).to eq([]) expect(assigns(:commits)).to eq([]) end @@ -94,7 +94,7 @@ describe Projects::CompareController do it 'sets empty diff and commit ivars' do show_request - expect(response).to be_success + expect(response).to be_successful expect(assigns(:diffs)).to eq([]) expect(assigns(:commits)).to eq([]) end diff --git a/spec/controllers/projects/cycle_analytics/events_controller_spec.rb b/spec/controllers/projects/cycle_analytics/events_controller_spec.rb index 8fc3ae0aa32..b828c678d0c 100644 --- a/spec/controllers/projects/cycle_analytics/events_controller_spec.rb +++ b/spec/controllers/projects/cycle_analytics/events_controller_spec.rb @@ -16,7 +16,7 @@ describe Projects::CycleAnalytics::EventsController do it 'is empty' do get_issue - expect(response).to be_success + expect(response).to be_successful expect(JSON.parse(response.body)['events']).to be_empty end end @@ -32,7 +32,7 @@ describe Projects::CycleAnalytics::EventsController do it 'is not empty' do get_issue - expect(response).to be_success + expect(response).to be_successful end it 'contains event detais' do @@ -49,7 +49,7 @@ describe Projects::CycleAnalytics::EventsController do it 'is empty' do get_issue(additional_params: { cycle_analytics: { start_date: 7 } }) - expect(response).to be_success + expect(response).to be_successful expect(JSON.parse(response.body)['events']).to be_empty end diff --git a/spec/controllers/projects/cycle_analytics_controller_spec.rb b/spec/controllers/projects/cycle_analytics_controller_spec.rb index 2dc97e18113..65eee7b8ead 100644 --- a/spec/controllers/projects/cycle_analytics_controller_spec.rb +++ b/spec/controllers/projects/cycle_analytics_controller_spec.rb @@ -11,6 +11,20 @@ describe Projects::CycleAnalyticsController do project.add_maintainer(user) end + context "counting page views for 'show'" do + it 'increases the counter' do + expect(Gitlab::UsageDataCounters::CycleAnalyticsCounter).to receive(:count).with(:views) + + get(:show, + params: { + namespace_id: project.namespace, + project_id: project + }) + + expect(response).to be_successful + end + end + describe 'cycle analytics not set up flag' do context 'with no data' do it 'is true' do @@ -20,7 +34,7 @@ describe Projects::CycleAnalyticsController do project_id: project }) - expect(response).to be_success + expect(response).to be_successful expect(assigns(:cycle_analytics_no_data)).to eq(true) end end @@ -41,7 +55,7 @@ describe Projects::CycleAnalyticsController do project_id: project }) - expect(response).to be_success + expect(response).to be_successful expect(assigns(:cycle_analytics_no_data)).to eq(false) end end diff --git a/spec/controllers/projects/environments_controller_spec.rb b/spec/controllers/projects/environments_controller_spec.rb index 8872e8d38e7..71ee1fd03bf 100644 --- a/spec/controllers/projects/environments_controller_spec.rb +++ b/spec/controllers/projects/environments_controller_spec.rb @@ -518,10 +518,10 @@ describe Projects::EnvironmentsController do end end - shared_examples_for 'the default dynamic dashboard' do + shared_examples_for 'specified dashboard embed' do |expected_titles| it_behaves_like '200 response' - it 'contains only the Memory and CPU charts' do + it 'contains only the specified charts' do get :metrics_dashboard, params: environment_params(dashboard_params) dashboard = json_response['dashboard'] @@ -531,10 +531,14 @@ describe Projects::EnvironmentsController do expect(dashboard['dashboard']).to be_nil expect(dashboard['panel_groups'].length).to eq 1 expect(panel_group['group']).to be_nil - expect(titles).to eq ['Memory Usage (Total)', 'Core Usage (Total)'] + expect(titles).to eq expected_titles end end + shared_examples_for 'the default dynamic dashboard' do + it_behaves_like 'specified dashboard embed', ['Memory Usage (Total)', 'Core Usage (Total)'] + end + shared_examples_for 'dashboard can be specified' do context 'when dashboard is specified' do let(:dashboard_path) { '.gitlab/dashboards/test.yml' } @@ -551,7 +555,7 @@ describe Projects::EnvironmentsController do end context 'when the specified dashboard is the default dashboard' do - let(:dashboard_path) { ::Metrics::Dashboard::SystemDashboardService::SYSTEM_DASHBOARD_PATH } + let(:dashboard_path) { system_dashboard_path } it_behaves_like 'the default dashboard' end @@ -564,12 +568,40 @@ describe Projects::EnvironmentsController do it_behaves_like 'the default dynamic dashboard' - context 'when the dashboard is specified' do - let(:dashboard_params) { { format: :json, embedded: true, dashboard: '.gitlab/dashboards/fake.yml' } } + context 'when incomplete dashboard params are provided' do + let(:dashboard_params) { { format: :json, embedded: true, title: 'Title' } } + + # The title param should be ignored. + it_behaves_like 'the default dynamic dashboard' + end + + context 'when invalid params are provided' do + let(:dashboard_params) { { format: :json, embedded: true, metric_id: 16 } } - # The dashboard param should be ignored. + # The superfluous param should be ignored. it_behaves_like 'the default dynamic dashboard' end + + context 'when the dashboard is correctly specified' do + let(:dashboard_params) do + { + format: :json, + embedded: true, + dashboard: system_dashboard_path, + group: business_metric_title, + title: 'title', + y_label: 'y_label' + } + end + + it_behaves_like 'error response', :not_found + + context 'and exists' do + let!(:metric) { create(:prometheus_metric, project: project) } + + it_behaves_like 'specified dashboard embed', ['title'] + end + end end end @@ -581,31 +613,13 @@ describe Projects::EnvironmentsController do end end - shared_examples_for 'dashboard cannot be embedded' do - context 'when the embedded flag is included' do - let(:dashboard_params) { { format: :json, embedded: true } } - - it_behaves_like 'the default dashboard' - end - end - let(:dashboard_params) { { format: :json } } it_behaves_like 'the default dashboard' it_behaves_like 'dashboard can be specified' it_behaves_like 'dashboard can be embedded' - context 'when multiple dashboards is enabled and embedding metrics is disabled' do - before do - stub_feature_flags(gfm_embedded_metrics: false) - end - - it_behaves_like 'the default dashboard' - it_behaves_like 'dashboard can be specified' - it_behaves_like 'dashboard cannot be embedded' - end - - context 'when multiple dashboards is disabled and embedding metrics is enabled' do + context 'when multiple dashboards is disabled' do before do stub_feature_flags(environment_metrics_show_multiple_dashboards: false) end @@ -614,19 +628,6 @@ describe Projects::EnvironmentsController do it_behaves_like 'dashboard cannot be specified' it_behaves_like 'dashboard can be embedded' end - - context 'when multiple dashboards and embedding metrics are disabled' do - before do - stub_feature_flags( - environment_metrics_show_multiple_dashboards: false, - gfm_embedded_metrics: false - ) - end - - it_behaves_like 'the default dashboard' - it_behaves_like 'dashboard cannot be specified' - it_behaves_like 'dashboard cannot be embedded' - end end describe 'GET #search' do diff --git a/spec/controllers/projects/git_http_controller_spec.rb b/spec/controllers/projects/git_http_controller_spec.rb index bf099e8deeb..88fa2236e33 100644 --- a/spec/controllers/projects/git_http_controller_spec.rb +++ b/spec/controllers/projects/git_http_controller_spec.rb @@ -12,4 +12,15 @@ describe Projects::GitHttpController do expect(response.status).to eq(403) end end + + describe 'GET #info_refs' do + it 'returns 401 for unauthenticated requests to public repositories when http protocol is disabled' do + stub_application_setting(enabled_git_access_protocol: 'ssh') + project = create(:project, :public, :repository) + + get :info_refs, params: { service: 'git-upload-pack', namespace_id: project.namespace.to_param, project_id: project.path + '.git' } + + expect(response.status).to eq(401) + end + end end diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index fab47aa4701..187c7864ad7 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -1104,18 +1104,39 @@ describe Projects::IssuesController do project.add_developer(user) end + subject do + post(:toggle_award_emoji, params: { + namespace_id: project.namespace, + project_id: project, + id: issue.iid, + name: emoji_name + }) + end + let(:emoji_name) { 'thumbsup' } + it "toggles the award emoji" do expect do - post(:toggle_award_emoji, params: { - namespace_id: project.namespace, - project_id: project, - id: issue.iid, - name: "thumbsup" - }) + subject end.to change { issue.award_emoji.count }.by(1) expect(response).to have_gitlab_http_status(200) end + + it "removes the already awarded emoji" do + create(:award_emoji, awardable: issue, name: emoji_name, user: user) + + expect { subject }.to change { AwardEmoji.count }.by(-1) + + expect(response).to have_gitlab_http_status(200) + end + + it 'marks Todos on the Issue as done' do + todo = create(:todo, target: issue, project: project, user: user) + + subject + + expect(todo.reload).to be_done + end end describe 'POST create_merge_request' do diff --git a/spec/controllers/projects/merge_requests/content_controller_spec.rb b/spec/controllers/projects/merge_requests/content_controller_spec.rb index 2879e06aee4..818cf794ec6 100644 --- a/spec/controllers/projects/merge_requests/content_controller_spec.rb +++ b/spec/controllers/projects/merge_requests/content_controller_spec.rb @@ -11,8 +11,8 @@ describe Projects::MergeRequests::ContentController do sign_in(user) end - def do_request - get :widget, params: { + def do_request(action = :cached_widget) + get action, params: { namespace_id: project.namespace.to_param, project_id: project, id: merge_request.iid, @@ -20,41 +20,65 @@ describe Projects::MergeRequests::ContentController do } end - describe 'GET widget' do - context 'user has access to the project' do - before do - expect(::Gitlab::GitalyClient).to receive(:allow_ref_name_caching).and_call_original + context 'user has access to the project' do + before do + expect(::Gitlab::GitalyClient).to receive(:allow_ref_name_caching).and_call_original - project.add_maintainer(user) - end + project.add_maintainer(user) + end + describe 'GET cached_widget' do it 'renders widget MR entity as json' do do_request - expect(response).to match_response_schema('entities/merge_request_widget') + expect(response).to match_response_schema('entities/merge_request_poll_cached_widget') end + it 'closes an MR with moved source project' do + merge_request.update_column(:source_project_id, nil) + + expect { do_request }.to change { merge_request.reload.open? }.from(true).to(false) + end + end + + describe 'GET widget' do it 'checks whether the MR can be merged' do controller.instance_variable_set(:@merge_request, merge_request) expect(merge_request).to receive(:check_mergeability) - do_request + do_request(:widget) end - it 'closes an MR with moved source project' do - merge_request.update_column(:source_project_id, nil) + context 'merged merge request' do + let(:merge_request) do + create(:merged_merge_request, :with_test_reports, target_project: project, source_project: project) + end - expect { do_request }.to change { merge_request.reload.open? }.from(true).to(false) + it 'renders widget MR entity as json' do + do_request(:widget) + + expect(response).to match_response_schema('entities/merge_request_poll_widget') + end end end + end - context 'user does not have access to the project' do - it 'renders widget MR entity as json' do + context 'user does not have access to the project' do + describe 'GET cached_widget' do + it 'returns 404' do do_request expect(response).to have_http_status(:not_found) end end + + describe 'GET widget' do + it 'returns 404' do + do_request(:widget) + + expect(response).to have_http_status(:not_found) + end + end end end diff --git a/spec/controllers/projects/merge_requests/creations_controller_spec.rb b/spec/controllers/projects/merge_requests/creations_controller_spec.rb index 3816e1c7a31..ce977f26ec6 100644 --- a/spec/controllers/projects/merge_requests/creations_controller_spec.rb +++ b/spec/controllers/projects/merge_requests/creations_controller_spec.rb @@ -28,7 +28,7 @@ describe Projects::MergeRequests::CreationsController do it 'renders new merge request widget template' do get :new, params: get_diff_params - expect(response).to be_success + expect(response).to be_successful end end @@ -56,7 +56,7 @@ describe Projects::MergeRequests::CreationsController do it 'limits total commits' do get :new, params: large_diff_params - expect(response).to be_success + expect(response).to be_successful total = assigns(:total_commit_count) expect(assigns(:commits)).to be_an Array @@ -70,7 +70,7 @@ describe Projects::MergeRequests::CreationsController do it 'shows total commits' do get :new, params: large_diff_params - expect(response).to be_success + expect(response).to be_successful total = assigns(:total_commit_count) expect(assigns(:commits)).to be_an CommitCollection @@ -89,7 +89,7 @@ describe Projects::MergeRequests::CreationsController do get :diffs, params: get_diff_params.merge(format: 'json') - expect(response).to be_success + expect(response).to be_successful expect(assigns[:diffs]).to be_nil end end diff --git a/spec/controllers/projects/merge_requests/diffs_controller_spec.rb b/spec/controllers/projects/merge_requests/diffs_controller_spec.rb index d940d226176..ac3e9901123 100644 --- a/spec/controllers/projects/merge_requests/diffs_controller_spec.rb +++ b/spec/controllers/projects/merge_requests/diffs_controller_spec.rb @@ -66,7 +66,7 @@ describe Projects::MergeRequests::DiffsController do end it 'renders' do - expect(response).to be_success + expect(response).to be_successful expect(response.body).to have_content('Subproject commit') end end diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index b1dc6a65dd4..4b9b6913997 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -57,7 +57,7 @@ describe Projects::MergeRequestsController do go(format: :html) - expect(response).to be_success + expect(response).to be_successful end end @@ -66,7 +66,7 @@ describe Projects::MergeRequestsController do go(format: :html) - expect(response).to be_success + expect(response).to be_successful end context "that is invalid" do @@ -75,7 +75,7 @@ describe Projects::MergeRequestsController do it "renders merge request page" do go(format: :html) - expect(response).to be_success + expect(response).to be_successful end end end @@ -124,7 +124,7 @@ describe Projects::MergeRequestsController do it "renders merge request page" do go(format: :json) - expect(response).to be_success + expect(response).to be_successful end end end @@ -719,19 +719,63 @@ describe Projects::MergeRequestsController do end describe 'GET test_reports' do + let(:merge_request) do + create(:merge_request, + :with_diffs, + :with_merge_request_pipeline, + target_project: project, + source_project: project + ) + end + subject do - get :test_reports, - params: { - namespace_id: project.namespace.to_param, - project_id: project, - id: merge_request.iid - }, - format: :json + get :test_reports, params: { + namespace_id: project.namespace.to_param, + project_id: project, + id: merge_request.iid + }, + format: :json end before do allow_any_instance_of(MergeRequest) - .to receive(:compare_test_reports).and_return(comparison_status) + .to receive(:compare_test_reports) + .and_return(comparison_status) + + allow_any_instance_of(MergeRequest) + .to receive(:actual_head_pipeline) + .and_return(merge_request.all_pipelines.take) + end + + describe 'permissions on a public project with private CI/CD' do + let(:project) { create :project, :repository, :public, :builds_private } + let(:comparison_status) { { status: :parsed, data: { summary: 1 } } } + + context 'while signed out' do + before do + sign_out(user) + end + + it 'responds with a 404' do + subject + + expect(response).to have_gitlab_http_status(404) + expect(response.body).to be_blank + end + end + + context 'while signed in as an unrelated user' do + before do + sign_in(create(:user)) + end + + it 'responds with a 404' do + subject + + expect(response).to have_gitlab_http_status(404) + expect(response.body).to be_blank + end + end end context 'when comparison is being processed' do @@ -1052,17 +1096,39 @@ describe Projects::MergeRequestsController do let(:status) { pipeline.detailed_status(double('user')) } - before do + it 'returns a detailed head_pipeline status in json' do get_pipeline_status - end - it 'return a detailed head_pipeline status in json' do expect(response).to have_gitlab_http_status(:ok) expect(json_response['text']).to eq status.text expect(json_response['label']).to eq status.label expect(json_response['icon']).to eq status.icon expect(json_response['favicon']).to match_asset_path "/assets/ci_favicons/#{status.favicon}.png" end + + context 'with project member visibility on a public project' do + let(:user) { create(:user) } + let(:project) { create(:project, :repository, :public, :builds_private) } + + it 'returns pipeline data to project members' do + project.add_developer(user) + + get_pipeline_status + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['text']).to eq status.text + expect(json_response['label']).to eq status.label + expect(json_response['icon']).to eq status.icon + expect(json_response['favicon']).to match_asset_path "/assets/ci_favicons/#{status.favicon}.png" + end + + it 'returns blank OK response to non-project-members' do + get_pipeline_status + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to be_empty + end + end end context 'when head_pipeline does not exist' do @@ -1070,7 +1136,7 @@ describe Projects::MergeRequestsController do get_pipeline_status end - it 'return empty' do + it 'returns blank OK response' do expect(response).to have_gitlab_http_status(:ok) expect(json_response).to be_empty end diff --git a/spec/controllers/projects/milestones_controller_spec.rb b/spec/controllers/projects/milestones_controller_spec.rb index 9b2025b836c..cbf9d437909 100644 --- a/spec/controllers/projects/milestones_controller_spec.rb +++ b/spec/controllers/projects/milestones_controller_spec.rb @@ -139,7 +139,7 @@ describe Projects::MilestonesController do expect(issue.milestone_id).to eq(milestone.id) delete :destroy, params: { namespace_id: project.namespace.id, project_id: project.id, id: milestone.iid }, format: :js - expect(response).to be_success + expect(response).to be_successful expect(Event.recent.first.action).to eq(Event::DESTROYED) diff --git a/spec/controllers/projects/notes_controller_spec.rb b/spec/controllers/projects/notes_controller_spec.rb index 99808dce016..4db77921f24 100644 --- a/spec/controllers/projects/notes_controller_spec.rb +++ b/spec/controllers/projects/notes_controller_spec.rb @@ -712,23 +712,32 @@ describe Projects::NotesController do project.add_developer(user) end + subject { post(:toggle_award_emoji, params: request_params.merge(name: emoji_name)) } + let(:emoji_name) { 'thumbsup' } + it "toggles the award emoji" do expect do - post(:toggle_award_emoji, params: request_params.merge(name: "thumbsup")) + subject end.to change { note.award_emoji.count }.by(1) expect(response).to have_gitlab_http_status(200) end it "removes the already awarded emoji" do - post(:toggle_award_emoji, params: request_params.merge(name: "thumbsup")) + create(:award_emoji, awardable: note, name: emoji_name, user: user) - expect do - post(:toggle_award_emoji, params: request_params.merge(name: "thumbsup")) - end.to change { AwardEmoji.count }.by(-1) + expect { subject }.to change { AwardEmoji.count }.by(-1) expect(response).to have_gitlab_http_status(200) end + + it 'marks Todos on the Noteable as done' do + todo = create(:todo, target: note.noteable, project: project, user: user) + + subject + + expect(todo.reload).to be_done + end end describe "resolving and unresolving" do diff --git a/spec/controllers/projects/pipelines_controller_spec.rb b/spec/controllers/projects/pipelines_controller_spec.rb index 9a50ea79f5e..212d8b15252 100644 --- a/spec/controllers/projects/pipelines_controller_spec.rb +++ b/spec/controllers/projects/pipelines_controller_spec.rb @@ -177,18 +177,22 @@ describe Projects::PipelinesController do end it 'does not perform N + 1 queries' do + # Set up all required variables + get_pipeline_json + control_count = ActiveRecord::QueryRecorder.new { get_pipeline_json }.count - create_build('test', 1, 'rspec 1') - create_build('test', 1, 'spinach 0') - create_build('test', 1, 'spinach 1') - create_build('test', 1, 'audit') - create_build('post deploy', 3, 'pages 1') - create_build('post deploy', 3, 'pages 2') + first_build = pipeline.builds.first + first_build.tag_list << [:hello, :world] + create(:deployment, deployable: first_build) + + second_build = pipeline.builds.second + second_build.tag_list << [:docker, :ruby] + create(:deployment, deployable: second_build) new_count = ActiveRecord::QueryRecorder.new { get_pipeline_json }.count - expect(new_count).to be_within(12).of(control_count) + expect(new_count).to be_within(1).of(control_count) end end @@ -393,4 +397,69 @@ describe Projects::PipelinesController do end end end + + describe 'GET latest' do + let(:branch_main) { project.repository.branches[0] } + let(:branch_secondary) { project.repository.branches[1] } + + let!(:pipeline_master) do + create(:ci_pipeline, + ref: branch_main.name, + sha: branch_main.target, + project: project) + end + + let!(:pipeline_secondary) do + create(:ci_pipeline, + ref: branch_secondary.name, + sha: branch_secondary.target, + project: project) + end + + before do + project.change_head(branch_main.name) + project.reload_default_branch + end + + context 'no ref provided' do + it 'shows latest pipeline for the default project branch' do + get :show, params: { namespace_id: project.namespace, project_id: project, latest: true, ref: nil } + + expect(response).to have_gitlab_http_status(200) + expect(assigns(:pipeline)).to have_attributes(id: pipeline_master.id) + end + end + + context 'ref provided' do + before do + create(:ci_pipeline, ref: 'master', project: project) + end + + it 'shows the latest pipeline for the provided ref' do + get :show, params: { namespace_id: project.namespace, project_id: project, latest: true, ref: branch_secondary.name } + + expect(response).to have_gitlab_http_status(200) + expect(assigns(:pipeline)).to have_attributes(id: pipeline_secondary.id) + end + + context 'newer pipeline exists for older sha' do + before do + create(:ci_pipeline, ref: branch_secondary.name, sha: project.commit(branch_secondary.name).parent, project: project) + end + + it 'shows the provided ref with the last sha/pipeline combo' do + get :show, params: { namespace_id: project.namespace, project_id: project, latest: true, ref: branch_secondary.name } + + expect(response).to have_gitlab_http_status(200) + expect(assigns(:pipeline)).to have_attributes(id: pipeline_secondary.id) + end + end + end + + it 'renders a 404 if no pipeline is found for the ref' do + get :show, params: { namespace_id: project.namespace, project_id: project, ref: 'no-branch' } + + expect(response).to have_gitlab_http_status(404) + end + end end diff --git a/spec/controllers/projects/project_members_controller_spec.rb b/spec/controllers/projects/project_members_controller_spec.rb index 4141e41c7a7..5130e26c928 100644 --- a/spec/controllers/projects/project_members_controller_spec.rb +++ b/spec/controllers/projects/project_members_controller_spec.rb @@ -158,7 +158,7 @@ describe Projects::ProjectMembersController do id: member }, xhr: true - expect(response).to be_success + expect(response).to be_successful expect(project.members).not_to include member end end diff --git a/spec/controllers/projects/raw_controller_spec.rb b/spec/controllers/projects/raw_controller_spec.rb index 8ee3168273f..8b43d1264b2 100644 --- a/spec/controllers/projects/raw_controller_spec.rb +++ b/spec/controllers/projects/raw_controller_spec.rb @@ -60,16 +60,16 @@ describe Projects::RawController do execute_raw_requests(requests: 6, project: project, file_path: file_path) expect(flash[:alert]).to eq('You cannot access the raw file. Please wait a minute.') - expect(response).to redirect_to(project_blob_path(project, file_path)) + expect(response).to have_gitlab_http_status(429) end it 'logs the event on auth.log' do attributes = { message: 'Action_Rate_Limiter_Request', env: :raw_blob_request_limit, - ip: '0.0.0.0', + remote_ip: '0.0.0.0', request_method: 'GET', - fullpath: "/#{project.full_path}/raw/#{file_path}" + path: "/#{project.full_path}/raw/#{file_path}" } expect(Gitlab::AuthLogger).to receive(:error).with(attributes).once @@ -92,7 +92,7 @@ describe Projects::RawController do execute_raw_requests(requests: 3, project: project, file_path: modified_path) expect(flash[:alert]).to eq('You cannot access the raw file. Please wait a minute.') - expect(response).to redirect_to(project_blob_path(project, modified_path)) + expect(response).to have_gitlab_http_status(429) end end @@ -120,7 +120,7 @@ describe Projects::RawController do execute_raw_requests(requests: 6, project: project, file_path: file_path) expect(flash[:alert]).to eq('You cannot access the raw file. Please wait a minute.') - expect(response).to redirect_to(project_blob_path(project, file_path)) + expect(response).to have_gitlab_http_status(429) # Accessing upcase version of readme file_path = "#{commit_sha}/README.md" diff --git a/spec/controllers/projects/refs_controller_spec.rb b/spec/controllers/projects/refs_controller_spec.rb index 6db98f2428b..646c7a7db7c 100644 --- a/spec/controllers/projects/refs_controller_spec.rb +++ b/spec/controllers/projects/refs_controller_spec.rb @@ -49,7 +49,7 @@ describe Projects::RefsController do expect(::Gitlab::GitalyClient).to receive(:allow_ref_name_caching).and_call_original xhr_get(:js) - expect(response).to be_success + expect(response).to be_successful end it 'renders JSON' do @@ -57,7 +57,7 @@ describe Projects::RefsController do xhr_get(:json) - expect(response).to be_success + expect(response).to be_successful expect(json_response).to be_kind_of(Array) end end diff --git a/spec/controllers/projects/registry/tags_controller_spec.rb b/spec/controllers/projects/registry/tags_controller_spec.rb index ff35139ae2e..c6e063d8229 100644 --- a/spec/controllers/projects/registry/tags_controller_spec.rb +++ b/spec/controllers/projects/registry/tags_controller_spec.rb @@ -113,4 +113,37 @@ describe Projects::Registry::TagsController do format: :json end end + + describe 'POST bulk_destroy' do + context 'when user has access to registry' do + before do + project.add_developer(user) + end + + context 'when there is matching tag present' do + before do + stub_container_registry_tags(repository: repository.path, tags: %w[rc1 test.]) + end + + it 'makes it possible to delete tags in bulk' do + allow_any_instance_of(ContainerRegistry::Tag).to receive(:delete) { |*args| ContainerRegistry::Tag.delete(*args) } + expect(ContainerRegistry::Tag).to receive(:delete).exactly(2).times + + bulk_destroy_tags(['rc1', 'test.']) + end + end + end + + private + + def bulk_destroy_tags(names) + post :bulk_destroy, params: { + namespace_id: project.namespace, + project_id: project, + repository_id: repository, + ids: names + }, + format: :json + end + end end diff --git a/spec/controllers/projects/serverless/functions_controller_spec.rb b/spec/controllers/projects/serverless/functions_controller_spec.rb index 18c594acae0..9f1ef3a4be8 100644 --- a/spec/controllers/projects/serverless/functions_controller_spec.rb +++ b/spec/controllers/projects/serverless/functions_controller_spec.rb @@ -10,12 +10,16 @@ describe Projects::Serverless::FunctionsController do let(:cluster) { create(:cluster, :project, :provided_by_gcp) } let(:service) { cluster.platform_kubernetes } let(:project) { cluster.project } + let(:environment) { create(:environment, project: project) } + let!(:deployment) { create(:deployment, :success, environment: environment, cluster: cluster) } + let(:knative_services_finder) { environment.knative_services_finder } let(:namespace) do create(:cluster_kubernetes_namespace, cluster: cluster, cluster_project: cluster.cluster_project, - project: cluster.cluster_project.project) + project: cluster.cluster_project.project, + environment: environment) end before do @@ -47,12 +51,11 @@ describe Projects::Serverless::FunctionsController do end context 'when cache is ready' do - let(:knative_services_finder) { project.clusters.first.knative_services_finder(project) } let(:knative_state) { true } before do - allow_any_instance_of(Clusters::Cluster) - .to receive(:knative_services_finder) + allow(Clusters::KnativeServicesFinder) + .to receive(:new) .and_return(knative_services_finder) synchronous_reactive_cache(knative_services_finder) stub_kubeclient_service_pods( @@ -107,12 +110,12 @@ describe Projects::Serverless::FunctionsController do context 'valid data', :use_clean_rails_memory_store_caching do before do stub_kubeclient_service_pods - stub_reactive_cache(cluster.knative_services_finder(project), + stub_reactive_cache(knative_services_finder, { services: kube_knative_services_body(namespace: namespace.namespace, name: cluster.project.name)["items"], pods: kube_knative_pods_body(cluster.project.name, namespace.namespace)["items"] }, - *cluster.knative_services_finder(project).cache_args) + *knative_services_finder.cache_args) end it 'has a valid function name' do @@ -140,12 +143,12 @@ describe Projects::Serverless::FunctionsController do describe 'GET #index with data', :use_clean_rails_memory_store_caching do before do stub_kubeclient_service_pods - stub_reactive_cache(cluster.knative_services_finder(project), + stub_reactive_cache(knative_services_finder, { services: kube_knative_services_body(namespace: namespace.namespace, name: cluster.project.name)["items"], pods: kube_knative_pods_body(cluster.project.name, namespace.namespace)["items"] }, - *cluster.knative_services_finder(project).cache_args) + *knative_services_finder.cache_args) end it 'has data' do diff --git a/spec/controllers/projects/services_controller_spec.rb b/spec/controllers/projects/services_controller_spec.rb index 68eabce8513..180d997a8e8 100644 --- a/spec/controllers/projects/services_controller_spec.rb +++ b/spec/controllers/projects/services_controller_spec.rb @@ -11,6 +11,7 @@ describe Projects::ServicesController do before do sign_in(user) project.add_maintainer(user) + allow(Gitlab::UrlBlocker).to receive(:validate!).and_return([URI.parse('http://example.com'), nil]) end describe '#test' do @@ -56,6 +57,8 @@ describe Projects::ServicesController do stub_request(:get, 'http://example.com/rest/api/2/serverInfo') .to_return(status: 200, body: '{}') + expect(Gitlab::HTTP).to receive(:get).with("/rest/api/2/serverInfo", any_args).and_call_original + put :test, params: { namespace_id: project.namespace, project_id: project, id: service.to_param, service: service_params } expect(response.status).to eq(200) @@ -66,6 +69,8 @@ describe Projects::ServicesController do stub_request(:get, 'http://example.com/rest/api/2/serverInfo') .to_return(status: 200, body: '{}') + expect(Gitlab::HTTP).to receive(:get).with("/rest/api/2/serverInfo", any_args).and_call_original + put :test, params: { namespace_id: project.namespace, project_id: project, id: service.to_param, service: service_params } expect(response.status).to eq(200) @@ -159,7 +164,7 @@ describe Projects::ServicesController do context 'with approved services' do it 'renders edit page' do - expect(response).to be_success + expect(response).to be_successful end end end diff --git a/spec/controllers/projects/starrers_controller_spec.rb b/spec/controllers/projects/starrers_controller_spec.rb new file mode 100644 index 00000000000..5774ff7c576 --- /dev/null +++ b/spec/controllers/projects/starrers_controller_spec.rb @@ -0,0 +1,196 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Projects::StarrersController do + let(:user_1) { create(:user, name: 'John') } + let(:user_2) { create(:user, name: 'Michael') } + let(:private_user) { create(:user, name: 'Michael Douglas', private_profile: true) } + let(:admin) { create(:user, admin: true) } + let(:project) { create(:project, :public) } + + before do + user_1.toggle_star(project) + user_2.toggle_star(project) + private_user.toggle_star(project) + end + + describe 'GET index' do + def get_starrers(search: nil) + get :index, params: { namespace_id: project.namespace, project_id: project, search: search } + end + + def user_ids + assigns[:starrers].map { |s| s['user_id'] } + end + + shared_examples 'starrers counts' do + it 'starrers counts are correct' do + expect(assigns[:total_count]).to eq(3) + expect(assigns[:public_count]).to eq(2) + expect(assigns[:private_count]).to eq(1) + end + end + + context 'N+1 queries' do + render_views + + it 'avoids N+1s loading users', :request_store do + get_starrers + + control_count = ActiveRecord::QueryRecorder.new { get_starrers }.count + + create_list(:user, 5).each { |user| user.toggle_star(project) } + + expect { get_starrers }.not_to exceed_query_limit(control_count) + end + end + + context 'when project is public' do + before do + project.update_attribute(:visibility_level, Project::PUBLIC) + end + + context 'when no user is logged in' do + context 'with no searching' do + before do + get_starrers + end + + it 'only users with public profiles are visible' do + expect(user_ids).to contain_exactly(user_1.id, user_2.id) + end + + include_examples 'starrers counts' + end + + context 'when searching by user' do + before do + get_starrers(search: 'Michael') + end + + it 'only users with public profiles are visible' do + expect(user_ids).to contain_exactly(user_2.id) + end + + include_examples 'starrers counts' + end + end + + context 'when public user is logged in' do + before do + sign_in(user_1) + end + + context 'with no searching' do + before do + get_starrers + end + + it 'their star is also visible' do + expect(user_ids).to contain_exactly(user_1.id, user_2.id) + end + + include_examples 'starrers counts' + end + + context 'when searching by user' do + before do + get_starrers(search: 'Michael') + end + + it 'only users with public profiles are visible' do + expect(user_ids).to contain_exactly(user_2.id) + end + + include_examples 'starrers counts' + end + end + + context 'when private user is logged in' do + before do + sign_in(private_user) + end + + context 'with no searching' do + before do + get_starrers + end + + it 'their star is also visible' do + expect(user_ids).to contain_exactly(user_1.id, user_2.id, private_user.id) + end + + include_examples 'starrers counts' + end + + context 'when searching by user' do + before do + get_starrers(search: 'Michael') + end + + it 'only users with public profiles are visible' do + expect(user_ids).to contain_exactly(user_2.id, private_user.id) + end + + include_examples 'starrers counts' + end + end + + context 'when admin is logged in' do + before do + sign_in(admin) + end + + context 'with no searching' do + before do + get_starrers + end + + it 'all users are visible' do + expect(user_ids).to include(user_1.id, user_2.id, private_user.id) + end + + include_examples 'starrers counts' + end + + context 'when searching by user' do + before do + get_starrers(search: 'Michael') + end + + it 'public and private starrers are visible' do + expect(user_ids).to contain_exactly(user_2.id, private_user.id) + end + + include_examples 'starrers counts' + end + end + end + + context 'when project is private' do + before do + project.update(visibility_level: Project::PRIVATE) + end + + it 'starrers are not visible for non logged in users' do + get_starrers + + expect(assigns[:starrers]).to be_blank + end + + context 'when user is logged in' do + before do + sign_in(project.creator) + get_starrers + end + + it 'only users with public profiles are visible' do + expect(user_ids).to contain_exactly(user_1.id, user_2.id) + end + + include_examples 'starrers counts' + end + end + end +end diff --git a/spec/controllers/projects/variables_controller_spec.rb b/spec/controllers/projects/variables_controller_spec.rb index a2a09e2580f..21e106660d0 100644 --- a/spec/controllers/projects/variables_controller_spec.rb +++ b/spec/controllers/projects/variables_controller_spec.rb @@ -36,5 +36,70 @@ describe Projects::VariablesController do end include_examples 'PATCH #update updates variables' + + context 'with environment scope' do + let!(:variable) { create(:ci_variable, project: project, environment_scope: 'custom_scope') } + + let(:variable_attributes) do + { id: variable.id, + key: variable.key, + secret_value: variable.value, + protected: variable.protected?.to_s, + environment_scope: variable.environment_scope } + end + let(:new_variable_attributes) do + { key: 'new_key', + secret_value: 'dummy_value', + protected: 'false', + environment_scope: 'new_scope' } + end + + context 'with same key and different environment scope' do + let(:variables_attributes) do + [ + variable_attributes, + new_variable_attributes.merge(key: variable.key) + ] + end + + it 'does not update the existing variable' do + expect { subject }.not_to change { variable.reload.value } + end + + it 'creates the new variable' do + expect { subject }.to change { owner.variables.count }.by(1) + end + + it 'returns a successful response including all variables' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('variables') + end + end + + context 'with same key and same environment scope' do + let(:variables_attributes) do + [ + variable_attributes, + new_variable_attributes.merge(key: variable.key, environment_scope: variable.environment_scope) + ] + end + + it 'does not update the existing variable' do + expect { subject }.not_to change { variable.reload.value } + end + + it 'does not create the new variable' do + expect { subject }.not_to change { owner.variables.count } + end + + it 'returns a bad request response' do + subject + + expect(response).to have_gitlab_http_status(:bad_request) + end + end + end end end diff --git a/spec/controllers/projects/wikis_controller_spec.rb b/spec/controllers/projects/wikis_controller_spec.rb index fbca1d5740f..6fea6bca4f2 100644 --- a/spec/controllers/projects/wikis_controller_spec.rb +++ b/spec/controllers/projects/wikis_controller_spec.rb @@ -3,11 +3,11 @@ require 'spec_helper' describe Projects::WikisController do - let(:project) { create(:project, :public, :repository) } - let(:user) { project.owner } + set(:project) { create(:project, :public, :repository) } + set(:user) { project.owner } let(:project_wiki) { ProjectWiki.new(project, user) } let(:wiki) { project_wiki.wiki } - let(:wiki_title) { 'page-title-test' } + let(:wiki_title) { 'page title test' } before do create_page(wiki_title, 'hello world') @@ -19,6 +19,21 @@ describe Projects::WikisController do destroy_page(wiki_title) end + describe 'GET #new' do + subject { get :new, params: { namespace_id: project.namespace, project_id: project } } + + it 'redirects to #show and appends a `random_title` param' do + subject + + expect(response).to have_http_status(302) + expect(Rails.application.routes.recognize_path(response.redirect_url)).to include( + controller: 'projects/wikis', + action: 'show' + ) + expect(response.redirect_url).to match(/\?random_title=true\Z/) + end + end + describe 'GET #pages' do subject { get :pages, params: { namespace_id: project.namespace, project_id: project, id: wiki_title } } @@ -75,40 +90,62 @@ describe Projects::WikisController do describe 'GET #show' do render_views - subject { get :show, params: { namespace_id: project.namespace, project_id: project, id: wiki_title } } + let(:random_title) { nil } - it 'limits the retrieved pages for the sidebar' do - expect(controller).to receive(:load_wiki).and_return(project_wiki) + subject { get :show, params: { namespace_id: project.namespace, project_id: project, id: id, random_title: random_title } } - # empty? call - expect(project_wiki).to receive(:list_pages).with(limit: 1).and_call_original - # Sidebar entries - expect(project_wiki).to receive(:list_pages).with(limit: 15).and_call_original + context 'when page exists' do + let(:id) { wiki_title } - subject + it 'limits the retrieved pages for the sidebar' do + expect(controller).to receive(:load_wiki).and_return(project_wiki) + expect(project_wiki).to receive(:list_pages).with(limit: 15).and_call_original + + subject + + expect(response).to have_http_status(:ok) + expect(assigns(:page).title).to eq(wiki_title) + end + + context 'when page content encoding is invalid' do + it 'sets flash error' do + allow(controller).to receive(:valid_encoding?).and_return(false) - expect(response).to have_http_status(:ok) - expect(response.body).to include(wiki_title) + subject + + expect(response).to have_http_status(:ok) + expect(flash[:notice]).to eq('The content of this page is not encoded in UTF-8. Edits can only be made via the Git repository.') + end + end end - context 'when page content encoding is invalid' do - it 'sets flash error' do - allow(controller).to receive(:valid_encoding?).and_return(false) + context 'when the page does not exist' do + let(:id) { 'does not exist' } + before do subject + end - expect(response).to have_http_status(:ok) - expect(flash[:notice]).to eq 'The content of this page is not encoded in UTF-8. Edits can only be made via the Git repository.' + it 'builds a new wiki page with the id as the title' do + expect(assigns(:page).title).to eq(id) + end + + context 'when a random_title param is present' do + let(:random_title) { true } + + it 'builds a new wiki page with no title' do + expect(assigns(:page).title).to be_empty + end end end context 'when page is a file' do include WikiHelpers - let(:path) { upload_file_to_wiki(project, user, file_name) } + let(:id) { upload_file_to_wiki(project, user, file_name) } before do - get :show, params: { namespace_id: project.namespace, project_id: project, id: path } + subject end context 'when file is an image' do diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index 083a1c1383a..c732caa6160 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -318,6 +318,102 @@ describe ProjectsController do end end + describe 'POST #archive' do + let(:group) { create(:group) } + let(:project) { create(:project, group: group) } + + before do + sign_in(user) + end + + context 'for a user with the ability to archive a project' do + before do + group.add_owner(user) + + post :archive, params: { + namespace_id: project.namespace.path, + id: project.path + } + end + + it 'archives the project' do + expect(project.reload.archived?).to be_truthy + end + + it 'redirects to projects path' do + expect(response).to have_gitlab_http_status(302) + expect(response).to redirect_to(project_path(project)) + end + end + + context 'for a user that does not have the ability to archive a project' do + before do + project.add_maintainer(user) + + post :archive, params: { + namespace_id: project.namespace.path, + id: project.path + } + end + + it 'does not archive the project' do + expect(project.reload.archived?).to be_falsey + end + + it 'returns 404' do + expect(response).to have_gitlab_http_status(404) + end + end + end + + describe 'POST #unarchive' do + let(:group) { create(:group) } + let(:project) { create(:project, :archived, group: group) } + + before do + sign_in(user) + end + + context 'for a user with the ability to unarchive a project' do + before do + group.add_owner(user) + + post :unarchive, params: { + namespace_id: project.namespace.path, + id: project.path + } + end + + it 'unarchives the project' do + expect(project.reload.archived?).to be_falsey + end + + it 'redirects to projects path' do + expect(response).to have_gitlab_http_status(302) + expect(response).to redirect_to(project_path(project)) + end + end + + context 'for a user that does not have the ability to unarchive a project' do + before do + project.add_maintainer(user) + + post :unarchive, params: { + namespace_id: project.namespace.path, + id: project.path + } + end + + it 'does not unarchive the project' do + expect(project.reload.archived?).to be_truthy + end + + it 'returns 404' do + expect(response).to have_gitlab_http_status(404) + end + end + end + describe '#housekeeping' do let(:group) { create(:group) } let(:project) { create(:project, group: group) } diff --git a/spec/controllers/registrations_controller_spec.rb b/spec/controllers/registrations_controller_spec.rb index faf3c990cb2..35487682462 100644 --- a/spec/controllers/registrations_controller_spec.rb +++ b/spec/controllers/registrations_controller_spec.rb @@ -5,6 +5,10 @@ require 'spec_helper' describe RegistrationsController do include TermsHelper + before do + stub_feature_flags(invisible_captcha: false) + end + describe '#create' do let(:base_user_params) { { name: 'new_user', username: 'new_username', email: 'new@user.com', password: 'Any_password' } } let(:user_params) { { user: base_user_params } } @@ -26,13 +30,36 @@ describe RegistrationsController do end context 'when send_user_confirmation_email is true' do - it 'does not authenticate user and sends confirmation email' do + before do stub_application_setting(send_user_confirmation_email: true) + end + + context 'when soft email confirmation is not enabled' do + before do + stub_feature_flags(soft_email_confirmation: false) + allow(User).to receive(:allow_unconfirmed_access_for).and_return 0 + end - post(:create, params: user_params) + it 'does not authenticate the user and sends a confirmation email' do + post(:create, params: user_params) - expect(ActionMailer::Base.deliveries.last.to.first).to eq(user_params[:user][:email]) - expect(subject.current_user).to be_nil + expect(ActionMailer::Base.deliveries.last.to.first).to eq(user_params[:user][:email]) + expect(subject.current_user).to be_nil + end + end + + context 'when soft email confirmation is enabled' do + before do + stub_feature_flags(soft_email_confirmation: true) + allow(User).to receive(:allow_unconfirmed_access_for).and_return 2.days + end + + it 'authenticates the user and sends a confirmation email' do + post(:create, params: user_params) + + expect(ActionMailer::Base.deliveries.last.to.first).to eq(user_params[:user][:email]) + expect(response).to redirect_to(dashboard_projects_path) + end end end @@ -88,6 +115,88 @@ describe RegistrationsController do end end + context 'when invisible captcha is enabled' do + before do + stub_feature_flags(invisible_captcha: true) + InvisibleCaptcha.timestamp_threshold = treshold + end + + let(:treshold) { 4 } + let(:session_params) { { invisible_captcha_timestamp: form_rendered_time.iso8601 } } + let(:form_rendered_time) { Time.current } + let(:submit_time) { form_rendered_time + treshold } + let(:auth_log_attributes) do + { + message: auth_log_message, + env: :invisible_captcha_signup_bot_detected, + remote_ip: '0.0.0.0', + request_method: 'POST', + path: '/users' + } + end + + describe 'the honeypot has not been filled and the signup form has not been submitted too quickly' do + it 'creates an account' do + travel_to(submit_time) do + expect { post(:create, params: user_params, session: session_params) }.to change(User, :count).by(1) + end + end + end + + describe 'honeypot spam detection' do + let(:user_params) { super().merge(firstname: 'Roy', lastname: 'Batty') } + let(:auth_log_message) { 'Invisible_Captcha_Honeypot_Request' } + + it 'logs the request, refuses to create an account and renders an empty body' do + travel_to(submit_time) do + expect(Gitlab::Metrics).to receive(:counter) + .with(:bot_blocked_by_invisible_captcha_honeypot, 'Counter of blocked sign up attempts with filled honeypot') + .and_call_original + expect(Gitlab::AuthLogger).to receive(:error).with(auth_log_attributes).once + expect { post(:create, params: user_params, session: session_params) }.not_to change(User, :count) + expect(response).to have_gitlab_http_status(200) + expect(response.body).to be_empty + end + end + end + + describe 'timestamp spam detection' do + let(:auth_log_message) { 'Invisible_Captcha_Timestamp_Request' } + + context 'the sign up form has been submitted without the invisible_captcha_timestamp parameter' do + let(:session_params) { nil } + + it 'logs the request, refuses to create an account and displays a flash alert' do + travel_to(submit_time) do + expect(Gitlab::Metrics).to receive(:counter) + .with(:bot_blocked_by_invisible_captcha_timestamp, 'Counter of blocked sign up attempts with invalid timestamp') + .and_call_original + expect(Gitlab::AuthLogger).to receive(:error).with(auth_log_attributes).once + expect { post(:create, params: user_params, session: session_params) }.not_to change(User, :count) + expect(response).to redirect_to(new_user_session_path) + expect(flash[:alert]).to include 'That was a bit too quick! Please resubmit.' + end + end + end + + context 'the sign up form has been submitted too quickly' do + let(:submit_time) { form_rendered_time } + + it 'logs the request, refuses to create an account and displays a flash alert' do + travel_to(submit_time) do + expect(Gitlab::Metrics).to receive(:counter) + .with(:bot_blocked_by_invisible_captcha_timestamp, 'Counter of blocked sign up attempts with invalid timestamp') + .and_call_original + expect(Gitlab::AuthLogger).to receive(:error).with(auth_log_attributes).once + expect { post(:create, params: user_params, session: session_params) }.not_to change(User, :count) + expect(response).to redirect_to(new_user_session_path) + expect(flash[:alert]).to include 'That was a bit too quick! Please resubmit.' + end + end + end + end + end + context 'when terms are enforced' do before do enforce_terms diff --git a/spec/controllers/search_controller_spec.rb b/spec/controllers/search_controller_spec.rb index 5a5c0a1f6ac..3e0d53a6573 100644 --- a/spec/controllers/search_controller_spec.rb +++ b/spec/controllers/search_controller_spec.rb @@ -11,151 +11,173 @@ describe SearchController do sign_in(user) end - context 'uses the right partials depending on scope' do - using RSpec::Parameterized::TableSyntax - render_views - - set(:project) { create(:project, :public, :repository, :wiki_repo) } - + shared_examples_for 'when the user cannot read cross project' do |action, params| before do - expect(::Gitlab::GitalyClient).to receive(:allow_ref_name_caching).and_call_original + allow(Ability).to receive(:allowed?).and_call_original + allow(Ability).to receive(:allowed?) + .with(user, :read_cross_project, :global) { false } end - subject { get(:show, params: { project_id: project.id, scope: scope, search: 'merge' }) } + it 'blocks access without a project_id' do + get action, params: params - where(:partial, :scope) do - '_blob' | :blobs - '_wiki_blob' | :wiki_blobs - '_commit' | :commits + expect(response).to have_gitlab_http_status(403) end - with_them do - it do - project_wiki = create(:project_wiki, project: project, user: user) - create(:wiki_page, wiki: project_wiki, attrs: { title: 'merge', content: 'merge' }) + it 'allows access with a project_id' do + get action, params: params.merge(project_id: create(:project, :public).id) - expect(subject).to render_template("search/results/#{partial}") - end + expect(response).to have_gitlab_http_status(200) end end - context 'global search' do - render_views - - it 'omits pipeline status from load' do - project = create(:project, :public) - expect(Gitlab::Cache::Ci::ProjectPipelineStatus).not_to receive(:load_in_batch_for_projects) - - get :show, params: { scope: 'projects', search: project.name } + shared_examples_for 'with external authorization service enabled' do |action, params| + let(:project) { create(:project, namespace: user.namespace) } + let(:note) { create(:note_on_issue, project: project) } - expect(assigns[:search_objects].first).to eq project + before do + enable_external_authorization_service_check end - end - - it 'finds issue comments' do - project = create(:project, :public) - note = create(:note_on_issue, project: project) - get :show, params: { project_id: project.id, scope: 'notes', search: note.note } + it 'renders a 403 when no project is given' do + get action, params: params - expect(assigns[:search_objects].first).to eq note - end - - context 'when the user cannot read cross project' do - before do - allow(Ability).to receive(:allowed?).and_call_original - allow(Ability).to receive(:allowed?) - .with(user, :read_cross_project, :global) { false } + expect(response).to have_gitlab_http_status(403) end - it 'still allows accessing the search page' do - get :show + it 'renders a 200 when a project was set' do + get action, params: params.merge(project_id: project.id) expect(response).to have_gitlab_http_status(200) end + end - it 'still blocks searches without a project_id' do - get :show, params: { search: 'hello' } + describe 'GET #show' do + it_behaves_like 'when the user cannot read cross project', :show, { search: 'hello' } do + it 'still allows accessing the search page' do + get :show - expect(response).to have_gitlab_http_status(403) + expect(response).to have_gitlab_http_status(200) + end end - it 'allows searches with a project_id' do - get :show, params: { search: 'hello', project_id: create(:project, :public).id } + it_behaves_like 'with external authorization service enabled', :show, { search: 'hello' } - expect(response).to have_gitlab_http_status(200) - end - end + context 'uses the right partials depending on scope' do + using RSpec::Parameterized::TableSyntax + render_views + + set(:project) { create(:project, :public, :repository, :wiki_repo) } - context 'on restricted projects' do - context 'when signed out' do before do - sign_out(user) + expect(::Gitlab::GitalyClient).to receive(:allow_ref_name_caching).and_call_original end - it "doesn't expose comments on issues" do - project = create(:project, :public, :issues_private) - note = create(:note_on_issue, project: project) + subject { get(:show, params: { project_id: project.id, scope: scope, search: 'merge' }) } - get :show, params: { project_id: project.id, scope: 'notes', search: note.note } + where(:partial, :scope) do + '_blob' | :blobs + '_wiki_blob' | :wiki_blobs + '_commit' | :commits + end - expect(assigns[:search_objects].count).to eq(0) + with_them do + it do + project_wiki = create(:project_wiki, project: project, user: user) + create(:wiki_page, wiki: project_wiki, attrs: { title: 'merge', content: 'merge' }) + + expect(subject).to render_template("search/results/#{partial}") + end end end - it "doesn't expose comments on merge_requests" do - project = create(:project, :public, :merge_requests_private) - note = create(:note_on_merge_request, project: project) + context 'global search' do + render_views - get :show, params: { project_id: project.id, scope: 'notes', search: note.note } + it 'omits pipeline status from load' do + project = create(:project, :public) + expect(Gitlab::Cache::Ci::ProjectPipelineStatus).not_to receive(:load_in_batch_for_projects) + + get :show, params: { scope: 'projects', search: project.name } - expect(assigns[:search_objects].count).to eq(0) + expect(assigns[:search_objects].first).to eq project + end end - it "doesn't expose comments on snippets" do - project = create(:project, :public, :snippets_private) - note = create(:note_on_project_snippet, project: project) + it 'finds issue comments' do + project = create(:project, :public) + note = create(:note_on_issue, project: project) get :show, params: { project_id: project.id, scope: 'notes', search: note.note } - expect(assigns[:search_objects].count).to eq(0) + expect(assigns[:search_objects].first).to eq note end - end - context 'with external authorization service enabled' do - let(:project) { create(:project, namespace: user.namespace) } - let(:note) { create(:note_on_issue, project: project) } + context 'on restricted projects' do + context 'when signed out' do + before do + sign_out(user) + end - before do - enable_external_authorization_service_check - end + it "doesn't expose comments on issues" do + project = create(:project, :public, :issues_private) + note = create(:note_on_issue, project: project) - describe 'GET #show' do - it 'renders a 403 when no project is given' do - get :show, params: { scope: 'notes', search: note.note } + get :show, params: { project_id: project.id, scope: 'notes', search: note.note } - expect(response).to have_gitlab_http_status(403) + expect(assigns[:search_objects].count).to eq(0) + end end - it 'renders a 200 when a project was set' do + it "doesn't expose comments on merge_requests" do + project = create(:project, :public, :merge_requests_private) + note = create(:note_on_merge_request, project: project) + get :show, params: { project_id: project.id, scope: 'notes', search: note.note } - expect(response).to have_gitlab_http_status(200) + expect(assigns[:search_objects].count).to eq(0) end - end - describe 'GET #autocomplete' do - it 'renders a 403 when no project is given' do - get :autocomplete, params: { term: 'hello' } + it "doesn't expose comments on snippets" do + project = create(:project, :public, :snippets_private) + note = create(:note_on_project_snippet, project: project) - expect(response).to have_gitlab_http_status(403) + get :show, params: { project_id: project.id, scope: 'notes', search: note.note } + + expect(assigns[:search_objects].count).to eq(0) end + end + end - it 'renders a 200 when a project was set' do - get :autocomplete, params: { project_id: project.id, term: 'hello' } + describe 'GET #count' do + it_behaves_like 'when the user cannot read cross project', :count, { search: 'hello', scope: 'projects' } + it_behaves_like 'with external authorization service enabled', :count, { search: 'hello', scope: 'projects' } - expect(response).to have_gitlab_http_status(200) - end + it 'returns the result count for the given term and scope' do + create(:project, :public, name: 'hello world') + create(:project, :public, name: 'foo bar') + + get :count, params: { search: 'hello', scope: 'projects' } + + expect(response).to have_gitlab_http_status(200) + expect(json_response).to eq({ 'count' => '1' }) + end + + it 'raises an error if search term is missing' do + expect do + get :count, params: { scope: 'projects' } + end.to raise_error(ActionController::ParameterMissing) end + + it 'raises an error if search scope is missing' do + expect do + get :count, params: { search: 'hello' } + end.to raise_error(ActionController::ParameterMissing) + end + end + + describe 'GET #autocomplete' do + it_behaves_like 'when the user cannot read cross project', :autocomplete, { term: 'hello' } + it_behaves_like 'with external authorization service enabled', :autocomplete, { term: 'hello' } end end diff --git a/spec/controllers/sessions_controller_spec.rb b/spec/controllers/sessions_controller_spec.rb index 9c4ddce5409..68b7bf61231 100644 --- a/spec/controllers/sessions_controller_spec.rb +++ b/spec/controllers/sessions_controller_spec.rb @@ -100,16 +100,8 @@ describe SessionsController do end end - context 'when reCAPTCHA is enabled' do - let(:user) { create(:user) } - let(:user_params) { { login: user.username, password: user.password } } - - before do - stub_application_setting(recaptcha_enabled: true) - request.headers[described_class::CAPTCHA_HEADER] = 1 - end - - it 'displays an error when the reCAPTCHA is not solved' do + context 'with reCAPTCHA' do + def unsuccesful_login(user_params, sesion_params: {}) # Without this, `verify_recaptcha` arbitrarily returns true in test env Recaptcha.configuration.skip_verify_env.delete('test') counter = double(:counter) @@ -119,14 +111,10 @@ describe SessionsController do .with(:failed_login_captcha_total, anything) .and_return(counter) - post(:create, params: { user: user_params }) - - expect(response).to render_template(:new) - expect(flash[:alert]).to include 'There was an error with the reCAPTCHA. Please solve the reCAPTCHA again.' - expect(subject.current_user).to be_nil + post(:create, params: { user: user_params }, session: sesion_params) end - it 'successfully logs in a user when reCAPTCHA is solved' do + def succesful_login(user_params, sesion_params: {}) # Avoid test ordering issue and ensure `verify_recaptcha` returns true Recaptcha.configuration.skip_verify_env << 'test' counter = double(:counter) @@ -137,9 +125,80 @@ describe SessionsController do .and_return(counter) expect(Gitlab::Metrics).to receive(:counter).and_call_original - post(:create, params: { user: user_params }) + post(:create, params: { user: user_params }, session: sesion_params) + end - expect(subject.current_user).to eq user + context 'when reCAPTCHA is enabled' do + let(:user) { create(:user) } + let(:user_params) { { login: user.username, password: user.password } } + + before do + stub_application_setting(recaptcha_enabled: true) + request.headers[described_class::CAPTCHA_HEADER] = 1 + end + + it 'displays an error when the reCAPTCHA is not solved' do + # Without this, `verify_recaptcha` arbitrarily returns true in test env + + unsuccesful_login(user_params) + + expect(response).to render_template(:new) + expect(flash[:alert]).to include 'There was an error with the reCAPTCHA. Please solve the reCAPTCHA again.' + expect(subject.current_user).to be_nil + end + + it 'successfully logs in a user when reCAPTCHA is solved' do + succesful_login(user_params) + + expect(subject.current_user).to eq user + end + end + + context 'when reCAPTCHA login protection is enabled' do + let(:user) { create(:user) } + let(:user_params) { { login: user.username, password: user.password } } + + before do + stub_application_setting(login_recaptcha_protection_enabled: true) + end + + context 'when user tried to login 5 times' do + it 'displays an error when the reCAPTCHA is not solved' do + unsuccesful_login(user_params, sesion_params: { failed_login_attempts: 6 }) + + expect(response).to render_template(:new) + expect(flash[:alert]).to include 'There was an error with the reCAPTCHA. Please solve the reCAPTCHA again.' + expect(subject.current_user).to be_nil + end + + it 'successfully logs in a user when reCAPTCHA is solved' do + succesful_login(user_params, sesion_params: { failed_login_attempts: 6 }) + + expect(subject.current_user).to eq user + end + end + + context 'when there are more than 5 anonymous session with the same IP' do + before do + allow(Gitlab::AnonymousSession).to receive_message_chain(:new, :stored_sessions).and_return(6) + end + + it 'displays an error when the reCAPTCHA is not solved' do + unsuccesful_login(user_params) + + expect(response).to render_template(:new) + expect(flash[:alert]).to include 'There was an error with the reCAPTCHA. Please solve the reCAPTCHA again.' + expect(subject.current_user).to be_nil + end + + it 'successfully logs in a user when reCAPTCHA is solved' do + expect(Gitlab::AnonymousSession).to receive_message_chain(:new, :cleanup_session_per_ip_entries) + + succesful_login(user_params) + + expect(subject.current_user).to eq user + end + end end end end @@ -348,4 +407,17 @@ describe SessionsController do expect(controller.stored_location_for(:redirect)).to eq(search_path) end end + + context 'when login fails' do + before do + set_devise_mapping(context: @request) + @request.env["warden.options"] = { action: 'unauthenticated' } + end + + it 'does increment failed login counts for session' do + get(:new, params: { user: { login: 'failed' } }) + + expect(session[:failed_login_attempts]).to eq(1) + end + end end diff --git a/spec/controllers/snippets/notes_controller_spec.rb b/spec/controllers/snippets/notes_controller_spec.rb index 652533ac49f..fd4b95ce226 100644 --- a/spec/controllers/snippets/notes_controller_spec.rb +++ b/spec/controllers/snippets/notes_controller_spec.rb @@ -288,11 +288,13 @@ describe Snippets::NotesController do describe 'POST toggle_award_emoji' do let(:note) { create(:note_on_personal_snippet, noteable: public_snippet) } + let(:emoji_name) { 'thumbsup'} + before do sign_in(user) end - subject { post(:toggle_award_emoji, params: { snippet_id: public_snippet, id: note.id, name: "thumbsup" }) } + subject { post(:toggle_award_emoji, params: { snippet_id: public_snippet, id: note.id, name: emoji_name }) } it "toggles the award emoji" do expect { subject }.to change { note.award_emoji.count }.by(1) @@ -301,7 +303,7 @@ describe Snippets::NotesController do end it "removes the already awarded emoji when it exists" do - note.toggle_award_emoji('thumbsup', user) # create award emoji before + create(:award_emoji, awardable: note, name: emoji_name, user: user) expect { subject }.to change { AwardEmoji.count }.by(-1) diff --git a/spec/controllers/uploads_controller_spec.rb b/spec/controllers/uploads_controller_spec.rb index 0876502a899..5f4a6bf8ee7 100644 --- a/spec/controllers/uploads_controller_spec.rb +++ b/spec/controllers/uploads_controller_spec.rb @@ -21,8 +21,20 @@ shared_examples 'content publicly cached' do end describe UploadsController do + include WorkhorseHelpers + let!(:user) { create(:user, avatar: fixture_file_upload("spec/fixtures/dk.png", "image/png")) } + describe 'POST #authorize' do + it_behaves_like 'handle uploads authorize' do + let(:uploader_class) { PersonalFileUploader } + let(:model) { create(:personal_snippet, :public) } + let(:params) do + { model: 'personal_snippet', id: model.id } + end + end + end + describe 'POST create' do let(:jpg) { fixture_file_upload('spec/fixtures/rails_sample.jpg', 'image/jpg') } let(:txt) { fixture_file_upload('spec/fixtures/doc_sample.txt', 'text/plain') } @@ -636,4 +648,10 @@ describe UploadsController do end end end + + def post_authorize(verified: true) + request.headers.merge!(workhorse_internal_api_request_header) if verified + + post :authorize, params: { model: 'personal_snippet', id: model.id }, format: :json + end end diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index 8b8d4c57000..5566df0c216 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -19,7 +19,7 @@ describe UsersController do it 'renders the show template' do get :show, params: { username: user.username } - expect(response).to be_success + expect(response).to be_successful expect(response).to render_template('show') end end @@ -362,7 +362,7 @@ describe UsersController do it 'responds with success' do get :show, params: { username: user.username } - expect(response).to be_success + expect(response).to be_successful end end @@ -418,7 +418,7 @@ describe UsersController do it 'responds with success' do get :projects, params: { username: user.username } - expect(response).to be_success + expect(response).to be_successful end end |