diff options
Diffstat (limited to 'spec/controllers')
51 files changed, 3817 insertions, 519 deletions
diff --git a/spec/controllers/admin/application_settings_controller_spec.rb b/spec/controllers/admin/application_settings_controller_spec.rb index 5dd8f66343f..2565622f8df 100644 --- a/spec/controllers/admin/application_settings_controller_spec.rb +++ b/spec/controllers/admin/application_settings_controller_spec.rb @@ -3,12 +3,49 @@ require 'spec_helper' describe Admin::ApplicationSettingsController do include StubENV + let(:group) { create(:group) } + let(:project) { create(:project, namespace: group) } let(:admin) { create(:admin) } + let(:user) { create(:user)} before do stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false') end + describe 'GET #usage_data with no access' do + before do + sign_in(user) + end + + it 'returns 404' do + get :usage_data, format: :html + + expect(response.status).to eq(404) + end + end + + describe 'GET #usage_data' do + before do + sign_in(admin) + end + + it 'returns HTML data' do + get :usage_data, format: :html + + expect(response.body).to start_with('<span') + expect(response.status).to eq(200) + end + + it 'returns JSON data' do + get :usage_data, format: :json + + body = JSON.parse(response.body) + expect(body["version"]).to eq(Gitlab::VERSION) + expect(body).to include('counts') + expect(response.status).to eq(200) + end + end + describe 'PUT #update' do before do sign_in(admin) diff --git a/spec/controllers/admin/groups_controller_spec.rb b/spec/controllers/admin/groups_controller_spec.rb index 84db26a958a..c29b2fe8946 100644 --- a/spec/controllers/admin/groups_controller_spec.rb +++ b/spec/controllers/admin/groups_controller_spec.rb @@ -22,4 +22,28 @@ describe Admin::GroupsController do expect(response).to redirect_to(admin_groups_path) end end + + describe 'PUT #members_update' do + let(:group_user) { create(:user) } + + it 'adds user to members' do + put :members_update, id: group, + user_ids: group_user.id, + access_level: Gitlab::Access::GUEST + + expect(response).to set_flash.to 'Users were successfully added.' + expect(response).to redirect_to(admin_group_path(group)) + expect(group.users).to include group_user + end + + it 'adds no user to members' do + put :members_update, id: group, + user_ids: '', + access_level: Gitlab::Access::GUEST + + expect(response).to set_flash.to 'No users specified.' + expect(response).to redirect_to(admin_group_path(group)) + expect(group.users).not_to include group_user + end + end end diff --git a/spec/controllers/admin/hooks_controller_spec.rb b/spec/controllers/admin/hooks_controller_spec.rb new file mode 100644 index 00000000000..1d1070e90f4 --- /dev/null +++ b/spec/controllers/admin/hooks_controller_spec.rb @@ -0,0 +1,28 @@ +require 'spec_helper' + +describe Admin::HooksController do + let(:admin) { create(:admin) } + + before do + sign_in(admin) + end + + describe 'POST #create' do + it 'sets all parameters' do + hook_params = { + enable_ssl_verification: true, + push_events: true, + tag_push_events: true, + repository_update_events: true, + token: "TEST TOKEN", + url: "http://example.com" + } + + post :create, hook: hook_params + + expect(response).to have_http_status(302) + expect(SystemHook.all.size).to eq(1) + expect(SystemHook.first).to have_attributes(hook_params) + end + end +end diff --git a/spec/controllers/admin/services_controller_spec.rb b/spec/controllers/admin/services_controller_spec.rb index e5cdd52307e..c94616d8508 100644 --- a/spec/controllers/admin/services_controller_spec.rb +++ b/spec/controllers/admin/services_controller_spec.rb @@ -23,4 +23,36 @@ describe Admin::ServicesController do end end end + + describe "#update" do + let(:project) { create(:empty_project) } + let!(:service) do + RedmineService.create( + project: project, + active: false, + template: true, + properties: { + project_url: 'http://abc', + issues_url: 'http://abc', + new_issue_url: 'http://abc' + } + ) + end + + it 'calls the propagation worker when service is active' do + expect(PropagateServiceTemplateWorker).to receive(:perform_async).with(service.id) + + put :update, id: service.id, service: { active: true } + + expect(response).to have_http_status(302) + end + + it 'does not call the propagation worker when service is not active' do + expect(PropagateServiceTemplateWorker).not_to receive(:perform_async) + + put :update, id: service.id, service: { properties: {} } + + expect(response).to have_http_status(302) + end + end end diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index 81cbccd5436..d40aae04fc3 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -4,7 +4,7 @@ describe ApplicationController do let(:user) { create(:user) } describe '#check_password_expiration' do - let(:controller) { ApplicationController.new } + let(:controller) { described_class.new } it 'redirects if the user is over their password expiry' do user.password_expires_at = Time.new(2002) @@ -34,7 +34,7 @@ describe ApplicationController do describe "#authenticate_user_from_token!" do describe "authenticating a user from a private token" do - controller(ApplicationController) do + controller(described_class) do def index render text: "authenticated" end @@ -66,7 +66,7 @@ describe ApplicationController do end describe "authenticating a user from a personal access token" do - controller(ApplicationController) do + controller(described_class) do def index render text: 'authenticated' end @@ -100,19 +100,215 @@ describe ApplicationController do end describe '#route_not_found' do - let(:controller) { ApplicationController.new } - it 'renders 404 if authenticated' do allow(controller).to receive(:current_user).and_return(user) expect(controller).to receive(:not_found) controller.send(:route_not_found) end - it 'does redirect to login page if not authenticated' do + it 'does redirect to login page via authenticate_user! if not authenticated' do allow(controller).to receive(:current_user).and_return(nil) - expect(controller).to receive(:redirect_to) - expect(controller).to receive(:new_user_session_path) + expect(controller).to receive(:authenticate_user!) controller.send(:route_not_found) end end + + context 'two-factor authentication' do + let(:controller) { described_class.new } + + describe '#check_two_factor_requirement' do + subject { controller.send :check_two_factor_requirement } + + it 'does not redirect if 2FA is not required' do + allow(controller).to receive(:two_factor_authentication_required?).and_return(false) + expect(controller).not_to receive(:redirect_to) + + subject + end + + it 'does not redirect if user is not logged in' do + allow(controller).to receive(:two_factor_authentication_required?).and_return(true) + allow(controller).to receive(:current_user).and_return(nil) + expect(controller).not_to receive(:redirect_to) + + subject + end + + it 'does not redirect if user has 2FA enabled' do + allow(controller).to receive(:two_factor_authentication_required?).and_return(true) + allow(controller).to receive(:current_user).twice.and_return(user) + allow(user).to receive(:two_factor_enabled?).and_return(true) + expect(controller).not_to receive(:redirect_to) + + subject + end + + it 'does not redirect if 2FA setup can be skipped' do + allow(controller).to receive(:two_factor_authentication_required?).and_return(true) + allow(controller).to receive(:current_user).twice.and_return(user) + allow(user).to receive(:two_factor_enabled?).and_return(false) + allow(controller).to receive(:skip_two_factor?).and_return(true) + expect(controller).not_to receive(:redirect_to) + + subject + end + + it 'redirects to 2FA setup otherwise' do + allow(controller).to receive(:two_factor_authentication_required?).and_return(true) + allow(controller).to receive(:current_user).twice.and_return(user) + allow(user).to receive(:two_factor_enabled?).and_return(false) + allow(controller).to receive(:skip_two_factor?).and_return(false) + allow(controller).to receive(:profile_two_factor_auth_path) + expect(controller).to receive(:redirect_to) + + subject + end + end + + describe '#two_factor_authentication_required?' do + subject { controller.send :two_factor_authentication_required? } + + it 'returns false if no 2FA requirement is present' do + allow(controller).to receive(:current_user).and_return(nil) + + expect(subject).to be_falsey + end + + it 'returns true if a 2FA requirement is set in the application settings' do + stub_application_setting require_two_factor_authentication: true + allow(controller).to receive(:current_user).and_return(nil) + + expect(subject).to be_truthy + end + + it 'returns true if a 2FA requirement is set on the user' do + user.require_two_factor_authentication_from_group = true + allow(controller).to receive(:current_user).and_return(user) + + expect(subject).to be_truthy + end + end + + describe '#two_factor_grace_period' do + subject { controller.send :two_factor_grace_period } + + it 'returns the grace period from the application settings' do + stub_application_setting two_factor_grace_period: 23 + allow(controller).to receive(:current_user).and_return(nil) + + expect(subject).to eq 23 + end + + context 'with a 2FA requirement set on the user' do + let(:user) { create :user, require_two_factor_authentication_from_group: true, two_factor_grace_period: 23 } + + it 'returns the user grace period if lower than the application grace period' do + stub_application_setting two_factor_grace_period: 24 + allow(controller).to receive(:current_user).and_return(user) + + expect(subject).to eq 23 + end + + it 'returns the application grace period if lower than the user grace period' do + stub_application_setting two_factor_grace_period: 22 + allow(controller).to receive(:current_user).and_return(user) + + expect(subject).to eq 22 + end + end + end + + describe '#two_factor_grace_period_expired?' do + subject { controller.send :two_factor_grace_period_expired? } + + before do + allow(controller).to receive(:current_user).and_return(user) + end + + it 'returns false if the user has not started their grace period yet' do + expect(subject).to be_falsey + end + + context 'with grace period started' do + let(:user) { create :user, otp_grace_period_started_at: 2.hours.ago } + + it 'returns true if the grace period has expired' do + allow(controller).to receive(:two_factor_grace_period).and_return(1) + + expect(subject).to be_truthy + end + + it 'returns false if the grace period is still active' do + allow(controller).to receive(:two_factor_grace_period).and_return(3) + + expect(subject).to be_falsey + end + end + end + + describe '#two_factor_skippable' do + subject { controller.send :two_factor_skippable? } + + before do + allow(controller).to receive(:current_user).and_return(user) + end + + it 'returns false if 2FA is not required' do + allow(controller).to receive(:two_factor_authentication_required?).and_return(false) + + expect(subject).to be_falsey + end + + it 'returns false if the user has already enabled 2FA' do + allow(controller).to receive(:two_factor_authentication_required?).and_return(true) + allow(user).to receive(:two_factor_enabled?).and_return(true) + + expect(subject).to be_falsey + end + + it 'returns false if the 2FA grace period has expired' do + allow(controller).to receive(:two_factor_authentication_required?).and_return(true) + allow(user).to receive(:two_factor_enabled?).and_return(false) + allow(controller).to receive(:two_factor_grace_period_expired?).and_return(true) + + expect(subject).to be_falsey + end + + it 'returns true otherwise' do + allow(controller).to receive(:two_factor_authentication_required?).and_return(true) + allow(user).to receive(:two_factor_enabled?).and_return(false) + allow(controller).to receive(:two_factor_grace_period_expired?).and_return(false) + + expect(subject).to be_truthy + end + end + + describe '#skip_two_factor?' do + subject { controller.send :skip_two_factor? } + + it 'returns false if 2FA setup was not skipped' do + allow(controller).to receive(:session).and_return({}) + + expect(subject).to be_falsey + end + + context 'with 2FA setup skipped' do + before do + allow(controller).to receive(:session).and_return({ skip_two_factor: 2.hours.from_now }) + end + + it 'returns false if the grace period has expired' do + Timecop.freeze(3.hours.from_now) do + expect(subject).to be_falsey + end + end + + it 'returns true if the grace period is still active' do + Timecop.freeze(1.hour.from_now) do + expect(subject).to be_truthy + end + end + end + end + end end diff --git a/spec/controllers/blob_controller_spec.rb b/spec/controllers/blob_controller_spec.rb deleted file mode 100644 index 44e011fd3a8..00000000000 --- a/spec/controllers/blob_controller_spec.rb +++ /dev/null @@ -1,67 +0,0 @@ -require 'spec_helper' - -describe Projects::BlobController do - let(:project) { create(:project, :repository) } - let(:user) { create(:user) } - - before do - sign_in(user) - - project.team << [user, :master] - - allow(project).to receive(:branches).and_return(['master', 'foo/bar/baz']) - allow(project).to receive(:tags).and_return(['v1.0.0', 'v2.0.0']) - controller.instance_variable_set(:@project, project) - end - - describe "GET show" do - render_views - - before do - get(:show, - namespace_id: project.namespace, - project_id: project, - id: id) - end - - context "valid branch, valid file" do - let(:id) { 'master/README.md' } - it { is_expected.to respond_with(:success) } - end - - context "valid branch, invalid file" do - let(:id) { 'master/invalid-path.rb' } - it { is_expected.to respond_with(:not_found) } - end - - context "invalid branch, valid file" do - let(:id) { 'invalid-branch/README.md' } - it { is_expected.to respond_with(:not_found) } - end - - context "binary file" do - let(:id) { 'binary-encoding/encoding/binary-1.bin' } - it { is_expected.to respond_with(:success) } - end - end - - describe 'GET show with tree path' do - render_views - - before do - get(:show, - namespace_id: project.namespace, - project_id: project, - id: id) - controller.instance_variable_set(:@blob, nil) - end - - context 'redirect to tree' do - let(:id) { 'markdown/doc' } - it 'redirects' do - expect(subject). - to redirect_to("/#{project.path_with_namespace}/tree/markdown/doc") - end - end - end -end diff --git a/spec/controllers/dashboard/todos_controller_spec.rb b/spec/controllers/dashboard/todos_controller_spec.rb index 71a4a2c43c7..085f3fd8543 100644 --- a/spec/controllers/dashboard/todos_controller_spec.rb +++ b/spec/controllers/dashboard/todos_controller_spec.rb @@ -1,8 +1,6 @@ require 'spec_helper' describe Dashboard::TodosController do - include ApiHelpers - let(:user) { create(:user) } let(:author) { create(:user) } let(:project) { create(:empty_project) } @@ -16,7 +14,7 @@ describe Dashboard::TodosController do describe 'GET #index' do context 'when using pagination' do let(:last_page) { user.todos.page.total_pages } - let!(:issues) { create_list(:issue, 2, project: project, assignee: user) } + let!(:issues) { create_list(:issue, 2, project: project, assignees: [user]) } before do issues.each { |issue| todo_service.new_issue(issue, user) } @@ -35,6 +33,13 @@ describe Dashboard::TodosController do expect(assigns(:todos).current_page).to eq(last_page) expect(response).to have_http_status(200) end + + it 'does not redirect to external sites when provided a host field' do + external_host = "www.example.com" + get :index, page: (last_page + 1).to_param, host: external_host + + expect(response).to redirect_to(dashboard_todos_path(page: last_page)) + end end end diff --git a/spec/controllers/groups/milestones_controller_spec.rb b/spec/controllers/groups/milestones_controller_spec.rb index 6e4b5f78e33..f3263bc177d 100644 --- a/spec/controllers/groups/milestones_controller_spec.rb +++ b/spec/controllers/groups/milestones_controller_spec.rb @@ -6,18 +6,29 @@ describe Groups::MilestonesController do let(:project2) { create(:empty_project, group: group) } let(:user) { create(:user) } let(:title) { '肯定不是中文的问题' } + let(:milestone) do + project_milestone = create(:milestone, project: project) + + GroupMilestone.build( + group, + [project], + project_milestone.title + ) + end + let(:milestone_path) { group_milestone_path(group, milestone.safe_title, title: milestone.title) } before do sign_in(user) group.add_owner(user) project.team << [user, :master] - controller.instance_variable_set(:@group, group) end + it_behaves_like 'milestone tabs' + describe "#create" do it "creates group milestone with Chinese title" do post :create, - group_id: group.id, + group_id: group.to_param, milestone: { project_ids: [project.id, project2.id], title: title } expect(response).to redirect_to(group_milestone_path(group, title.to_slug.to_s, title: title)) @@ -25,9 +36,139 @@ describe Groups::MilestonesController do end it "redirects to new when there are no project ids" do - post :create, group_id: group.id, milestone: { title: title, project_ids: [""] } + post :create, group_id: group.to_param, milestone: { title: title, project_ids: [""] } expect(response).to render_template :new expect(assigns(:milestone).errors).not_to be_nil end end + + describe '#ensure_canonical_path' do + before do + sign_in(user) + end + + context 'for a GET request' do + context 'when requesting the canonical path' do + context 'non-show path' do + context 'with exactly matching casing' do + it 'does not redirect' do + get :index, group_id: group.to_param + + expect(response).not_to have_http_status(301) + end + end + + context 'with different casing' do + it 'redirects to the correct casing' do + get :index, group_id: group.to_param.upcase + + expect(response).to redirect_to(group_milestones_path(group.to_param)) + expect(controller).not_to set_flash[:notice] + end + end + end + + context 'show path' do + context 'with exactly matching casing' do + it 'does not redirect' do + get :show, group_id: group.to_param, id: title + + expect(response).not_to have_http_status(301) + end + end + + context 'with different casing' do + it 'redirects to the correct casing' do + get :show, group_id: group.to_param.upcase, id: title + + expect(response).to redirect_to(group_milestone_path(group.to_param, title)) + expect(controller).not_to set_flash[:notice] + end + end + end + end + + context 'when requesting a redirected path' do + let(:redirect_route) { group.redirect_routes.create(path: 'old-path') } + + it 'redirects to the canonical path' do + get :merge_requests, group_id: redirect_route.path, id: title + + expect(response).to redirect_to(merge_requests_group_milestone_path(group.to_param, title)) + expect(controller).to set_flash[:notice].to(group_moved_message(redirect_route, group)) + end + + context 'when the old group path is a substring of the scheme or host' do + let(:redirect_route) { group.redirect_routes.create(path: 'http') } + + it 'does not modify the requested host' do + get :merge_requests, group_id: redirect_route.path, id: title + + expect(response).to redirect_to(merge_requests_group_milestone_path(group.to_param, title)) + expect(controller).to set_flash[:notice].to(group_moved_message(redirect_route, group)) + end + end + + context 'when the old group path is substring of groups' do + # I.e. /groups/oups should not become /grfoo/oups + let(:redirect_route) { group.redirect_routes.create(path: 'oups') } + + it 'does not modify the /groups part of the path' do + get :merge_requests, group_id: redirect_route.path, id: title + + expect(response).to redirect_to(merge_requests_group_milestone_path(group.to_param, title)) + expect(controller).to set_flash[:notice].to(group_moved_message(redirect_route, group)) + end + end + + context 'when the old group path is substring of groups plus the new path' do + # I.e. /groups/oups/oup should not become /grfoos + let(:redirect_route) { group.redirect_routes.create(path: 'oups/oup') } + + it 'does not modify the /groups part of the path' do + get :merge_requests, group_id: redirect_route.path, id: title + + expect(response).to redirect_to(merge_requests_group_milestone_path(group.to_param, title)) + expect(controller).to set_flash[:notice].to(group_moved_message(redirect_route, group)) + end + end + end + end + end + + context 'for a non-GET request' do + context 'when requesting the canonical path with different casing' do + it 'does not 404' do + post :create, + group_id: group.to_param, + milestone: { project_ids: [project.id, project2.id], title: title } + + expect(response).not_to have_http_status(404) + end + + it 'does not redirect to the correct casing' do + post :create, + group_id: group.to_param, + milestone: { project_ids: [project.id, project2.id], title: title } + + expect(response).not_to have_http_status(301) + end + end + + context 'when requesting a redirected path' do + let(:redirect_route) { group.redirect_routes.create(path: 'old-path') } + + it 'returns not found' do + post :create, + group_id: redirect_route.path, + milestone: { project_ids: [project.id, project2.id], title: title } + + expect(response).to have_http_status(404) + end + end + end + + def group_moved_message(redirect_route, group) + "Group '#{redirect_route.path}' was moved to '#{group.full_path}'. Please update any links and bookmarks that may still have the old path." + end end diff --git a/spec/controllers/groups_controller_spec.rb b/spec/controllers/groups_controller_spec.rb index cad82a34fb0..4626f1ebc29 100644 --- a/spec/controllers/groups_controller_spec.rb +++ b/spec/controllers/groups_controller_spec.rb @@ -26,6 +26,41 @@ describe GroupsController do end end + describe 'GET #subgroups' do + let!(:public_subgroup) { create(:group, :public, parent: group) } + let!(:private_subgroup) { create(:group, :private, parent: group) } + + context 'as a user' do + before do + sign_in(user) + end + + it 'shows the public subgroups' do + get :subgroups, id: group.to_param + + expect(assigns(:nested_groups)).to contain_exactly(public_subgroup) + end + + context 'being member' do + it 'shows public and private subgroups the user is member of' do + private_subgroup.add_guest(user) + + get :subgroups, id: group.to_param + + expect(assigns(:nested_groups)).to contain_exactly(public_subgroup, private_subgroup) + end + end + end + + context 'as a guest' do + it 'shows the public subgroups' do + get :subgroups, id: group.to_param + + expect(assigns(:nested_groups)).to contain_exactly(public_subgroup) + end + end + end + describe 'GET #issues' do let(:issue_1) { create(:issue, project: project) } let(:issue_2) { create(:issue, project: project) } @@ -33,7 +68,7 @@ describe GroupsController do before do create_list(:award_emoji, 3, awardable: issue_2) create_list(:award_emoji, 2, awardable: issue_1) - create_list(:award_emoji, 2, :downvote, awardable: issue_2,) + create_list(:award_emoji, 2, :downvote, awardable: issue_2) sign_in(user) end @@ -81,7 +116,7 @@ describe GroupsController do it 'returns 404' do sign_in(create(:user)) - delete :destroy, id: group.path + delete :destroy, id: group.to_param expect(response.status).to eq(404) end @@ -94,12 +129,12 @@ describe GroupsController do it 'schedules a group destroy' do Sidekiq::Testing.fake! do - expect { delete :destroy, id: group.path }.to change(GroupDestroyWorker.jobs, :size).by(1) + expect { delete :destroy, id: group.to_param }.to change(GroupDestroyWorker.jobs, :size).by(1) end end it 'redirects to the root path' do - delete :destroy, id: group.path + delete :destroy, id: group.to_param expect(response).to redirect_to(root_path) end @@ -111,7 +146,7 @@ describe GroupsController do sign_in(user) end - it 'updates the path succesfully' do + it 'updates the path successfully' do post :update, id: group.to_param, group: { path: 'new_path' } expect(response).to have_http_status(302) @@ -126,4 +161,201 @@ describe GroupsController do expect(assigns(:group).path).not_to eq('new_path') end end + + describe '#ensure_canonical_path' do + before do + sign_in(user) + end + + context 'for a GET request' do + context 'when requesting groups at the root path' do + before do + allow(request).to receive(:original_fullpath).and_return("/#{group_full_path}") + get :show, id: group_full_path + end + + context 'when requesting the canonical path with different casing' do + let(:group_full_path) { group.to_param.upcase } + + it 'redirects to the correct casing' do + expect(response).to redirect_to(group) + expect(controller).not_to set_flash[:notice] + end + end + + context 'when requesting a redirected path' do + let(:redirect_route) { group.redirect_routes.create(path: 'old-path') } + let(:group_full_path) { redirect_route.path } + + it 'redirects to the canonical path' do + expect(response).to redirect_to(group) + expect(controller).to set_flash[:notice].to(group_moved_message(redirect_route, group)) + end + + context 'when the old group path is a substring of the scheme or host' do + let(:redirect_route) { group.redirect_routes.create(path: 'http') } + + it 'does not modify the requested host' do + expect(response).to redirect_to(group) + expect(controller).to set_flash[:notice].to(group_moved_message(redirect_route, group)) + end + end + + context 'when the old group path is substring of groups' do + # I.e. /groups/oups should not become /grfoo/oups + let(:redirect_route) { group.redirect_routes.create(path: 'oups') } + + it 'does not modify the /groups part of the path' do + expect(response).to redirect_to(group) + expect(controller).to set_flash[:notice].to(group_moved_message(redirect_route, group)) + end + end + end + end + + context 'when requesting groups under the /groups path' do + context 'when requesting the canonical path' do + context 'non-show path' do + context 'with exactly matching casing' do + it 'does not redirect' do + get :issues, id: group.to_param + + expect(response).not_to have_http_status(301) + end + end + + context 'with different casing' do + it 'redirects to the correct casing' do + get :issues, id: group.to_param.upcase + + expect(response).to redirect_to(issues_group_path(group.to_param)) + expect(controller).not_to set_flash[:notice] + end + end + end + + context 'show path' do + context 'with exactly matching casing' do + it 'does not redirect' do + get :show, id: group.to_param + + expect(response).not_to have_http_status(301) + end + end + + context 'with different casing' do + it 'redirects to the correct casing at the root path' do + get :show, id: group.to_param.upcase + + expect(response).to redirect_to(group) + expect(controller).not_to set_flash[:notice] + end + end + end + end + + context 'when requesting a redirected path' do + let(:redirect_route) { group.redirect_routes.create(path: 'old-path') } + + it 'redirects to the canonical path' do + get :issues, id: redirect_route.path + + expect(response).to redirect_to(issues_group_path(group.to_param)) + expect(controller).to set_flash[:notice].to(group_moved_message(redirect_route, group)) + end + + context 'when the old group path is a substring of the scheme or host' do + let(:redirect_route) { group.redirect_routes.create(path: 'http') } + + it 'does not modify the requested host' do + get :issues, id: redirect_route.path + + expect(response).to redirect_to(issues_group_path(group.to_param)) + expect(controller).to set_flash[:notice].to(group_moved_message(redirect_route, group)) + end + end + + context 'when the old group path is substring of groups' do + # I.e. /groups/oups should not become /grfoo/oups + let(:redirect_route) { group.redirect_routes.create(path: 'oups') } + + it 'does not modify the /groups part of the path' do + get :issues, id: redirect_route.path + + expect(response).to redirect_to(issues_group_path(group.to_param)) + expect(controller).to set_flash[:notice].to(group_moved_message(redirect_route, group)) + end + end + + context 'when the old group path is substring of groups plus the new path' do + # I.e. /groups/oups/oup should not become /grfoos + let(:redirect_route) { group.redirect_routes.create(path: 'oups/oup') } + + it 'does not modify the /groups part of the path' do + get :issues, id: redirect_route.path + + expect(response).to redirect_to(issues_group_path(group.to_param)) + expect(controller).to set_flash[:notice].to(group_moved_message(redirect_route, group)) + end + end + end + end + end + + context 'for a POST request' do + context 'when requesting the canonical path with different casing' do + it 'does not 404' do + post :update, id: group.to_param.upcase, group: { path: 'new_path' } + + expect(response).not_to have_http_status(404) + end + + it 'does not redirect to the correct casing' do + post :update, id: group.to_param.upcase, group: { path: 'new_path' } + + expect(response).not_to have_http_status(301) + end + end + + context 'when requesting a redirected path' do + let(:redirect_route) { group.redirect_routes.create(path: 'old-path') } + + it 'returns not found' do + post :update, id: redirect_route.path, group: { path: 'new_path' } + + expect(response).to have_http_status(404) + end + end + end + + context 'for a DELETE request' do + context 'when requesting the canonical path with different casing' do + it 'does not 404' do + delete :destroy, id: group.to_param.upcase + + expect(response).not_to have_http_status(404) + end + + it 'does not redirect to the correct casing' do + delete :destroy, id: group.to_param.upcase + + expect(response).not_to have_http_status(301) + end + end + + context 'when requesting a redirected path' do + let(:redirect_route) { group.redirect_routes.create(path: 'old-path') } + + it 'returns not found' do + delete :destroy, id: redirect_route.path + + expect(response).to have_http_status(404) + end + end + end + end + + def group_moved_message(redirect_route, group) + "Group '#{redirect_route.path}' was moved to '#{group.full_path}'. Please update any links and bookmarks that may still have the old path." + end end diff --git a/spec/controllers/health_controller_spec.rb b/spec/controllers/health_controller_spec.rb new file mode 100644 index 00000000000..b8b6e0c3a88 --- /dev/null +++ b/spec/controllers/health_controller_spec.rb @@ -0,0 +1,96 @@ +require 'spec_helper' + +describe HealthController do + include StubENV + + let(:token) { current_application_settings.health_check_access_token } + let(:json_response) { JSON.parse(response.body) } + + before do + stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false') + end + + describe '#readiness' do + context 'authorization token provided' do + before do + request.headers['TOKEN'] = token + end + + it 'returns proper response' do + get :readiness + expect(json_response['db_check']['status']).to eq('ok') + expect(json_response['redis_check']['status']).to eq('ok') + expect(json_response['fs_shards_check']['status']).to eq('ok') + expect(json_response['fs_shards_check']['labels']['shard']).to eq('default') + end + end + + context 'without authorization token' do + it 'returns proper response' do + get :readiness + expect(response.status).to eq(404) + end + end + end + + describe '#liveness' do + context 'authorization token provided' do + before do + request.headers['TOKEN'] = token + end + + it 'returns proper response' do + get :liveness + expect(json_response['db_check']['status']).to eq('ok') + expect(json_response['redis_check']['status']).to eq('ok') + expect(json_response['fs_shards_check']['status']).to eq('ok') + end + end + + context 'without authorization token' do + it 'returns proper response' do + get :liveness + expect(response.status).to eq(404) + end + end + end + + describe '#metrics' do + context 'authorization token provided' do + before do + request.headers['TOKEN'] = token + end + + it 'returns DB ping metrics' do + get :metrics + expect(response.body).to match(/^db_ping_timeout 0$/) + expect(response.body).to match(/^db_ping_success 1$/) + expect(response.body).to match(/^db_ping_latency [0-9\.]+$/) + end + + it 'returns Redis ping metrics' do + get :metrics + expect(response.body).to match(/^redis_ping_timeout 0$/) + expect(response.body).to match(/^redis_ping_success 1$/) + expect(response.body).to match(/^redis_ping_latency [0-9\.]+$/) + end + + it 'returns file system check metrics' do + get :metrics + expect(response.body).to match(/^filesystem_access_latency{shard="default"} [0-9\.]+$/) + expect(response.body).to match(/^filesystem_accessible{shard="default"} 1$/) + expect(response.body).to match(/^filesystem_write_latency{shard="default"} [0-9\.]+$/) + expect(response.body).to match(/^filesystem_writable{shard="default"} 1$/) + expect(response.body).to match(/^filesystem_read_latency{shard="default"} [0-9\.]+$/) + expect(response.body).to match(/^filesystem_readable{shard="default"} 1$/) + end + end + + context 'without authorization token' do + it 'returns proper response' do + get :metrics + expect(response.status).to eq(404) + end + end + end +end diff --git a/spec/controllers/import/bitbucket_controller_spec.rb b/spec/controllers/import/bitbucket_controller_spec.rb index 51f23e4eeb9..010e3180ea4 100644 --- a/spec/controllers/import/bitbucket_controller_spec.rb +++ b/spec/controllers/import/bitbucket_controller_spec.rb @@ -200,5 +200,72 @@ describe Import::BitbucketController do end end end + + context 'user has chosen an existing nested namespace and name for the project' do + let(:parent_namespace) { create(:namespace, name: 'foo', owner: user) } + let(:nested_namespace) { create(:namespace, name: 'bar', parent: parent_namespace, owner: user) } + let(:test_name) { 'test_name' } + + it 'takes the selected namespace and name' do + expect(Gitlab::BitbucketImport::ProjectCreator). + to receive(:new).with(bitbucket_repo, test_name, nested_namespace, user, access_params). + and_return(double(execute: true)) + + post :create, { target_namespace: nested_namespace.full_path, new_name: test_name, format: :js } + end + end + + context 'user has chosen a non-existent nested namespaces and name for the project' do + let(:test_name) { 'test_name' } + + it 'takes the selected namespace and name' do + expect(Gitlab::BitbucketImport::ProjectCreator). + to receive(:new).with(bitbucket_repo, test_name, kind_of(Namespace), user, access_params). + and_return(double(execute: true)) + + post :create, { target_namespace: 'foo/bar', new_name: test_name, format: :js } + end + + it 'creates the namespaces' do + allow(Gitlab::BitbucketImport::ProjectCreator). + to receive(:new).with(bitbucket_repo, test_name, kind_of(Namespace), user, access_params). + and_return(double(execute: true)) + + expect { post :create, { target_namespace: 'foo/bar', new_name: test_name, format: :js } } + .to change { Namespace.count }.by(2) + end + + it 'new namespace has the right parent' do + allow(Gitlab::BitbucketImport::ProjectCreator). + to receive(:new).with(bitbucket_repo, test_name, kind_of(Namespace), user, access_params). + and_return(double(execute: true)) + + post :create, { target_namespace: 'foo/bar', new_name: test_name, format: :js } + + expect(Namespace.find_by_path_or_name('bar').parent.path).to eq('foo') + end + end + + context 'user has chosen existent and non-existent nested namespaces and name for the project' do + let(:test_name) { 'test_name' } + let!(:parent_namespace) { create(:namespace, name: 'foo', owner: user) } + + it 'takes the selected namespace and name' do + expect(Gitlab::BitbucketImport::ProjectCreator). + to receive(:new).with(bitbucket_repo, test_name, kind_of(Namespace), user, access_params). + and_return(double(execute: true)) + + post :create, { target_namespace: 'foo/foobar/bar', new_name: test_name, format: :js } + end + + it 'creates the namespaces' do + allow(Gitlab::BitbucketImport::ProjectCreator). + to receive(:new).with(bitbucket_repo, test_name, kind_of(Namespace), user, access_params). + and_return(double(execute: true)) + + expect { post :create, { target_namespace: 'foo/foobar/bar', new_name: test_name, format: :js } } + .to change { Namespace.count }.by(2) + end + end end end diff --git a/spec/controllers/import/gitlab_controller_spec.rb b/spec/controllers/import/gitlab_controller_spec.rb index 3f73ea000ae..2dbb89219d0 100644 --- a/spec/controllers/import/gitlab_controller_spec.rb +++ b/spec/controllers/import/gitlab_controller_spec.rb @@ -174,6 +174,72 @@ describe Import::GitlabController do end end end + + context 'user has chosen an existing nested namespace for the project' do + let(:parent_namespace) { create(:namespace, name: 'foo', owner: user) } + let(:nested_namespace) { create(:namespace, name: 'bar', parent: parent_namespace, owner: user) } + + it 'takes the selected namespace and name' do + expect(Gitlab::GitlabImport::ProjectCreator). + to receive(:new).with(gitlab_repo, nested_namespace, user, access_params). + and_return(double(execute: true)) + + post :create, { target_namespace: nested_namespace.full_path, format: :js } + end + end + + context 'user has chosen a non-existent nested namespaces for the project' do + let(:test_name) { 'test_name' } + + it 'takes the selected namespace and name' do + expect(Gitlab::GitlabImport::ProjectCreator). + to receive(:new).with(gitlab_repo, kind_of(Namespace), user, access_params). + and_return(double(execute: true)) + + post :create, { target_namespace: 'foo/bar', format: :js } + end + + it 'creates the namespaces' do + allow(Gitlab::GitlabImport::ProjectCreator). + to receive(:new).with(gitlab_repo, kind_of(Namespace), user, access_params). + and_return(double(execute: true)) + + expect { post :create, { target_namespace: 'foo/bar', format: :js } } + .to change { Namespace.count }.by(2) + end + + it 'new namespace has the right parent' do + allow(Gitlab::GitlabImport::ProjectCreator). + to receive(:new).with(gitlab_repo, kind_of(Namespace), user, access_params). + and_return(double(execute: true)) + + post :create, { target_namespace: 'foo/bar', format: :js } + + expect(Namespace.find_by_path_or_name('bar').parent.path).to eq('foo') + end + end + + context 'user has chosen existent and non-existent nested namespaces and name for the project' do + let(:test_name) { 'test_name' } + let!(:parent_namespace) { create(:namespace, name: 'foo', owner: user) } + + it 'takes the selected namespace and name' do + expect(Gitlab::GitlabImport::ProjectCreator). + to receive(:new).with(gitlab_repo, kind_of(Namespace), user, access_params). + and_return(double(execute: true)) + + post :create, { target_namespace: 'foo/foobar/bar', format: :js } + end + + it 'creates the namespaces' do + allow(Gitlab::GitlabImport::ProjectCreator). + to receive(:new).with(gitlab_repo, kind_of(Namespace), user, access_params). + and_return(double(execute: true)) + + expect { post :create, { target_namespace: 'foo/foobar/bar', format: :js } } + .to change { Namespace.count }.by(2) + end + end end end end diff --git a/spec/controllers/oauth/authorizations_controller_spec.rb b/spec/controllers/oauth/authorizations_controller_spec.rb new file mode 100644 index 00000000000..d321bfcea9d --- /dev/null +++ b/spec/controllers/oauth/authorizations_controller_spec.rb @@ -0,0 +1,55 @@ +require 'spec_helper' + +describe Oauth::AuthorizationsController do + let(:user) { create(:user) } + + let(:doorkeeper) do + Doorkeeper::Application.create( + name: "MyApp", + redirect_uri: 'http://example.com', + scopes: "") + end + + let(:params) do + { + response_type: "code", + client_id: doorkeeper.uid, + redirect_uri: doorkeeper.redirect_uri, + state: 'state' + } + end + + before do + sign_in(user) + end + + describe 'GET #new' do + context 'without valid params' do + it 'returns 200 code and renders error view' do + get :new + + expect(response).to have_http_status(200) + expect(response).to render_template('doorkeeper/authorizations/error') + end + end + + context 'with valid params' do + it 'returns 200 code and renders view' do + get :new, params + + expect(response).to have_http_status(200) + expect(response).to render_template('doorkeeper/authorizations/new') + end + + it 'deletes session.user_return_to and redirects when skip authorization' do + request.session['user_return_to'] = 'http://example.com' + allow(controller).to receive(:skip_authorization?).and_return(true) + + get :new, params + + expect(request.session['user_return_to']).to be_nil + expect(response).to have_http_status(302) + end + end + end +end diff --git a/spec/controllers/profiles/accounts_controller_spec.rb b/spec/controllers/profiles/accounts_controller_spec.rb index 18148acde3e..2f9d18e3a0e 100644 --- a/spec/controllers/profiles/accounts_controller_spec.rb +++ b/spec/controllers/profiles/accounts_controller_spec.rb @@ -1,25 +1,47 @@ require 'spec_helper' describe Profiles::AccountsController do - let(:user) { create(:omniauth_user, provider: 'saml') } + describe 'DELETE unlink' do + let(:user) { create(:omniauth_user) } - before do - sign_in(user) - end + before do + sign_in(user) + end - it 'does not allow to unlink SAML connected account' do - identity = user.identities.last - delete :unlink, provider: 'saml' - updated_user = User.find(user.id) + it 'renders 404 if someone tries to unlink a non existent provider' do + delete :unlink, provider: 'github' - expect(response).to have_http_status(302) - expect(updated_user.identities.size).to eq(1) - expect(updated_user.identities).to include(identity) - end + expect(response).to have_http_status(404) + end + + [:saml, :cas3].each do |provider| + describe "#{provider} provider" do + let(:user) { create(:omniauth_user, provider: provider.to_s) } + + it "does not allow to unlink connected account" do + identity = user.identities.last + + delete :unlink, provider: provider.to_s + + expect(response).to have_http_status(302) + expect(user.reload.identities).to include(identity) + end + end + end + + [:twitter, :facebook, :google_oauth2, :gitlab, :github, :bitbucket, :crowd, :auth0].each do |provider| + describe "#{provider} provider" do + let(:user) { create(:omniauth_user, provider: provider.to_s) } + + it 'allows to unlink connected account' do + identity = user.identities.last - it 'does allow to delete other linked accounts' do - user.identities.create(provider: 'twitter', extern_uid: 'twitter_123') + delete :unlink, provider: provider.to_s - expect { delete :unlink, provider: 'twitter' }.to change(Identity.all, :size).by(-1) + expect(response).to have_http_status(302) + expect(user.reload.identities).not_to include(identity) + end + end + end end end diff --git a/spec/controllers/profiles/personal_access_tokens_spec.rb b/spec/controllers/profiles/personal_access_tokens_controller_spec.rb index dfed1de2046..98a43e278b2 100644 --- a/spec/controllers/profiles/personal_access_tokens_spec.rb +++ b/spec/controllers/profiles/personal_access_tokens_controller_spec.rb @@ -12,7 +12,7 @@ describe Profiles::PersonalAccessTokensController do end it "allows creation of a token with scopes" do - name = FFaker::Product.brand + name = 'My PAT' scopes = %w[api read_user] post :create, personal_access_token: token_attributes.merge(scopes: scopes, name: name) diff --git a/spec/controllers/projects/artifacts_controller_spec.rb b/spec/controllers/projects/artifacts_controller_spec.rb new file mode 100644 index 00000000000..eff9fab8da2 --- /dev/null +++ b/spec/controllers/projects/artifacts_controller_spec.rb @@ -0,0 +1,188 @@ +require 'spec_helper' + +describe Projects::ArtifactsController do + let(:user) { create(:user) } + let(:project) { create(:project, :repository) } + + let(:pipeline) do + create(:ci_pipeline, + project: project, + sha: project.commit.sha, + ref: project.default_branch, + status: 'success') + end + + let(:build) { create(:ci_build, :success, :artifacts, pipeline: pipeline) } + + before do + project.team << [user, :developer] + + sign_in(user) + end + + describe 'GET download' do + it 'sends the artifacts file' do + expect(controller).to receive(:send_file).with(build.artifacts_file.path, disposition: 'attachment').and_call_original + + get :download, namespace_id: project.namespace, project_id: project, build_id: build + end + end + + describe 'GET browse' do + context 'when the directory exists' do + it 'renders the browse view' do + get :browse, namespace_id: project.namespace, project_id: project, build_id: build, path: 'other_artifacts_0.1.2' + + expect(response).to render_template('projects/artifacts/browse') + end + end + + context 'when the directory does not exist' do + it 'responds Not Found' do + get :browse, namespace_id: project.namespace, project_id: project, build_id: build, path: 'unknown' + + expect(response).to be_not_found + end + end + end + + describe 'GET file' do + context 'when the file exists' do + it 'renders the file view' do + get :file, namespace_id: project.namespace, project_id: project, build_id: build, path: 'ci_artifacts.txt' + + expect(response).to render_template('projects/artifacts/file') + end + end + + context 'when the file does not exist' do + it 'responds Not Found' do + get :file, namespace_id: project.namespace, project_id: project, build_id: build, path: 'unknown' + + expect(response).to be_not_found + end + end + end + + describe 'GET raw' do + context 'when the file exists' do + it 'serves the file using workhorse' do + get :raw, namespace_id: project.namespace, project_id: project, build_id: build, path: 'ci_artifacts.txt' + + send_data = response.headers[Gitlab::Workhorse::SEND_DATA_HEADER] + + expect(send_data).to start_with('artifacts-entry:') + + base64_params = send_data.sub(/\Aartifacts\-entry:/, '') + params = JSON.parse(Base64.urlsafe_decode64(base64_params)) + + expect(params.keys).to eq(%w(Archive Entry)) + expect(params['Archive']).to end_with('build_artifacts.zip') + expect(params['Entry']).to eq(Base64.encode64('ci_artifacts.txt')) + end + end + + context 'when the file does not exist' do + it 'responds Not Found' do + get :raw, namespace_id: project.namespace, project_id: project, build_id: build, path: 'unknown' + + expect(response).to be_not_found + end + end + end + + describe 'GET latest_succeeded' do + def params_from_ref(ref = pipeline.ref, job = build.name, path = 'browse') + { + namespace_id: project.namespace, + project_id: project, + ref_name_and_path: File.join(ref, path), + job: job + } + end + + context 'cannot find the build' do + shared_examples 'not found' do + it { expect(response).to have_http_status(:not_found) } + end + + context 'has no such ref' do + before do + get :latest_succeeded, params_from_ref('TAIL', build.name) + end + + it_behaves_like 'not found' + end + + context 'has no such build' do + before do + get :latest_succeeded, params_from_ref(pipeline.ref, 'NOBUILD') + end + + it_behaves_like 'not found' + end + + context 'has no path' do + before do + get :latest_succeeded, params_from_ref(pipeline.sha, build.name, '') + end + + it_behaves_like 'not found' + end + end + + context 'found the build and redirect' do + shared_examples 'redirect to the build' do + it 'redirects' do + path = browse_namespace_project_build_artifacts_path( + project.namespace, + project, + build) + + expect(response).to redirect_to(path) + end + end + + context 'with regular branch' do + before do + pipeline.update(ref: 'master', + sha: project.commit('master').sha) + + get :latest_succeeded, params_from_ref('master') + end + + it_behaves_like 'redirect to the build' + end + + context 'with branch name containing slash' do + before do + pipeline.update(ref: 'improve/awesome', + sha: project.commit('improve/awesome').sha) + + get :latest_succeeded, params_from_ref('improve/awesome') + end + + it_behaves_like 'redirect to the build' + end + + context 'with branch name and path containing slashes' do + before do + pipeline.update(ref: 'improve/awesome', + sha: project.commit('improve/awesome').sha) + + get :latest_succeeded, params_from_ref('improve/awesome', build.name, 'file/README.md') + end + + it 'redirects' do + path = file_namespace_project_build_artifacts_path( + project.namespace, + project, + build, + 'README.md') + + expect(response).to redirect_to(path) + end + end + end + end +end diff --git a/spec/controllers/projects/blob_controller_spec.rb b/spec/controllers/projects/blob_controller_spec.rb index ec36a64b415..3b3caa9d3e6 100644 --- a/spec/controllers/projects/blob_controller_spec.rb +++ b/spec/controllers/projects/blob_controller_spec.rb @@ -2,15 +2,61 @@ require 'rails_helper' describe Projects::BlobController do let(:project) { create(:project, :public, :repository) } - let(:user) { create(:user) } - before do - project.team << [user, :master] + describe "GET show" do + render_views + + context 'with file path' do + before do + get(:show, + namespace_id: project.namespace, + project_id: project, + id: id) + end + + context "valid branch, valid file" do + let(:id) { 'master/README.md' } + it { is_expected.to respond_with(:success) } + end + + context "valid branch, invalid file" do + let(:id) { 'master/invalid-path.rb' } + it { is_expected.to respond_with(:not_found) } + end + + context "invalid branch, valid file" do + let(:id) { 'invalid-branch/README.md' } + it { is_expected.to respond_with(:not_found) } + end + + context "binary file" do + let(:id) { 'binary-encoding/encoding/binary-1.bin' } + it { is_expected.to respond_with(:success) } + end + end + + context 'with tree path' do + before do + get(:show, + namespace_id: project.namespace, + project_id: project, + id: id) + controller.instance_variable_set(:@blob, nil) + end - sign_in(user) + context 'redirect to tree' do + let(:id) { 'markdown/doc' } + it 'redirects' do + expect(subject). + to redirect_to("/#{project.path_with_namespace}/tree/markdown/doc") + end + end + end end describe 'GET diff' do + let(:user) { create(:user) } + render_views def do_get(opts = {}) @@ -20,6 +66,12 @@ describe Projects::BlobController do get :diff, params.merge(opts) end + before do + project.team << [user, :master] + + sign_in(user) + end + context 'when essential params are missing' do it 'renders nothing' do do_get @@ -37,13 +89,75 @@ describe Projects::BlobController do end end + describe 'GET edit' do + let(:default_params) do + { + namespace_id: project.namespace, + project_id: project, + id: 'master/CHANGELOG' + } + end + + context 'anonymous' do + before do + get :edit, default_params + end + + it 'redirects to sign in and returns' do + expect(response).to redirect_to(new_user_session_path) + end + end + + context 'as guest' do + let(:guest) { create(:user) } + + before do + sign_in(guest) + get :edit, default_params + end + + it 'redirects to blob show' do + expect(response).to redirect_to(namespace_project_blob_path(project.namespace, project, 'master/CHANGELOG')) + end + end + + context 'as developer' do + let(:developer) { create(:user) } + + before do + project.team << [developer, :developer] + sign_in(developer) + get :edit, default_params + end + + it 'redirects to blob show' do + expect(response).to have_http_status(200) + end + end + + context 'as master' do + let(:master) { create(:user) } + + before do + project.team << [master, :master] + sign_in(master) + get :edit, default_params + end + + it 'redirects to blob show' do + expect(response).to have_http_status(200) + end + end + end + describe 'PUT update' do + let(:user) { create(:user) } let(:default_params) do { namespace_id: project.namespace, project_id: project, id: 'master/CHANGELOG', - target_branch: 'master', + branch_name: 'master', content: 'Added changes', commit_message: 'Update CHANGELOG' } @@ -53,6 +167,12 @@ describe Projects::BlobController do namespace_project_blob_path(project.namespace, project, 'master/CHANGELOG') end + before do + project.team << [user, :master] + + sign_in(user) + end + it 'redirects to blob' do put :update, default_params @@ -109,7 +229,7 @@ describe Projects::BlobController do context 'when editing on the original repository' do it "redirects to forked project new merge request" do - default_params[:target_branch] = "fork-test-1" + default_params[:branch_name] = "fork-test-1" default_params[:create_merge_request] = 1 put :update, default_params diff --git a/spec/controllers/projects/boards/issues_controller_spec.rb b/spec/controllers/projects/boards/issues_controller_spec.rb index 15667e8d4b1..dc3b72c6de4 100644 --- a/spec/controllers/projects/boards/issues_controller_spec.rb +++ b/spec/controllers/projects/boards/issues_controller_spec.rb @@ -34,7 +34,7 @@ describe Projects::Boards::IssuesController do issue = create(:labeled_issue, project: project, labels: [planning]) create(:labeled_issue, project: project, labels: [planning]) create(:labeled_issue, project: project, labels: [development], due_date: Date.tomorrow) - create(:labeled_issue, project: project, labels: [development], assignee: johndoe) + create(:labeled_issue, project: project, labels: [development], assignees: [johndoe]) issue.subscribe(johndoe, project) list_issues user: user, board: board, list: list2 diff --git a/spec/controllers/projects/branches_controller_spec.rb b/spec/controllers/projects/branches_controller_spec.rb index d20e7368086..f285e5333d6 100644 --- a/spec/controllers/projects/branches_controller_spec.rb +++ b/spec/controllers/projects/branches_controller_spec.rb @@ -14,7 +14,7 @@ describe Projects::BranchesController do controller.instance_variable_set(:@project, project) end - describe "POST create" do + describe "POST create with HTML format" do render_views context "on creation of a new branch" do @@ -152,6 +152,42 @@ describe Projects::BranchesController do end end + describe 'POST create with JSON format' do + before do + sign_in(user) + end + + context 'with valid params' do + it 'returns a successful 200 response' do + create_branch name: 'my-branch', ref: 'master' + + expect(response).to have_http_status(200) + end + + it 'returns the created branch' do + create_branch name: 'my-branch', ref: 'master' + + expect(response).to match_response_schema('branch') + end + end + + context 'with invalid params' do + it 'returns an unprocessable entity 422 response' do + create_branch name: "<script>alert('merge');</script>", ref: "<script>alert('ref');</script>" + + expect(response).to have_http_status(422) + end + end + + def create_branch(name:, ref:) + post :create, namespace_id: project.namespace.to_param, + project_id: project.to_param, + branch_name: name, + ref: ref, + format: :json + end + end + describe "POST destroy with HTML format" do render_views @@ -177,33 +213,98 @@ describe Projects::BranchesController do sign_in(user) post :destroy, - format: :js, - id: branch, - namespace_id: project.namespace, - project_id: project + format: format, + id: branch, + namespace_id: project.namespace, + project_id: project end - context "valid branch name, valid source" do + context 'as JS' do let(:branch) { "feature" } + let(:format) { :js } - it { expect(response).to have_http_status(200) } - end + context "valid branch name, valid source" do + let(:branch) { "feature" } - context "valid branch name with unencoded slashes" do - let(:branch) { "improve/awesome" } + it { expect(response).to have_http_status(200) } + it { expect(response.body).to be_blank } + end + + context "valid branch name with unencoded slashes" do + let(:branch) { "improve/awesome" } + + it { expect(response).to have_http_status(200) } + it { expect(response.body).to be_blank } + end - it { expect(response).to have_http_status(200) } + context "valid branch name with encoded slashes" do + let(:branch) { "improve%2Fawesome" } + + it { expect(response).to have_http_status(200) } + it { expect(response.body).to be_blank } + end + + context "invalid branch name, valid ref" do + let(:branch) { "no-branch" } + + it { expect(response).to have_http_status(404) } + it { expect(response.body).to be_blank } + end end - context "valid branch name with encoded slashes" do - let(:branch) { "improve%2Fawesome" } + context 'as JSON' do + let(:branch) { "feature" } + let(:format) { :json } + + context 'valid branch name, valid source' do + let(:branch) { "feature" } + + it 'returns JSON response with message' do + expect(json_response).to eql("message" => 'Branch was removed') + end + + it { expect(response).to have_http_status(200) } + end + + context 'valid branch name with unencoded slashes' do + let(:branch) { "improve/awesome" } + + it 'returns JSON response with message' do + expect(json_response).to eql('message' => 'Branch was removed') + end + + it { expect(response).to have_http_status(200) } + end + + context "valid branch name with encoded slashes" do + let(:branch) { 'improve%2Fawesome' } + + it 'returns JSON response with message' do + expect(json_response).to eql('message' => 'Branch was removed') + end + + it { expect(response).to have_http_status(200) } + end - it { expect(response).to have_http_status(200) } + context 'invalid branch name, valid ref' do + let(:branch) { 'no-branch' } + + it 'returns JSON response with message' do + expect(json_response).to eql('message' => 'No such branch') + end + + it { expect(response).to have_http_status(404) } + end end - context "invalid branch name, valid ref" do - let(:branch) { "no-branch" } - it { expect(response).to have_http_status(404) } + context 'as HTML' do + let(:branch) { "feature" } + let(:format) { :html } + + it 'redirects to branches path' do + expect(response) + .to redirect_to(namespace_project_branches_path(project.namespace, project)) + end end end diff --git a/spec/controllers/projects/builds_controller_spec.rb b/spec/controllers/projects/builds_controller_spec.rb index 683667129e5..3ce23c17cdc 100644 --- a/spec/controllers/projects/builds_controller_spec.rb +++ b/spec/controllers/projects/builds_controller_spec.rb @@ -3,15 +3,169 @@ require 'spec_helper' describe Projects::BuildsController do include ApiHelpers - let(:user) { create(:user) } let(:project) { create(:empty_project, :public) } + let(:pipeline) { create(:ci_pipeline, project: project) } + let(:user) { create(:user) } + + describe 'GET index' do + context 'when scope is pending' do + before do + create(:ci_build, :pending, pipeline: pipeline) + + get_index(scope: 'pending') + end + + it 'has only pending builds' do + expect(response).to have_http_status(:ok) + expect(assigns(:builds).first.status).to eq('pending') + end + end + + context 'when scope is running' do + before do + create(:ci_build, :running, pipeline: pipeline) + + get_index(scope: 'running') + end + + it 'has only running builds' do + expect(response).to have_http_status(:ok) + expect(assigns(:builds).first.status).to eq('running') + end + end + + context 'when scope is finished' do + before do + create(:ci_build, :success, pipeline: pipeline) + + get_index(scope: 'finished') + end + + it 'has only finished builds' do + expect(response).to have_http_status(:ok) + expect(assigns(:builds).first.status).to eq('success') + end + end + + context 'when page is specified' do + let(:last_page) { project.builds.page.total_pages } + + context 'when page number is eligible' do + before do + create_list(:ci_build, 2, pipeline: pipeline) + + get_index(page: last_page.to_param) + end + + it 'redirects to the page' do + expect(response).to have_http_status(:ok) + expect(assigns(:builds).current_page).to eq(last_page) + end + end + end - before do - sign_in(user) + context 'number of queries' do + before do + Ci::Build::AVAILABLE_STATUSES.each do |status| + create_build(status, status) + end + + RequestStore.begin! + end + + after do + RequestStore.end! + RequestStore.clear! + end + + it "verifies number of queries" do + recorded = ActiveRecord::QueryRecorder.new { get_index } + expect(recorded.count).to be_within(5).of(8) + end + + def create_build(name, status) + pipeline = create(:ci_pipeline, project: project) + create(:ci_build, :tags, :triggered, :artifacts, + pipeline: pipeline, name: name, status: status) + end + end + + def get_index(**extra_params) + params = { + namespace_id: project.namespace.to_param, + project_id: project + } + + get :index, params.merge(extra_params) + end + end + + describe 'GET show' do + context 'when build exists' do + let!(:build) { create(:ci_build, pipeline: pipeline) } + + before do + get_show(id: build.id) + end + + it 'has a build' do + expect(response).to have_http_status(:ok) + expect(assigns(:build).id).to eq(build.id) + end + end + + context 'when build does not exist' do + before do + get_show(id: 1234) + end + + it 'renders not_found' do + expect(response).to have_http_status(:not_found) + end + end + + def get_show(**extra_params) + params = { + namespace_id: project.namespace.to_param, + project_id: project + } + + get :show, params.merge(extra_params) + end + end + + describe 'GET trace.json' do + before do + get_trace + end + + context 'when build has a trace' do + let(:build) { create(:ci_build, :trace, pipeline: pipeline) } + + it 'returns a trace' do + expect(response).to have_http_status(:ok) + expect(json_response['html']).to eq('BUILD TRACE') + end + end + + context 'when build has no traces' do + let(:build) { create(:ci_build, pipeline: pipeline) } + + it 'returns no traces' do + expect(response).to have_http_status(:ok) + expect(json_response['html']).to be_nil + end + end + + def get_trace + get :trace, namespace_id: project.namespace, + project_id: project, + id: build.id, + format: :json + end end describe 'GET status.json' do - let(:pipeline) { create(:ci_pipeline, project: project) } let(:build) { create(:ci_build, pipeline: pipeline) } let(:status) { build.detailed_status(double('user')) } @@ -27,7 +181,266 @@ describe Projects::BuildsController do 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 eq status.favicon + expect(json_response['favicon']).to eq "/assets/ci_favicons/#{status.favicon}.ico" + end + end + + describe 'GET trace.json' do + let(:pipeline) { create(:ci_pipeline, project: project) } + let(:build) { create(:ci_build, pipeline: pipeline) } + let(:user) { create(:user) } + + context 'when user is logged in as developer' do + before do + project.add_developer(user) + sign_in(user) + + get_trace + end + + it 'traces build log' do + expect(response).to have_http_status(:ok) + expect(json_response['id']).to eq build.id + expect(json_response['status']).to eq build.status + end + end + + context 'when user is logged in as non member' do + before do + sign_in(user) + + get_trace + end + + it 'traces build log' do + expect(response).to have_http_status(:ok) + expect(json_response['id']).to eq build.id + expect(json_response['status']).to eq build.status + end + end + + def get_trace + get :trace, namespace_id: project.namespace, + project_id: project, + id: build.id, + format: :json + end + end + + describe 'POST retry' do + before do + project.add_developer(user) + sign_in(user) + + post_retry + end + + context 'when build is retryable' do + let(:build) { create(:ci_build, :retryable, pipeline: pipeline) } + + it 'redirects to the retried build page' do + expect(response).to have_http_status(:found) + expect(response).to redirect_to(namespace_project_build_path(id: Ci::Build.last.id)) + end + end + + context 'when build is not retryable' do + let(:build) { create(:ci_build, pipeline: pipeline) } + + it 'renders unprocessable_entity' do + expect(response).to have_http_status(:unprocessable_entity) + end + end + + def post_retry + post :retry, namespace_id: project.namespace, + project_id: project, + id: build.id + end + end + + describe 'POST play' do + before do + project.add_master(user) + sign_in(user) + + post_play + end + + context 'when build is playable' do + let(:build) { create(:ci_build, :playable, pipeline: pipeline) } + + it 'redirects to the played build page' do + expect(response).to have_http_status(:found) + expect(response).to redirect_to(namespace_project_build_path(id: build.id)) + end + + it 'transits to pending' do + expect(build.reload).to be_pending + end + end + + context 'when build is not playable' do + let(:build) { create(:ci_build, pipeline: pipeline) } + + it 'renders unprocessable_entity' do + expect(response).to have_http_status(:unprocessable_entity) + end + end + + def post_play + post :play, namespace_id: project.namespace, + project_id: project, + id: build.id + end + end + + describe 'POST cancel' do + before do + project.add_developer(user) + sign_in(user) + + post_cancel + end + + context 'when build is cancelable' do + let(:build) { create(:ci_build, :cancelable, pipeline: pipeline) } + + it 'redirects to the canceled build page' do + expect(response).to have_http_status(:found) + expect(response).to redirect_to(namespace_project_build_path(id: build.id)) + end + + it 'transits to canceled' do + expect(build.reload).to be_canceled + end + end + + context 'when build is not cancelable' do + let(:build) { create(:ci_build, :canceled, pipeline: pipeline) } + + it 'returns unprocessable_entity' do + expect(response).to have_http_status(:unprocessable_entity) + end + end + + def post_cancel + post :cancel, namespace_id: project.namespace, + project_id: project, + id: build.id + end + end + + describe 'POST cancel_all' do + before do + project.add_developer(user) + sign_in(user) + end + + context 'when builds are cancelable' do + before do + create_list(:ci_build, 2, :cancelable, pipeline: pipeline) + + post_cancel_all + end + + it 'redirects to a index page' do + expect(response).to have_http_status(:found) + expect(response).to redirect_to(namespace_project_builds_path) + end + + it 'transits to canceled' do + expect(Ci::Build.all).to all(be_canceled) + end + end + + context 'when builds are not cancelable' do + before do + create_list(:ci_build, 2, :canceled, pipeline: pipeline) + + post_cancel_all + end + + it 'redirects to a index page' do + expect(response).to have_http_status(:found) + expect(response).to redirect_to(namespace_project_builds_path) + end + end + + def post_cancel_all + post :cancel_all, namespace_id: project.namespace, + project_id: project + end + end + + describe 'POST erase' do + before do + project.add_developer(user) + sign_in(user) + + post_erase + end + + context 'when build is erasable' do + let(:build) { create(:ci_build, :erasable, :trace, pipeline: pipeline) } + + it 'redirects to the erased build page' do + expect(response).to have_http_status(:found) + expect(response).to redirect_to(namespace_project_build_path(id: build.id)) + end + + it 'erases artifacts' do + expect(build.artifacts_file.exists?).to be_falsey + expect(build.artifacts_metadata.exists?).to be_falsey + end + + it 'erases trace' do + expect(build.trace.exist?).to be_falsey + end + end + + context 'when build is not erasable' do + let(:build) { create(:ci_build, :erased, pipeline: pipeline) } + + it 'returns unprocessable_entity' do + expect(response).to have_http_status(:unprocessable_entity) + end + end + + def post_erase + post :erase, namespace_id: project.namespace, + project_id: project, + id: build.id + end + end + + describe 'GET raw' do + before do + get_raw + end + + context 'when build has a trace file' do + let(:build) { create(:ci_build, :trace, pipeline: pipeline) } + + it 'send a trace file' do + expect(response).to have_http_status(:ok) + expect(response.content_type).to eq 'text/plain; charset=utf-8' + expect(response.body).to eq 'BUILD TRACE' + end + end + + context 'when build does not have a trace file' do + let(:build) { create(:ci_build, pipeline: pipeline) } + + it 'returns not_found' do + expect(response).to have_http_status(:not_found) + end + end + + def get_raw + post :raw, namespace_id: project.namespace, + project_id: project, + id: build.id end end end diff --git a/spec/controllers/projects/builds_controller_specs.rb b/spec/controllers/projects/builds_controller_specs.rb deleted file mode 100644 index d501f7b3155..00000000000 --- a/spec/controllers/projects/builds_controller_specs.rb +++ /dev/null @@ -1,47 +0,0 @@ -require 'spec_helper' - -describe Projects::BuildsController do - include ApiHelpers - - let(:project) { create(:empty_project, :public) } - - describe 'GET trace.json' do - let(:pipeline) { create(:ci_pipeline, project: project) } - let(:build) { create(:ci_build, pipeline: pipeline) } - let(:user) { create(:user) } - - context 'when user is logged in as developer' do - before do - project.add_developer(user) - sign_in(user) - get_trace - end - - it 'traces build log' do - expect(response).to have_http_status(:ok) - expect(json_response['id']).to eq build.id - expect(json_response['status']).to eq build.status - end - end - - context 'when user is logged in as non member' do - before do - sign_in(user) - get_trace - end - - it 'traces build log' do - expect(response).to have_http_status(:ok) - expect(json_response['id']).to eq build.id - expect(json_response['status']).to eq build.status - end - end - - def get_trace - get :trace, namespace_id: project.namespace, - project_id: project, - id: build.id, - format: :json - end - end -end diff --git a/spec/controllers/projects/commit_controller_spec.rb b/spec/controllers/projects/commit_controller_spec.rb index b223a22ae60..69e4706dc71 100644 --- a/spec/controllers/projects/commit_controller_spec.rb +++ b/spec/controllers/projects/commit_controller_spec.rb @@ -266,8 +266,8 @@ describe Projects::CommitController do diff_for_path(id: commit2.id, old_path: existing_path, new_path: existing_path) expect(assigns(:diff_notes_disabled)).to be_falsey - expect(assigns(:comments_target)).to eq(noteable_type: 'Commit', - commit_id: commit2.id) + expect(assigns(:new_diff_note_attrs)).to eq(noteable_type: 'Commit', + commit_id: commit2.id) end it 'only renders the diffs for the path given' do diff --git a/spec/controllers/projects/deploy_keys_controller_spec.rb b/spec/controllers/projects/deploy_keys_controller_spec.rb new file mode 100644 index 00000000000..efe1a78415b --- /dev/null +++ b/spec/controllers/projects/deploy_keys_controller_spec.rb @@ -0,0 +1,66 @@ +require 'spec_helper' + +describe Projects::DeployKeysController do + let(:project) { create(:project, :repository) } + let(:user) { create(:user) } + + before do + project.team << [user, :master] + + sign_in(user) + end + + describe 'GET index' do + let(:params) do + { namespace_id: project.namespace, project_id: project } + end + + context 'when html requested' do + it 'redirects to blob' do + get :index, params + + expect(response).to redirect_to(namespace_project_settings_repository_path(params)) + end + end + + context 'when json requested' do + let(:project2) { create(:empty_project, :internal)} + let(:project_private) { create(:empty_project, :private)} + + let(:deploy_key_internal) do + create(:deploy_key, key: 'ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAAAgQCdMHEHyhRjbhEZVddFn6lTWdgEy5Q6Bz4nwGB76xWZI5YT/1WJOMEW+sL5zYd31kk7sd3FJ5L9ft8zWMWrr/iWXQikC2cqZK24H1xy+ZUmrRuJD4qGAaIVoyyzBL+avL+lF8J5lg6YSw8gwJY/lX64/vnJHUlWw2n5BF8IFOWhiw== dummy@gitlab.com') + end + let(:deploy_key_actual) do + create(:deploy_key, key: 'ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAAAgQDNd/UJWhPrpb+b/G5oL109y57yKuCxE+WUGJGYaj7WQKsYRJmLYh1mgjrl+KVyfsWpq4ylOxIfFSnN9xBBFN8mlb0Fma5DC7YsSsibJr3MZ19ZNBprwNcdogET7aW9I0In7Wu5f2KqI6e5W/spJHCy4JVxzVMUvk6Myab0LnJ2iQ== dummy@gitlab.com') + end + let!(:deploy_key_public) { create(:deploy_key, public: true) } + + let!(:deploy_keys_project_internal) do + create(:deploy_keys_project, project: project2, deploy_key: deploy_key_internal) + end + + let!(:deploy_keys_actual_project) do + create(:deploy_keys_project, project: project, deploy_key: deploy_key_actual) + end + + let!(:deploy_keys_project_private) do + create(:deploy_keys_project, project: project_private, deploy_key: create(:another_deploy_key)) + end + + before do + project2.team << [user, :developer] + end + + it 'returns json in a correct format' do + get :index, params.merge(format: :json) + + json = JSON.parse(response.body) + + expect(json.keys).to match_array(%w(enabled_keys available_project_keys public_keys)) + expect(json['enabled_keys'].count).to eq(1) + expect(json['available_project_keys'].count).to eq(1) + expect(json['public_keys'].count).to eq(1) + end + end + end +end diff --git a/spec/controllers/projects/deployments_controller_spec.rb b/spec/controllers/projects/deployments_controller_spec.rb new file mode 100644 index 00000000000..4c69443314d --- /dev/null +++ b/spec/controllers/projects/deployments_controller_spec.rb @@ -0,0 +1,116 @@ +require 'spec_helper' + +describe Projects::DeploymentsController do + include ApiHelpers + + let(:user) { create(:user) } + let(:project) { create(:empty_project) } + let(:environment) { create(:environment, name: 'production', project: project) } + + before do + project.team << [user, :master] + + sign_in(user) + end + + describe 'GET #index' do + it 'returns list of deployments from last 8 hours' do + create(:deployment, environment: environment, created_at: 9.hours.ago) + create(:deployment, environment: environment, created_at: 7.hours.ago) + create(:deployment, environment: environment) + + get :index, deployment_params(after: 8.hours.ago) + + expect(response).to be_ok + + expect(json_response['deployments'].count).to eq(2) + end + + it 'returns a list with deployments information' do + create(:deployment, environment: environment) + + get :index, deployment_params + + expect(response).to be_ok + expect(response).to match_response_schema('deployments') + end + end + + describe 'GET #metrics' do + let(:deployment) { create(:deployment, project: project, environment: environment) } + + before do + allow(controller).to receive(:deployment).and_return(deployment) + end + context 'when metrics are disabled' do + before do + allow(deployment).to receive(:has_metrics?).and_return false + end + + it 'responds with not found' do + get :metrics, deployment_params(id: deployment.id) + + expect(response).to be_not_found + end + end + + context 'when metrics are enabled' do + before do + allow(deployment).to receive(:has_metrics?).and_return true + end + + context 'when environment has no metrics' do + before do + expect(deployment).to receive(:metrics).and_return(nil) + end + + it 'returns a empty response 204 resposne' do + get :metrics, deployment_params(id: deployment.id) + expect(response).to have_http_status(204) + expect(response.body).to eq('') + end + end + + context 'when environment has some metrics' do + let(:empty_metrics) do + { + success: true, + metrics: {}, + last_update: 42 + } + end + + before do + expect(deployment).to receive(:metrics).and_return(empty_metrics) + end + + it 'returns a metrics JSON document' do + get :metrics, deployment_params(id: deployment.id) + + expect(response).to be_ok + expect(json_response['success']).to be(true) + expect(json_response['metrics']).to eq({}) + expect(json_response['last_update']).to eq(42) + end + end + + context 'when metrics service does not implement deployment metrics' do + before do + allow(deployment).to receive(:metrics).and_raise(NotImplementedError) + end + + it 'responds with not found' do + get :metrics, deployment_params(id: deployment.id) + + expect(response).to be_not_found + end + end + end + end + + def deployment_params(opts = {}) + opts.reverse_merge(namespace_id: project.namespace, + project_id: project, + environment_id: environment.id) + end +end diff --git a/spec/controllers/projects/discussions_controller_spec.rb b/spec/controllers/projects/discussions_controller_spec.rb index 79ab364a6f3..fe62898fa9b 100644 --- a/spec/controllers/projects/discussions_controller_spec.rb +++ b/spec/controllers/projects/discussions_controller_spec.rb @@ -4,7 +4,7 @@ describe Projects::DiscussionsController do let(:user) { create(:user) } let(:merge_request) { create(:merge_request) } let(:project) { merge_request.source_project } - let(:note) { create(:diff_note_on_merge_request, noteable: merge_request, project: project) } + let(:note) { create(:discussion_note_on_merge_request, noteable: merge_request, project: project) } let(:discussion) { note.discussion } let(:request_params) do diff --git a/spec/controllers/projects/environments_controller_spec.rb b/spec/controllers/projects/environments_controller_spec.rb index 5525fbd8130..c0f8c36a018 100644 --- a/spec/controllers/projects/environments_controller_spec.rb +++ b/spec/controllers/projects/environments_controller_spec.rb @@ -1,8 +1,6 @@ require 'spec_helper' describe Projects::EnvironmentsController do - include ApiHelpers - let(:user) { create(:user) } let(:project) { create(:empty_project) } @@ -151,6 +149,48 @@ describe Projects::EnvironmentsController do end end + describe 'PATCH #stop' do + context 'when env not available' do + it 'returns 404' do + allow_any_instance_of(Environment).to receive(:available?) { false } + + patch :stop, environment_params(format: :json) + + expect(response).to have_http_status(404) + end + end + + context 'when stop action' do + it 'returns action url' do + action = create(:ci_build, :manual) + + allow_any_instance_of(Environment) + .to receive_messages(available?: true, stop_with_action!: action) + + patch :stop, environment_params(format: :json) + + expect(response).to have_http_status(200) + expect(json_response).to eq( + { 'redirect_url' => + "http://test.host/#{project.path_with_namespace}/builds/#{action.id}" }) + end + end + + context 'when no stop action' do + it 'returns env url' do + allow_any_instance_of(Environment) + .to receive_messages(available?: true, stop_with_action!: nil) + + patch :stop, environment_params(format: :json) + + expect(response).to have_http_status(200) + expect(json_response).to eq( + { 'redirect_url' => + "http://test.host/#{project.path_with_namespace}/environments/#{environment.id}" }) + end + end + end + describe 'GET #terminal' do context 'with valid id' do it 'responds with a status code 200' do diff --git a/spec/controllers/projects/imports_controller_spec.rb b/spec/controllers/projects/imports_controller_spec.rb index 7c75815f3c4..6724b474179 100644 --- a/spec/controllers/projects/imports_controller_spec.rb +++ b/spec/controllers/projects/imports_controller_spec.rb @@ -96,12 +96,19 @@ describe Projects::ImportsController do } end - it 'redirects to params[:to]' do + it 'redirects to internal params[:to]' do get :show, namespace_id: project.namespace.to_param, project_id: project, continue: params expect(flash[:notice]).to eq params[:notice] expect(response).to redirect_to params[:to] end + + it 'does not redirect to external params[:to]' do + params[:to] = "//google.com" + + get :show, namespace_id: project.namespace.to_param, project_id: project, continue: params + expect(response).not_to redirect_to params[:to] + end end end diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index 734966d50b2..04afd07c59e 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -83,6 +83,17 @@ describe Projects::IssuesController do expect(assigns(:issues).current_page).to eq(last_page) expect(response).to have_http_status(200) end + + it 'does not redirect to external sites when provided a host field' do + external_host = "www.example.com" + get :index, + namespace_id: project.namespace.to_param, + project_id: project, + page: (last_page + 1).to_param, + host: external_host + + expect(response).to redirect_to(namespace_project_issues_path(page: last_page, state: controller.params[:state], scope: controller.params[:scope])) + end end end @@ -145,6 +156,32 @@ describe Projects::IssuesController do end end + describe 'Redirect after sign in' do + context 'with an AJAX request' do + it 'does not store the visited URL' do + xhr :get, + :show, + format: :json, + namespace_id: project.namespace, + project_id: project, + id: issue.iid + + expect(session['user_return_to']).to be_blank + end + end + + context 'without an AJAX request' do + it 'stores the visited URL' do + get :show, + namespace_id: project.namespace.to_param, + project_id: project, + id: issue.iid + + expect(session['user_return_to']).to eq("/#{project.namespace.to_param}/#{project.to_param}/issues/#{issue.iid}") + end + end + end + describe 'PUT #update' do before do sign_in(user) @@ -162,12 +199,12 @@ describe Projects::IssuesController do namespace_id: project.namespace.to_param, project_id: project, id: issue.iid, - issue: { assignee_id: assignee.id }, + issue: { assignee_ids: [assignee.id] }, format: :json body = JSON.parse(response.body) - expect(body['assignee'].keys) - .to match_array(%w(name username avatar_url)) + expect(body['assignees'].first.keys) + .to match_array(%w(id name username avatar_url)) end end @@ -337,7 +374,7 @@ describe Projects::IssuesController do let(:admin) { create(:admin) } let!(:issue) { create(:issue, project: project) } let!(:unescaped_parameter_value) { create(:issue, :confidential, project: project, author: author) } - let!(:request_forgery_timing_attack) { create(:issue, :confidential, project: project, assignee: assignee) } + let!(:request_forgery_timing_attack) { create(:issue, :confidential, project: project, assignees: [assignee]) } describe 'GET #index' do it 'does not list confidential issues for guests' do @@ -508,7 +545,7 @@ describe Projects::IssuesController do end context 'resolving discussions in MergeRequest' do - let(:discussion) { Discussion.for_diff_notes([create(:diff_note_on_merge_request)]).first } + let(:discussion) { create(:diff_note_on_merge_request).to_discussion } let(:merge_request) { discussion.noteable } let(:project) { merge_request.source_project } @@ -745,4 +782,28 @@ describe Projects::IssuesController do expect(response).to have_http_status(200) end end + + describe 'POST create_merge_request' do + before do + project.add_developer(user) + sign_in(user) + end + + it 'creates a new merge request' do + expect { create_merge_request }.to change(project.merge_requests, :count).by(1) + end + + it 'render merge request as json' do + create_merge_request + + expect(response).to match_response_schema('merge_request') + end + + def create_merge_request + post :create_merge_request, namespace_id: project.namespace.to_param, + project_id: project.to_param, + id: issue.to_param, + format: :json + end + end end diff --git a/spec/controllers/projects/labels_controller_spec.rb b/spec/controllers/projects/labels_controller_spec.rb index 6a6e9bf378a..130b0b744b5 100644 --- a/spec/controllers/projects/labels_controller_spec.rb +++ b/spec/controllers/projects/labels_controller_spec.rb @@ -127,7 +127,7 @@ describe Projects::LabelsController do context 'group owner' do before do - GroupMember.add_users_to_group(group, [user], :owner) + GroupMember.add_users(group, [user], :owner) end it 'gives access' do @@ -157,4 +157,74 @@ describe Projects::LabelsController do end end end + + describe '#ensure_canonical_path' do + before do + sign_in(user) + end + + context 'for a GET request' do + context 'when requesting the canonical path' do + context 'non-show path' do + context 'with exactly matching casing' do + it 'does not redirect' do + get :index, namespace_id: project.namespace, project_id: project.to_param + + expect(response).not_to have_http_status(301) + end + end + + context 'with different casing' do + it 'redirects to the correct casing' do + get :index, namespace_id: project.namespace, project_id: project.to_param.upcase + + expect(response).to redirect_to(namespace_project_labels_path(project.namespace, project)) + expect(controller).not_to set_flash[:notice] + end + end + end + end + + context 'when requesting a redirected path' do + let!(:redirect_route) { project.redirect_routes.create(path: project.full_path + 'old') } + + it 'redirects to the canonical path' do + get :index, namespace_id: project.namespace, project_id: project.to_param + 'old' + + expect(response).to redirect_to(namespace_project_labels_path(project.namespace, project)) + expect(controller).to set_flash[:notice].to(project_moved_message(redirect_route, project)) + end + end + end + end + + context 'for a non-GET request' do + context 'when requesting the canonical path with different casing' do + it 'does not 404' do + post :generate, namespace_id: project.namespace, project_id: project + + expect(response).not_to have_http_status(404) + end + + it 'does not redirect to the correct casing' do + post :generate, namespace_id: project.namespace, project_id: project + + expect(response).not_to have_http_status(301) + end + end + + context 'when requesting a redirected path' do + let!(:redirect_route) { project.redirect_routes.create(path: project.full_path + 'old') } + + it 'returns not found' do + post :generate, namespace_id: project.namespace, project_id: project.to_param + 'old' + + expect(response).to have_http_status(404) + end + end + end + + def project_moved_message(redirect_route, project) + "Project '#{redirect_route.path}' was moved to '#{project.full_path}'. Please update any links and bookmarks that may still have the old path." + end end diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index 72f41f7209a..f0dc6df15ee 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -1,8 +1,6 @@ require 'spec_helper' describe Projects::MergeRequestsController do - include ApiHelpers - let(:project) { create(:project) } let(:user) { create(:user) } let(:merge_request) { create(:merge_request_with_diffs, target_project: project, source_project: project) } @@ -61,6 +59,18 @@ describe Projects::MergeRequestsController do end end + describe 'GET commit_change_content' do + it 'renders commit_change_content template' do + get :commit_change_content, + namespace_id: project.namespace.to_param, + project_id: project, + id: merge_request.iid, + format: 'html' + + expect(response).to render_template('_commit_change_content') + end + end + shared_examples "loads labels" do |action| it "loads labels into the @labels variable" do get action, @@ -73,63 +83,59 @@ describe Projects::MergeRequestsController do end describe "GET show" do - shared_examples "export merge as" do |format| - it "does generally work" do - get(:show, - namespace_id: project.namespace.to_param, - project_id: project, - id: merge_request.iid, - format: format) + def go(extra_params = {}) + params = { + namespace_id: project.namespace.to_param, + project_id: project, + id: merge_request.iid + } + + get :show, params.merge(extra_params) + end + + it_behaves_like "loads labels", :show + + describe 'as html' do + it "renders merge request page" do + go(format: :html) expect(response).to be_success end + end - it_behaves_like "loads labels", :show - - it "generates it" do - expect_any_instance_of(MergeRequest).to receive(:"to_#{format}") + describe 'as json' do + context 'with basic param' do + it 'renders basic MR entity as json' do + go(basic: true, format: :json) - get(:show, - namespace_id: project.namespace.to_param, - project_id: project, - id: merge_request.iid, - format: format) + expect(response).to match_response_schema('entities/merge_request_basic') + end end - it "renders it" do - get(:show, - namespace_id: project.namespace.to_param, - project_id: project, - id: merge_request.iid, - format: format) + context 'without basic param' do + it 'renders the merge request in the json format' do + go(format: :json) - expect(response.body).to eq(merge_request.send(:"to_#{format}").to_s) + expect(response).to match_response_schema('entities/merge_request') + end end - it "does not escape Html" do - allow_any_instance_of(MergeRequest).to receive(:"to_#{format}"). - and_return('HTML entities &<>" ') + context 'number of queries' do + it 'verifies number of queries' do + # pre-create objects + merge_request - get(:show, - namespace_id: project.namespace.to_param, - project_id: project, - id: merge_request.iid, - format: format) + recorded = ActiveRecord::QueryRecorder.new { go(format: :json) } - expect(response.body).not_to include('&') - expect(response.body).not_to include('>') - expect(response.body).not_to include('<') - expect(response.body).not_to include('"') + expect(recorded.count).to be_within(1).of(51) + expect(recorded.cached_count).to eq(0) + end end end describe "as diff" do it "triggers workhorse to serve the request" do - get(:show, - namespace_id: project.namespace.to_param, - project_id: project, - id: merge_request.iid, - format: :diff) + go(format: :diff) expect(response.headers[Gitlab::Workhorse::SEND_DATA_HEADER]).to start_with("git-diff:") end @@ -137,11 +143,7 @@ describe Projects::MergeRequestsController do describe "as patch" do it 'triggers workhorse to serve the request' do - get(:show, - namespace_id: project.namespace.to_param, - project_id: project, - id: merge_request.iid, - format: :patch) + go(format: :patch) expect(response.headers[Gitlab::Workhorse::SEND_DATA_HEADER]).to start_with("git-format-patch:") end @@ -176,6 +178,18 @@ describe Projects::MergeRequestsController do expect(assigns(:merge_requests).current_page).to eq(last_page) expect(response).to have_http_status(200) end + + it 'does not redirect to external sites when provided a host field' do + external_host = "www.example.com" + get :index, + namespace_id: project.namespace.to_param, + project_id: project, + state: 'opened', + page: (last_page + 1).to_param, + host: external_host + + expect(response).to redirect_to(namespace_project_merge_requests_path(page: last_page, state: controller.params[:state], scope: controller.params[:scope])) + end end context 'when filtering by opened state' do @@ -285,19 +299,18 @@ describe Projects::MergeRequestsController do namespace_id: project.namespace, project_id: project, id: merge_request.iid, - format: 'raw' + format: 'json' } end - context 'when the user does not have access' do + context 'when user cannot access' do before do - project.team.truncate - project.team << [user, :reporter] - post :merge, base_params + project.add_reporter(user) + xhr :post, :merge, base_params end - it 'returns not found' do - expect(response).to be_not_found + it 'returns 404' do + expect(response).to have_http_status(404) end end @@ -309,7 +322,7 @@ describe Projects::MergeRequestsController do end it 'returns :failed' do - expect(assigns(:status)).to eq(:failed) + expect(json_response).to eq('status' => 'failed') end end @@ -317,7 +330,7 @@ describe Projects::MergeRequestsController do before { post :merge, base_params.merge(sha: 'foo') } it 'returns :sha_mismatch' do - expect(assigns(:status)).to eq(:sha_mismatch) + expect(json_response).to eq('status' => 'sha_mismatch') end end @@ -329,7 +342,7 @@ describe Projects::MergeRequestsController do it 'returns :success' do merge_with_sha - expect(assigns(:status)).to eq(:success) + expect(json_response).to eq('status' => 'success') end it 'starts the merge immediately' do @@ -344,13 +357,14 @@ describe Projects::MergeRequestsController do end before do - create(:ci_empty_pipeline, project: project, sha: merge_request.diff_head_sha, ref: merge_request.source_branch) + pipeline = create(:ci_empty_pipeline, project: project, sha: merge_request.diff_head_sha, ref: merge_request.source_branch) + merge_request.update(head_pipeline: pipeline) end it 'returns :merge_when_pipeline_succeeds' do merge_when_pipeline_succeeds - expect(assigns(:status)).to eq(:merge_when_pipeline_succeeds) + expect(json_response).to eq('status' => 'merge_when_pipeline_succeeds') end it 'sets the MR to merge when the pipeline succeeds' do @@ -372,7 +386,7 @@ describe Projects::MergeRequestsController do it 'returns :merge_when_pipeline_succeeds' do merge_when_pipeline_succeeds - expect(assigns(:status)).to eq(:merge_when_pipeline_succeeds) + expect(json_response).to eq('status' => 'merge_when_pipeline_succeeds') end end end @@ -393,7 +407,7 @@ describe Projects::MergeRequestsController do it 'returns :failed' do merge_with_sha - expect(assigns(:status)).to eq(:failed) + expect(json_response).to eq('status' => 'failed') end end @@ -406,7 +420,7 @@ describe Projects::MergeRequestsController do it 'returns :success' do merge_with_sha - expect(assigns(:status)).to eq(:success) + expect(json_response).to eq('status' => 'success') end end end @@ -424,7 +438,7 @@ describe Projects::MergeRequestsController do it 'returns :success' do merge_with_sha - expect(assigns(:status)).to eq(:success) + expect(json_response).to eq('status' => 'success') end end @@ -437,7 +451,7 @@ describe Projects::MergeRequestsController do it 'returns :success' do merge_with_sha - expect(assigns(:status)).to eq(:success) + expect(json_response).to eq('status' => 'success') end end end @@ -574,8 +588,8 @@ describe Projects::MergeRequestsController do diff_for_path(id: merge_request.iid, old_path: existing_path, new_path: existing_path) expect(assigns(:diff_notes_disabled)).to be_falsey - expect(assigns(:comments_target)).to eq(noteable_type: 'MergeRequest', - noteable_id: merge_request.id) + expect(assigns(:new_diff_note_attrs)).to eq(noteable_type: 'MergeRequest', + noteable_id: merge_request.id) end it 'only renders the diffs for the path given' do @@ -821,18 +835,55 @@ describe Projects::MergeRequestsController do end end - context 'POST remove_wip' do - it 'removes the wip status' do + describe 'POST remove_wip' do + before do merge_request.title = merge_request.wip_title merge_request.save - post :remove_wip, - namespace_id: merge_request.project.namespace.to_param, - project_id: merge_request.project, - id: merge_request.iid + xhr :post, :remove_wip, + namespace_id: merge_request.project.namespace.to_param, + project_id: merge_request.project, + id: merge_request.iid, + format: :json + end + it 'removes the wip status' do expect(merge_request.reload.title).to eq(merge_request.wipless_title) end + + it 'renders MergeRequest as JSON' do + expect(json_response.keys).to include('id', 'iid', 'description') + end + end + + describe 'POST cancel_merge_when_pipeline_succeeds' do + subject do + xhr :post, :cancel_merge_when_pipeline_succeeds, + namespace_id: merge_request.project.namespace.to_param, + project_id: merge_request.project, + id: merge_request.iid, + format: :json + end + + it 'calls MergeRequests::MergeWhenPipelineSucceedsService' do + mwps_service = double + + allow(MergeRequests::MergeWhenPipelineSucceedsService) + .to receive(:new) + .and_return(mwps_service) + + expect(mwps_service).to receive(:cancel).with(merge_request) + + subject + end + + it { is_expected.to have_http_status(:success) } + + it 'renders MergeRequest as JSON' do + subject + + expect(json_response.keys).to include('id', 'iid', 'description') + end end describe 'GET conflict_for_path' do @@ -877,7 +928,9 @@ describe Projects::MergeRequestsController do end it 'returns the file in JSON format' do - content = merge_request_with_conflicts.conflicts.file_for_path(path, path).content + content = MergeRequests::Conflicts::ListService.new(merge_request_with_conflicts). + file_for_path(path, path). + content expect(json_response).to include('old_path' => path, 'new_path' => path, @@ -1001,11 +1054,15 @@ describe Projects::MergeRequestsController do context 'when a file has identical content to the conflict' do before do + content = MergeRequests::Conflicts::ListService.new(merge_request_with_conflicts). + file_for_path('files/ruby/popen.rb', 'files/ruby/popen.rb'). + content + resolved_files = [ { 'new_path' => 'files/ruby/popen.rb', 'old_path' => 'files/ruby/popen.rb', - 'content' => merge_request_with_conflicts.conflicts.file_for_path('files/ruby/popen.rb', 'files/ruby/popen.rb').content + 'content' => content }, { 'new_path' => 'files/ruby/regex.rb', 'old_path' => 'files/ruby/regex.rb', @@ -1057,7 +1114,7 @@ describe Projects::MergeRequestsController do end it 'correctly pluralizes flash message on success' do - issue2.update!(assignee: user) + issue2.assignees = [user] post_assign_issues @@ -1111,74 +1168,6 @@ describe Projects::MergeRequestsController do end end - describe 'GET merge_widget_refresh' do - let(:params) do - { - namespace_id: project.namespace, - project_id: project, - id: merge_request.iid, - format: :raw - } - end - - before do - project.team << [user, :developer] - xhr :get, :merge_widget_refresh, params - end - - context 'when merge in progress' do - let(:merge_request) { create(:merge_request, source_project: project, in_progress_merge_commit_sha: 'sha') } - - it 'returns an OK response' do - expect(response).to have_http_status(:ok) - end - - it 'sets status to :success' do - expect(assigns(:status)).to eq(:success) - expect(response).to render_template('merge') - end - end - - context 'when merge request was merged already' do - let(:merge_request) { create(:merge_request, source_project: project, state: :merged) } - - it 'returns an OK response' do - expect(response).to have_http_status(:ok) - end - - it 'sets status to :success' do - expect(assigns(:status)).to eq(:success) - expect(response).to render_template('merge') - end - end - - context 'when waiting for build' do - let(:merge_request) { create(:merge_request, source_project: project, merge_when_pipeline_succeeds: true, merge_user: user) } - - it 'returns an OK response' do - expect(response).to have_http_status(:ok) - end - - it 'sets status to :merge_when_pipeline_succeeds' do - expect(assigns(:status)).to eq(:merge_when_pipeline_succeeds) - expect(response).to render_template('merge') - end - end - - context 'when MR does not have special state' do - let(:merge_request) { create(:merge_request, source_project: project) } - - it 'returns an OK response' do - expect(response).to have_http_status(:ok) - end - - it 'sets status to success' do - expect(assigns(:status)).to eq(:success) - expect(response).to render_template('merge') - end - end - end - describe 'GET pipeline_status.json' do context 'when head_pipeline exists' do let!(:pipeline) do @@ -1189,14 +1178,17 @@ describe Projects::MergeRequestsController do let(:status) { pipeline.detailed_status(double('user')) } - before { get_pipeline_status } + before do + merge_request.update(head_pipeline: pipeline) + get_pipeline_status + end it 'return a detailed head_pipeline status in json' do expect(response).to have_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 eq status.favicon + expect(json_response['favicon']).to eq "/assets/ci_favicons/#{status.favicon}.ico" end end diff --git a/spec/controllers/projects/milestones_controller_spec.rb b/spec/controllers/projects/milestones_controller_spec.rb index 14207bf6b7a..84a61b2784e 100644 --- a/spec/controllers/projects/milestones_controller_spec.rb +++ b/spec/controllers/projects/milestones_controller_spec.rb @@ -5,7 +5,9 @@ describe Projects::MilestonesController do let(:user) { create(:user) } let(:milestone) { create(:milestone, project: project) } let(:issue) { create(:issue, project: project, milestone: milestone) } + let!(:label) { create(:label, project: project, title: 'Issue Label', issues: [issue]) } let!(:merge_request) { create(:merge_request, source_project: project, target_project: project, milestone: milestone) } + let(:milestone_path) { namespace_project_milestone_path } before do sign_in(user) @@ -13,6 +15,22 @@ describe Projects::MilestonesController do controller.instance_variable_set(:@project, project) end + it_behaves_like 'milestone tabs' + + describe "#show" do + render_views + + def view_milestone + get :show, namespace_id: project.namespace.id, project_id: project.id, id: milestone.iid + end + + it 'shows milestone page' do + view_milestone + + expect(response).to have_http_status(200) + end + end + describe "#destroy" do it "removes milestone" do expect(issue.milestone_id).to eq(milestone.id) diff --git a/spec/controllers/projects/notes_controller_spec.rb b/spec/controllers/projects/notes_controller_spec.rb index d80780b1d90..45f4cf9180d 100644 --- a/spec/controllers/projects/notes_controller_spec.rb +++ b/spec/controllers/projects/notes_controller_spec.rb @@ -14,6 +14,109 @@ describe Projects::NotesController do } end + describe 'GET index' do + let(:request_params) do + { + namespace_id: project.namespace, + project_id: project, + target_type: 'issue', + target_id: issue.id, + format: 'json' + } + end + + let(:parsed_response) { JSON.parse(response.body).with_indifferent_access } + let(:note_json) { parsed_response[:notes].first } + + before do + sign_in(user) + project.team << [user, :developer] + end + + it 'passes last_fetched_at from headers to NotesFinder' do + last_fetched_at = 3.hours.ago.to_i + + request.headers['X-Last-Fetched-At'] = last_fetched_at + + expect(NotesFinder).to receive(:new) + .with(anything, anything, hash_including(last_fetched_at: last_fetched_at)) + .and_call_original + + get :index, request_params + end + + context 'for a discussion note' do + let!(:note) { create(:discussion_note_on_issue, noteable: issue, project: project) } + + it 'responds with the expected attributes' do + get :index, request_params + + expect(note_json[:id]).to eq(note.id) + expect(note_json[:discussion_html]).not_to be_nil + expect(note_json[:diff_discussion_html]).to be_nil + end + end + + context 'for a diff discussion note' do + let(:project) { create(:project, :repository) } + let!(:note) { create(:diff_note_on_merge_request, project: project) } + + let(:params) { request_params.merge(target_type: 'merge_request', target_id: note.noteable_id) } + + it 'responds with the expected attributes' do + get :index, params + + expect(note_json[:id]).to eq(note.id) + expect(note_json[:discussion_html]).not_to be_nil + expect(note_json[:diff_discussion_html]).not_to be_nil + end + end + + context 'for a commit note' do + let(:project) { create(:project, :repository) } + let!(:note) { create(:note_on_commit, project: project) } + + context 'when displayed on a merge request' do + let(:merge_request) { create(:merge_request, source_project: project) } + + let(:params) { request_params.merge(target_type: 'merge_request', target_id: merge_request.id) } + + it 'responds with the expected attributes' do + get :index, params + + expect(note_json[:id]).to eq(note.id) + expect(note_json[:discussion_html]).not_to be_nil + expect(note_json[:diff_discussion_html]).to be_nil + end + end + + context 'when displayed on the commit' do + let(:params) { request_params.merge(target_type: 'commit', target_id: note.commit_id) } + + it 'responds with the expected attributes' do + get :index, params + + expect(note_json[:id]).to eq(note.id) + expect(note_json[:discussion_html]).to be_nil + expect(note_json[:diff_discussion_html]).to be_nil + end + end + end + + context 'for a regular note' do + let!(:note) { create(:note, noteable: issue, project: project) } + + it 'responds with the expected attributes' do + get :index, request_params + + expect(note_json[:id]).to eq(note.id) + expect(note_json[:html]).not_to be_nil + expect(note_json[:discussion_html]).to be_nil + expect(note_json[:diff_discussion_html]).to be_nil + end + end + end + describe 'POST create' do let(:merge_request) { create(:merge_request) } let(:project) { merge_request.source_project } @@ -49,7 +152,8 @@ describe Projects::NotesController do note: 'some note', noteable_id: merge_request.id.to_s, noteable_type: 'MergeRequest', - merge_request_diff_head_sha: 'sha' + merge_request_diff_head_sha: 'sha', + in_reply_to_discussion_id: nil } expect(Notes::CreateService).to receive(:new).with(project, user, service_params).and_return(double(execute: true)) @@ -63,6 +167,47 @@ describe Projects::NotesController do end end + describe 'DELETE destroy' do + let(:request_params) do + { + namespace_id: project.namespace, + project_id: project, + id: note, + format: :js + } + end + + context 'user is the author of a note' do + before do + sign_in(note.author) + project.team << [note.author, :developer] + end + + it "returns status 200 for html" do + delete :destroy, request_params + + expect(response).to have_http_status(200) + end + + it "deletes the note" do + expect { delete :destroy, request_params }.to change { Note.count }.from(1).to(0) + end + end + + context 'user is not the author of a note' do + before do + sign_in(user) + project.team << [user, :developer] + end + + it "returns status 404" do + delete :destroy, request_params + + expect(response).to have_http_status(404) + end + end + end + describe 'POST toggle_award_emoji' do before do sign_in(user) @@ -200,31 +345,4 @@ describe Projects::NotesController do end end end - - describe 'GET index' do - let(:last_fetched_at) { '1487756246' } - let(:request_params) do - { - namespace_id: project.namespace, - project_id: project, - target_type: 'issue', - target_id: issue.id - } - end - - before do - sign_in(user) - project.team << [user, :developer] - end - - it 'passes last_fetched_at from headers to NotesFinder' do - request.headers['X-Last-Fetched-At'] = last_fetched_at - - expect(NotesFinder).to receive(:new) - .with(anything, anything, hash_including(last_fetched_at: last_fetched_at)) - .and_call_original - - get :index, request_params - end - end end diff --git a/spec/controllers/projects/pages_controller_spec.rb b/spec/controllers/projects/pages_controller_spec.rb new file mode 100644 index 00000000000..df35d8e86b9 --- /dev/null +++ b/spec/controllers/projects/pages_controller_spec.rb @@ -0,0 +1,57 @@ +require 'spec_helper' + +describe Projects::PagesController do + let(:user) { create(:user) } + let(:project) { create(:empty_project, :public, :access_requestable) } + + let(:request_params) do + { + namespace_id: project.namespace, + project_id: project + } + end + + before do + allow(Gitlab.config.pages).to receive(:enabled).and_return(true) + sign_in(user) + project.add_master(user) + end + + describe 'GET show' do + it 'returns 200 status' do + get :show, request_params + + expect(response).to have_http_status(200) + end + end + + describe 'DELETE destroy' do + it 'returns 302 status' do + delete :destroy, request_params + + expect(response).to have_http_status(302) + end + end + + context 'pages disabled' do + before do + allow(Gitlab.config.pages).to receive(:enabled).and_return(false) + end + + describe 'GET show' do + it 'returns 404 status' do + get :show, request_params + + expect(response).to have_http_status(404) + end + end + + describe 'DELETE destroy' do + it 'returns 404 status' do + delete :destroy, request_params + + expect(response).to have_http_status(404) + end + end + end +end diff --git a/spec/controllers/projects/pages_domains_controller_spec.rb b/spec/controllers/projects/pages_domains_controller_spec.rb index 2362df895a8..33853c4b9d0 100644 --- a/spec/controllers/projects/pages_domains_controller_spec.rb +++ b/spec/controllers/projects/pages_domains_controller_spec.rb @@ -1,8 +1,9 @@ require 'spec_helper' describe Projects::PagesDomainsController do - let(:user) { create(:user) } - let(:project) { create(:project) } + let(:user) { create(:user) } + let(:project) { create(:empty_project) } + let!(:pages_domain) { create(:pages_domain, project: project) } let(:request_params) do { @@ -11,14 +12,17 @@ describe Projects::PagesDomainsController do } end + let(:pages_domain_params) do + build(:pages_domain, :with_certificate, :with_key, domain: 'my.otherdomain.com').slice(:key, :certificate, :domain) + end + before do + allow(Gitlab.config.pages).to receive(:enabled).and_return(true) sign_in(user) - project.team << [user, :master] + project.add_master(user) end describe 'GET show' do - let!(:pages_domain) { create(:pages_domain, project: project) } - it "displays the 'show' page" do get(:show, request_params.merge(id: pages_domain.domain)) @@ -37,10 +41,6 @@ describe Projects::PagesDomainsController do end describe 'POST create' do - let(:pages_domain_params) do - build(:pages_domain, :with_certificate, :with_key).slice(:key, :certificate, :domain) - end - it "creates a new pages domain" do expect do post(:create, request_params.merge(pages_domain: pages_domain_params)) @@ -51,8 +51,6 @@ describe Projects::PagesDomainsController do end describe 'DELETE destroy' do - let!(:pages_domain) { create(:pages_domain, project: project) } - it "deletes the pages domain" do expect do delete(:destroy, request_params.merge(id: pages_domain.domain)) @@ -61,4 +59,42 @@ describe Projects::PagesDomainsController do expect(response).to redirect_to(namespace_project_pages_path(project.namespace, project)) end end + + context 'pages disabled' do + before do + allow(Gitlab.config.pages).to receive(:enabled).and_return(false) + end + + describe 'GET show' do + it 'returns 404 status' do + get(:show, request_params.merge(id: pages_domain.domain)) + + expect(response).to have_http_status(404) + end + end + + describe 'GET new' do + it 'returns 404 status' do + get :new, request_params + + expect(response).to have_http_status(404) + end + end + + describe 'POST create' do + it "returns 404 status" do + post(:create, request_params.merge(pages_domain: pages_domain_params)) + + expect(response).to have_http_status(404) + end + end + + describe 'DELETE destroy' do + it "deletes the pages domain" do + delete(:destroy, request_params.merge(id: pages_domain.domain)) + + expect(response).to have_http_status(404) + end + end + end end diff --git a/spec/controllers/projects/pipeline_schedules_controller_spec.rb b/spec/controllers/projects/pipeline_schedules_controller_spec.rb new file mode 100644 index 00000000000..f8f95dd9bc8 --- /dev/null +++ b/spec/controllers/projects/pipeline_schedules_controller_spec.rb @@ -0,0 +1,87 @@ +require 'spec_helper' + +describe Projects::PipelineSchedulesController do + set(:project) { create(:empty_project, :public) } + let!(:pipeline_schedule) { create(:ci_pipeline_schedule, project: project) } + + describe 'GET #index' do + let(:scope) { nil } + let!(:inactive_pipeline_schedule) do + create(:ci_pipeline_schedule, :inactive, project: project) + end + + it 'renders the index view' do + visit_pipelines_schedules + + expect(response).to have_http_status(:ok) + expect(response).to render_template(:index) + end + + context 'when the scope is set to active' do + let(:scope) { 'active' } + + before do + visit_pipelines_schedules + end + + it 'only shows active pipeline schedules' do + expect(response).to have_http_status(:ok) + expect(assigns(:schedules)).to include(pipeline_schedule) + expect(assigns(:schedules)).not_to include(inactive_pipeline_schedule) + end + end + + def visit_pipelines_schedules + get :index, namespace_id: project.namespace.to_param, project_id: project, scope: scope + end + end + + describe 'GET edit' do + let(:user) { create(:user) } + + before do + project.add_master(user) + + sign_in(user) + end + + it 'loads the pipeline schedule' do + get :edit, namespace_id: project.namespace.to_param, project_id: project, id: pipeline_schedule.id + + expect(response).to have_http_status(:ok) + expect(assigns(:schedule)).to eq(pipeline_schedule) + end + end + + describe 'DELETE #destroy' do + set(:user) { create(:user) } + + context 'when a developer makes the request' do + before do + project.add_developer(user) + sign_in(user) + + delete :destroy, namespace_id: project.namespace.to_param, project_id: project, id: pipeline_schedule.id + end + + it 'does not delete the pipeline schedule' do + expect(response).not_to have_http_status(:ok) + end + end + + context 'when a master makes the request' do + before do + project.add_master(user) + sign_in(user) + end + + it 'destroys the pipeline schedule' do + expect do + delete :destroy, namespace_id: project.namespace.to_param, project_id: project, id: pipeline_schedule.id + end.to change { project.pipeline_schedules.count }.by(-1) + + expect(response).to have_http_status(302) + end + end + end +end diff --git a/spec/controllers/projects/pipelines_controller_spec.rb b/spec/controllers/projects/pipelines_controller_spec.rb index d8f9bfd0d37..c880da1e36a 100644 --- a/spec/controllers/projects/pipelines_controller_spec.rb +++ b/spec/controllers/projects/pipelines_controller_spec.rb @@ -7,6 +7,8 @@ describe Projects::PipelinesController do let(:project) { create(:empty_project, :public) } before do + project.add_developer(user) + sign_in(user) end @@ -24,6 +26,7 @@ describe Projects::PipelinesController do it 'returns JSON with serialized pipelines' do expect(response).to have_http_status(:ok) + expect(response).to match_response_schema('pipeline') expect(json_response).to include('pipelines') expect(json_response['pipelines'].count).to eq 4 @@ -34,6 +37,62 @@ describe Projects::PipelinesController do end end + describe 'GET show JSON' do + let(:pipeline) { create(:ci_pipeline_with_one_job, project: project) } + + it 'returns the pipeline' do + get_pipeline_json + + expect(response).to have_http_status(:ok) + expect(json_response).not_to be_an(Array) + expect(json_response['id']).to be(pipeline.id) + expect(json_response['details']).to have_key 'stages' + end + + context 'when the pipeline has multiple stages and groups' do + before do + RequestStore.begin! + + create_build('build', 0, 'build') + create_build('test', 1, 'rspec 0') + create_build('deploy', 2, 'production') + create_build('post deploy', 3, 'pages 0') + end + + after do + RequestStore.end! + RequestStore.clear! + end + + let(:project) { create(:project) } + let(:pipeline) do + create(:ci_empty_pipeline, project: project, user: user, sha: project.commit.id) + end + + it 'does not perform N + 1 queries' do + 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') + + new_count = ActiveRecord::QueryRecorder.new { get_pipeline_json }.count + expect(new_count).to be_within(12).of(control_count) + end + end + + def get_pipeline_json + get :show, namespace_id: project.namespace, project_id: project, id: pipeline, format: :json + end + + def create_build(stage, stage_idx, name) + create(:ci_build, pipeline: pipeline, stage: stage, stage_idx: stage_idx, name: name) + end + end + describe 'GET stages.json' do let(:pipeline) { create(:ci_pipeline, project: project) } @@ -86,7 +145,41 @@ describe Projects::PipelinesController do 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 eq status.favicon + expect(json_response['favicon']).to eq "/assets/ci_favicons/#{status.favicon}.ico" + end + end + + describe 'POST retry.json' do + let!(:pipeline) { create(:ci_pipeline, :failed, project: project) } + let!(:build) { create(:ci_build, :failed, pipeline: pipeline) } + + before do + post :retry, namespace_id: project.namespace, + project_id: project, + id: pipeline.id, + format: :json + end + + it 'retries a pipeline without returning any content' do + expect(response).to have_http_status(:no_content) + expect(build.reload).to be_retried + end + end + + describe 'POST cancel.json' do + let!(:pipeline) { create(:ci_pipeline, project: project) } + let!(:build) { create(:ci_build, :running, pipeline: pipeline) } + + before do + post :cancel, namespace_id: project.namespace, + project_id: project, + id: pipeline.id, + format: :json + end + + it 'cancels a pipeline without returning any content' do + expect(response).to have_http_status(:no_content) + expect(pipeline.reload).to be_canceled end end end diff --git a/spec/controllers/projects/project_members_controller_spec.rb b/spec/controllers/projects/project_members_controller_spec.rb index 416eaa0037e..a4b4392d7cc 100644 --- a/spec/controllers/projects/project_members_controller_spec.rb +++ b/spec/controllers/projects/project_members_controller_spec.rb @@ -55,7 +55,7 @@ describe Projects::ProjectMembersController do user_ids: '', access_level: Gitlab::Access::GUEST - expect(response).to set_flash.to 'No users or groups specified.' + expect(response).to set_flash.to 'No users specified.' expect(response).to redirect_to(namespace_project_settings_members_path(project.namespace, project)) end end @@ -225,7 +225,7 @@ describe Projects::ProjectMembersController do id: member expect(response).to redirect_to( - namespace_project_project_members_path(project.namespace, project) + namespace_project_settings_members_path(project.namespace, project) ) expect(project.members).to include member end diff --git a/spec/controllers/projects/protected_branches_controller_spec.rb b/spec/controllers/projects/protected_branches_controller_spec.rb index e378b5714fe..80be135b5d8 100644 --- a/spec/controllers/projects/protected_branches_controller_spec.rb +++ b/spec/controllers/projects/protected_branches_controller_spec.rb @@ -3,6 +3,7 @@ require('spec_helper') describe Projects::ProtectedBranchesController do describe "GET #index" do let(:project) { create(:project_empty_repo, :public) } + it "redirects empty repo to projects page" do get(:index, namespace_id: project.namespace.to_param, project_id: project) end diff --git a/spec/controllers/projects/protected_tags_controller_spec.rb b/spec/controllers/projects/protected_tags_controller_spec.rb new file mode 100644 index 00000000000..64658988b3f --- /dev/null +++ b/spec/controllers/projects/protected_tags_controller_spec.rb @@ -0,0 +1,11 @@ +require('spec_helper') + +describe Projects::ProtectedTagsController do + describe "GET #index" do + let(:project) { create(:project_empty_repo, :public) } + + it "redirects empty repo to projects page" do + get(:index, namespace_id: project.namespace.to_param, project_id: project) + end + end +end diff --git a/spec/controllers/projects/registry/repositories_controller_spec.rb b/spec/controllers/projects/registry/repositories_controller_spec.rb new file mode 100644 index 00000000000..464302824a8 --- /dev/null +++ b/spec/controllers/projects/registry/repositories_controller_spec.rb @@ -0,0 +1,84 @@ +require 'spec_helper' + +describe Projects::Registry::RepositoriesController do + let(:user) { create(:user) } + let(:project) { create(:empty_project, :private) } + + before do + sign_in(user) + stub_container_registry_config(enabled: true) + end + + context 'when user has access to registry' do + before do + project.add_developer(user) + end + + describe 'GET index' do + context 'when root container repository exists' do + before do + create(:container_repository, :root, project: project) + end + + it 'does not create root container repository' do + expect { go_to_index }.not_to change { ContainerRepository.all.count } + end + end + + context 'when root container repository is not created' do + context 'when there are tags for this repository' do + before do + stub_container_registry_tags(repository: project.full_path, + tags: %w[rc1 latest]) + end + + it 'successfully renders container repositories' do + go_to_index + + expect(response).to have_http_status(:ok) + end + + it 'creates a root container repository' do + expect { go_to_index }.to change { ContainerRepository.all.count }.by(1) + expect(ContainerRepository.first).to be_root_repository + end + end + + context 'when there are no tags for this repository' do + before do + stub_container_registry_tags(repository: :any, tags: []) + end + + it 'successfully renders container repositories' do + go_to_index + + expect(response).to have_http_status(:ok) + end + + it 'does not ensure root container repository' do + expect { go_to_index }.not_to change { ContainerRepository.all.count } + end + end + end + end + end + + context 'when user does not have access to registry' do + describe 'GET index' do + it 'responds with 404' do + go_to_index + + expect(response).to have_http_status(:not_found) + end + + it 'does not ensure root container repository' do + expect { go_to_index }.not_to change { ContainerRepository.all.count } + end + end + end + + def go_to_index + get :index, namespace_id: project.namespace, + project_id: project + end +end diff --git a/spec/controllers/projects/services_controller_spec.rb b/spec/controllers/projects/services_controller_spec.rb index 16365642a34..2d892f4a2b7 100644 --- a/spec/controllers/projects/services_controller_spec.rb +++ b/spec/controllers/projects/services_controller_spec.rb @@ -8,6 +8,7 @@ describe Projects::ServicesController do before do sign_in(user) project.team << [user, :master] + controller.instance_variable_set(:@project, project) controller.instance_variable_set(:@service, service) end @@ -18,20 +19,60 @@ describe Projects::ServicesController do end describe "#test" do + context 'when can_test? returns false' do + it 'renders 404' do + allow_any_instance_of(Service).to receive(:can_test?).and_return(false) + + get :test, namespace_id: project.namespace.id, project_id: project.id, id: service.id, format: :html + + expect(response).to have_http_status(404) + end + end + context 'success' do + context 'with empty project' do + let(:project) { create(:empty_project) } + + context 'with chat notification service' do + let(:service) { project.create_microsoft_teams_service(webhook: 'http://webhook.com') } + + it 'redirects and show success message' do + allow_any_instance_of(MicrosoftTeams::Notifier).to receive(:ping).and_return(true) + + get :test, namespace_id: project.namespace.id, project_id: project.id, id: service.id, format: :html + + expect(response).to redirect_to(root_path) + expect(flash[:notice]).to eq('We sent a request to the provided URL') + end + end + + it 'redirects and show success message' do + expect(service).to receive(:test).and_return(success: true, result: 'done') + + get :test, namespace_id: project.namespace.id, project_id: project.id, id: service.id, format: :html + + expect(response).to redirect_to(root_path) + expect(flash[:notice]).to eq('We sent a request to the provided URL') + end + end + it "redirects and show success message" do - expect(service).to receive(:test).and_return({ success: true, result: 'done' }) + expect(service).to receive(:test).and_return(success: true, result: 'done') + get :test, namespace_id: project.namespace.id, project_id: project.id, id: service.id, format: :html - expect(response.status).to redirect_to('/') + + expect(response).to redirect_to(root_path) expect(flash[:notice]).to eq('We sent a request to the provided URL') end end context 'failure' do it "redirects and show failure message" do - expect(service).to receive(:test).and_return({ success: false, result: 'Bad test' }) + expect(service).to receive(:test).and_return(success: false, result: 'Bad test') + get :test, namespace_id: project.namespace.id, project_id: project.id, id: service.id, format: :html - expect(response.status).to redirect_to('/') + + expect(response).to redirect_to(root_path) expect(flash[:alert]).to eq('We tried to send a request to the provided URL but an error occurred: Bad test') end end diff --git a/spec/controllers/projects/todo_controller_spec.rb b/spec/controllers/projects/todos_controller_spec.rb index 9a7beeff6fe..c5a4153d991 100644 --- a/spec/controllers/projects/todo_controller_spec.rb +++ b/spec/controllers/projects/todos_controller_spec.rb @@ -1,8 +1,6 @@ require('spec_helper') describe Projects::TodosController do - include ApiHelpers - let(:user) { create(:user) } let(:project) { create(:empty_project) } let(:issue) { create(:issue, project: project) } diff --git a/spec/controllers/projects/tree_controller_spec.rb b/spec/controllers/projects/tree_controller_spec.rb index ab94e292e48..a43dad5756d 100644 --- a/spec/controllers/projects/tree_controller_spec.rb +++ b/spec/controllers/projects/tree_controller_spec.rb @@ -97,29 +97,29 @@ describe Projects::TreeController do project_id: project, id: 'master', dir_name: path, - target_branch: target_branch, + branch_name: branch_name, commit_message: 'Test commit message') end context 'successful creation' do let(:path) { 'files/new_dir'} - let(:target_branch) { 'master-test'} + let(:branch_name) { 'master-test'} it 'redirects to the new directory' do expect(subject). - to redirect_to("/#{project.path_with_namespace}/tree/#{target_branch}/#{path}") + to redirect_to("/#{project.path_with_namespace}/tree/#{branch_name}/#{path}") expect(flash[:notice]).to eq('The directory has been successfully created.') end end context 'unsuccessful creation' do let(:path) { 'README.md' } - let(:target_branch) { 'master'} + let(:branch_name) { 'master'} it 'does not allow overwriting of existing files' do expect(subject). to redirect_to("/#{project.path_with_namespace}/tree/master") - expect(flash[:alert]).to eq('Directory already exists as a file') + expect(flash[:alert]).to eq('A file with this name already exists') end end end diff --git a/spec/controllers/projects/wikis_controller_spec.rb b/spec/controllers/projects/wikis_controller_spec.rb new file mode 100644 index 00000000000..92addf30307 --- /dev/null +++ b/spec/controllers/projects/wikis_controller_spec.rb @@ -0,0 +1,16 @@ +require 'spec_helper' + +describe Projects::WikisController do + let(:project) { create(:project_empty_repo, :public) } + let(:user) { create(:user) } + + describe 'POST #preview_markdown' do + it 'renders json in a correct format' do + sign_in(user) + + post :preview_markdown, namespace_id: project.namespace, project_id: project, id: 'page/path', text: '*Markdown* text' + + expect(JSON.parse(response.body).keys).to match_array(%w(body references)) + end + end +end diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index a88ffc1ea6a..a8be6768a47 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -169,26 +169,6 @@ describe ProjectsController do end end - context "when requested with case sensitive namespace and project path" do - context "when there is a match with the same casing" do - it "loads the project" do - get :show, namespace_id: public_project.namespace, id: public_project - - expect(assigns(:project)).to eq(public_project) - expect(response).to have_http_status(200) - end - end - - context "when there is a match with different casing" do - it "redirects to the normalized path" do - get :show, namespace_id: public_project.namespace, id: public_project.path.upcase - - expect(assigns(:project)).to eq(public_project) - expect(response).to redirect_to("/#{public_project.full_path}") - end - end - end - context "when the url contains .atom" do let(:public_project_with_dot_atom) { build(:empty_project, :public, name: 'my.atom', path: 'my.atom') } @@ -224,13 +204,16 @@ describe ProjectsController do render_views let(:admin) { create(:admin) } + let(:project) { create(:project, :repository) } + let(:new_path) { 'renamed_path' } + let(:project_params) { { path: new_path } } + + before do + sign_in(admin) + end it "sets the repository to the right path after a rename" do - project = create(:project, :repository) - new_path = 'renamed_path' - project_params = { path: new_path } controller.instance_variable_set(:@project, project) - sign_in(admin) put :update, namespace_id: project.namespace, @@ -398,4 +381,121 @@ describe ProjectsController do expect(parsed_body["Commits"]).to include("123456") end end + + describe 'POST #preview_markdown' do + it 'renders json in a correct format' do + sign_in(user) + + post :preview_markdown, namespace_id: public_project.namespace, id: public_project, text: '*Markdown* text' + + expect(JSON.parse(response.body).keys).to match_array(%w(body references)) + end + end + + describe '#ensure_canonical_path' do + before do + sign_in(user) + end + + context 'for a GET request' do + context 'when requesting the canonical path' do + context "with exactly matching casing" do + it "loads the project" do + get :show, namespace_id: public_project.namespace, id: public_project + + expect(assigns(:project)).to eq(public_project) + expect(response).to have_http_status(200) + end + end + + context "with different casing" do + it "redirects to the normalized path" do + get :show, namespace_id: public_project.namespace, id: public_project.path.upcase + + expect(assigns(:project)).to eq(public_project) + expect(response).to redirect_to("/#{public_project.full_path}") + expect(controller).not_to set_flash[:notice] + end + end + end + + context 'when requesting a redirected path' do + let!(:redirect_route) { public_project.redirect_routes.create!(path: "foo/bar") } + + it 'redirects to the canonical path' do + get :show, namespace_id: 'foo', id: 'bar' + + expect(response).to redirect_to(public_project) + expect(controller).to set_flash[:notice].to(project_moved_message(redirect_route, public_project)) + end + + it 'redirects to the canonical path (testing non-show action)' do + get :refs, namespace_id: 'foo', id: 'bar' + + expect(response).to redirect_to(refs_namespace_project_path(namespace_id: public_project.namespace, id: public_project)) + expect(controller).to set_flash[:notice].to(project_moved_message(redirect_route, public_project)) + end + end + end + + context 'for a POST request' do + context 'when requesting the canonical path with different casing' do + it 'does not 404' do + post :toggle_star, namespace_id: public_project.namespace, id: public_project.path.upcase + + expect(response).not_to have_http_status(404) + end + + it 'does not redirect to the correct casing' do + post :toggle_star, namespace_id: public_project.namespace, id: public_project.path.upcase + + expect(response).not_to have_http_status(301) + end + end + + context 'when requesting a redirected path' do + let!(:redirect_route) { public_project.redirect_routes.create!(path: "foo/bar") } + + it 'returns not found' do + post :toggle_star, namespace_id: 'foo', id: 'bar' + + expect(response).to have_http_status(404) + end + end + end + + context 'for a DELETE request' do + before do + sign_in(create(:admin)) + end + + context 'when requesting the canonical path with different casing' do + it 'does not 404' do + delete :destroy, namespace_id: project.namespace, id: project.path.upcase + + expect(response).not_to have_http_status(404) + end + + it 'does not redirect to the correct casing' do + delete :destroy, namespace_id: project.namespace, id: project.path.upcase + + expect(response).not_to have_http_status(301) + end + end + + context 'when requesting a redirected path' do + let!(:redirect_route) { project.redirect_routes.create!(path: "foo/bar") } + + it 'returns not found' do + delete :destroy, namespace_id: 'foo', id: 'bar' + + expect(response).to have_http_status(404) + end + end + end + end + + def project_moved_message(redirect_route, project) + "Project '#{redirect_route.path}' was moved to '#{project.full_path}'. Please update any links and bookmarks that may still have the old path." + end end diff --git a/spec/controllers/registrations_controller_spec.rb b/spec/controllers/registrations_controller_spec.rb index 902911071c4..71dd9ef3eb4 100644 --- a/spec/controllers/registrations_controller_spec.rb +++ b/spec/controllers/registrations_controller_spec.rb @@ -68,4 +68,20 @@ describe RegistrationsController do end end end + + describe '#destroy' do + let(:user) { create(:user) } + + before do + sign_in(user) + end + + it 'schedules the user for destruction' do + expect(DeleteUserWorker).to receive(:perform_async).with(user.id, user.id) + + post(:destroy) + + expect(response.status).to eq(302) + end + end end diff --git a/spec/controllers/sessions_controller_spec.rb b/spec/controllers/sessions_controller_spec.rb index a06c29dd91a..038132cffe0 100644 --- a/spec/controllers/sessions_controller_spec.rb +++ b/spec/controllers/sessions_controller_spec.rb @@ -16,7 +16,9 @@ describe SessionsController do end end - context 'when using valid password' do + context 'when using valid password', :redis do + include UserActivitiesHelpers + let(:user) { create(:user) } it 'authenticates user correctly' do @@ -37,6 +39,12 @@ describe SessionsController do subject.sign_out user end end + + it 'updates the user activity' do + expect do + post(:create, user: { login: user.username, password: user.password }) + end.to change { user_activity(user) } + end end end @@ -211,4 +219,20 @@ describe SessionsController do end end end + + describe '#new' do + before do + @request.env['devise.mapping'] = Devise.mappings[:user] + end + + it 'redirects correctly for referer on same host with params' do + search_path = '/search?search=seed_project' + allow(controller.request).to receive(:referer). + and_return('http://%{host}%{path}' % { host: Gitlab.config.gitlab.host, path: search_path }) + + get(:new, redirect_to_referer: :yes) + + expect(controller.stored_location_for(:redirect)).to eq(search_path) + end + end end diff --git a/spec/controllers/snippets/notes_controller_spec.rb b/spec/controllers/snippets/notes_controller_spec.rb new file mode 100644 index 00000000000..1c494b8c7ab --- /dev/null +++ b/spec/controllers/snippets/notes_controller_spec.rb @@ -0,0 +1,196 @@ +require 'spec_helper' + +describe Snippets::NotesController do + let(:user) { create(:user) } + + let(:private_snippet) { create(:personal_snippet, :private) } + let(:internal_snippet) { create(:personal_snippet, :internal) } + let(:public_snippet) { create(:personal_snippet, :public) } + + let(:note_on_private) { create(:note_on_personal_snippet, noteable: private_snippet) } + let(:note_on_internal) { create(:note_on_personal_snippet, noteable: internal_snippet) } + let(:note_on_public) { create(:note_on_personal_snippet, noteable: public_snippet) } + + describe 'GET index' do + context 'when a snippet is public' do + before do + note_on_public + + get :index, { snippet_id: public_snippet } + end + + it "returns status 200" do + expect(response).to have_http_status(200) + end + + it "returns not empty array of notes" do + expect(JSON.parse(response.body)["notes"].empty?).to be_falsey + end + end + + context 'when a snippet is internal' do + before do + note_on_internal + end + + context 'when user not logged in' do + it "returns status 404" do + get :index, { snippet_id: internal_snippet } + + expect(response).to have_http_status(404) + end + end + + context 'when user logged in' do + before do + sign_in(user) + end + + it "returns status 200" do + get :index, { snippet_id: internal_snippet } + + expect(response).to have_http_status(200) + end + end + end + + context 'when a snippet is private' do + before do + note_on_private + end + + context 'when user not logged in' do + it "returns status 404" do + get :index, { snippet_id: private_snippet } + + expect(response).to have_http_status(404) + end + end + + context 'when user other than author logged in' do + before do + sign_in(user) + end + + it "returns status 404" do + get :index, { snippet_id: private_snippet } + + expect(response).to have_http_status(404) + end + end + + context 'when author logged in' do + before do + note_on_private + + sign_in(private_snippet.author) + end + + it "returns status 200" do + get :index, { snippet_id: private_snippet } + + expect(response).to have_http_status(200) + end + + it "returns 1 note" do + get :index, { snippet_id: private_snippet } + + expect(JSON.parse(response.body)['notes'].count).to eq(1) + end + end + end + + context 'dont show non visible notes' do + before do + note_on_public + + sign_in(user) + + expect_any_instance_of(Note).to receive(:cross_reference_not_visible_for?).and_return(true) + end + + it "does not return any note" do + get :index, { snippet_id: public_snippet } + + expect(JSON.parse(response.body)['notes'].count).to eq(0) + end + end + end + + describe 'DELETE destroy' do + let(:request_params) do + { + snippet_id: public_snippet, + id: note_on_public, + format: :js + } + end + + context 'when user is the author of a note' do + before do + sign_in(note_on_public.author) + end + + it "returns status 200" do + delete :destroy, request_params + + expect(response).to have_http_status(200) + end + + it "deletes the note" do + expect{ delete :destroy, request_params }.to change{ Note.count }.from(1).to(0) + end + + context 'system note' do + before do + expect_any_instance_of(Note).to receive(:system?).and_return(true) + end + + it "does not delete the note" do + expect{ delete :destroy, request_params }.not_to change{ Note.count } + end + end + end + + context 'when user is not the author of a note' do + before do + sign_in(user) + + note_on_public + end + + it "returns status 404" do + delete :destroy, request_params + + expect(response).to have_http_status(404) + end + + it "does not update the note" do + expect{ delete :destroy, request_params }.not_to change{ Note.count } + end + end + end + + describe 'POST toggle_award_emoji' do + let(:note) { create(:note_on_personal_snippet, noteable: public_snippet) } + before do + sign_in(user) + end + + subject { post(:toggle_award_emoji, snippet_id: public_snippet, id: note.id, name: "thumbsup") } + + it "toggles the award emoji" do + expect { subject }.to change { note.award_emoji.count }.by(1) + + expect(response).to have_http_status(200) + end + + it "removes the already awarded emoji when it exists" do + note.toggle_award_emoji('thumbsup', user) # create award emoji before + + expect { subject }.to change { AwardEmoji.count }.by(-1) + + expect(response).to have_http_status(200) + end + end +end diff --git a/spec/controllers/snippets_controller_spec.rb b/spec/controllers/snippets_controller_spec.rb index 5de3b9890ef..930415a4778 100644 --- a/spec/controllers/snippets_controller_spec.rb +++ b/spec/controllers/snippets_controller_spec.rb @@ -3,6 +3,34 @@ require 'spec_helper' describe SnippetsController do let(:user) { create(:user) } + describe 'GET #index' do + let(:user) { create(:user) } + + context 'when username parameter is present' do + it 'renders snippets of a user when username is present' do + get :index, username: user.username + + expect(response).to render_template(:index) + end + end + + context 'when username parameter is not present' do + it 'redirects to explore snippets page when user is not logged in' do + get :index + + expect(response).to redirect_to(explore_snippets_path) + end + + it 'redirects to snippets dashboard page when user is logged in' do + sign_in(user) + + get :index + + expect(response).to redirect_to(dashboard_snippets_path) + end + end + end + describe 'GET #new' do context 'when signed in' do before do @@ -132,7 +160,7 @@ describe SnippetsController do it 'responds with status 404' do get :show, id: 'doesntexist' - expect(response).to have_http_status(404) + expect(response).to redirect_to(new_user_session_path) end end end @@ -350,144 +378,138 @@ describe SnippetsController do end end - %w(raw download).each do |action| - describe "GET #{action}" do - context 'when the personal snippet is private' do - let(:personal_snippet) { create(:personal_snippet, :private, author: user) } + describe "GET #raw" do + context 'when the personal snippet is private' do + let(:personal_snippet) { create(:personal_snippet, :private, author: user) } - context 'when signed in' do - before do - sign_in(user) - end + context 'when signed in' do + before do + sign_in(user) + end - context 'when signed in user is not the author' do - let(:other_author) { create(:author) } - let(:other_personal_snippet) { create(:personal_snippet, :private, author: other_author) } + context 'when signed in user is not the author' do + let(:other_author) { create(:author) } + let(:other_personal_snippet) { create(:personal_snippet, :private, author: other_author) } - it 'responds with status 404' do - get action, id: other_personal_snippet.to_param + it 'responds with status 404' do + get :raw, id: other_personal_snippet.to_param - expect(response).to have_http_status(404) - end + expect(response).to have_http_status(404) end + end - context 'when signed in user is the author' do - before { get action, id: personal_snippet.to_param } + context 'when signed in user is the author' do + before { get :raw, id: personal_snippet.to_param } - it 'responds with status 200' do - expect(assigns(:snippet)).to eq(personal_snippet) - expect(response).to have_http_status(200) - end + it 'responds with status 200' do + expect(assigns(:snippet)).to eq(personal_snippet) + expect(response).to have_http_status(200) + end - it 'has expected headers' do - expect(response.header['Content-Type']).to eq('text/plain; charset=utf-8') + it 'has expected headers' do + expect(response.header['Content-Type']).to eq('text/plain; charset=utf-8') - if action == :download - expect(response.header['Content-Disposition']).to match(/attachment/) - elsif action == :raw - expect(response.header['Content-Disposition']).to match(/inline/) - end - end + expect(response.header['Content-Disposition']).to match(/inline/) end end + end - context 'when not signed in' do - it 'redirects to the sign in page' do - get action, id: personal_snippet.to_param + context 'when not signed in' do + it 'redirects to the sign in page' do + get :raw, id: personal_snippet.to_param - expect(response).to redirect_to(new_user_session_path) - end + expect(response).to redirect_to(new_user_session_path) end end + end - context 'when the personal snippet is internal' do - let(:personal_snippet) { create(:personal_snippet, :internal, author: user) } + context 'when the personal snippet is internal' do + let(:personal_snippet) { create(:personal_snippet, :internal, author: user) } - context 'when signed in' do - before do - sign_in(user) - end + context 'when signed in' do + before do + sign_in(user) + end - it 'responds with status 200' do - get action, id: personal_snippet.to_param + it 'responds with status 200' do + get :raw, id: personal_snippet.to_param - expect(assigns(:snippet)).to eq(personal_snippet) - expect(response).to have_http_status(200) - end + expect(assigns(:snippet)).to eq(personal_snippet) + expect(response).to have_http_status(200) end + end - context 'when not signed in' do - it 'redirects to the sign in page' do - get action, id: personal_snippet.to_param + context 'when not signed in' do + it 'redirects to the sign in page' do + get :raw, id: personal_snippet.to_param - expect(response).to redirect_to(new_user_session_path) - end + expect(response).to redirect_to(new_user_session_path) end end + end - context 'when the personal snippet is public' do - let(:personal_snippet) { create(:personal_snippet, :public, author: user) } + context 'when the personal snippet is public' do + let(:personal_snippet) { create(:personal_snippet, :public, author: user) } - context 'when signed in' do - before do - sign_in(user) - end + context 'when signed in' do + before do + sign_in(user) + end - it 'responds with status 200' do - get action, id: personal_snippet.to_param + it 'responds with status 200' do + get :raw, id: personal_snippet.to_param - expect(assigns(:snippet)).to eq(personal_snippet) - expect(response).to have_http_status(200) - end + expect(assigns(:snippet)).to eq(personal_snippet) + expect(response).to have_http_status(200) + end - context 'CRLF line ending' do - let(:personal_snippet) do - create(:personal_snippet, :public, author: user, content: "first line\r\nsecond line\r\nthird line") - end + context 'CRLF line ending' do + let(:personal_snippet) do + create(:personal_snippet, :public, author: user, content: "first line\r\nsecond line\r\nthird line") + end - it 'returns LF line endings by default' do - get action, id: personal_snippet.to_param + it 'returns LF line endings by default' do + get :raw, id: personal_snippet.to_param - expect(response.body).to eq("first line\nsecond line\nthird line") - end + expect(response.body).to eq("first line\nsecond line\nthird line") + end - it 'does not convert line endings when parameter present' do - get action, id: personal_snippet.to_param, line_ending: :raw + it 'does not convert line endings when parameter present' do + get :raw, id: personal_snippet.to_param, line_ending: :raw - expect(response.body).to eq("first line\r\nsecond line\r\nthird line") - end + expect(response.body).to eq("first line\r\nsecond line\r\nthird line") end end + end - context 'when not signed in' do - it 'responds with status 200' do - get action, id: personal_snippet.to_param + context 'when not signed in' do + it 'responds with status 200' do + get :raw, id: personal_snippet.to_param - expect(assigns(:snippet)).to eq(personal_snippet) - expect(response).to have_http_status(200) - end + expect(assigns(:snippet)).to eq(personal_snippet) + expect(response).to have_http_status(200) end end + end - context 'when the personal snippet does not exist' do - context 'when signed in' do - before do - sign_in(user) - end + context 'when the personal snippet does not exist' do + context 'when signed in' do + before do + sign_in(user) + end - it 'responds with status 404' do - get action, id: 'doesntexist' + it 'responds with status 404' do + get :raw, id: 'doesntexist' - expect(response).to have_http_status(404) - end + expect(response).to have_http_status(404) end + end - context 'when not signed in' do - it 'responds with status 404' do - get action, id: 'doesntexist' + context 'when not signed in' do + it 'redirects to the sign in path' do + get :raw, id: 'doesntexist' - expect(response).to have_http_status(404) - end + expect(response).to redirect_to(new_user_session_path) end end end @@ -521,4 +543,16 @@ describe SnippetsController do end end end + + describe 'POST #preview_markdown' do + let(:snippet) { create(:personal_snippet, :public) } + + it 'renders json in a correct format' do + sign_in(user) + + post :preview_markdown, id: snippet, text: '*Markdown* text' + + expect(JSON.parse(response.body).keys).to match_array(%w(body references)) + end + end end diff --git a/spec/controllers/uploads_controller_spec.rb b/spec/controllers/uploads_controller_spec.rb index f67d26da0ac..8000c9dec61 100644 --- a/spec/controllers/uploads_controller_spec.rb +++ b/spec/controllers/uploads_controller_spec.rb @@ -8,6 +8,93 @@ end describe UploadsController do let!(:user) { create(:user, avatar: fixture_file_upload(Rails.root + "spec/fixtures/dk.png", "image/png")) } + describe 'POST create' do + let(:model) { 'personal_snippet' } + let(:snippet) { create(:personal_snippet, :public) } + let(:jpg) { fixture_file_upload(Rails.root + 'spec/fixtures/rails_sample.jpg', 'image/jpg') } + let(:txt) { fixture_file_upload(Rails.root + 'spec/fixtures/doc_sample.txt', 'text/plain') } + + context 'when a user does not have permissions to upload a file' do + it "returns 401 when the user is not logged in" do + post :create, model: model, id: snippet.id, format: :json + + expect(response).to have_http_status(401) + end + + it "returns 404 when user can't comment on a snippet" do + private_snippet = create(:personal_snippet, :private) + + sign_in(user) + post :create, model: model, id: private_snippet.id, format: :json + + expect(response).to have_http_status(404) + end + end + + context 'when a user is logged in' do + before do + sign_in(user) + end + + it "returns an error without file" do + post :create, model: model, id: snippet.id, format: :json + + expect(response).to have_http_status(422) + end + + it "returns an error with invalid model" do + expect { post :create, model: 'invalid', id: snippet.id, format: :json } + .to raise_error(ActionController::UrlGenerationError) + end + + it "returns 404 status when object not found" do + post :create, model: model, id: 9999, format: :json + + expect(response).to have_http_status(404) + end + + context 'with valid image' do + before do + post :create, model: 'personal_snippet', id: snippet.id, file: jpg, format: :json + end + + it 'returns a content with original filename, new link, and correct type.' do + expect(response.body).to match '\"alt\":\"rails_sample\"' + expect(response.body).to match "\"url\":\"/uploads" + end + + it 'creates a corresponding Upload record' do + upload = Upload.last + + aggregate_failures do + expect(upload).to exist + expect(upload.model).to eq snippet + end + end + end + + context 'with valid non-image file' do + before do + post :create, model: 'personal_snippet', id: snippet.id, file: txt, format: :json + end + + it 'returns a content with original filename, new link, and correct type.' do + expect(response.body).to match '\"alt\":\"doc_sample.txt\"' + expect(response.body).to match "\"url\":\"/uploads" + end + + it 'creates a corresponding Upload record' do + upload = Upload.last + + aggregate_failures do + expect(upload).to exist + expect(upload.model).to eq snippet + end + end + end + end + end + describe "GET show" do context 'Content-Disposition security measures' do let(:project) { create(:empty_project, :public) } @@ -386,5 +473,45 @@ describe UploadsController do end end end + + context 'Appearance' do + context 'when viewing a custom header logo' do + let!(:appearance) { create :appearance, header_logo: fixture_file_upload(Rails.root.join('spec/fixtures/dk.png'), 'image/png') } + + context 'when not signed in' do + it 'responds with status 200' do + get :show, model: 'appearance', mounted_as: 'header_logo', id: appearance.id, filename: 'dk.png' + + expect(response).to have_http_status(200) + end + + it_behaves_like 'content not cached without revalidation' do + subject do + get :show, model: 'appearance', mounted_as: 'header_logo', id: appearance.id, filename: 'dk.png' + response + end + end + end + end + + context 'when viewing a custom logo' do + let!(:appearance) { create :appearance, logo: fixture_file_upload(Rails.root.join('spec/fixtures/dk.png'), 'image/png') } + + context 'when not signed in' do + it 'responds with status 200' do + get :show, model: 'appearance', mounted_as: 'logo', id: appearance.id, filename: 'dk.png' + + expect(response).to have_http_status(200) + end + + it_behaves_like 'content not cached without revalidation' do + subject do + get :show, model: 'appearance', mounted_as: 'logo', id: appearance.id, filename: 'dk.png' + response + end + end + end + end + end end end diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index bbe9aaf737f..d33e2ba1e53 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -4,15 +4,6 @@ describe UsersController do let(:user) { create(:user) } describe 'GET #show' do - it 'is case-insensitive' do - user = create(:user, username: 'CamelCaseUser') - sign_in(user) - - get :show, username: user.username.downcase - - expect(response).to be_success - end - context 'with rendered views' do render_views @@ -45,9 +36,9 @@ describe UsersController do end context 'when logged out' do - it 'renders 404' do + it 'redirects to login page' do get :show, username: user.username - expect(response).to have_http_status(404) + expect(response).to redirect_to new_user_session_path end end @@ -61,6 +52,24 @@ describe UsersController do end end end + + context 'when a user by that username does not exist' do + context 'when logged out' do + it 'redirects to login page' do + get :show, username: 'nonexistent' + expect(response).to redirect_to new_user_session_path + end + end + + context 'when logged in' do + before { sign_in(user) } + + it 'renders 404' do + get :show, username: 'nonexistent' + expect(response).to have_http_status(404) + end + end + end end describe 'GET #calendar' do @@ -92,7 +101,7 @@ describe UsersController do describe 'GET #calendar_activities' do let!(:project) { create(:empty_project) } - let!(:user) { create(:user) } + let(:user) { create(:user) } before do allow_any_instance_of(User).to receive(:contributed_projects_ids).and_return([project.id]) @@ -133,4 +142,175 @@ describe UsersController do end end end + + describe 'GET #exists' do + before do + sign_in(user) + end + + context 'when user exists' do + it 'returns JSON indicating the user exists' do + get :exists, username: user.username + + expected_json = { exists: true }.to_json + expect(response.body).to eq(expected_json) + end + + context 'when the casing is different' do + let(:user) { create(:user, username: 'CamelCaseUser') } + + it 'returns JSON indicating the user exists' do + get :exists, username: user.username.downcase + + expected_json = { exists: true }.to_json + expect(response.body).to eq(expected_json) + end + end + end + + context 'when the user does not exist' do + it 'returns JSON indicating the user does not exist' do + get :exists, username: 'foo' + + expected_json = { exists: false }.to_json + expect(response.body).to eq(expected_json) + end + + context 'when a user changed their username' do + let(:redirect_route) { user.namespace.redirect_routes.create(path: 'old-username') } + + it 'returns JSON indicating a user by that username does not exist' do + get :exists, username: 'old-username' + + expected_json = { exists: false }.to_json + expect(response.body).to eq(expected_json) + end + end + end + end + + describe '#ensure_canonical_path' do + before do + sign_in(user) + end + + context 'for a GET request' do + context 'when requesting users at the root path' do + context 'when requesting the canonical path' do + let(:user) { create(:user, username: 'CamelCaseUser') } + + context 'with exactly matching casing' do + it 'responds with success' do + get :show, username: user.username + + expect(response).to be_success + end + end + + context 'with different casing' do + it 'redirects to the correct casing' do + get :show, username: user.username.downcase + + expect(response).to redirect_to(user) + expect(controller).not_to set_flash[:notice] + end + end + end + + context 'when requesting a redirected path' do + let(:redirect_route) { user.namespace.redirect_routes.create(path: 'old-path') } + + it 'redirects to the canonical path' do + get :show, username: redirect_route.path + + expect(response).to redirect_to(user) + expect(controller).to set_flash[:notice].to(user_moved_message(redirect_route, user)) + end + + context 'when the old path is a substring of the scheme or host' do + let(:redirect_route) { user.namespace.redirect_routes.create(path: 'http') } + + it 'does not modify the requested host' do + get :show, username: redirect_route.path + + expect(response).to redirect_to(user) + expect(controller).to set_flash[:notice].to(user_moved_message(redirect_route, user)) + end + end + + context 'when the old path is substring of users' do + let(:redirect_route) { user.namespace.redirect_routes.create(path: 'ser') } + + it 'redirects to the canonical path' do + get :show, username: redirect_route.path + + expect(response).to redirect_to(user) + expect(controller).to set_flash[:notice].to(user_moved_message(redirect_route, user)) + end + end + end + end + + context 'when requesting users under the /users path' do + context 'when requesting the canonical path' do + let(:user) { create(:user, username: 'CamelCaseUser') } + + context 'with exactly matching casing' do + it 'responds with success' do + get :projects, username: user.username + + expect(response).to be_success + end + end + + context 'with different casing' do + it 'redirects to the correct casing' do + get :projects, username: user.username.downcase + + expect(response).to redirect_to(user_projects_path(user)) + expect(controller).not_to set_flash[:notice] + end + end + end + + context 'when requesting a redirected path' do + let(:redirect_route) { user.namespace.redirect_routes.create(path: 'old-path') } + + it 'redirects to the canonical path' do + get :projects, username: redirect_route.path + + expect(response).to redirect_to(user_projects_path(user)) + expect(controller).to set_flash[:notice].to(user_moved_message(redirect_route, user)) + end + + context 'when the old path is a substring of the scheme or host' do + let(:redirect_route) { user.namespace.redirect_routes.create(path: 'http') } + + it 'does not modify the requested host' do + get :projects, username: redirect_route.path + + expect(response).to redirect_to(user_projects_path(user)) + expect(controller).to set_flash[:notice].to(user_moved_message(redirect_route, user)) + end + end + + context 'when the old path is substring of users' do + let(:redirect_route) { user.namespace.redirect_routes.create(path: 'ser') } + + # I.e. /users/ser should not become /ufoos/ser + it 'does not modify the /users part of the path' do + get :projects, username: redirect_route.path + + expect(response).to redirect_to(user_projects_path(user)) + expect(controller).to set_flash[:notice].to(user_moved_message(redirect_route, user)) + end + end + end + end + end + end + + def user_moved_message(redirect_route, user) + "User '#{redirect_route.path}' was moved to '#{user.full_path}'. Please update any links and bookmarks that may still have the old path." + end end |