diff options
author | Douwe Maan <douwe@selenight.nl> | 2017-03-07 16:39:52 -0600 |
---|---|---|
committer | Douwe Maan <douwe@selenight.nl> | 2017-03-07 16:39:52 -0600 |
commit | 3420c6cc4ef53d78db276bde06dbd9ab47165fb3 (patch) | |
tree | 2e34fd77be6424157243b67911f5c53d3a585692 /spec | |
parent | 9895d6707d51140b3cc75e925cfd775c6bd93f83 (diff) | |
parent | 7f2819b778b055278a7fafe9c782d12d09dbd2ea (diff) | |
download | gitlab-ce-3420c6cc4ef53d78db276bde06dbd9ab47165fb3.tar.gz |
Merge branch 'master' into orderable-issues
Diffstat (limited to 'spec')
79 files changed, 5113 insertions, 343 deletions
diff --git a/spec/controllers/admin/applications_controller_spec.rb b/spec/controllers/admin/applications_controller_spec.rb new file mode 100644 index 00000000000..e311b8a63b2 --- /dev/null +++ b/spec/controllers/admin/applications_controller_spec.rb @@ -0,0 +1,65 @@ +require 'spec_helper' + +describe Admin::ApplicationsController do + let(:admin) { create(:admin) } + let(:application) { create(:oauth_application, owner_id: nil, owner_type: nil) } + + before do + sign_in(admin) + end + + describe 'GET #new' do + it 'renders the application form' do + get :new + + expect(response).to render_template :new + expect(assigns[:scopes]).to be_kind_of(Doorkeeper::OAuth::Scopes) + end + end + + describe 'GET #edit' do + it 'renders the application form' do + get :edit, id: application.id + + expect(response).to render_template :edit + expect(assigns[:scopes]).to be_kind_of(Doorkeeper::OAuth::Scopes) + end + end + + describe 'POST #create' do + it 'creates the application' do + expect do + post :create, doorkeeper_application: attributes_for(:application) + end.to change { Doorkeeper::Application.count }.by(1) + + application = Doorkeeper::Application.last + + expect(response).to redirect_to(admin_application_path(application)) + end + + it 'renders the application form on errors' do + expect do + post :create, doorkeeper_application: attributes_for(:application).merge(redirect_uri: nil) + end.not_to change { Doorkeeper::Application.count } + + expect(response).to render_template :new + expect(assigns[:scopes]).to be_kind_of(Doorkeeper::OAuth::Scopes) + end + end + + describe 'PATCH #update' do + it 'updates the application' do + patch :update, id: application.id, doorkeeper_application: { redirect_uri: 'http://example.com/' } + + expect(response).to redirect_to(admin_application_path(application)) + expect(application.reload.redirect_uri).to eq 'http://example.com/' + end + + it 'renders the application form on errors' do + patch :update, id: application.id, doorkeeper_application: { redirect_uri: nil } + + expect(response).to render_template :edit + expect(assigns[:scopes]).to be_kind_of(Doorkeeper::OAuth::Scopes) + end + end +end diff --git a/spec/controllers/profiles/personal_access_tokens_spec.rb b/spec/controllers/profiles/personal_access_tokens_spec.rb index 9d5f4c99f6d..dfed1de2046 100644 --- a/spec/controllers/profiles/personal_access_tokens_spec.rb +++ b/spec/controllers/profiles/personal_access_tokens_spec.rb @@ -2,48 +2,55 @@ require 'spec_helper' describe Profiles::PersonalAccessTokensController do let(:user) { create(:user) } + let(:token_attributes) { attributes_for(:personal_access_token) } + + before { sign_in(user) } describe '#create' do def created_token PersonalAccessToken.order(:created_at).last end - before { sign_in(user) } - - it "allows creation of a token" do + it "allows creation of a token with scopes" do name = FFaker::Product.brand + scopes = %w[api read_user] - post :create, personal_access_token: { name: name } + post :create, personal_access_token: token_attributes.merge(scopes: scopes, name: name) expect(created_token).not_to be_nil expect(created_token.name).to eq(name) - expect(created_token.expires_at).to be_nil + expect(created_token.scopes).to eq(scopes) expect(PersonalAccessToken.active).to include(created_token) end it "allows creation of a token with an expiry date" do - expires_at = 5.days.from_now + expires_at = 5.days.from_now.to_date - post :create, personal_access_token: { name: FFaker::Product.brand, expires_at: expires_at } + post :create, personal_access_token: token_attributes.merge(expires_at: expires_at) expect(created_token).not_to be_nil - expect(created_token.expires_at.to_i).to eq(expires_at.to_i) + expect(created_token.expires_at).to eq(expires_at) end + end - context "scopes" do - it "allows creation of a token with scopes" do - post :create, personal_access_token: { name: FFaker::Product.brand, scopes: %w(api read_user) } + describe '#index' do + let!(:active_personal_access_token) { create(:personal_access_token, user: user) } + let!(:inactive_personal_access_token) { create(:personal_access_token, :revoked, user: user) } + let!(:impersonation_personal_access_token) { create(:personal_access_token, :impersonation, user: user) } - expect(created_token).not_to be_nil - expect(created_token.scopes).to eq(%w(api read_user)) - end + before { get :index } - it "allows creation of a token with no scopes" do - post :create, personal_access_token: { name: FFaker::Product.brand, scopes: [] } + it "retrieves active personal access tokens" do + expect(assigns(:active_personal_access_tokens)).to include(active_personal_access_token) + end + + it "retrieves inactive personal access tokens" do + expect(assigns(:inactive_personal_access_tokens)).to include(inactive_personal_access_token) + end - expect(created_token).not_to be_nil - expect(created_token.scopes).to eq([]) - end + it "does not retrieve impersonation personal access tokens" do + expect(assigns(:active_personal_access_tokens)).not_to include(impersonation_personal_access_token) + expect(assigns(:inactive_personal_access_tokens)).not_to include(impersonation_personal_access_token) end end end diff --git a/spec/controllers/projects/environments_controller_spec.rb b/spec/controllers/projects/environments_controller_spec.rb index 84d119f1867..83d80b376fb 100644 --- a/spec/controllers/projects/environments_controller_spec.rb +++ b/spec/controllers/projects/environments_controller_spec.rb @@ -187,6 +187,52 @@ describe Projects::EnvironmentsController do end end + describe 'GET #metrics' do + before do + allow(controller).to receive(:environment).and_return(environment) + end + + context 'when environment has no metrics' do + before do + expect(environment).to receive(:metrics).and_return(nil) + end + + it 'returns a metrics page' do + get :metrics, environment_params + + expect(response).to be_ok + end + + context 'when requesting metrics as JSON' do + it 'returns a metrics JSON document' do + get :metrics, environment_params(format: :json) + + expect(response).to have_http_status(204) + expect(json_response).to eq({}) + end + end + end + + context 'when environment has some metrics' do + before do + expect(environment).to receive(:metrics).and_return({ + success: true, + metrics: {}, + last_update: 42 + }) + end + + it 'returns a metrics JSON document' do + get :metrics, environment_params(format: :json) + + 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 + end + def environment_params(opts = {}) opts.reverse_merge(namespace_id: project.namespace, project_id: project, diff --git a/spec/controllers/projects/settings/repository_controller_spec.rb b/spec/controllers/projects/settings/repository_controller_spec.rb new file mode 100644 index 00000000000..f73471f8ca8 --- /dev/null +++ b/spec/controllers/projects/settings/repository_controller_spec.rb @@ -0,0 +1,20 @@ +require 'spec_helper' + +describe Projects::Settings::RepositoryController do + let(:project) { create(:project_empty_repo, :public) } + let(:user) { create(:user) } + + before do + project.add_master(user) + sign_in(user) + end + + describe 'GET show' do + it 'renders show with 200 status code' do + get :show, namespace_id: project.namespace, project_id: project + + expect(response).to have_http_status(200) + expect(response).to render_template(:show) + end + end +end diff --git a/spec/controllers/uploads_controller_spec.rb b/spec/controllers/uploads_controller_spec.rb index c9584ddf18c..f67d26da0ac 100644 --- a/spec/controllers/uploads_controller_spec.rb +++ b/spec/controllers/uploads_controller_spec.rb @@ -1,4 +1,9 @@ require 'spec_helper' +shared_examples 'content not cached without revalidation' do + it 'ensures content will not be cached without revalidation' do + expect(subject['Cache-Control']).to eq('max-age=0, private, must-revalidate') + end +end describe UploadsController do let!(:user) { create(:user, avatar: fixture_file_upload(Rails.root + "spec/fixtures/dk.png", "image/png")) } @@ -50,6 +55,13 @@ describe UploadsController do expect(response).to have_http_status(200) end + + it_behaves_like 'content not cached without revalidation' do + subject do + get :show, model: 'user', mounted_as: 'avatar', id: user.id, filename: 'image.png' + response + end + end end end @@ -59,6 +71,13 @@ describe UploadsController do expect(response).to have_http_status(200) end + + it_behaves_like 'content not cached without revalidation' do + subject do + get :show, model: 'user', mounted_as: 'avatar', id: user.id, filename: 'image.png' + response + end + end end end @@ -76,6 +95,13 @@ describe UploadsController do expect(response).to have_http_status(200) end + + it_behaves_like 'content not cached without revalidation' do + subject do + get :show, model: 'project', mounted_as: 'avatar', id: project.id, filename: 'image.png' + response + end + end end context "when signed in" do @@ -88,6 +114,13 @@ describe UploadsController do expect(response).to have_http_status(200) end + + it_behaves_like 'content not cached without revalidation' do + subject do + get :show, model: 'project', mounted_as: 'avatar', id: project.id, filename: 'image.png' + response + end + end end end @@ -133,6 +166,13 @@ describe UploadsController do expect(response).to have_http_status(200) end + + it_behaves_like 'content not cached without revalidation' do + subject do + get :show, model: 'project', mounted_as: 'avatar', id: project.id, filename: 'image.png' + response + end + end end end @@ -157,6 +197,13 @@ describe UploadsController do expect(response).to have_http_status(200) end + + it_behaves_like 'content not cached without revalidation' do + subject do + get :show, model: 'group', mounted_as: 'avatar', id: group.id, filename: 'image.png' + response + end + end end context "when signed in" do @@ -169,6 +216,13 @@ describe UploadsController do expect(response).to have_http_status(200) end + + it_behaves_like 'content not cached without revalidation' do + subject do + get :show, model: 'group', mounted_as: 'avatar', id: group.id, filename: 'image.png' + response + end + end end end @@ -205,6 +259,13 @@ describe UploadsController do expect(response).to have_http_status(200) end + + it_behaves_like 'content not cached without revalidation' do + subject do + get :show, model: 'group', mounted_as: 'avatar', id: group.id, filename: 'image.png' + response + end + end end end @@ -234,6 +295,13 @@ describe UploadsController do expect(response).to have_http_status(200) end + + it_behaves_like 'content not cached without revalidation' do + subject do + get :show, model: 'note', mounted_as: 'attachment', id: note.id, filename: 'image.png' + response + end + end end context "when signed in" do @@ -246,6 +314,13 @@ describe UploadsController do expect(response).to have_http_status(200) end + + it_behaves_like 'content not cached without revalidation' do + subject do + get :show, model: 'note', mounted_as: 'attachment', id: note.id, filename: 'image.png' + response + end + end end end @@ -291,6 +366,13 @@ describe UploadsController do expect(response).to have_http_status(200) end + + it_behaves_like 'content not cached without revalidation' do + subject do + get :show, model: 'note', mounted_as: 'attachment', id: note.id, filename: 'image.png' + response + end + end end end diff --git a/spec/factories/chat_teams.rb b/spec/factories/chat_teams.rb new file mode 100644 index 00000000000..82f44fa3d15 --- /dev/null +++ b/spec/factories/chat_teams.rb @@ -0,0 +1,9 @@ +FactoryGirl.define do + factory :chat_team, class: ChatTeam do + sequence :team_id do |n| + "abcdefghijklm#{n}" + end + + namespace factory: :group + end +end diff --git a/spec/factories/ci/builds.rb b/spec/factories/ci/builds.rb index 279583c2c44..6b0d084614b 100644 --- a/spec/factories/ci/builds.rb +++ b/spec/factories/ci/builds.rb @@ -15,8 +15,8 @@ FactoryGirl.define do options do { - image: "ruby:2.1", - services: ["postgres"] + image: 'ruby:2.1', + services: ['postgres'] } end @@ -166,5 +166,31 @@ FactoryGirl.define do allow(build).to receive(:commit).and_return build(:commit) end end + + trait :extended_options do + options do + { + image: 'ruby:2.1', + services: ['postgres'], + after_script: "ls\ndate", + artifacts: { + name: 'artifacts_file', + untracked: false, + paths: ['out/'], + when: 'always', + expire_in: '7d' + }, + cache: { + key: 'cache_key', + untracked: false, + paths: ['vendor/*'] + } + } + end + end + + trait :no_options do + options { {} } + end end end diff --git a/spec/factories/oauth_access_grants.rb b/spec/factories/oauth_access_grants.rb new file mode 100644 index 00000000000..543b3e99274 --- /dev/null +++ b/spec/factories/oauth_access_grants.rb @@ -0,0 +1,11 @@ +FactoryGirl.define do + factory :oauth_access_grant do + resource_owner_id { create(:user).id } + application + token { Doorkeeper::OAuth::Helpers::UniqueToken.generate } + expires_in 2.hours + + redirect_uri { application.redirect_uri } + scopes { application.scopes } + end +end diff --git a/spec/factories/oauth_access_tokens.rb b/spec/factories/oauth_access_tokens.rb index ccf02d0719b..a46bc1d8ce8 100644 --- a/spec/factories/oauth_access_tokens.rb +++ b/spec/factories/oauth_access_tokens.rb @@ -2,6 +2,7 @@ FactoryGirl.define do factory :oauth_access_token do resource_owner application - token '123456' + token { Doorkeeper::OAuth::Helpers::UniqueToken.generate } + scopes { application.scopes } end end diff --git a/spec/factories/oauth_applications.rb b/spec/factories/oauth_applications.rb index d116a573830..86cdc208268 100644 --- a/spec/factories/oauth_applications.rb +++ b/spec/factories/oauth_applications.rb @@ -1,7 +1,7 @@ FactoryGirl.define do factory :oauth_application, class: 'Doorkeeper::Application', aliases: [:application] do name { FFaker::Name.name } - uid { FFaker::Name.name } + uid { Doorkeeper::OAuth::Helpers::UniqueToken.generate } redirect_uri { FFaker::Internet.uri('http') } owner owner_type 'User' diff --git a/spec/factories/personal_access_tokens.rb b/spec/factories/personal_access_tokens.rb index 811eab7e15b..7b15ba47de1 100644 --- a/spec/factories/personal_access_tokens.rb +++ b/spec/factories/personal_access_tokens.rb @@ -6,5 +6,22 @@ FactoryGirl.define do revoked false expires_at { 5.days.from_now } scopes ['api'] + impersonation false + + trait :impersonation do + impersonation true + end + + trait :revoked do + revoked true + end + + trait :expired do + expires_at { 1.day.ago } + end + + trait :invalid do + token nil + end end end diff --git a/spec/factories/projects.rb b/spec/factories/projects.rb index 70c65bc693a..c6f91e05d83 100644 --- a/spec/factories/projects.rb +++ b/spec/factories/projects.rb @@ -195,4 +195,15 @@ FactoryGirl.define do factory :kubernetes_project, parent: :empty_project do kubernetes_service end + + factory :prometheus_project, parent: :empty_project do + after :create do |project| + project.create_prometheus_service( + active: true, + properties: { + api_url: 'https://prometheus.example.com' + } + ) + end + end end diff --git a/spec/features/admin/admin_users_impersonation_tokens_spec.rb b/spec/features/admin/admin_users_impersonation_tokens_spec.rb new file mode 100644 index 00000000000..9ff5c2f9d40 --- /dev/null +++ b/spec/features/admin/admin_users_impersonation_tokens_spec.rb @@ -0,0 +1,72 @@ +require 'spec_helper' + +describe 'Admin > Users > Impersonation Tokens', feature: true, js: true do + let(:admin) { create(:admin) } + let!(:user) { create(:user) } + + def active_impersonation_tokens + find(".table.active-tokens") + end + + def inactive_impersonation_tokens + find(".table.inactive-tokens") + end + + before { login_as(admin) } + + describe "token creation" do + it "allows creation of a token" do + name = FFaker::Product.brand + + visit admin_user_impersonation_tokens_path(user_id: user.username) + fill_in "Name", with: name + + # Set date to 1st of next month + find_field("Expires at").trigger('focus') + find(".pika-next").click + click_on "1" + + # Scopes + check "api" + check "read_user" + + expect { click_on "Create Impersonation Token" }.to change { PersonalAccessTokensFinder.new(impersonation: true).execute.count } + expect(active_impersonation_tokens).to have_text(name) + expect(active_impersonation_tokens).to have_text('In') + expect(active_impersonation_tokens).to have_text('api') + expect(active_impersonation_tokens).to have_text('read_user') + end + end + + describe 'active tokens' do + let!(:impersonation_token) { create(:personal_access_token, :impersonation, user: user) } + let!(:personal_access_token) { create(:personal_access_token, user: user) } + + it 'only shows impersonation tokens' do + visit admin_user_impersonation_tokens_path(user_id: user.username) + + expect(active_impersonation_tokens).to have_text(impersonation_token.name) + expect(active_impersonation_tokens).not_to have_text(personal_access_token.name) + end + end + + describe "inactive tokens" do + let!(:impersonation_token) { create(:personal_access_token, :impersonation, user: user) } + + it "allows revocation of an active impersonation token" do + visit admin_user_impersonation_tokens_path(user_id: user.username) + + click_on "Revoke" + + expect(inactive_impersonation_tokens).to have_text(impersonation_token.name) + end + + it "moves expired tokens to the 'inactive' section" do + impersonation_token.update(expires_at: 5.days.ago) + + visit admin_user_impersonation_tokens_path(user_id: user.username) + + expect(inactive_impersonation_tokens).to have_text(impersonation_token.name) + end + end +end diff --git a/spec/features/issues/filtered_search/dropdown_assignee_spec.rb b/spec/features/issues/filtered_search/dropdown_assignee_spec.rb index 93763f092fb..ede6aa0c201 100644 --- a/spec/features/issues/filtered_search/dropdown_assignee_spec.rb +++ b/spec/features/issues/filtered_search/dropdown_assignee_spec.rb @@ -1,25 +1,16 @@ require 'rails_helper' -describe 'Dropdown assignee', js: true, feature: true do - include WaitForAjax - +describe 'Dropdown assignee', :feature, :js do let!(:project) { create(:empty_project) } let!(:user) { create(:user, name: 'administrator', username: 'root') } let!(:user_john) { create(:user, name: 'John', username: 'th0mas') } let!(:user_jacob) { create(:user, name: 'Jacob', username: 'otter32') } let(:filtered_search) { find('.filtered-search') } let(:js_dropdown_assignee) { '#js-dropdown-assignee' } - - def send_keys_to_filtered_search(input) - input.split("").each do |i| - filtered_search.send_keys(i) - sleep 5 - wait_for_ajax - end - end + let(:filter_dropdown) { find("#{js_dropdown_assignee} .filter-dropdown") } def dropdown_assignee_size - page.all('#js-dropdown-assignee .filter-dropdown .filter-dropdown-item').size + filter_dropdown.all('.filter-dropdown-item').size end def click_assignee(text) @@ -56,63 +47,80 @@ describe 'Dropdown assignee', js: true, feature: true do end it 'should hide loading indicator when loaded' do - send_keys_to_filtered_search('assignee:') + filtered_search.set('assignee:') - expect(page).not_to have_css('#js-dropdown-assignee .filter-dropdown-loading') + expect(find(js_dropdown_assignee)).to have_css('.filter-dropdown-loading') + expect(find(js_dropdown_assignee)).not_to have_css('.filter-dropdown-loading') end it 'should load all the assignees when opened' do - send_keys_to_filtered_search('assignee:') + filtered_search.set('assignee:') expect(dropdown_assignee_size).to eq(3) end it 'shows current user at top of dropdown' do - send_keys_to_filtered_search('assignee:') + filtered_search.set('assignee:') - expect(first('#js-dropdown-assignee .filter-dropdown li')).to have_content(user.name) + expect(filter_dropdown.first('.filter-dropdown-item')).to have_content(user.name) end end describe 'filtering' do before do - send_keys_to_filtered_search('assignee:') + filtered_search.set('assignee:') + + expect(find("#{js_dropdown_assignee} .filter-dropdown")).to have_content(user_john.name) + expect(find("#{js_dropdown_assignee} .filter-dropdown")).to have_content(user_jacob.name) + expect(find("#{js_dropdown_assignee} .filter-dropdown")).to have_content(user.name) end it 'filters by name' do - send_keys_to_filtered_search('j') + filtered_search.send_keys('j') - expect(dropdown_assignee_size).to eq(2) + expect(find("#{js_dropdown_assignee} .filter-dropdown")).to have_content(user_john.name) + expect(find("#{js_dropdown_assignee} .filter-dropdown")).to have_content(user_jacob.name) + expect(find("#{js_dropdown_assignee} .filter-dropdown")).to have_no_content(user.name) end it 'filters by case insensitive name' do - send_keys_to_filtered_search('J') + filtered_search.send_keys('J') - expect(dropdown_assignee_size).to eq(2) + expect(find("#{js_dropdown_assignee} .filter-dropdown")).to have_content(user_john.name) + expect(find("#{js_dropdown_assignee} .filter-dropdown")).to have_content(user_jacob.name) + expect(find("#{js_dropdown_assignee} .filter-dropdown")).to have_no_content(user.name) end it 'filters by username with symbol' do - send_keys_to_filtered_search('@ot') + filtered_search.send_keys('@ot') - expect(dropdown_assignee_size).to eq(2) + expect(find("#{js_dropdown_assignee} .filter-dropdown")).to have_content(user_jacob.name) + expect(find("#{js_dropdown_assignee} .filter-dropdown")).to have_content(user.name) + expect(find("#{js_dropdown_assignee} .filter-dropdown")).to have_no_content(user_john.name) end it 'filters by case insensitive username with symbol' do - send_keys_to_filtered_search('@OT') + filtered_search.send_keys('@OT') - expect(dropdown_assignee_size).to eq(2) + expect(find("#{js_dropdown_assignee} .filter-dropdown")).to have_content(user_jacob.name) + expect(find("#{js_dropdown_assignee} .filter-dropdown")).to have_content(user.name) + expect(find("#{js_dropdown_assignee} .filter-dropdown")).to have_no_content(user_john.name) end it 'filters by username without symbol' do - send_keys_to_filtered_search('ot') + filtered_search.send_keys('ot') - expect(dropdown_assignee_size).to eq(2) + expect(find("#{js_dropdown_assignee} .filter-dropdown")).to have_content(user_jacob.name) + expect(find("#{js_dropdown_assignee} .filter-dropdown")).to have_content(user.name) + expect(find("#{js_dropdown_assignee} .filter-dropdown")).to have_no_content(user_john.name) end it 'filters by case insensitive username without symbol' do - send_keys_to_filtered_search('OT') + filtered_search.send_keys('OT') - expect(dropdown_assignee_size).to eq(2) + expect(find("#{js_dropdown_assignee} .filter-dropdown")).to have_content(user_jacob.name) + expect(find("#{js_dropdown_assignee} .filter-dropdown")).to have_content(user.name) + expect(find("#{js_dropdown_assignee} .filter-dropdown")).to have_no_content(user_john.name) end end @@ -129,7 +137,7 @@ describe 'Dropdown assignee', js: true, feature: true do end it 'fills in the assignee username when the assignee has been filtered' do - send_keys_to_filtered_search('roo') + filtered_search.send_keys('roo') click_assignee(user.name) expect(page).to have_css(js_dropdown_assignee, visible: false) @@ -173,7 +181,7 @@ describe 'Dropdown assignee', js: true, feature: true do describe 'caching requests' do it 'caches requests after the first load' do filtered_search.set('assignee') - send_keys_to_filtered_search(':') + filtered_search.send_keys(':') initial_size = dropdown_assignee_size expect(initial_size).to be > 0 @@ -182,7 +190,7 @@ describe 'Dropdown assignee', js: true, feature: true do project.team << [new_user, :master] find('.filtered-search-input-container .clear-search').click filtered_search.set('assignee') - send_keys_to_filtered_search(':') + filtered_search.send_keys(':') expect(dropdown_assignee_size).to eq(initial_size) end diff --git a/spec/features/merge_requests/widget_spec.rb b/spec/features/merge_requests/widget_spec.rb index b575aeff0d8..c2db7d8da3c 100644 --- a/spec/features/merge_requests/widget_spec.rb +++ b/spec/features/merge_requests/widget_spec.rb @@ -37,7 +37,12 @@ describe 'Merge request', :feature, :js do context 'view merge request' do let!(:environment) { create(:environment, project: project) } - let!(:deployment) { create(:deployment, environment: environment, ref: 'feature', sha: merge_request.diff_head_sha) } + + let!(:deployment) do + create(:deployment, environment: environment, + ref: 'feature', + sha: merge_request.diff_head_sha) + end before do visit namespace_project_merge_request_path(project.namespace, project, merge_request) @@ -96,6 +101,26 @@ describe 'Merge request', :feature, :js do end end + context 'when merge request is in the blocked pipeline state' do + before do + create(:ci_pipeline, project: project, + sha: merge_request.diff_head_sha, + ref: merge_request.source_branch, + status: :manual) + + visit namespace_project_merge_request_path(project.namespace, + project, + merge_request) + end + + it 'shows information about blocked pipeline' do + expect(page).to have_content("Pipeline blocked") + expect(page).to have_content( + "The pipeline for this merge request requires a manual action") + expect(page).to have_css('.ci-status-icon-manual') + end + end + context 'view merge request with MWBS button' do before do commit_status = create(:commit_status, project: project, status: 'pending') diff --git a/spec/features/profiles/personal_access_tokens_spec.rb b/spec/features/profiles/personal_access_tokens_spec.rb index eb7b8a24669..0917d4dc3ef 100644 --- a/spec/features/profiles/personal_access_tokens_spec.rb +++ b/spec/features/profiles/personal_access_tokens_spec.rb @@ -4,11 +4,11 @@ describe 'Profile > Personal Access Tokens', feature: true, js: true do let(:user) { create(:user) } def active_personal_access_tokens - find(".table.active-personal-access-tokens") + find(".table.active-tokens") end def inactive_personal_access_tokens - find(".table.inactive-personal-access-tokens") + find(".table.inactive-tokens") end def created_personal_access_token @@ -26,7 +26,7 @@ describe 'Profile > Personal Access Tokens', feature: true, js: true do end describe "token creation" do - it "allows creation of a token" do + it "allows creation of a personal access token" do name = FFaker::Product.brand visit profile_personal_access_tokens_path @@ -43,7 +43,7 @@ describe 'Profile > Personal Access Tokens', feature: true, js: true do click_on "Create Personal Access Token" expect(active_personal_access_tokens).to have_text(name) - expect(active_personal_access_tokens).to have_text(Date.today.next_month.at_beginning_of_month.to_s(:medium)) + expect(active_personal_access_tokens).to have_text('In') expect(active_personal_access_tokens).to have_text('api') expect(active_personal_access_tokens).to have_text('read_user') end @@ -60,6 +60,18 @@ describe 'Profile > Personal Access Tokens', feature: true, js: true do end end + describe 'active tokens' do + let!(:impersonation_token) { create(:personal_access_token, :impersonation, user: user) } + let!(:personal_access_token) { create(:personal_access_token, user: user) } + + it 'only shows personal access tokens' do + visit profile_personal_access_tokens_path + + expect(active_personal_access_tokens).to have_text(personal_access_token.name) + expect(active_personal_access_tokens).not_to have_text(impersonation_token.name) + end + end + describe "inactive tokens" do let!(:personal_access_token) { create(:personal_access_token, user: user) } diff --git a/spec/features/projects/environments/environment_metrics_spec.rb b/spec/features/projects/environments/environment_metrics_spec.rb new file mode 100644 index 00000000000..ee925e811e1 --- /dev/null +++ b/spec/features/projects/environments/environment_metrics_spec.rb @@ -0,0 +1,39 @@ +require 'spec_helper' + +feature 'Environment > Metrics', :feature do + include PrometheusHelpers + + given(:user) { create(:user) } + given(:project) { create(:prometheus_project) } + given(:pipeline) { create(:ci_pipeline, project: project) } + given(:build) { create(:ci_build, pipeline: pipeline) } + given(:environment) { create(:environment, project: project) } + given(:current_time) { Time.now.utc } + + background do + project.add_developer(user) + create(:deployment, environment: environment, deployable: build) + stub_all_prometheus_requests(environment.slug) + + login_as(user) + visit_environment(environment) + end + + around do |example| + Timecop.freeze(current_time) { example.run } + end + + context 'with deployments and related deployable present' do + scenario 'shows metrics' do + click_link('See metrics') + + expect(page).to have_css('svg.prometheus-graph') + end + end + + def visit_environment(environment) + visit namespace_project_environment_path(environment.project.namespace, + environment.project, + environment) + end +end diff --git a/spec/features/environment_spec.rb b/spec/features/projects/environments/environment_spec.rb index 65373e3f77d..e2d16e0830a 100644 --- a/spec/features/environment_spec.rb +++ b/spec/features/projects/environments/environment_spec.rb @@ -37,13 +37,7 @@ feature 'Environment', :feature do scenario 'does show deployment SHA' do expect(page).to have_link(deployment.short_sha) - end - - scenario 'does not show a re-deploy button for deployment without build' do expect(page).not_to have_link('Re-deploy') - end - - scenario 'does not show terminal button' do expect(page).not_to have_terminal_button end end @@ -58,13 +52,7 @@ feature 'Environment', :feature do scenario 'does show build name' do expect(page).to have_link("#{build.name} (##{build.id})") - end - - scenario 'does show re-deploy button' do expect(page).to have_link('Re-deploy') - end - - scenario 'does not show terminal button' do expect(page).not_to have_terminal_button end @@ -117,9 +105,6 @@ feature 'Environment', :feature do it 'displays a web terminal' do expect(page).to have_selector('#terminal') - end - - it 'displays a link to the environment external url' do expect(page).to have_link(nil, href: environment.external_url) end end @@ -147,10 +132,6 @@ feature 'Environment', :feature do on_stop: 'close_app') end - scenario 'does show stop button' do - expect(page).to have_link('Stop') - end - scenario 'does allow to stop environment' do click_link('Stop') diff --git a/spec/features/environments_spec.rb b/spec/features/projects/environments/environments_spec.rb index 25f31b423b8..25f31b423b8 100644 --- a/spec/features/environments_spec.rb +++ b/spec/features/projects/environments/environments_spec.rb diff --git a/spec/features/projects/labels/issues_sorted_by_priority_spec.rb b/spec/features/projects/labels/issues_sorted_by_priority_spec.rb index 7414ce21f59..de3c6eceb82 100644 --- a/spec/features/projects/labels/issues_sorted_by_priority_spec.rb +++ b/spec/features/projects/labels/issues_sorted_by_priority_spec.rb @@ -32,7 +32,7 @@ feature 'Issue prioritization', feature: true do visit namespace_project_issues_path(project.namespace, project, sort: 'priority') # Ensure we are indicating that issues are sorted by priority - expect(page).to have_selector('.dropdown-toggle', text: 'Priority') + expect(page).to have_selector('.dropdown-toggle', text: 'Label priority') page.within('.issues-holder') do issue_titles = all('.issues-list .issue-title-text').map(&:text) @@ -70,7 +70,7 @@ feature 'Issue prioritization', feature: true do login_as user visit namespace_project_issues_path(project.namespace, project, sort: 'priority') - expect(page).to have_selector('.dropdown-toggle', text: 'Priority') + expect(page).to have_selector('.dropdown-toggle', text: 'Label priority') page.within('.issues-holder') do issue_titles = all('.issues-list .issue-title-text').map(&:text) diff --git a/spec/features/security/project/internal_access_spec.rb b/spec/features/security/project/internal_access_spec.rb index 24af062d763..1a66d1a6a1e 100644 --- a/spec/features/security/project/internal_access_spec.rb +++ b/spec/features/security/project/internal_access_spec.rb @@ -110,6 +110,20 @@ describe "Internal Project Access", feature: true do it { is_expected.to be_denied_for(:external) } end + describe "GET /:project_path/settings/repository" do + subject { namespace_project_settings_repository_path(project.namespace, project) } + + it { is_expected.to be_allowed_for(:admin) } + it { is_expected.to be_allowed_for(:owner).of(project) } + it { is_expected.to be_allowed_for(:master).of(project) } + it { is_expected.to be_denied_for(:developer).of(project) } + it { is_expected.to be_denied_for(:reporter).of(project) } + it { is_expected.to be_denied_for(:guest).of(project) } + it { is_expected.to be_denied_for(:user) } + it { is_expected.to be_denied_for(:visitor) } + it { is_expected.to be_denied_for(:external) } + end + describe "GET /:project_path/blob" do let(:commit) { project.repository.commit } subject { namespace_project_blob_path(project.namespace, project, File.join(commit.id, '.gitignore')) } diff --git a/spec/features/security/project/private_access_spec.rb b/spec/features/security/project/private_access_spec.rb index c511dcfa18e..ad3bd60a313 100644 --- a/spec/features/security/project/private_access_spec.rb +++ b/spec/features/security/project/private_access_spec.rb @@ -110,6 +110,20 @@ describe "Private Project Access", feature: true do it { is_expected.to be_denied_for(:external) } end + describe "GET /:project_path/settings/repository" do + subject { namespace_project_settings_repository_path(project.namespace, project) } + + it { is_expected.to be_allowed_for(:admin) } + it { is_expected.to be_allowed_for(:owner).of(project) } + it { is_expected.to be_allowed_for(:master).of(project) } + it { is_expected.to be_denied_for(:developer).of(project) } + it { is_expected.to be_denied_for(:reporter).of(project) } + it { is_expected.to be_denied_for(:guest).of(project) } + it { is_expected.to be_denied_for(:user) } + it { is_expected.to be_denied_for(:external) } + it { is_expected.to be_denied_for(:visitor) } + end + describe "GET /:project_path/blob" do let(:commit) { project.repository.commit } subject { namespace_project_blob_path(project.namespace, project, File.join(commit.id, '.gitignore'))} diff --git a/spec/features/security/project/public_access_spec.rb b/spec/features/security/project/public_access_spec.rb index d8cc012c27e..e06aab4e0b2 100644 --- a/spec/features/security/project/public_access_spec.rb +++ b/spec/features/security/project/public_access_spec.rb @@ -110,6 +110,20 @@ describe "Public Project Access", feature: true do it { is_expected.to be_denied_for(:external) } end + describe "GET /:project_path/settings/repository" do + subject { namespace_project_settings_repository_path(project.namespace, project) } + + it { is_expected.to be_allowed_for(:admin) } + it { is_expected.to be_allowed_for(:owner).of(project) } + it { is_expected.to be_allowed_for(:master).of(project) } + it { is_expected.to be_denied_for(:developer).of(project) } + it { is_expected.to be_denied_for(:reporter).of(project) } + it { is_expected.to be_denied_for(:guest).of(project) } + it { is_expected.to be_denied_for(:user) } + it { is_expected.to be_denied_for(:visitor) } + it { is_expected.to be_denied_for(:external) } + end + describe "GET /:project_path/pipelines" do subject { namespace_project_pipelines_path(project.namespace, project) } diff --git a/spec/features/todos/todos_sorting_spec.rb b/spec/features/todos/todos_sorting_spec.rb index fec28c55d30..4d5bd476301 100644 --- a/spec/features/todos/todos_sorting_spec.rb +++ b/spec/features/todos/todos_sorting_spec.rb @@ -56,8 +56,8 @@ describe "Dashboard > User sorts todos", feature: true do expect(results_list.all('p')[4]).to have_content("merge_request_1") end - it "sorts by priority" do - click_link "Priority" + it "sorts by label priority" do + click_link "Label priority" results_list = page.find('.todos-list') expect(results_list.all('p')[0]).to have_content("issue_3") @@ -85,8 +85,8 @@ describe "Dashboard > User sorts todos", feature: true do visit dashboard_todos_path end - it "doesn't mix issues and merge requests priorities" do - click_link "Priority" + it "doesn't mix issues and merge requests label priorities" do + click_link "Label priority" results_list = page.find('.todos-list') expect(results_list.all('p')[0]).to have_content("issue_1") diff --git a/spec/features/triggers_spec.rb b/spec/features/triggers_spec.rb index 4a7511589d6..c1ae6db00c6 100644 --- a/spec/features/triggers_spec.rb +++ b/spec/features/triggers_spec.rb @@ -1,28 +1,175 @@ require 'spec_helper' -describe 'Triggers' do +feature 'Triggers', feature: true, js: true do + let(:trigger_title) { 'trigger desc' } let(:user) { create(:user) } + let(:user2) { create(:user) } + let(:guest_user) { create(:user) } before { login_as(user) } before do - @project = FactoryGirl.create :empty_project + @project = create(:empty_project) @project.team << [user, :master] + @project.team << [user2, :master] + @project.team << [guest_user, :guest] visit namespace_project_settings_ci_cd_path(@project.namespace, @project) end - context 'create a trigger' do - before do - click_on 'Add trigger' - expect(@project.triggers.count).to eq(1) + describe 'create trigger workflow' do + scenario 'prevents adding new trigger with no description' do + fill_in 'trigger_description', with: '' + click_button 'Add trigger' + + # See if input has error due to empty value + expect(page.find('form.gl-show-field-errors .gl-field-error')['style']).to eq 'display: block;' + end + + scenario 'adds new trigger with description' do + fill_in 'trigger_description', with: 'trigger desc' + click_button 'Add trigger' + + # See if "trigger creation successful" message displayed and description and owner are correct + expect(page.find('.flash-notice')).to have_content 'Trigger was created successfully.' + expect(page.find('.triggers-list')).to have_content 'trigger desc' + expect(page.find('.triggers-list .trigger-owner')).to have_content @user.name + end + end + + describe 'edit trigger workflow' do + let(:new_trigger_title) { 'new trigger' } + + scenario 'click on edit trigger opens edit trigger page' do + create(:ci_trigger, owner: user, project: @project, description: trigger_title) + visit namespace_project_settings_ci_cd_path(@project.namespace, @project) + + # See if edit page has correct descrption + find('a[title="Edit"]').click + expect(page.find('#trigger_description').value).to have_content 'trigger desc' + end + + scenario 'edit trigger and save' do + create(:ci_trigger, owner: user, project: @project, description: trigger_title) + visit namespace_project_settings_ci_cd_path(@project.namespace, @project) + + # See if edit page opens, then fill in new description and save + find('a[title="Edit"]').click + fill_in 'trigger_description', with: new_trigger_title + click_button 'Save trigger' + + # See if "trigger updated successfully" message displayed and description and owner are correct + expect(page.find('.flash-notice')).to have_content 'Trigger was successfully updated.' + expect(page.find('.triggers-list')).to have_content new_trigger_title + expect(page.find('.triggers-list .trigger-owner')).to have_content @user.name + end + + scenario 'edit "legacy" trigger and save' do + # Create new trigger without owner association, i.e. Legacy trigger + create(:ci_trigger, owner: nil, project: @project) + visit namespace_project_settings_ci_cd_path(@project.namespace, @project) + + # See if the trigger can be edited and description is blank + find('a[title="Edit"]').click + expect(page.find('#trigger_description').value).to have_content '' + + # See if trigger can be updated with description and saved successfully + fill_in 'trigger_description', with: new_trigger_title + click_button 'Save trigger' + expect(page.find('.flash-notice')).to have_content 'Trigger was successfully updated.' + expect(page.find('.triggers-list')).to have_content new_trigger_title + end + end + + describe 'trigger "Take ownership" workflow' do + before(:each) do + create(:ci_trigger, owner: user2, project: @project, description: trigger_title) + visit namespace_project_settings_ci_cd_path(@project.namespace, @project) + end + + scenario 'button "Take ownership" has correct alert' do + expected_alert = 'By taking ownership you will bind this trigger to your user account. With this the trigger will have access to all your projects as if it was you. Are you sure?' + expect(page.find('a.btn-trigger-take-ownership')['data-confirm']).to eq expected_alert end - it 'contains trigger token' do - expect(page).to have_content(@project.triggers.first.token) + scenario 'take trigger ownership' do + # See if "Take ownership" on trigger works post trigger creation + find('a.btn-trigger-take-ownership').click + page.accept_confirm do + expect(page.find('.flash-notice')).to have_content 'Trigger was re-assigned.' + expect(page.find('.triggers-list')).to have_content trigger_title + expect(page.find('.triggers-list .trigger-owner')).to have_content @user.name + end end + end + + describe 'trigger "Revoke" workflow' do + before(:each) do + create(:ci_trigger, owner: user2, project: @project, description: trigger_title) + visit namespace_project_settings_ci_cd_path(@project.namespace, @project) + end + + scenario 'button "Revoke" has correct alert' do + expected_alert = 'By revoking a trigger you will break any processes making use of it. Are you sure?' + expect(page.find('a.btn-trigger-revoke')['data-confirm']).to eq expected_alert + end + + scenario 'revoke trigger' do + # See if "Revoke" on trigger works post trigger creation + find('a.btn-trigger-revoke').click + page.accept_confirm do + expect(page.find('.flash-notice')).to have_content 'Trigger removed' + expect(page).to have_selector('p.settings-message.text-center.append-bottom-default') + end + end + end + + describe 'show triggers workflow' do + scenario 'contains trigger description placeholder' do + expect(page.find('#trigger_description')['placeholder']).to eq 'Trigger description' + end + + scenario 'show "legacy" badge for legacy trigger' do + create(:ci_trigger, owner: nil, project: @project) + visit namespace_project_settings_ci_cd_path(@project.namespace, @project) + + # See if trigger without owner (i.e. legacy) shows "legacy" badge and is editable + expect(page.find('.triggers-list')).to have_content 'legacy' + expect(page.find('.triggers-list')).to have_selector('a[title="Edit"]') + end + + scenario 'show "invalid" badge for trigger with owner having insufficient permissions' do + create(:ci_trigger, owner: guest_user, project: @project, description: trigger_title) + visit namespace_project_settings_ci_cd_path(@project.namespace, @project) + + # See if trigger without owner (i.e. legacy) shows "legacy" badge and is non-editable + expect(page.find('.triggers-list')).to have_content 'invalid' + expect(page.find('.triggers-list')).not_to have_selector('a[title="Edit"]') + end + + scenario 'do not show "Edit" or full token for not owned trigger' do + # Create trigger with user different from current_user + create(:ci_trigger, owner: user2, project: @project, description: trigger_title) + visit namespace_project_settings_ci_cd_path(@project.namespace, @project) + + # See if trigger not owned by current_user shows only first few token chars and doesn't have copy-to-clipboard button + expect(page.find('.triggers-list')).to have_content(@project.triggers.first.token[0..3]) + expect(page.find('.triggers-list')).not_to have_selector('button.btn-clipboard') + + # See if trigger owner name doesn't match with current_user and trigger is non-editable + expect(page.find('.triggers-list .trigger-owner')).not_to have_content @user.name + expect(page.find('.triggers-list')).not_to have_selector('a[title="Edit"]') + end + + scenario 'show "Edit" and full token for owned trigger' do + create(:ci_trigger, owner: user, project: @project, description: trigger_title) + visit namespace_project_settings_ci_cd_path(@project.namespace, @project) + + # See if trigger shows full token and has copy-to-clipboard button + expect(page.find('.triggers-list')).to have_content @project.triggers.first.token + expect(page.find('.triggers-list')).to have_selector('button.btn-clipboard') - it 'revokes the trigger' do - click_on 'Revoke' - expect(@project.triggers.count).to eq(0) + # See if trigger owner name matches with current_user and is editable + expect(page.find('.triggers-list .trigger-owner')).to have_content @user.name + expect(page.find('.triggers-list')).to have_selector('a[title="Edit"]') end end end diff --git a/spec/finders/personal_access_tokens_finder_spec.rb b/spec/finders/personal_access_tokens_finder_spec.rb new file mode 100644 index 00000000000..fd92664ca24 --- /dev/null +++ b/spec/finders/personal_access_tokens_finder_spec.rb @@ -0,0 +1,196 @@ +require 'spec_helper' + +describe PersonalAccessTokensFinder do + def finder(options = {}) + described_class.new(options) + end + + describe '#execute' do + let(:user) { create(:user) } + let(:params) { {} } + let!(:active_personal_access_token) { create(:personal_access_token, user: user) } + let!(:expired_personal_access_token) { create(:personal_access_token, :expired, user: user) } + let!(:revoked_personal_access_token) { create(:personal_access_token, :revoked, user: user) } + let!(:active_impersonation_token) { create(:personal_access_token, :impersonation, user: user) } + let!(:expired_impersonation_token) { create(:personal_access_token, :expired, :impersonation, user: user) } + let!(:revoked_impersonation_token) { create(:personal_access_token, :revoked, :impersonation, user: user) } + + subject { finder(params).execute } + + describe 'without user' do + it do + is_expected.to contain_exactly(active_personal_access_token, active_impersonation_token, + revoked_personal_access_token, expired_personal_access_token, + revoked_impersonation_token, expired_impersonation_token) + end + + describe 'without impersonation' do + before { params[:impersonation] = false } + + it { is_expected.to contain_exactly(active_personal_access_token, revoked_personal_access_token, expired_personal_access_token) } + + describe 'with active state' do + before { params[:state] = 'active' } + + it { is_expected.to contain_exactly(active_personal_access_token) } + end + + describe 'with inactive state' do + before { params[:state] = 'inactive' } + + it { is_expected.to contain_exactly(revoked_personal_access_token, expired_personal_access_token) } + end + end + + describe 'with impersonation' do + before { params[:impersonation] = true } + + it { is_expected.to contain_exactly(active_impersonation_token, revoked_impersonation_token, expired_impersonation_token) } + + describe 'with active state' do + before { params[:state] = 'active' } + + it { is_expected.to contain_exactly(active_impersonation_token) } + end + + describe 'with inactive state' do + before { params[:state] = 'inactive' } + + it { is_expected.to contain_exactly(revoked_impersonation_token, expired_impersonation_token) } + end + end + + describe 'with active state' do + before { params[:state] = 'active' } + + it { is_expected.to contain_exactly(active_personal_access_token, active_impersonation_token) } + end + + describe 'with inactive state' do + before { params[:state] = 'inactive' } + + it do + is_expected.to contain_exactly(expired_personal_access_token, revoked_personal_access_token, + expired_impersonation_token, revoked_impersonation_token) + end + end + + describe 'with id' do + subject { finder(params).find_by(id: active_personal_access_token.id) } + + it { is_expected.to eq(active_personal_access_token) } + + describe 'with impersonation' do + before { params[:impersonation] = true } + + it { is_expected.to be_nil } + end + end + + describe 'with token' do + subject { finder(params).find_by(token: active_personal_access_token.token) } + + it { is_expected.to eq(active_personal_access_token) } + + describe 'with impersonation' do + before { params[:impersonation] = true } + + it { is_expected.to be_nil } + end + end + end + + describe 'with user' do + let(:user2) { create(:user) } + let!(:other_user_active_personal_access_token) { create(:personal_access_token, user: user2) } + let!(:other_user_expired_personal_access_token) { create(:personal_access_token, :expired, user: user2) } + let!(:other_user_revoked_personal_access_token) { create(:personal_access_token, :revoked, user: user2) } + let!(:other_user_active_impersonation_token) { create(:personal_access_token, :impersonation, user: user2) } + let!(:other_user_expired_impersonation_token) { create(:personal_access_token, :expired, :impersonation, user: user2) } + let!(:other_user_revoked_impersonation_token) { create(:personal_access_token, :revoked, :impersonation, user: user2) } + + before { params[:user] = user } + + it do + is_expected.to contain_exactly(active_personal_access_token, active_impersonation_token, + revoked_personal_access_token, expired_personal_access_token, + revoked_impersonation_token, expired_impersonation_token) + end + + describe 'without impersonation' do + before { params[:impersonation] = false } + + it { is_expected.to contain_exactly(active_personal_access_token, revoked_personal_access_token, expired_personal_access_token) } + + describe 'with active state' do + before { params[:state] = 'active' } + + it { is_expected.to contain_exactly(active_personal_access_token) } + end + + describe 'with inactive state' do + before { params[:state] = 'inactive' } + + it { is_expected.to contain_exactly(revoked_personal_access_token, expired_personal_access_token) } + end + end + + describe 'with impersonation' do + before { params[:impersonation] = true } + + it { is_expected.to contain_exactly(active_impersonation_token, revoked_impersonation_token, expired_impersonation_token) } + + describe 'with active state' do + before { params[:state] = 'active' } + + it { is_expected.to contain_exactly(active_impersonation_token) } + end + + describe 'with inactive state' do + before { params[:state] = 'inactive' } + + it { is_expected.to contain_exactly(revoked_impersonation_token, expired_impersonation_token) } + end + end + + describe 'with active state' do + before { params[:state] = 'active' } + + it { is_expected.to contain_exactly(active_personal_access_token, active_impersonation_token) } + end + + describe 'with inactive state' do + before { params[:state] = 'inactive' } + + it do + is_expected.to contain_exactly(expired_personal_access_token, revoked_personal_access_token, + expired_impersonation_token, revoked_impersonation_token) + end + end + + describe 'with id' do + subject { finder(params).find_by(id: active_personal_access_token.id) } + + it { is_expected.to eq(active_personal_access_token) } + + describe 'with impersonation' do + before { params[:impersonation] = true } + + it { is_expected.to be_nil } + end + end + + describe 'with token' do + subject { finder(params).find_by(token: active_personal_access_token.token) } + + it { is_expected.to eq(active_personal_access_token) } + + describe 'with impersonation' do + before { params[:impersonation] = true } + + it { is_expected.to be_nil } + end + end + end + end +end diff --git a/spec/helpers/events_helper_spec.rb b/spec/helpers/events_helper_spec.rb index 594b40303bc..81ba693f2f3 100644 --- a/spec/helpers/events_helper_spec.rb +++ b/spec/helpers/events_helper_spec.rb @@ -61,6 +61,13 @@ describe EventsHelper do '</code></pre>' expect(helper.event_note(input)).to eq(expected) end + + it 'preserves style attribute within a tag' do + input = '<span class="" style="background-color: #44ad8e; color: #FFFFFF;"></span>' + expected = '<p><span style="background-color: #44ad8e; color: #FFFFFF;"></span></p>' + + expect(helper.event_note(input)).to eq(expected) + end end describe '#event_commit_title' do diff --git a/spec/initializers/6_validations_spec.rb b/spec/initializers/6_validations_spec.rb index baab30f482f..cf182e6d221 100644 --- a/spec/initializers/6_validations_spec.rb +++ b/spec/initializers/6_validations_spec.rb @@ -14,7 +14,7 @@ describe '6_validations', lib: true do context 'with correct settings' do before do - mock_storages('foo' => 'tmp/tests/paths/a/b/c', 'bar' => 'tmp/tests/paths/a/b/d') + mock_storages('foo' => { 'path' => 'tmp/tests/paths/a/b/c' }, 'bar' => { 'path' => 'tmp/tests/paths/a/b/d' }) end it 'passes through' do @@ -24,7 +24,7 @@ describe '6_validations', lib: true do context 'with invalid storage names' do before do - mock_storages('name with spaces' => 'tmp/tests/paths/a/b/c') + mock_storages('name with spaces' => { 'path' => 'tmp/tests/paths/a/b/c' }) end it 'throws an error' do @@ -34,7 +34,7 @@ describe '6_validations', lib: true do context 'with nested storage paths' do before do - mock_storages('foo' => 'tmp/tests/paths/a/b/c', 'bar' => 'tmp/tests/paths/a/b/c/d') + mock_storages('foo' => { 'path' => 'tmp/tests/paths/a/b/c' }, 'bar' => { 'path' => 'tmp/tests/paths/a/b/c/d' }) end it 'throws an error' do @@ -44,7 +44,7 @@ describe '6_validations', lib: true do context 'with similar but un-nested storage paths' do before do - mock_storages('foo' => 'tmp/tests/paths/a/b/c', 'bar' => 'tmp/tests/paths/a/b/c2') + mock_storages('foo' => { 'path' => 'tmp/tests/paths/a/b/c' }, 'bar' => { 'path' => 'tmp/tests/paths/a/b/c2' }) end it 'passes through' do @@ -52,6 +52,26 @@ describe '6_validations', lib: true do end end + context 'with incomplete settings' do + before do + mock_storages('foo' => {}) + end + + it 'throws an error suggesting the user to update its settings' do + expect { validate_storages }.to raise_error('foo is not a valid storage, because it has no `path` key. Refer to gitlab.yml.example for an updated example. Please fix this in your gitlab.yml before starting GitLab.') + end + end + + context 'with deprecated settings structure' do + before do + mock_storages('foo' => 'tmp/tests/paths/a/b/c') + end + + it 'throws an error suggesting the user to update its settings' do + expect { validate_storages }.to raise_error("foo is not a valid storage, because it has no `path` key. It may be configured as:\n\nfoo:\n path: tmp/tests/paths/a/b/c\n\nRefer to gitlab.yml.example for an updated example. Please fix this in your gitlab.yml before starting GitLab.") + end + end + def mock_storages(storages) allow(Gitlab.config.repositories).to receive(:storages).and_return(storages) end diff --git a/spec/initializers/doorkeeper_spec.rb b/spec/initializers/doorkeeper_spec.rb new file mode 100644 index 00000000000..74bdbb01166 --- /dev/null +++ b/spec/initializers/doorkeeper_spec.rb @@ -0,0 +1,71 @@ +require 'spec_helper' +require_relative '../../config/initializers/doorkeeper' + +describe Doorkeeper.configuration do + describe '#default_scopes' do + it 'matches Gitlab::Auth::DEFAULT_SCOPES' do + expect(subject.default_scopes).to eq Gitlab::Auth::DEFAULT_SCOPES + end + end + + describe '#optional_scopes' do + it 'matches Gitlab::Auth::OPTIONAL_SCOPES' do + expect(subject.optional_scopes).to eq Gitlab::Auth::OPTIONAL_SCOPES + end + end + + describe '#resource_owner_authenticator' do + subject { controller.instance_exec(&Doorkeeper.configuration.authenticate_resource_owner) } + + let(:controller) { double } + + before do + allow(controller).to receive(:current_user).and_return(current_user) + allow(controller).to receive(:session).and_return({}) + allow(controller).to receive(:request).and_return(OpenStruct.new(fullpath: '/return-path')) + allow(controller).to receive(:redirect_to) + allow(controller).to receive(:new_user_session_url).and_return('/login') + end + + context 'with a user present' do + let(:current_user) { create(:user) } + + it 'returns the user' do + expect(subject).to eq current_user + end + + it 'does not redirect' do + expect(controller).not_to receive(:redirect_to) + + subject + end + + it 'does not store the return path' do + subject + + expect(controller.session).not_to include :user_return_to + end + end + + context 'without a user present' do + let(:current_user) { nil } + + # NOTE: this is required for doorkeeper-openid_connect + it 'returns nil' do + expect(subject).to eq nil + end + + it 'redirects to the login form' do + expect(controller).to receive(:redirect_to).with('/login') + + subject + end + + it 'stores the return path' do + subject + + expect(controller.session[:user_return_to]).to eq '/return-path' + end + end + end +end diff --git a/spec/initializers/secret_token_spec.rb b/spec/initializers/secret_token_spec.rb index ad7f032d1e5..65c97da2efd 100644 --- a/spec/initializers/secret_token_spec.rb +++ b/spec/initializers/secret_token_spec.rb @@ -6,6 +6,9 @@ describe 'create_tokens', lib: true do let(:secrets) { ActiveSupport::OrderedOptions.new } + HEX_KEY = /\h{128}/ + RSA_KEY = /\A-----BEGIN RSA PRIVATE KEY-----\n.+\n-----END RSA PRIVATE KEY-----\n\Z/m + before do allow(File).to receive(:write) allow(File).to receive(:delete) @@ -15,7 +18,7 @@ describe 'create_tokens', lib: true do allow(self).to receive(:exit) end - context 'setting secret_key_base and otp_key_base' do + context 'setting secret keys' do context 'when none of the secrets exist' do before do stub_env('SECRET_KEY_BASE', nil) @@ -24,19 +27,29 @@ describe 'create_tokens', lib: true do allow(self).to receive(:warn_missing_secret) end - it 'generates different secrets for secret_key_base, otp_key_base, and db_key_base' do + it 'generates different hashes for secret_key_base, otp_key_base, and db_key_base' do create_tokens keys = secrets.values_at(:secret_key_base, :otp_key_base, :db_key_base) expect(keys.uniq).to eq(keys) - expect(keys.map(&:length)).to all(eq(128)) + expect(keys).to all(match(HEX_KEY)) + end + + it 'generates an RSA key for jws_private_key' do + create_tokens + + keys = secrets.values_at(:jws_private_key) + + expect(keys.uniq).to eq(keys) + expect(keys).to all(match(RSA_KEY)) end it 'warns about the secrets to add to secrets.yml' do expect(self).to receive(:warn_missing_secret).with('secret_key_base') expect(self).to receive(:warn_missing_secret).with('otp_key_base') expect(self).to receive(:warn_missing_secret).with('db_key_base') + expect(self).to receive(:warn_missing_secret).with('jws_private_key') create_tokens end @@ -48,6 +61,7 @@ describe 'create_tokens', lib: true do expect(new_secrets['secret_key_base']).to eq(secrets.secret_key_base) expect(new_secrets['otp_key_base']).to eq(secrets.otp_key_base) expect(new_secrets['db_key_base']).to eq(secrets.db_key_base) + expect(new_secrets['jws_private_key']).to eq(secrets.jws_private_key) end create_tokens @@ -63,6 +77,7 @@ describe 'create_tokens', lib: true do context 'when the other secrets all exist' do before do secrets.db_key_base = 'db_key_base' + secrets.jws_private_key = 'jws_private_key' allow(File).to receive(:exist?).with('.secret').and_return(true) allow(File).to receive(:read).with('.secret').and_return('file_key') @@ -73,6 +88,7 @@ describe 'create_tokens', lib: true do stub_env('SECRET_KEY_BASE', 'env_key') secrets.secret_key_base = 'secret_key_base' secrets.otp_key_base = 'otp_key_base' + secrets.jws_private_key = 'jws_private_key' end it 'does not issue a warning' do @@ -98,6 +114,7 @@ describe 'create_tokens', lib: true do before do secrets.secret_key_base = 'secret_key_base' secrets.otp_key_base = 'otp_key_base' + secrets.jws_private_key = 'jws_private_key' end it 'does not write any files' do @@ -112,6 +129,7 @@ describe 'create_tokens', lib: true do expect(secrets.secret_key_base).to eq('secret_key_base') expect(secrets.otp_key_base).to eq('otp_key_base') expect(secrets.db_key_base).to eq('db_key_base') + expect(secrets.jws_private_key).to eq('jws_private_key') end it 'deletes the .secret file' do @@ -135,6 +153,7 @@ describe 'create_tokens', lib: true do expect(new_secrets['secret_key_base']).to eq('file_key') expect(new_secrets['otp_key_base']).to eq('file_key') expect(new_secrets['db_key_base']).to eq('db_key_base') + expect(new_secrets['jws_private_key']).to eq('jws_private_key') end create_tokens diff --git a/spec/javascripts/fixtures/environments/metrics.html.haml b/spec/javascripts/fixtures/environments/metrics.html.haml new file mode 100644 index 00000000000..483063fb889 --- /dev/null +++ b/spec/javascripts/fixtures/environments/metrics.html.haml @@ -0,0 +1,12 @@ +%div + .top-area + .row + .col-sm-6 + %h3.page-title + Metrics for environment + .row + .col-sm-12 + %svg.prometheus-graph{ 'graph-type' => 'cpu_values' } + .row + .col-sm-12 + %svg.prometheus-graph{ 'graph-type' => 'memory_values' }
\ No newline at end of file diff --git a/spec/javascripts/monitoring/prometheus_graph_spec.js b/spec/javascripts/monitoring/prometheus_graph_spec.js new file mode 100644 index 00000000000..823b4bab7fc --- /dev/null +++ b/spec/javascripts/monitoring/prometheus_graph_spec.js @@ -0,0 +1,78 @@ +import 'jquery'; +import es6Promise from 'es6-promise'; +import '~/lib/utils/common_utils'; +import PrometheusGraph from '~/monitoring/prometheus_graph'; +import { prometheusMockData } from './prometheus_mock_data'; + +es6Promise.polyfill(); + +describe('PrometheusGraph', () => { + const fixtureName = 'static/environments/metrics.html.raw'; + const prometheusGraphContainer = '.prometheus-graph'; + const prometheusGraphContents = `${prometheusGraphContainer}[graph-type=cpu_values]`; + + preloadFixtures(fixtureName); + + beforeEach(() => { + loadFixtures(fixtureName); + this.prometheusGraph = new PrometheusGraph(); + const self = this; + const fakeInit = (metricsResponse) => { + self.prometheusGraph.transformData(metricsResponse); + self.prometheusGraph.createGraph(); + }; + spyOn(this.prometheusGraph, 'init').and.callFake(fakeInit); + }); + + it('initializes graph properties', () => { + // Test for the measurements + expect(this.prometheusGraph.margin).toBeDefined(); + expect(this.prometheusGraph.marginLabelContainer).toBeDefined(); + expect(this.prometheusGraph.originalWidth).toBeDefined(); + expect(this.prometheusGraph.originalHeight).toBeDefined(); + expect(this.prometheusGraph.height).toBeDefined(); + expect(this.prometheusGraph.width).toBeDefined(); + expect(this.prometheusGraph.backOffRequestCounter).toBeDefined(); + // Test for the graph properties (colors, radius, etc.) + expect(this.prometheusGraph.graphSpecificProperties).toBeDefined(); + expect(this.prometheusGraph.commonGraphProperties).toBeDefined(); + }); + + it('transforms the data', () => { + this.prometheusGraph.init(prometheusMockData.metrics); + expect(this.prometheusGraph.data).toBeDefined(); + expect(this.prometheusGraph.data.cpu_values.length).toBe(121); + expect(this.prometheusGraph.data.memory_values.length).toBe(121); + }); + + it('creates two graphs', () => { + this.prometheusGraph.init(prometheusMockData.metrics); + expect($(prometheusGraphContainer).length).toBe(2); + }); + + describe('Graph contents', () => { + beforeEach(() => { + this.prometheusGraph.init(prometheusMockData.metrics); + }); + + it('has axis, an area, a line and a overlay', () => { + const $graphContainer = $(prometheusGraphContents).find('.x-axis').parent(); + expect($graphContainer.find('.x-axis')).toBeDefined(); + expect($graphContainer.find('.y-axis')).toBeDefined(); + expect($graphContainer.find('.prometheus-graph-overlay')).toBeDefined(); + expect($graphContainer.find('.metric-line')).toBeDefined(); + expect($graphContainer.find('.metric-area')).toBeDefined(); + }); + + it('has legends, labels and an extra axis that labels the metrics', () => { + const $prometheusGraphContents = $(prometheusGraphContents); + const $axisLabelContainer = $(prometheusGraphContents).find('.label-x-axis-line').parent(); + expect($prometheusGraphContents.find('.label-x-axis-line')).toBeDefined(); + expect($prometheusGraphContents.find('.label-y-axis-line')).toBeDefined(); + expect($prometheusGraphContents.find('.label-axis-text')).toBeDefined(); + expect($prometheusGraphContents.find('.rect-axis-text')).toBeDefined(); + expect($axisLabelContainer.find('rect').length).toBe(2); + expect($axisLabelContainer.find('text').length).toBe(4); + }); + }); +}); diff --git a/spec/javascripts/monitoring/prometheus_mock_data.js b/spec/javascripts/monitoring/prometheus_mock_data.js new file mode 100644 index 00000000000..1cdc14faaa8 --- /dev/null +++ b/spec/javascripts/monitoring/prometheus_mock_data.js @@ -0,0 +1,1014 @@ +/* eslint-disable import/prefer-default-export*/ +export const prometheusMockData = { + status: 200, + metrics: { + success: true, + metrics: { + memory_values: [ + { + metric: { + }, + values: [ + [ + 1488462917.256, + '10.12890625', + ], + [ + 1488462977.256, + '10.140625', + ], + [ + 1488463037.256, + '10.140625', + ], + [ + 1488463097.256, + '10.14453125', + ], + [ + 1488463157.256, + '10.1484375', + ], + [ + 1488463217.256, + '10.15625', + ], + [ + 1488463277.256, + '10.15625', + ], + [ + 1488463337.256, + '10.15625', + ], + [ + 1488463397.256, + '10.1640625', + ], + [ + 1488463457.256, + '10.171875', + ], + [ + 1488463517.256, + '10.171875', + ], + [ + 1488463577.256, + '10.171875', + ], + [ + 1488463637.256, + '10.18359375', + ], + [ + 1488463697.256, + '10.1953125', + ], + [ + 1488463757.256, + '10.203125', + ], + [ + 1488463817.256, + '10.20703125', + ], + [ + 1488463877.256, + '10.20703125', + ], + [ + 1488463937.256, + '10.20703125', + ], + [ + 1488463997.256, + '10.20703125', + ], + [ + 1488464057.256, + '10.2109375', + ], + [ + 1488464117.256, + '10.2109375', + ], + [ + 1488464177.256, + '10.2109375', + ], + [ + 1488464237.256, + '10.2109375', + ], + [ + 1488464297.256, + '10.21484375', + ], + [ + 1488464357.256, + '10.22265625', + ], + [ + 1488464417.256, + '10.22265625', + ], + [ + 1488464477.256, + '10.2265625', + ], + [ + 1488464537.256, + '10.23046875', + ], + [ + 1488464597.256, + '10.23046875', + ], + [ + 1488464657.256, + '10.234375', + ], + [ + 1488464717.256, + '10.234375', + ], + [ + 1488464777.256, + '10.234375', + ], + [ + 1488464837.256, + '10.234375', + ], + [ + 1488464897.256, + '10.234375', + ], + [ + 1488464957.256, + '10.234375', + ], + [ + 1488465017.256, + '10.23828125', + ], + [ + 1488465077.256, + '10.23828125', + ], + [ + 1488465137.256, + '10.2421875', + ], + [ + 1488465197.256, + '10.2421875', + ], + [ + 1488465257.256, + '10.2421875', + ], + [ + 1488465317.256, + '10.2421875', + ], + [ + 1488465377.256, + '10.2421875', + ], + [ + 1488465437.256, + '10.2421875', + ], + [ + 1488465497.256, + '10.2421875', + ], + [ + 1488465557.256, + '10.2421875', + ], + [ + 1488465617.256, + '10.2421875', + ], + [ + 1488465677.256, + '10.2421875', + ], + [ + 1488465737.256, + '10.2421875', + ], + [ + 1488465797.256, + '10.24609375', + ], + [ + 1488465857.256, + '10.25', + ], + [ + 1488465917.256, + '10.25390625', + ], + [ + 1488465977.256, + '9.98828125', + ], + [ + 1488466037.256, + '9.9921875', + ], + [ + 1488466097.256, + '9.9921875', + ], + [ + 1488466157.256, + '9.99609375', + ], + [ + 1488466217.256, + '10', + ], + [ + 1488466277.256, + '10.00390625', + ], + [ + 1488466337.256, + '10.0078125', + ], + [ + 1488466397.256, + '10.01171875', + ], + [ + 1488466457.256, + '10.0234375', + ], + [ + 1488466517.256, + '10.02734375', + ], + [ + 1488466577.256, + '10.02734375', + ], + [ + 1488466637.256, + '10.03125', + ], + [ + 1488466697.256, + '10.03125', + ], + [ + 1488466757.256, + '10.03125', + ], + [ + 1488466817.256, + '10.03125', + ], + [ + 1488466877.256, + '10.03125', + ], + [ + 1488466937.256, + '10.03125', + ], + [ + 1488466997.256, + '10.03125', + ], + [ + 1488467057.256, + '10.0390625', + ], + [ + 1488467117.256, + '10.0390625', + ], + [ + 1488467177.256, + '10.04296875', + ], + [ + 1488467237.256, + '10.05078125', + ], + [ + 1488467297.256, + '10.05859375', + ], + [ + 1488467357.256, + '10.06640625', + ], + [ + 1488467417.256, + '10.06640625', + ], + [ + 1488467477.256, + '10.0703125', + ], + [ + 1488467537.256, + '10.07421875', + ], + [ + 1488467597.256, + '10.0859375', + ], + [ + 1488467657.256, + '10.0859375', + ], + [ + 1488467717.256, + '10.09765625', + ], + [ + 1488467777.256, + '10.1015625', + ], + [ + 1488467837.256, + '10.10546875', + ], + [ + 1488467897.256, + '10.10546875', + ], + [ + 1488467957.256, + '10.125', + ], + [ + 1488468017.256, + '10.13671875', + ], + [ + 1488468077.256, + '10.1484375', + ], + [ + 1488468137.256, + '10.15625', + ], + [ + 1488468197.256, + '10.16796875', + ], + [ + 1488468257.256, + '10.171875', + ], + [ + 1488468317.256, + '10.171875', + ], + [ + 1488468377.256, + '10.171875', + ], + [ + 1488468437.256, + '10.171875', + ], + [ + 1488468497.256, + '10.171875', + ], + [ + 1488468557.256, + '10.171875', + ], + [ + 1488468617.256, + '10.171875', + ], + [ + 1488468677.256, + '10.17578125', + ], + [ + 1488468737.256, + '10.17578125', + ], + [ + 1488468797.256, + '10.265625', + ], + [ + 1488468857.256, + '10.19921875', + ], + [ + 1488468917.256, + '10.19921875', + ], + [ + 1488468977.256, + '10.19921875', + ], + [ + 1488469037.256, + '10.19921875', + ], + [ + 1488469097.256, + '10.19921875', + ], + [ + 1488469157.256, + '10.203125', + ], + [ + 1488469217.256, + '10.43359375', + ], + [ + 1488469277.256, + '10.20703125', + ], + [ + 1488469337.256, + '10.2109375', + ], + [ + 1488469397.256, + '10.22265625', + ], + [ + 1488469457.256, + '10.21484375', + ], + [ + 1488469517.256, + '10.21484375', + ], + [ + 1488469577.256, + '10.21484375', + ], + [ + 1488469637.256, + '10.22265625', + ], + [ + 1488469697.256, + '10.234375', + ], + [ + 1488469757.256, + '10.234375', + ], + [ + 1488469817.256, + '10.234375', + ], + [ + 1488469877.256, + '10.2421875', + ], + [ + 1488469937.256, + '10.25', + ], + [ + 1488469997.256, + '10.25390625', + ], + [ + 1488470057.256, + '10.26171875', + ], + [ + 1488470117.256, + '10.2734375', + ], + ], + }, + ], + memory_current: [ + { + metric: { + }, + value: [ + 1488470117.737, + '10.2734375', + ], + }, + ], + cpu_values: [ + { + metric: { + }, + values: [ + [ + 1488462918.15, + '0.0002996458625058103', + ], + [ + 1488462978.15, + '0.0002652382333333314', + ], + [ + 1488463038.15, + '0.0003485461333333421', + ], + [ + 1488463098.15, + '0.0003420421999999886', + ], + [ + 1488463158.15, + '0.00023107150000001297', + ], + [ + 1488463218.15, + '0.00030463981666664826', + ], + [ + 1488463278.15, + '0.0002477177833333677', + ], + [ + 1488463338.15, + '0.00026936656666665115', + ], + [ + 1488463398.15, + '0.000406264750000022', + ], + [ + 1488463458.15, + '0.00029592802026561453', + ], + [ + 1488463518.15, + '0.00023426999683316343', + ], + [ + 1488463578.15, + '0.0003057080666666915', + ], + [ + 1488463638.15, + '0.0003408470500000149', + ], + [ + 1488463698.15, + '0.00025497336666665166', + ], + [ + 1488463758.15, + '0.0003009282833333534', + ], + [ + 1488463818.15, + '0.0003119383499999924', + ], + [ + 1488463878.15, + '0.00028719019999998705', + ], + [ + 1488463938.15, + '0.000327864749999988', + ], + [ + 1488463998.15, + '0.0002514917333333422', + ], + [ + 1488464058.15, + '0.0003614651166666742', + ], + [ + 1488464118.15, + '0.0003221668000000122', + ], + [ + 1488464178.15, + '0.00023323083333330884', + ], + [ + 1488464238.15, + '0.00028531499475009274', + ], + [ + 1488464298.15, + '0.0002627695294921391', + ], + [ + 1488464358.15, + '0.00027145463333333453', + ], + [ + 1488464418.15, + '0.00025669488333335266', + ], + [ + 1488464478.15, + '0.00022307761666665965', + ], + [ + 1488464538.15, + '0.0003307265833333517', + ], + [ + 1488464598.15, + '0.0002817050666666709', + ], + [ + 1488464658.15, + '0.00022357458333332285', + ], + [ + 1488464718.15, + '0.00032648590000000275', + ], + [ + 1488464778.15, + '0.00028410750000000816', + ], + [ + 1488464838.15, + '0.0003038076999999954', + ], + [ + 1488464898.15, + '0.00037568226666667335', + ], + [ + 1488464958.15, + '0.00020160354999999202', + ], + [ + 1488465018.15, + '0.0003229403333333399', + ], + [ + 1488465078.15, + '0.00033516069999999236', + ], + [ + 1488465138.15, + '0.0003365978333333371', + ], + [ + 1488465198.15, + '0.00020262178333331585', + ], + [ + 1488465258.15, + '0.00040567498333331876', + ], + [ + 1488465318.15, + '0.00029114155000001436', + ], + [ + 1488465378.15, + '0.0002498841000000122', + ], + [ + 1488465438.15, + '0.00027296763333331715', + ], + [ + 1488465498.15, + '0.0002958794000000135', + ], + [ + 1488465558.15, + '0.0002922354666666867', + ], + [ + 1488465618.15, + '0.00034186624999999653', + ], + [ + 1488465678.15, + '0.0003397984166666627', + ], + [ + 1488465738.15, + '0.0002658284166666469', + ], + [ + 1488465798.15, + '0.00026221139999999346', + ], + [ + 1488465858.15, + '0.00029467960000001034', + ], + [ + 1488465918.15, + '0.0002634141333333358', + ], + [ + 1488465978.15, + '0.0003202958333333209', + ], + [ + 1488466038.15, + '0.00037890760000000394', + ], + [ + 1488466098.15, + '0.00023453356666666518', + ], + [ + 1488466158.15, + '0.0002866827333333433', + ], + [ + 1488466218.15, + '0.0003335935499999998', + ], + [ + 1488466278.15, + '0.00022787131666666125', + ], + [ + 1488466338.15, + '0.00033821938333333064', + ], + [ + 1488466398.15, + '0.00029233375000001043', + ], + [ + 1488466458.15, + '0.00026562758333333514', + ], + [ + 1488466518.15, + '0.0003142600999999819', + ], + [ + 1488466578.15, + '0.00027392178333333444', + ], + [ + 1488466638.15, + '0.00028178598333334173', + ], + [ + 1488466698.15, + '0.0002463400666666911', + ], + [ + 1488466758.15, + '0.00040234373333332125', + ], + [ + 1488466818.15, + '0.00023677453333332822', + ], + [ + 1488466878.15, + '0.00030852703333333523', + ], + [ + 1488466938.15, + '0.0003582272833333455', + ], + [ + 1488466998.15, + '0.0002176380833332973', + ], + [ + 1488467058.15, + '0.00026180203333335447', + ], + [ + 1488467118.15, + '0.00027862966666667436', + ], + [ + 1488467178.15, + '0.0002769731166666567', + ], + [ + 1488467238.15, + '0.0002832899166666477', + ], + [ + 1488467298.15, + '0.0003446533500000311', + ], + [ + 1488467358.15, + '0.0002691345999999761', + ], + [ + 1488467418.15, + '0.000284919933333357', + ], + [ + 1488467478.15, + '0.0002396026166666528', + ], + [ + 1488467538.15, + '0.00035625295000002075', + ], + [ + 1488467598.15, + '0.00036759816666664946', + ], + [ + 1488467658.15, + '0.00030326608333333855', + ], + [ + 1488467718.15, + '0.00023584972418043393', + ], + [ + 1488467778.15, + '0.00025744508892115107', + ], + [ + 1488467838.15, + '0.00036737541666663395', + ], + [ + 1488467898.15, + '0.00034325741666666094', + ], + [ + 1488467958.15, + '0.00026390046666667407', + ], + [ + 1488468018.15, + '0.0003302534500000102', + ], + [ + 1488468078.15, + '0.00035243794999999527', + ], + [ + 1488468138.15, + '0.00020149738333333407', + ], + [ + 1488468198.15, + '0.0003183469666666679', + ], + [ + 1488468258.15, + '0.0003835329166666845', + ], + [ + 1488468318.15, + '0.0002485075333333124', + ], + [ + 1488468378.15, + '0.0003011457166666768', + ], + [ + 1488468438.15, + '0.00032242785497684965', + ], + [ + 1488468498.15, + '0.0002659713747457531', + ], + [ + 1488468558.15, + '0.0003476860333333202', + ], + [ + 1488468618.15, + '0.00028336403333334794', + ], + [ + 1488468678.15, + '0.00017132354999998728', + ], + [ + 1488468738.15, + '0.0003001915833333276', + ], + [ + 1488468798.15, + '0.0003025715666666725', + ], + [ + 1488468858.15, + '0.0003012370166666815', + ], + [ + 1488468918.15, + '0.00030203619999997025', + ], + [ + 1488468978.15, + '0.0002804355000000314', + ], + [ + 1488469038.15, + '0.00033194884999998564', + ], + [ + 1488469098.15, + '0.00025201496666665455', + ], + [ + 1488469158.15, + '0.0002777531500000189', + ], + [ + 1488469218.15, + '0.0003314885833333392', + ], + [ + 1488469278.15, + '0.0002234891422095589', + ], + [ + 1488469338.15, + '0.000349117355867791', + ], + [ + 1488469398.15, + '0.0004036731333333303', + ], + [ + 1488469458.15, + '0.00024553911666667835', + ], + [ + 1488469518.15, + '0.0003056456833333184', + ], + [ + 1488469578.15, + '0.0002618737166666681', + ], + [ + 1488469638.15, + '0.00022972643333331414', + ], + [ + 1488469698.15, + '0.0003713522500000307', + ], + [ + 1488469758.15, + '0.00018322576666666515', + ], + [ + 1488469818.15, + '0.00034534762753952466', + ], + [ + 1488469878.15, + '0.00028200510008501677', + ], + [ + 1488469938.15, + '0.0002773708499999768', + ], + [ + 1488469998.15, + '0.00027547160000001013', + ], + [ + 1488470058.15, + '0.00031713610000000023', + ], + [ + 1488470118.15, + '0.00035276853333332525', + ], + ], + }, + ], + cpu_current: [ + { + metric: { + }, + value: [ + 1488470118.566, + '0.00035276853333332525', + ], + }, + ], + last_update: '2017-03-02T15:55:18.981Z', + }, + }, +}; diff --git a/spec/lib/gitlab/auth_spec.rb b/spec/lib/gitlab/auth_spec.rb index daf8f5c1d6c..03c4879ed6f 100644 --- a/spec/lib/gitlab/auth_spec.rb +++ b/spec/lib/gitlab/auth_spec.rb @@ -3,6 +3,24 @@ require 'spec_helper' describe Gitlab::Auth, lib: true do let(:gl_auth) { described_class } + describe 'constants' do + it 'API_SCOPES contains all scopes for API access' do + expect(subject::API_SCOPES).to eq [:api, :read_user] + end + + it 'OPENID_SCOPES contains all scopes for OpenID Connect' do + expect(subject::OPENID_SCOPES).to eq [:openid] + end + + it 'DEFAULT_SCOPES contains all default scopes' do + expect(subject::DEFAULT_SCOPES).to eq [:api] + end + + it 'OPTIONAL_SCOPES contains all non-default scopes' do + expect(subject::OPTIONAL_SCOPES).to eq [:read_user, :openid] + end + end + describe 'find_for_git_client' do context 'build token' do subject { gl_auth.find_for_git_client('gitlab-ci-token', build.token, project: project, ip: 'ip') } @@ -118,25 +136,37 @@ describe Gitlab::Auth, lib: true do end context 'while using personal access tokens as passwords' do - let(:user) { create(:user) } - let(:token_w_api_scope) { create(:personal_access_token, user: user, scopes: ['api']) } - it 'succeeds for personal access tokens with the `api` scope' do - expect(gl_auth).to receive(:rate_limit!).with('ip', success: true, login: user.email) - expect(gl_auth.find_for_git_client(user.email, token_w_api_scope.token, project: nil, ip: 'ip')).to eq(Gitlab::Auth::Result.new(user, nil, :personal_token, full_authentication_abilities)) + personal_access_token = create(:personal_access_token, scopes: ['api']) + + expect(gl_auth).to receive(:rate_limit!).with('ip', success: true, login: '') + expect(gl_auth.find_for_git_client('', personal_access_token.token, project: nil, ip: 'ip')).to eq(Gitlab::Auth::Result.new(personal_access_token.user, nil, :personal_token, full_authentication_abilities)) + end + + it 'succeeds if it is an impersonation token' do + impersonation_token = create(:personal_access_token, :impersonation, scopes: ['api']) + + expect(gl_auth).to receive(:rate_limit!).with('ip', success: true, login: '') + expect(gl_auth.find_for_git_client('', impersonation_token.token, project: nil, ip: 'ip')).to eq(Gitlab::Auth::Result.new(impersonation_token.user, nil, :personal_token, full_authentication_abilities)) end it 'fails for personal access tokens with other scopes' do - personal_access_token = create(:personal_access_token, user: user, scopes: ['read_user']) + personal_access_token = create(:personal_access_token, scopes: ['read_user']) - expect(gl_auth).to receive(:rate_limit!).with('ip', success: false, login: user.email) - expect(gl_auth.find_for_git_client(user.email, personal_access_token.token, project: nil, ip: 'ip')).to eq(Gitlab::Auth::Result.new(nil, nil)) + expect(gl_auth).to receive(:rate_limit!).with('ip', success: false, login: '') + expect(gl_auth.find_for_git_client('', personal_access_token.token, project: nil, ip: 'ip')).to eq(Gitlab::Auth::Result.new(nil, nil)) end - it 'does not try password auth before personal access tokens' do - expect(gl_auth).not_to receive(:find_with_user_password) + it 'fails for impersonation token with other scopes' do + impersonation_token = create(:personal_access_token, scopes: ['read_user']) - gl_auth.find_for_git_client(user.email, token_w_api_scope.token, project: nil, ip: 'ip') + expect(gl_auth).to receive(:rate_limit!).with('ip', success: false, login: '') + expect(gl_auth.find_for_git_client('', impersonation_token.token, project: nil, ip: 'ip')).to eq(Gitlab::Auth::Result.new(nil, nil)) + end + + it 'fails if password is nil' do + expect(gl_auth).to receive(:rate_limit!).with('ip', success: false, login: '') + expect(gl_auth.find_for_git_client('', nil, project: nil, ip: 'ip')).to eq(Gitlab::Auth::Result.new(nil, nil)) end end @@ -210,6 +240,18 @@ describe Gitlab::Auth, lib: true do end end + it "does not find user in blocked state" do + user.block + + expect( gl_auth.find_with_user_password(username, password) ).not_to eql user + end + + it "does not find user in ldap_blocked state" do + user.ldap_block + + expect( gl_auth.find_with_user_password(username, password) ).not_to eql user + end + context "with ldap enabled" do before do allow(Gitlab::LDAP::Config).to receive(:enabled?).and_return(true) diff --git a/spec/lib/gitlab/ci/build/image_spec.rb b/spec/lib/gitlab/ci/build/image_spec.rb new file mode 100644 index 00000000000..382385dfd6b --- /dev/null +++ b/spec/lib/gitlab/ci/build/image_spec.rb @@ -0,0 +1,67 @@ +require 'spec_helper' + +describe Gitlab::Ci::Build::Image do + let(:job) { create(:ci_build, :no_options) } + + describe '#from_image' do + subject { described_class.from_image(job) } + + context 'when image is defined in job' do + let(:image_name) { 'ruby:2.1' } + let(:job) { create(:ci_build, options: { image: image_name } ) } + + it 'fabricates an object of the proper class' do + is_expected.to be_kind_of(described_class) + end + + it 'populates fabricated object with the proper name attribute' do + expect(subject.name).to eq(image_name) + end + + context 'when image name is empty' do + let(:image_name) { '' } + + it 'does not fabricate an object' do + is_expected.to be_nil + end + end + end + + context 'when image is not defined in job' do + it 'does not fabricate an object' do + is_expected.to be_nil + end + end + end + + describe '#from_services' do + subject { described_class.from_services(job) } + + context 'when services are defined in job' do + let(:service_image_name) { 'postgres' } + let(:job) { create(:ci_build, options: { services: [service_image_name] }) } + + it 'fabricates an non-empty array of objects' do + is_expected.to be_kind_of(Array) + is_expected.not_to be_empty + expect(subject.first.name).to eq(service_image_name) + end + + context 'when service image name is empty' do + let(:service_image_name) { '' } + + it 'fabricates an empty array' do + is_expected.to be_kind_of(Array) + is_expected.to be_empty + end + end + end + + context 'when services are not defined in job' do + it 'fabricates an empty array' do + is_expected.to be_kind_of(Array) + is_expected.to be_empty + end + end + end +end diff --git a/spec/lib/gitlab/ci/build/step_spec.rb b/spec/lib/gitlab/ci/build/step_spec.rb new file mode 100644 index 00000000000..2a314a744ca --- /dev/null +++ b/spec/lib/gitlab/ci/build/step_spec.rb @@ -0,0 +1,39 @@ +require 'spec_helper' + +describe Gitlab::Ci::Build::Step do + let(:job) { create(:ci_build, :no_options, commands: "ls -la\ndate") } + + describe '#from_commands' do + subject { described_class.from_commands(job) } + + it 'fabricates an object' do + expect(subject.name).to eq(:script) + expect(subject.script).to eq(['ls -la', 'date']) + expect(subject.timeout).to eq(job.timeout) + expect(subject.when).to eq('on_success') + expect(subject.allow_failure).to be_falsey + end + end + + describe '#from_after_script' do + subject { described_class.from_after_script(job) } + + context 'when after_script is empty' do + it 'doesn not fabricate an object' do + is_expected.to be_nil + end + end + + context 'when after_script is not empty' do + let(:job) { create(:ci_build, options: { after_script: "ls -la\ndate" }) } + + it 'fabricates an object' do + expect(subject.name).to eq(:after_script) + expect(subject.script).to eq(['ls -la', 'date']) + expect(subject.timeout).to eq(job.timeout) + expect(subject.when).to eq('always') + expect(subject.allow_failure).to be_truthy + end + end + end +end diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index 1a1280e5198..e47956a365f 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -136,6 +136,7 @@ project: - slack_slash_commands_service - irker_service - pivotaltracker_service +- prometheus_service - hipchat_service - flowdock_service - assembla_service diff --git a/spec/lib/gitlab/prometheus_spec.rb b/spec/lib/gitlab/prometheus_spec.rb new file mode 100644 index 00000000000..280264188e2 --- /dev/null +++ b/spec/lib/gitlab/prometheus_spec.rb @@ -0,0 +1,143 @@ +require 'spec_helper' + +describe Gitlab::Prometheus, lib: true do + include PrometheusHelpers + + subject { described_class.new(api_url: 'https://prometheus.example.com') } + + describe '#ping' do + it 'issues a "query" request to the API endpoint' do + req_stub = stub_prometheus_request(prometheus_query_url('1'), body: prometheus_value_body('vector')) + + expect(subject.ping).to eq({ "resultType" => "vector", "result" => [{ "metric" => {}, "value" => [1488772511.004, "0.000041021495238095323"] }] }) + expect(req_stub).to have_been_requested + end + end + + # This shared examples expect: + # - query_url: A query URL + # - execute_query: A query call + shared_examples 'failure response' do + context 'when request returns 400 with an error message' do + it 'raises a Gitlab::PrometheusError error' do + req_stub = stub_prometheus_request(query_url, status: 400, body: { error: 'bar!' }) + + expect { execute_query } + .to raise_error(Gitlab::PrometheusError, 'bar!') + expect(req_stub).to have_been_requested + end + end + + context 'when request returns 400 without an error message' do + it 'raises a Gitlab::PrometheusError error' do + req_stub = stub_prometheus_request(query_url, status: 400) + + expect { execute_query } + .to raise_error(Gitlab::PrometheusError, 'Bad data received') + expect(req_stub).to have_been_requested + end + end + + context 'when request returns 500' do + it 'raises a Gitlab::PrometheusError error' do + req_stub = stub_prometheus_request(query_url, status: 500, body: { message: 'FAIL!' }) + + expect { execute_query } + .to raise_error(Gitlab::PrometheusError, '500 - {"message":"FAIL!"}') + expect(req_stub).to have_been_requested + end + end + end + + describe '#query' do + let(:prometheus_query) { prometheus_cpu_query('env-slug') } + let(:query_url) { prometheus_query_url(prometheus_query) } + + context 'when request returns vector results' do + it 'returns data from the API call' do + req_stub = stub_prometheus_request(query_url, body: prometheus_value_body('vector')) + + expect(subject.query(prometheus_query)).to eq [{ "metric" => {}, "value" => [1488772511.004, "0.000041021495238095323"] }] + expect(req_stub).to have_been_requested + end + end + + context 'when request returns matrix results' do + it 'returns nil' do + req_stub = stub_prometheus_request(query_url, body: prometheus_value_body('matrix')) + + expect(subject.query(prometheus_query)).to be_nil + expect(req_stub).to have_been_requested + end + end + + context 'when request returns no data' do + it 'returns []' do + req_stub = stub_prometheus_request(query_url, body: prometheus_empty_body('vector')) + + expect(subject.query(prometheus_query)).to be_empty + expect(req_stub).to have_been_requested + end + end + + it_behaves_like 'failure response' do + let(:execute_query) { subject.query(prometheus_query) } + end + end + + describe '#query_range' do + let(:prometheus_query) { prometheus_memory_query('env-slug') } + let(:query_url) { prometheus_query_range_url(prometheus_query) } + + around do |example| + Timecop.freeze { example.run } + end + + context 'when a start time is passed' do + let(:query_url) { prometheus_query_range_url(prometheus_query, start: 2.hours.ago) } + + it 'passed it in the requested URL' do + req_stub = stub_prometheus_request(query_url, body: prometheus_values_body('vector')) + + subject.query_range(prometheus_query, start: 2.hours.ago) + expect(req_stub).to have_been_requested + end + end + + context 'when request returns vector results' do + it 'returns nil' do + req_stub = stub_prometheus_request(query_url, body: prometheus_values_body('vector')) + + expect(subject.query_range(prometheus_query)).to be_nil + expect(req_stub).to have_been_requested + end + end + + context 'when request returns matrix results' do + it 'returns data from the API call' do + req_stub = stub_prometheus_request(query_url, body: prometheus_values_body('matrix')) + + expect(subject.query_range(prometheus_query)).to eq([ + { + "metric" => {}, + "values" => [[1488758662.506, "0.00002996364761904785"], [1488758722.506, "0.00003090239047619091"]] + } + ]) + expect(req_stub).to have_been_requested + end + end + + context 'when request returns no data' do + it 'returns []' do + req_stub = stub_prometheus_request(query_url, body: prometheus_empty_body('matrix')) + + expect(subject.query_range(prometheus_query)).to be_empty + expect(req_stub).to have_been_requested + end + end + + it_behaves_like 'failure response' do + let(:execute_query) { subject.query_range(prometheus_query) } + end + end +end diff --git a/spec/models/chat_team_spec.rb b/spec/models/chat_team_spec.rb index 1aab161ec13..5283561a83f 100644 --- a/spec/models/chat_team_spec.rb +++ b/spec/models/chat_team_spec.rb @@ -1,9 +1,14 @@ require 'spec_helper' describe ChatTeam, type: :model do + subject { create(:chat_team) } + # Associations it { is_expected.to belong_to(:namespace) } + # Validations + it { is_expected.to validate_uniqueness_of(:namespace) } + # Fields it { is_expected.to respond_to(:name) } it { is_expected.to respond_to(:team_id) } diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 2db42a94077..fd6ea2d6722 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -345,11 +345,11 @@ describe Ci::Build, :models do describe '#expanded_environment_name' do subject { build.expanded_environment_name } - context 'when environment uses $CI_BUILD_REF_NAME' do + context 'when environment uses $CI_COMMIT_REF_NAME' do let(:build) do create(:ci_build, ref: 'master', - environment: 'review/$CI_BUILD_REF_NAME') + environment: 'review/$CI_COMMIT_REF_NAME') end it { is_expected.to eq('review/master') } @@ -915,7 +915,7 @@ describe Ci::Build, :models do end context 'referenced with a variable' do - let(:build) { create(:ci_build, pipeline: pipeline, environment: "foo-$CI_BUILD_REF_NAME") } + let(:build) { create(:ci_build, pipeline: pipeline, environment: "foo-$CI_COMMIT_REF_NAME") } it { is_expected.to eq(@environment) } end @@ -1286,23 +1286,25 @@ describe Ci::Build, :models do [ { key: 'CI', value: 'true', public: true }, { key: 'GITLAB_CI', value: 'true', public: true }, - { key: 'CI_BUILD_ID', value: build.id.to_s, public: true }, - { key: 'CI_BUILD_TOKEN', value: build.token, public: false }, - { key: 'CI_BUILD_REF', value: build.sha, public: true }, - { key: 'CI_BUILD_BEFORE_SHA', value: build.before_sha, public: true }, - { key: 'CI_BUILD_REF_NAME', value: 'master', public: true }, - { key: 'CI_BUILD_REF_SLUG', value: 'master', public: true }, - { key: 'CI_BUILD_NAME', value: 'test', public: true }, - { key: 'CI_BUILD_STAGE', value: 'test', public: true }, { key: 'CI_SERVER_NAME', value: 'GitLab', public: true }, { key: 'CI_SERVER_VERSION', value: Gitlab::VERSION, public: true }, { key: 'CI_SERVER_REVISION', value: Gitlab::REVISION, public: true }, + { key: 'CI_JOB_ID', value: build.id.to_s, public: true }, + { key: 'CI_JOB_NAME', value: 'test', public: true }, + { key: 'CI_JOB_STAGE', value: 'test', public: true }, + { key: 'CI_JOB_TOKEN', value: build.token, public: false }, + { key: 'CI_COMMIT_SHA', value: build.sha, public: true }, + { key: 'CI_COMMIT_REF_NAME', value: build.ref, public: true }, + { key: 'CI_COMMIT_REF_SLUG', value: build.ref_slug, public: true }, { key: 'CI_PROJECT_ID', value: project.id.to_s, public: true }, { key: 'CI_PROJECT_NAME', value: project.path, public: true }, { key: 'CI_PROJECT_PATH', value: project.full_path, public: true }, { key: 'CI_PROJECT_NAMESPACE', value: project.namespace.full_path, public: true }, { key: 'CI_PROJECT_URL', value: project.web_url, public: true }, - { key: 'CI_PIPELINE_ID', value: pipeline.id.to_s, public: true } + { key: 'CI_PIPELINE_ID', value: pipeline.id.to_s, public: true }, + { key: 'CI_REGISTRY_USER', value: 'gitlab-ci-token', public: true }, + { key: 'CI_REGISTRY_PASSWORD', value: build.token, public: false }, + { key: 'CI_REPOSITORY_URL', value: build.repo_url, public: false }, ] end @@ -1317,7 +1319,7 @@ describe Ci::Build, :models do build.yaml_variables = [] end - it { is_expected.to eq(predefined_variables) } + it { is_expected.to include(*predefined_variables) } end context 'when build has user' do @@ -1355,7 +1357,7 @@ describe Ci::Build, :models do end let(:manual_variable) do - { key: 'CI_BUILD_MANUAL', value: 'true', public: true } + { key: 'CI_JOB_MANUAL', value: 'true', public: true } end it { is_expected.to include(manual_variable) } @@ -1363,7 +1365,7 @@ describe Ci::Build, :models do context 'when build is for tag' do let(:tag_variable) do - { key: 'CI_BUILD_TAG', value: 'master', public: true } + { key: 'CI_COMMIT_TAG', value: 'master', public: true } end before do @@ -1392,7 +1394,7 @@ describe Ci::Build, :models do { key: :TRIGGER_KEY_1, value: 'TRIGGER_VALUE_1', public: false } end let(:predefined_trigger_variable) do - { key: 'CI_BUILD_TRIGGERED', value: 'true', public: true } + { key: 'CI_PIPELINE_TRIGGERED', value: 'true', public: true } end before do @@ -1416,7 +1418,7 @@ describe Ci::Build, :models do context 'when config is not found' do let(:config) { nil } - it { is_expected.to eq(predefined_variables) } + it { is_expected.to include(*predefined_variables) } end context 'when config does not have a questioned job' do @@ -1428,7 +1430,7 @@ describe Ci::Build, :models do }) end - it { is_expected.to eq(predefined_variables) } + it { is_expected.to include(*predefined_variables) } end context 'when config has variables' do @@ -1446,7 +1448,8 @@ describe Ci::Build, :models do [{ key: 'KEY', value: 'value', public: true }] end - it { is_expected.to eq(predefined_variables + variables) } + it { is_expected.to include(*predefined_variables) } + it { is_expected.to include(*variables) } end end end diff --git a/spec/models/ci/trigger_spec.rb b/spec/models/ci/trigger_spec.rb index 074cf1a0bd7..1bcb673cb16 100644 --- a/spec/models/ci/trigger_spec.rb +++ b/spec/models/ci/trigger_spec.rb @@ -22,4 +22,62 @@ describe Ci::Trigger, models: true do expect(trigger.token).to eq('token') end end + + describe '#short_token' do + let(:trigger) { create(:ci_trigger, token: '12345678') } + + subject { trigger.short_token } + + it 'returns shortened token' do + is_expected.to eq('1234') + end + end + + describe '#legacy?' do + let(:trigger) { create(:ci_trigger, owner: owner, project: project) } + + subject { trigger } + + context 'when owner is blank' do + let(:owner) { nil } + + it { is_expected.to be_legacy } + end + + context 'when owner is set' do + let(:owner) { create(:user) } + + it { is_expected.not_to be_legacy } + end + end + + describe '#can_access_project?' do + let(:trigger) { create(:ci_trigger, owner: owner, project: project) } + + context 'when owner is blank' do + let(:owner) { nil } + + subject { trigger.can_access_project? } + + it { is_expected.to eq(true) } + end + + context 'when owner is set' do + let(:owner) { create(:user) } + + subject { trigger.can_access_project? } + + context 'and is member of the project' do + before do + project.team << [owner, :developer] + end + + it { is_expected.to eq(true) } + end + + context 'and is not member of the project' do + it { is_expected.to eq(false) } + end + end + end end diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index dce18f008f8..b4305e92812 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -271,7 +271,11 @@ describe Environment, models: true do context 'when the environment is unavailable' do let(:project) { create(:kubernetes_project) } - before { environment.stop } + + before do + environment.stop + end + it { is_expected.to be_falsy } end end @@ -281,20 +285,85 @@ describe Environment, models: true do subject { environment.terminals } context 'when the environment has terminals' do - before { allow(environment).to receive(:has_terminals?).and_return(true) } + before do + allow(environment).to receive(:has_terminals?).and_return(true) + end it 'returns the terminals from the deployment service' do - expect(project.deployment_service). - to receive(:terminals).with(environment). - and_return(:fake_terminals) + expect(project.deployment_service) + .to receive(:terminals).with(environment) + .and_return(:fake_terminals) is_expected.to eq(:fake_terminals) end end context 'when the environment does not have terminals' do - before { allow(environment).to receive(:has_terminals?).and_return(false) } - it { is_expected.to eq(nil) } + before do + allow(environment).to receive(:has_terminals?).and_return(false) + end + + it { is_expected.to be_nil } + end + end + + describe '#has_metrics?' do + subject { environment.has_metrics? } + + context 'when the enviroment is available' do + context 'with a deployment service' do + let(:project) { create(:prometheus_project) } + + context 'and a deployment' do + let!(:deployment) { create(:deployment, environment: environment) } + it { is_expected.to be_truthy } + end + + context 'but no deployments' do + it { is_expected.to be_falsy } + end + end + + context 'without a monitoring service' do + it { is_expected.to be_falsy } + end + end + + context 'when the environment is unavailable' do + let(:project) { create(:prometheus_project) } + + before do + environment.stop + end + + it { is_expected.to be_falsy } + end + end + + describe '#metrics' do + let(:project) { create(:prometheus_project) } + subject { environment.metrics } + + context 'when the environment has metrics' do + before do + allow(environment).to receive(:has_metrics?).and_return(true) + end + + it 'returns the metrics from the deployment service' do + expect(project.monitoring_service) + .to receive(:metrics).with(environment) + .and_return(:fake_metrics) + + is_expected.to eq(:fake_metrics) + end + end + + context 'when the environment does not have metrics' do + before do + allow(environment).to receive(:has_metrics?).and_return(false) + end + + it { is_expected.to be_nil } end end diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index 7525a1b79ee..757f3921450 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -165,7 +165,7 @@ describe Namespace, models: true do describe :rm_dir do let!(:project) { create(:empty_project, namespace: namespace) } - let!(:path) { File.join(Gitlab.config.repositories.storages.default, namespace.full_path) } + let!(:path) { File.join(Gitlab.config.repositories.storages.default['path'], namespace.full_path) } it "removes its dirs when deleted" do namespace.destroy diff --git a/spec/models/personal_access_token_spec.rb b/spec/models/personal_access_token_spec.rb index 46eb71cef14..823623d96fa 100644 --- a/spec/models/personal_access_token_spec.rb +++ b/spec/models/personal_access_token_spec.rb @@ -1,15 +1,61 @@ require 'spec_helper' describe PersonalAccessToken, models: true do - describe ".generate" do - it "generates a random token" do - personal_access_token = PersonalAccessToken.generate({}) - expect(personal_access_token.token).to be_present + describe '.build' do + let(:personal_access_token) { build(:personal_access_token) } + let(:invalid_personal_access_token) { build(:personal_access_token, :invalid) } + + it 'is a valid personal access token' do + expect(personal_access_token).to be_valid + end + + it 'ensures that the token is generated' do + invalid_personal_access_token.save! + + expect(invalid_personal_access_token).to be_valid + expect(invalid_personal_access_token.token).not_to be_nil end + end + + describe ".active?" do + let(:active_personal_access_token) { build(:personal_access_token) } + let(:revoked_personal_access_token) { build(:personal_access_token, :revoked) } + let(:expired_personal_access_token) { build(:personal_access_token, :expired) } + + it "returns false if the personal_access_token is revoked" do + expect(revoked_personal_access_token).not_to be_active + end + + it "returns false if the personal_access_token is expired" do + expect(expired_personal_access_token).not_to be_active + end + + it "returns true if the personal_access_token is not revoked and not expired" do + expect(active_personal_access_token).to be_active + end + end + + context "validations" do + let(:personal_access_token) { build(:personal_access_token) } + + it "requires at least one scope" do + personal_access_token.scopes = [] + + expect(personal_access_token).not_to be_valid + expect(personal_access_token.errors[:scopes].first).to eq "can't be blank" + end + + it "allows creating a token with API scopes" do + personal_access_token.scopes = [:api, :read_user] + + expect(personal_access_token).to be_valid + end + + it "rejects creating a token with non-API scopes" do + personal_access_token.scopes = [:openid, :api] - it "doesn't save the record" do - personal_access_token = PersonalAccessToken.generate({}) - expect(personal_access_token).not_to be_persisted + expect(personal_access_token).not_to be_valid + expect(personal_access_token.errors[:scopes].first).to eq "can only contain API scopes" end end end diff --git a/spec/models/project_services/prometheus_service_spec.rb b/spec/models/project_services/prometheus_service_spec.rb new file mode 100644 index 00000000000..d15079b686b --- /dev/null +++ b/spec/models/project_services/prometheus_service_spec.rb @@ -0,0 +1,104 @@ +require 'spec_helper' + +describe PrometheusService, models: true, caching: true do + include PrometheusHelpers + include ReactiveCachingHelpers + + let(:project) { create(:prometheus_project) } + let(:service) { project.prometheus_service } + + describe "Associations" do + it { is_expected.to belong_to :project } + end + + describe 'Validations' do + context 'when service is active' do + before { subject.active = true } + + it { is_expected.to validate_presence_of(:api_url) } + end + + context 'when service is inactive' do + before { subject.active = false } + + it { is_expected.not_to validate_presence_of(:api_url) } + end + end + + describe '#test' do + let!(:req_stub) { stub_prometheus_request(prometheus_query_url('1'), body: prometheus_value_body('vector')) } + + context 'success' do + it 'reads the discovery endpoint' do + expect(service.test[:success]).to be_truthy + expect(req_stub).to have_been_requested + end + end + + context 'failure' do + let!(:req_stub) { stub_prometheus_request(prometheus_query_url('1'), status: 404) } + + it 'fails to read the discovery endpoint' do + expect(service.test[:success]).to be_falsy + expect(req_stub).to have_been_requested + end + end + end + + describe '#metrics' do + let(:environment) { build_stubbed(:environment, slug: 'env-slug') } + subject { service.metrics(environment) } + + around do |example| + Timecop.freeze { example.run } + end + + context 'with valid data' do + before do + stub_reactive_cache(service, prometheus_data, 'env-slug') + end + + it 'returns reactive data' do + is_expected.to eq(prometheus_data) + end + end + end + + describe '#calculate_reactive_cache' do + let(:environment) { build_stubbed(:environment, slug: 'env-slug') } + + around do |example| + Timecop.freeze { example.run } + end + + subject do + service.calculate_reactive_cache(environment.slug) + end + + context 'when service is inactive' do + before do + service.active = false + end + + it { is_expected.to be_nil } + end + + context 'when Prometheus responds with valid data' do + before do + stub_all_prometheus_requests(environment.slug) + end + + it { expect(subject.to_json).to eq(prometheus_data.to_json) } + end + + [404, 500].each do |status| + context "when Prometheus responds with #{status}" do + before do + stub_all_prometheus_requests(environment.slug, status: status, body: 'QUERY FAILED!') + end + + it { is_expected.to eq(success: false, result: %(#{status} - "QUERY FAILED!")) } + end + end + end +end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 84bdcbe8e59..e120e21af06 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -179,7 +179,7 @@ describe Project, models: true do let(:project2) { build(:empty_project, repository_storage: 'missing') } before do - storages = { 'custom' => 'tmp/tests/custom_repositories' } + storages = { 'custom' => { 'path' => 'tmp/tests/custom_repositories' } } allow(Gitlab.config.repositories).to receive(:storages).and_return(storages) end @@ -381,7 +381,7 @@ describe Project, models: true do before do FileUtils.mkdir('tmp/tests/custom_repositories') - storages = { 'custom' => 'tmp/tests/custom_repositories' } + storages = { 'custom' => { 'path' => 'tmp/tests/custom_repositories' } } allow(Gitlab.config.repositories).to receive(:storages).and_return(storages) end @@ -947,8 +947,8 @@ describe Project, models: true do before do storages = { - 'default' => 'tmp/tests/repositories', - 'picked' => 'tmp/tests/repositories', + 'default' => { 'path' => 'tmp/tests/repositories' }, + 'picked' => { 'path' => 'tmp/tests/repositories' }, } allow(Gitlab.config.repositories).to receive(:storages).and_return(storages) end diff --git a/spec/policies/ci/trigger_policy_spec.rb b/spec/policies/ci/trigger_policy_spec.rb new file mode 100644 index 00000000000..63ad5eb7322 --- /dev/null +++ b/spec/policies/ci/trigger_policy_spec.rb @@ -0,0 +1,103 @@ +require 'spec_helper' + +describe Ci::TriggerPolicy, :models do + let(:user) { create(:user) } + let(:project) { create(:empty_project) } + let(:trigger) { create(:ci_trigger, project: project, owner: owner) } + + let(:policies) do + described_class.abilities(user, trigger).to_set + end + + shared_examples 'allows to admin and manage trigger' do + it 'does include ability to admin trigger' do + expect(policies).to include :admin_trigger + end + + it 'does include ability to manage trigger' do + expect(policies).to include :manage_trigger + end + end + + shared_examples 'allows to manage trigger' do + it 'does not include ability to admin trigger' do + expect(policies).not_to include :admin_trigger + end + + it 'does include ability to manage trigger' do + expect(policies).to include :manage_trigger + end + end + + shared_examples 'disallows to admin and manage trigger' do + it 'does not include ability to admin trigger' do + expect(policies).not_to include :admin_trigger + end + + it 'does not include ability to manage trigger' do + expect(policies).not_to include :manage_trigger + end + end + + describe '#rules' do + context 'when owner is undefined' do + let(:owner) { nil } + + context 'when user is master of the project' do + before do + project.team << [user, :master] + end + + it_behaves_like 'allows to admin and manage trigger' + end + + context 'when user is developer of the project' do + before do + project.team << [user, :developer] + end + + it_behaves_like 'disallows to admin and manage trigger' + end + + context 'when user is not member of the project' do + it_behaves_like 'disallows to admin and manage trigger' + end + end + + context 'when owner is an user' do + let(:owner) { user } + + context 'when user is master of the project' do + before do + project.team << [user, :master] + end + + it_behaves_like 'allows to admin and manage trigger' + end + end + + context 'when owner is another user' do + let(:owner) { create(:user) } + + context 'when user is master of the project' do + before do + project.team << [user, :master] + end + + it_behaves_like 'allows to manage trigger' + end + + context 'when user is developer of the project' do + before do + project.team << [user, :developer] + end + + it_behaves_like 'disallows to admin and manage trigger' + end + + context 'when user is not member of the project' do + it_behaves_like 'disallows to admin and manage trigger' + end + end + end +end diff --git a/spec/presenters/projects/settings/deploy_keys_presenter_spec.rb b/spec/presenters/projects/settings/deploy_keys_presenter_spec.rb new file mode 100644 index 00000000000..6443f86b6a1 --- /dev/null +++ b/spec/presenters/projects/settings/deploy_keys_presenter_spec.rb @@ -0,0 +1,66 @@ +require 'spec_helper' + +describe Projects::Settings::DeployKeysPresenter do + let(:project) { create(:empty_project) } + let(:user) { create(:user) } + let(:deploy_key) { create(:deploy_key, public: true) } + + let!(:deploy_keys_project) do + create(:deploy_keys_project, project: project, deploy_key: deploy_key) + end + + subject(:presenter) do + described_class.new(project, current_user: user) + end + + it 'inherits from Gitlab::View::Presenter::Simple' do + expect(described_class.superclass).to eq(Gitlab::View::Presenter::Simple) + end + + describe '#enabled_keys' do + it 'returns currently enabled keys' do + expect(presenter.enabled_keys).to eq [deploy_keys_project.deploy_key] + end + + it 'does not contain enabled_keys inside available_keys' do + expect(presenter.available_keys).not_to include deploy_key + end + + it 'returns the enabled_keys size' do + expect(presenter.enabled_keys_size).to eq(1) + end + + it 'returns true if there is any enabled_keys' do + expect(presenter.any_keys_enabled?).to eq(true) + end + end + + describe '#available_keys/#available_project_keys' do + let(:other_deploy_key) { create(:another_deploy_key) } + + before do + project_key = create(:deploy_keys_project, deploy_key: other_deploy_key) + project_key.project.add_developer(user) + end + + it 'returns the current available_keys' do + expect(presenter.available_keys).not_to be_empty + end + + it 'returns the current available_project_keys' do + expect(presenter.available_project_keys).not_to be_empty + end + + it 'returns false if any available_project_keys are enabled' do + expect(presenter.any_available_project_keys_enabled?).to eq(true) + end + + it 'returns the available_project_keys size' do + expect(presenter.available_project_keys_size).to eq(1) + end + + it 'shows if there is an available key' do + expect(presenter.key_available?(deploy_key)).to eq(false) + end + end +end diff --git a/spec/requests/api/api_internal_helpers_spec.rb b/spec/requests/api/api_internal_helpers_spec.rb index be4bc39ada2..f5265ea60ff 100644 --- a/spec/requests/api/api_internal_helpers_spec.rb +++ b/spec/requests/api/api_internal_helpers_spec.rb @@ -21,7 +21,7 @@ describe ::API::Helpers::InternalHelpers do # Relative and absolute storage paths, with and without trailing / ['.', './', Dir.pwd, Dir.pwd + '/'].each do |storage_path| context "storage path is #{storage_path}" do - subject { clean_project_path(project_path, [storage_path]) } + subject { clean_project_path(project_path, [{ 'path' => storage_path }]) } it { is_expected.to eq(expected) } end diff --git a/spec/requests/api/award_emoji_spec.rb b/spec/requests/api/award_emoji_spec.rb index 9756991162e..f4d4a8a2cc7 100644 --- a/spec/requests/api/award_emoji_spec.rb +++ b/spec/requests/api/award_emoji_spec.rb @@ -15,7 +15,7 @@ describe API::AwardEmoji, api: true do describe "GET /projects/:id/awardable/:awardable_id/award_emoji" do context 'on an issue' do it "returns an array of award_emoji" do - get api("/projects/#{project.id}/issues/#{issue.id}/award_emoji", user) + get api("/projects/#{project.id}/issues/#{issue.iid}/award_emoji", user) expect(response).to have_http_status(200) expect(json_response).to be_an Array @@ -31,7 +31,7 @@ describe API::AwardEmoji, api: true do context 'on a merge request' do it "returns an array of award_emoji" do - get api("/projects/#{project.id}/merge_requests/#{merge_request.id}/award_emoji", user) + get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/award_emoji", user) expect(response).to have_http_status(200) expect(response).to include_pagination_headers @@ -57,7 +57,7 @@ describe API::AwardEmoji, api: true do it 'returns a status code 404' do user1 = create(:user) - get api("/projects/#{project.id}/merge_requests/#{merge_request.id}/award_emoji", user1) + get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/award_emoji", user1) expect(response).to have_http_status(404) end @@ -68,7 +68,7 @@ describe API::AwardEmoji, api: true do let!(:rocket) { create(:award_emoji, awardable: note, name: 'rocket') } it 'returns an array of award emoji' do - get api("/projects/#{project.id}/issues/#{issue.id}/notes/#{note.id}/award_emoji", user) + get api("/projects/#{project.id}/issues/#{issue.iid}/notes/#{note.id}/award_emoji", user) expect(response).to have_http_status(200) expect(json_response).to be_an Array @@ -79,7 +79,7 @@ describe API::AwardEmoji, api: true do describe "GET /projects/:id/awardable/:awardable_id/award_emoji/:award_id" do context 'on an issue' do it "returns the award emoji" do - get api("/projects/#{project.id}/issues/#{issue.id}/award_emoji/#{award_emoji.id}", user) + get api("/projects/#{project.id}/issues/#{issue.iid}/award_emoji/#{award_emoji.id}", user) expect(response).to have_http_status(200) expect(json_response['name']).to eq(award_emoji.name) @@ -88,7 +88,7 @@ describe API::AwardEmoji, api: true do end it "returns a 404 error if the award is not found" do - get api("/projects/#{project.id}/issues/#{issue.id}/award_emoji/12345", user) + get api("/projects/#{project.id}/issues/#{issue.iid}/award_emoji/12345", user) expect(response).to have_http_status(404) end @@ -96,7 +96,7 @@ describe API::AwardEmoji, api: true do context 'on a merge request' do it 'returns the award emoji' do - get api("/projects/#{project.id}/merge_requests/#{merge_request.id}/award_emoji/#{downvote.id}", user) + get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/award_emoji/#{downvote.id}", user) expect(response).to have_http_status(200) expect(json_response['name']).to eq(downvote.name) @@ -123,7 +123,7 @@ describe API::AwardEmoji, api: true do it 'returns a status code 404' do user1 = create(:user) - get api("/projects/#{project.id}/merge_requests/#{merge_request.id}/award_emoji/#{downvote.id}", user1) + get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/award_emoji/#{downvote.id}", user1) expect(response).to have_http_status(404) end @@ -134,7 +134,7 @@ describe API::AwardEmoji, api: true do let!(:rocket) { create(:award_emoji, awardable: note, name: 'rocket') } it 'returns an award emoji' do - get api("/projects/#{project.id}/issues/#{issue.id}/notes/#{note.id}/award_emoji/#{rocket.id}", user) + get api("/projects/#{project.id}/issues/#{issue.iid}/notes/#{note.id}/award_emoji/#{rocket.id}", user) expect(response).to have_http_status(200) expect(json_response).not_to be_an Array @@ -147,7 +147,7 @@ describe API::AwardEmoji, api: true do context "on an issue" do it "creates a new award emoji" do - post api("/projects/#{project.id}/issues/#{issue.id}/award_emoji", user), name: 'blowfish' + post api("/projects/#{project.id}/issues/#{issue.iid}/award_emoji", user), name: 'blowfish' expect(response).to have_http_status(201) expect(json_response['name']).to eq('blowfish') @@ -155,13 +155,13 @@ describe API::AwardEmoji, api: true do end it "returns a 400 bad request error if the name is not given" do - post api("/projects/#{project.id}/issues/#{issue.id}/award_emoji", user) + post api("/projects/#{project.id}/issues/#{issue.iid}/award_emoji", user) expect(response).to have_http_status(400) end it "returns a 401 unauthorized error if the user is not authenticated" do - post api("/projects/#{project.id}/issues/#{issue.id}/award_emoji"), name: 'thumbsup' + post api("/projects/#{project.id}/issues/#{issue.iid}/award_emoji"), name: 'thumbsup' expect(response).to have_http_status(401) end @@ -173,15 +173,15 @@ describe API::AwardEmoji, api: true do end it "normalizes +1 as thumbsup award" do - post api("/projects/#{project.id}/issues/#{issue.id}/award_emoji", user), name: '+1' + post api("/projects/#{project.id}/issues/#{issue.iid}/award_emoji", user), name: '+1' expect(issue.award_emoji.last.name).to eq("thumbsup") end context 'when the emoji already has been awarded' do it 'returns a 404 status code' do - post api("/projects/#{project.id}/issues/#{issue.id}/award_emoji", user), name: 'thumbsup' - post api("/projects/#{project.id}/issues/#{issue.id}/award_emoji", user), name: 'thumbsup' + post api("/projects/#{project.id}/issues/#{issue.iid}/award_emoji", user), name: 'thumbsup' + post api("/projects/#{project.id}/issues/#{issue.iid}/award_emoji", user), name: 'thumbsup' expect(response).to have_http_status(404) expect(json_response["message"]).to match("has already been taken") @@ -207,7 +207,7 @@ describe API::AwardEmoji, api: true do it 'creates a new award emoji' do expect do - post api("/projects/#{project.id}/issues/#{issue.id}/notes/#{note.id}/award_emoji", user), name: 'rocket' + post api("/projects/#{project.id}/issues/#{issue.iid}/notes/#{note.id}/award_emoji", user), name: 'rocket' end.to change { note.award_emoji.count }.from(0).to(1) expect(response).to have_http_status(201) @@ -215,21 +215,21 @@ describe API::AwardEmoji, api: true do end it "it returns 404 error when user authored note" do - post api("/projects/#{project.id}/issues/#{issue.id}/notes/#{note2.id}/award_emoji", user), name: 'thumbsup' + post api("/projects/#{project.id}/issues/#{issue.iid}/notes/#{note2.id}/award_emoji", user), name: 'thumbsup' expect(response).to have_http_status(404) end it "normalizes +1 as thumbsup award" do - post api("/projects/#{project.id}/issues/#{issue.id}/notes/#{note.id}/award_emoji", user), name: '+1' + post api("/projects/#{project.id}/issues/#{issue.iid}/notes/#{note.id}/award_emoji", user), name: '+1' expect(note.award_emoji.last.name).to eq("thumbsup") end context 'when the emoji already has been awarded' do it 'returns a 404 status code' do - post api("/projects/#{project.id}/issues/#{issue.id}/notes/#{note.id}/award_emoji", user), name: 'rocket' - post api("/projects/#{project.id}/issues/#{issue.id}/notes/#{note.id}/award_emoji", user), name: 'rocket' + post api("/projects/#{project.id}/issues/#{issue.iid}/notes/#{note.id}/award_emoji", user), name: 'rocket' + post api("/projects/#{project.id}/issues/#{issue.iid}/notes/#{note.id}/award_emoji", user), name: 'rocket' expect(response).to have_http_status(404) expect(json_response["message"]).to match("has already been taken") @@ -241,14 +241,14 @@ describe API::AwardEmoji, api: true do context 'when the awardable is an Issue' do it 'deletes the award' do expect do - delete api("/projects/#{project.id}/issues/#{issue.id}/award_emoji/#{award_emoji.id}", user) + delete api("/projects/#{project.id}/issues/#{issue.iid}/award_emoji/#{award_emoji.id}", user) expect(response).to have_http_status(204) end.to change { issue.award_emoji.count }.from(1).to(0) end it 'returns a 404 error when the award emoji can not be found' do - delete api("/projects/#{project.id}/issues/#{issue.id}/award_emoji/12345", user) + delete api("/projects/#{project.id}/issues/#{issue.iid}/award_emoji/12345", user) expect(response).to have_http_status(404) end @@ -257,14 +257,14 @@ describe API::AwardEmoji, api: true do context 'when the awardable is a Merge Request' do it 'deletes the award' do expect do - delete api("/projects/#{project.id}/merge_requests/#{merge_request.id}/award_emoji/#{downvote.id}", user) + delete api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/award_emoji/#{downvote.id}", user) expect(response).to have_http_status(204) end.to change { merge_request.award_emoji.count }.from(1).to(0) end it 'returns a 404 error when note id not found' do - delete api("/projects/#{project.id}/merge_requests/#{merge_request.id}/notes/12345", user) + delete api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/notes/12345", user) expect(response).to have_http_status(404) end @@ -289,7 +289,7 @@ describe API::AwardEmoji, api: true do it 'deletes the award' do expect do - delete api("/projects/#{project.id}/issues/#{issue.id}/notes/#{note.id}/award_emoji/#{rocket.id}", user) + delete api("/projects/#{project.id}/issues/#{issue.iid}/notes/#{note.id}/award_emoji/#{rocket.id}", user) expect(response).to have_http_status(204) end.to change { note.award_emoji.count }.from(1).to(0) diff --git a/spec/requests/api/doorkeeper_access_spec.rb b/spec/requests/api/doorkeeper_access_spec.rb index 2974875510a..f6fd567eca5 100644 --- a/spec/requests/api/doorkeeper_access_spec.rb +++ b/spec/requests/api/doorkeeper_access_spec.rb @@ -39,4 +39,22 @@ describe API::API, api: true do end end end + + describe "when user is blocked" do + it "returns authentication error" do + user.block + get api("/user"), access_token: token.token + + expect(response).to have_http_status(401) + end + end + + describe "when user is ldap_blocked" do + it "returns authentication error" do + user.ldap_block + get api("/user"), access_token: token.token + + expect(response).to have_http_status(401) + end + end end diff --git a/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb index aca7c6a0734..2fc11a3b782 100644 --- a/spec/requests/api/issues_spec.rb +++ b/spec/requests/api/issues_spec.rb @@ -757,9 +757,9 @@ describe API::Issues, api: true do end end - describe "GET /projects/:id/issues/:issue_id" do + describe "GET /projects/:id/issues/:issue_iid" do it 'exposes known attributes' do - get api("/projects/#{project.id}/issues/#{issue.id}", user) + get api("/projects/#{project.id}/issues/#{issue.iid}", user) expect(response).to have_http_status(200) expect(json_response['id']).to eq(issue.id) @@ -777,8 +777,8 @@ describe API::Issues, api: true do expect(json_response['confidential']).to be_falsy end - it "returns a project issue by id" do - get api("/projects/#{project.id}/issues/#{issue.id}", user) + it "returns a project issue by internal id" do + get api("/projects/#{project.id}/issues/#{issue.iid}", user) expect(response).to have_http_status(200) expect(json_response['title']).to eq(issue.title) @@ -790,40 +790,52 @@ describe API::Issues, api: true do expect(response).to have_http_status(404) end + it "returns 404 if the issue ID is used" do + get api("/projects/#{project.id}/issues/#{issue.id}", user) + + expect(response).to have_http_status(404) + end + context 'confidential issues' do it "returns 404 for non project members" do - get api("/projects/#{project.id}/issues/#{confidential_issue.id}", non_member) + get api("/projects/#{project.id}/issues/#{confidential_issue.iid}", non_member) + expect(response).to have_http_status(404) end it "returns 404 for project members with guest role" do - get api("/projects/#{project.id}/issues/#{confidential_issue.id}", guest) + get api("/projects/#{project.id}/issues/#{confidential_issue.iid}", guest) + expect(response).to have_http_status(404) end it "returns confidential issue for project members" do - get api("/projects/#{project.id}/issues/#{confidential_issue.id}", user) + get api("/projects/#{project.id}/issues/#{confidential_issue.iid}", user) + expect(response).to have_http_status(200) expect(json_response['title']).to eq(confidential_issue.title) expect(json_response['iid']).to eq(confidential_issue.iid) end it "returns confidential issue for author" do - get api("/projects/#{project.id}/issues/#{confidential_issue.id}", author) + get api("/projects/#{project.id}/issues/#{confidential_issue.iid}", author) + expect(response).to have_http_status(200) expect(json_response['title']).to eq(confidential_issue.title) expect(json_response['iid']).to eq(confidential_issue.iid) end it "returns confidential issue for assignee" do - get api("/projects/#{project.id}/issues/#{confidential_issue.id}", assignee) + get api("/projects/#{project.id}/issues/#{confidential_issue.iid}", assignee) + expect(response).to have_http_status(200) expect(json_response['title']).to eq(confidential_issue.title) expect(json_response['iid']).to eq(confidential_issue.iid) end it "returns confidential issue for admin" do - get api("/projects/#{project.id}/issues/#{confidential_issue.id}", admin) + get api("/projects/#{project.id}/issues/#{confidential_issue.iid}", admin) + expect(response).to have_http_status(200) expect(json_response['title']).to eq(confidential_issue.title) expect(json_response['iid']).to eq(confidential_issue.iid) @@ -1004,23 +1016,29 @@ describe API::Issues, api: true do end end - describe "PUT /projects/:id/issues/:issue_id to update only title" do + describe "PUT /projects/:id/issues/:issue_iid to update only title" do it "updates a project issue" do - put api("/projects/#{project.id}/issues/#{issue.id}", user), + put api("/projects/#{project.id}/issues/#{issue.iid}", user), title: 'updated title' expect(response).to have_http_status(200) expect(json_response['title']).to eq('updated title') end - it "returns 404 error if issue id not found" do + it "returns 404 error if issue iid not found" do put api("/projects/#{project.id}/issues/44444", user), title: 'updated title' expect(response).to have_http_status(404) end - it 'allows special label names' do + it "returns 404 error if issue id is used instead of the iid" do put api("/projects/#{project.id}/issues/#{issue.id}", user), + title: 'updated title' + expect(response).to have_http_status(404) + end + + it 'allows special label names' do + put api("/projects/#{project.id}/issues/#{issue.iid}", user), title: 'updated title', labels: 'label, label?, label&foo, ?, &' @@ -1034,40 +1052,40 @@ describe API::Issues, api: true do context 'confidential issues' do it "returns 403 for non project members" do - put api("/projects/#{project.id}/issues/#{confidential_issue.id}", non_member), + put api("/projects/#{project.id}/issues/#{confidential_issue.iid}", non_member), title: 'updated title' expect(response).to have_http_status(403) end it "returns 403 for project members with guest role" do - put api("/projects/#{project.id}/issues/#{confidential_issue.id}", guest), + put api("/projects/#{project.id}/issues/#{confidential_issue.iid}", guest), title: 'updated title' expect(response).to have_http_status(403) end it "updates a confidential issue for project members" do - put api("/projects/#{project.id}/issues/#{confidential_issue.id}", user), + put api("/projects/#{project.id}/issues/#{confidential_issue.iid}", user), title: 'updated title' expect(response).to have_http_status(200) expect(json_response['title']).to eq('updated title') end it "updates a confidential issue for author" do - put api("/projects/#{project.id}/issues/#{confidential_issue.id}", author), + put api("/projects/#{project.id}/issues/#{confidential_issue.iid}", author), title: 'updated title' expect(response).to have_http_status(200) expect(json_response['title']).to eq('updated title') end it "updates a confidential issue for admin" do - put api("/projects/#{project.id}/issues/#{confidential_issue.id}", admin), + put api("/projects/#{project.id}/issues/#{confidential_issue.iid}", admin), title: 'updated title' expect(response).to have_http_status(200) expect(json_response['title']).to eq('updated title') end it 'sets an issue to confidential' do - put api("/projects/#{project.id}/issues/#{issue.id}", user), + put api("/projects/#{project.id}/issues/#{issue.iid}", user), confidential: true expect(response).to have_http_status(200) @@ -1075,7 +1093,7 @@ describe API::Issues, api: true do end it 'makes a confidential issue public' do - put api("/projects/#{project.id}/issues/#{confidential_issue.id}", user), + put api("/projects/#{project.id}/issues/#{confidential_issue.iid}", user), confidential: false expect(response).to have_http_status(200) @@ -1083,7 +1101,7 @@ describe API::Issues, api: true do end it 'does not update a confidential issue with wrong confidential flag' do - put api("/projects/#{project.id}/issues/#{confidential_issue.id}", user), + put api("/projects/#{project.id}/issues/#{confidential_issue.iid}", user), confidential: 'foo' expect(response).to have_http_status(400) @@ -1092,7 +1110,7 @@ describe API::Issues, api: true do end end - describe 'PUT /projects/:id/issues/:issue_id with spam filtering' do + describe 'PUT /projects/:id/issues/:issue_iid with spam filtering' do let(:params) do { title: 'updated title', @@ -1105,7 +1123,7 @@ describe API::Issues, api: true do allow_any_instance_of(SpamService).to receive_messages(check_for_spam?: true) allow_any_instance_of(AkismetService).to receive_messages(is_spam?: true) - put api("/projects/#{project.id}/issues/#{issue.id}", user), params + put api("/projects/#{project.id}/issues/#{issue.iid}", user), params expect(response).to have_http_status(400) expect(json_response['message']).to eq({ "error" => "Spam detected" }) @@ -1119,12 +1137,12 @@ describe API::Issues, api: true do end end - describe 'PUT /projects/:id/issues/:issue_id to update labels' do + describe 'PUT /projects/:id/issues/:issue_iid to update labels' do let!(:label) { create(:label, title: 'dummy', project: project) } let!(:label_link) { create(:label_link, label: label, target: issue) } it 'does not update labels if not present' do - put api("/projects/#{project.id}/issues/#{issue.id}", user), + put api("/projects/#{project.id}/issues/#{issue.iid}", user), title: 'updated title' expect(response).to have_http_status(200) expect(json_response['labels']).to eq([label.title]) @@ -1135,7 +1153,7 @@ describe API::Issues, api: true do label.toggle_subscription(user2, project) perform_enqueued_jobs do - put api("/projects/#{project.id}/issues/#{issue.id}", user), + put api("/projects/#{project.id}/issues/#{issue.iid}", user), title: 'updated title', labels: label.title end @@ -1143,14 +1161,14 @@ describe API::Issues, api: true do end it 'removes all labels' do - put api("/projects/#{project.id}/issues/#{issue.id}", user), labels: '' + put api("/projects/#{project.id}/issues/#{issue.iid}", user), labels: '' expect(response).to have_http_status(200) expect(json_response['labels']).to eq([]) end it 'updates labels' do - put api("/projects/#{project.id}/issues/#{issue.id}", user), + put api("/projects/#{project.id}/issues/#{issue.iid}", user), labels: 'foo,bar' expect(response).to have_http_status(200) expect(json_response['labels']).to include 'foo' @@ -1158,7 +1176,7 @@ describe API::Issues, api: true do end it 'allows special label names' do - put api("/projects/#{project.id}/issues/#{issue.id}", user), + put api("/projects/#{project.id}/issues/#{issue.iid}", user), labels: 'label:foo, label-bar,label_bar,label/bar,label?bar,label&bar,?,&' expect(response.status).to eq(200) expect(json_response['labels']).to include 'label:foo' @@ -1172,7 +1190,7 @@ describe API::Issues, api: true do end it 'returns 400 if title is too long' do - put api("/projects/#{project.id}/issues/#{issue.id}", user), + put api("/projects/#{project.id}/issues/#{issue.iid}", user), title: 'g' * 256 expect(response).to have_http_status(400) expect(json_response['message']['title']).to eq([ @@ -1181,9 +1199,9 @@ describe API::Issues, api: true do end end - describe "PUT /projects/:id/issues/:issue_id to update state and label" do + describe "PUT /projects/:id/issues/:issue_iid to update state and label" do it "updates a project issue" do - put api("/projects/#{project.id}/issues/#{issue.id}", user), + put api("/projects/#{project.id}/issues/#{issue.iid}", user), labels: 'label2', state_event: "close" expect(response).to have_http_status(200) @@ -1192,7 +1210,7 @@ describe API::Issues, api: true do end it 'reopens a project isssue' do - put api("/projects/#{project.id}/issues/#{closed_issue.id}", user), state_event: 'reopen' + put api("/projects/#{project.id}/issues/#{closed_issue.iid}", user), state_event: 'reopen' expect(response).to have_http_status(200) expect(json_response['state']).to eq 'reopened' @@ -1201,7 +1219,7 @@ describe API::Issues, api: true do context 'when an admin or owner makes the request' do it 'accepts the update date to be set' do update_time = 2.weeks.ago - put api("/projects/#{project.id}/issues/#{issue.id}", user), + put api("/projects/#{project.id}/issues/#{issue.iid}", user), labels: 'label3', state_event: 'close', updated_at: update_time expect(response).to have_http_status(200) @@ -1211,25 +1229,25 @@ describe API::Issues, api: true do end end - describe 'PUT /projects/:id/issues/:issue_id to update due date' do + describe 'PUT /projects/:id/issues/:issue_iid to update due date' do it 'creates a new project issue' do due_date = 2.weeks.from_now.strftime('%Y-%m-%d') - put api("/projects/#{project.id}/issues/#{issue.id}", user), due_date: due_date + put api("/projects/#{project.id}/issues/#{issue.iid}", user), due_date: due_date expect(response).to have_http_status(200) expect(json_response['due_date']).to eq(due_date) end end - describe "DELETE /projects/:id/issues/:issue_id" do + describe "DELETE /projects/:id/issues/:issue_iid" do it "rejects a non member from deleting an issue" do - delete api("/projects/#{project.id}/issues/#{issue.id}", non_member) + delete api("/projects/#{project.id}/issues/#{issue.iid}", non_member) expect(response).to have_http_status(403) end it "rejects a developer from deleting an issue" do - delete api("/projects/#{project.id}/issues/#{issue.id}", author) + delete api("/projects/#{project.id}/issues/#{issue.iid}", author) expect(response).to have_http_status(403) end @@ -1238,7 +1256,7 @@ describe API::Issues, api: true do let(:project) { create(:empty_project, namespace: owner.namespace) } it "deletes the issue if an admin requests it" do - delete api("/projects/#{project.id}/issues/#{issue.id}", owner) + delete api("/projects/#{project.id}/issues/#{issue.iid}", owner) expect(response).to have_http_status(204) end @@ -1251,14 +1269,20 @@ describe API::Issues, api: true do expect(response).to have_http_status(404) end end + + it 'returns 404 when using the issue ID instead of IID' do + delete api("/projects/#{project.id}/issues/#{issue.id}", user) + + expect(response).to have_http_status(404) + end end - describe '/projects/:id/issues/:issue_id/move' do + describe '/projects/:id/issues/:issue_iid/move' do let!(:target_project) { create(:empty_project, path: 'project2', creator_id: user.id, namespace: user.namespace ) } let!(:target_project2) { create(:empty_project, creator_id: non_member.id, namespace: non_member.namespace ) } it 'moves an issue' do - post api("/projects/#{project.id}/issues/#{issue.id}/move", user), + post api("/projects/#{project.id}/issues/#{issue.iid}/move", user), to_project_id: target_project.id expect(response).to have_http_status(201) @@ -1267,7 +1291,7 @@ describe API::Issues, api: true do context 'when source and target projects are the same' do it 'returns 400 when trying to move an issue' do - post api("/projects/#{project.id}/issues/#{issue.id}/move", user), + post api("/projects/#{project.id}/issues/#{issue.iid}/move", user), to_project_id: project.id expect(response).to have_http_status(400) @@ -1277,7 +1301,7 @@ describe API::Issues, api: true do context 'when the user does not have the permission to move issues' do it 'returns 400 when trying to move an issue' do - post api("/projects/#{project.id}/issues/#{issue.id}/move", user), + post api("/projects/#{project.id}/issues/#{issue.iid}/move", user), to_project_id: target_project2.id expect(response).to have_http_status(400) @@ -1286,13 +1310,23 @@ describe API::Issues, api: true do end it 'moves the issue to another namespace if I am admin' do - post api("/projects/#{project.id}/issues/#{issue.id}/move", admin), + post api("/projects/#{project.id}/issues/#{issue.iid}/move", admin), to_project_id: target_project2.id expect(response).to have_http_status(201) expect(json_response['project_id']).to eq(target_project2.id) end + context 'when using the issue ID instead of iid' do + it 'returns 404 when trying to move an issue' do + post api("/projects/#{project.id}/issues/#{issue.id}/move", user), + to_project_id: target_project.id + + expect(response).to have_http_status(404) + expect(json_response['message']).to eq('404 Issue Not Found') + end + end + context 'when issue does not exist' do it 'returns 404 when trying to move an issue' do post api("/projects/#{project.id}/issues/123/move", user), @@ -1305,7 +1339,7 @@ describe API::Issues, api: true do context 'when source project does not exist' do it 'returns 404 when trying to move an issue' do - post api("/projects/123/issues/#{issue.id}/move", user), + post api("/projects/123/issues/#{issue.iid}/move", user), to_project_id: target_project.id expect(response).to have_http_status(404) @@ -1315,7 +1349,7 @@ describe API::Issues, api: true do context 'when target project does not exist' do it 'returns 404 when trying to move an issue' do - post api("/projects/#{project.id}/issues/#{issue.id}/move", user), + post api("/projects/#{project.id}/issues/#{issue.iid}/move", user), to_project_id: 123 expect(response).to have_http_status(404) @@ -1323,16 +1357,16 @@ describe API::Issues, api: true do end end - describe 'POST :id/issues/:issue_id/subscribe' do + describe 'POST :id/issues/:issue_iid/subscribe' do it 'subscribes to an issue' do - post api("/projects/#{project.id}/issues/#{issue.id}/subscribe", user2) + post api("/projects/#{project.id}/issues/#{issue.iid}/subscribe", user2) expect(response).to have_http_status(201) expect(json_response['subscribed']).to eq(true) end it 'returns 304 if already subscribed' do - post api("/projects/#{project.id}/issues/#{issue.id}/subscribe", user) + post api("/projects/#{project.id}/issues/#{issue.iid}/subscribe", user) expect(response).to have_http_status(304) end @@ -1343,8 +1377,14 @@ describe API::Issues, api: true do expect(response).to have_http_status(404) end + it 'returns 404 if the issue ID is used instead of the iid' do + post api("/projects/#{project.id}/issues/#{issue.id}/subscribe", user) + + expect(response).to have_http_status(404) + end + it 'returns 404 if the issue is confidential' do - post api("/projects/#{project.id}/issues/#{confidential_issue.id}/subscribe", non_member) + post api("/projects/#{project.id}/issues/#{confidential_issue.iid}/subscribe", non_member) expect(response).to have_http_status(404) end @@ -1352,14 +1392,14 @@ describe API::Issues, api: true do describe 'POST :id/issues/:issue_id/unsubscribe' do it 'unsubscribes from an issue' do - post api("/projects/#{project.id}/issues/#{issue.id}/unsubscribe", user) + post api("/projects/#{project.id}/issues/#{issue.iid}/unsubscribe", user) expect(response).to have_http_status(201) expect(json_response['subscribed']).to eq(false) end it 'returns 304 if not subscribed' do - post api("/projects/#{project.id}/issues/#{issue.id}/unsubscribe", user2) + post api("/projects/#{project.id}/issues/#{issue.iid}/unsubscribe", user2) expect(response).to have_http_status(304) end @@ -1370,8 +1410,14 @@ describe API::Issues, api: true do expect(response).to have_http_status(404) end + it 'returns 404 if using the issue ID instead of iid' do + post api("/projects/#{project.id}/issues/#{issue.id}/unsubscribe", user) + + expect(response).to have_http_status(404) + end + it 'returns 404 if the issue is confidential' do - post api("/projects/#{project.id}/issues/#{confidential_issue.id}/unsubscribe", non_member) + post api("/projects/#{project.id}/issues/#{confidential_issue.iid}/unsubscribe", non_member) expect(response).to have_http_status(404) end diff --git a/spec/requests/api/merge_request_diffs_spec.rb b/spec/requests/api/merge_request_diffs_spec.rb index 1d02e827183..79f3151ba52 100644 --- a/spec/requests/api/merge_request_diffs_spec.rb +++ b/spec/requests/api/merge_request_diffs_spec.rb @@ -13,9 +13,9 @@ describe API::MergeRequestDiffs, 'MergeRequestDiffs', api: true do project.team << [user, :master] end - describe 'GET /projects/:id/merge_requests/:merge_request_id/versions' do + describe 'GET /projects/:id/merge_requests/:merge_request_iid/versions' do it 'returns 200 for a valid merge request' do - get api("/projects/#{project.id}/merge_requests/#{merge_request.id}/versions", user) + get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/versions", user) merge_request_diff = merge_request.merge_request_diffs.first expect(response.status).to eq 200 @@ -26,16 +26,22 @@ describe API::MergeRequestDiffs, 'MergeRequestDiffs', api: true do expect(json_response.first['head_commit_sha']).to eq(merge_request_diff.head_commit_sha) end - it 'returns a 404 when merge_request_id not found' do + it 'returns a 404 when merge_request id is used instead of the iid' do + get api("/projects/#{project.id}/merge_requests/#{merge_request.id}/versions", user) + expect(response).to have_http_status(404) + end + + it 'returns a 404 when merge_request_iid not found' do get api("/projects/#{project.id}/merge_requests/999/versions", user) expect(response).to have_http_status(404) end end - describe 'GET /projects/:id/merge_requests/:merge_request_id/versions/:version_id' do + describe 'GET /projects/:id/merge_requests/:merge_request_iid/versions/:version_id' do + let(:merge_request_diff) { merge_request.merge_request_diffs.first } + it 'returns a 200 for a valid merge request' do - merge_request_diff = merge_request.merge_request_diffs.first - get api("/projects/#{project.id}/merge_requests/#{merge_request.id}/versions/#{merge_request_diff.id}", user) + get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/versions/#{merge_request_diff.id}", user) expect(response.status).to eq 200 expect(json_response['id']).to eq(merge_request_diff.id) @@ -43,8 +49,18 @@ describe API::MergeRequestDiffs, 'MergeRequestDiffs', api: true do expect(json_response['diffs'].size).to eq(merge_request_diff.diffs.size) end - it 'returns a 404 when merge_request_id not found' do - get api("/projects/#{project.id}/merge_requests/#{merge_request.id}/versions/999", user) + it 'returns a 404 when merge_request id is used instead of the iid' do + get api("/projects/#{project.id}/merge_requests/#{merge_request.id}/versions/#{merge_request_diff.id}", user) + expect(response).to have_http_status(404) + end + + it 'returns a 404 when merge_request version_id is not found' do + get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/versions/999", user) + expect(response).to have_http_status(404) + end + + it 'returns a 404 when merge_request_iid is not found' do + get api("/projects/#{project.id}/merge_requests/12345/versions/#{merge_request_diff.id}", user) expect(response).to have_http_status(404) end end diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index 1083abf2ad3..9aba1d75612 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -153,9 +153,9 @@ describe API::MergeRequests, api: true do end end - describe "GET /projects/:id/merge_requests/:merge_request_id" do + describe "GET /projects/:id/merge_requests/:merge_request_iid" do it 'exposes known attributes' do - get api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user) + get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user) expect(response).to have_http_status(200) expect(json_response['id']).to eq(merge_request.id) @@ -184,7 +184,7 @@ describe API::MergeRequests, api: true do end it "returns merge_request" do - get api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user) + get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user) expect(response).to have_http_status(200) expect(json_response['title']).to eq(merge_request.title) expect(json_response['iid']).to eq(merge_request.iid) @@ -194,25 +194,31 @@ describe API::MergeRequests, api: true do expect(json_response['force_close_merge_request']).to be_falsy end - it "returns a 404 error if merge_request_id not found" do + it "returns a 404 error if merge_request_iid not found" do get api("/projects/#{project.id}/merge_requests/999", user) expect(response).to have_http_status(404) end + it "returns a 404 error if merge_request `id` is used instead of iid" do + get api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user) + + expect(response).to have_http_status(404) + end + context 'Work in Progress' do let!(:merge_request_wip) { create(:merge_request, author: user, assignee: user, source_project: project, target_project: project, title: "WIP: Test", created_at: base_time + 1.second) } it "returns merge_request" do - get api("/projects/#{project.id}/merge_requests/#{merge_request_wip.id}", user) + get api("/projects/#{project.id}/merge_requests/#{merge_request_wip.iid}", user) expect(response).to have_http_status(200) expect(json_response['work_in_progress']).to eq(true) end end end - describe 'GET /projects/:id/merge_requests/:merge_request_id/commits' do + describe 'GET /projects/:id/merge_requests/:merge_request_iid/commits' do it 'returns a 200 when merge request is valid' do - get api("/projects/#{project.id}/merge_requests/#{merge_request.id}/commits", user) + get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/commits", user) commit = merge_request.commits.first expect(response.status).to eq 200 @@ -223,24 +229,36 @@ describe API::MergeRequests, api: true do expect(json_response.first['title']).to eq(commit.title) end - it 'returns a 404 when merge_request_id not found' do + it 'returns a 404 when merge_request_iid not found' do get api("/projects/#{project.id}/merge_requests/999/commits", user) expect(response).to have_http_status(404) end + + it 'returns a 404 when merge_request id is used instead of iid' do + get api("/projects/#{project.id}/merge_requests/#{merge_request.id}/commits", user) + + expect(response).to have_http_status(404) + end end - describe 'GET /projects/:id/merge_requests/:merge_request_id/changes' do + describe 'GET /projects/:id/merge_requests/:merge_request_iid/changes' do it 'returns the change information of the merge_request' do - get api("/projects/#{project.id}/merge_requests/#{merge_request.id}/changes", user) + get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/changes", user) expect(response.status).to eq 200 expect(json_response['changes'].size).to eq(merge_request.diffs.size) end - it 'returns a 404 when merge_request_id not found' do + it 'returns a 404 when merge_request_iid not found' do get api("/projects/#{project.id}/merge_requests/999/changes", user) expect(response).to have_http_status(404) end + + it 'returns a 404 when merge_request id is used instead of iid' do + get api("/projects/#{project.id}/merge_requests/#{merge_request.id}/changes", user) + + expect(response).to have_http_status(404) + end end describe "POST /projects/:id/merge_requests" do @@ -400,7 +418,7 @@ describe API::MergeRequests, api: true do end end - describe "DELETE /projects/:id/merge_requests/:merge_request_id" do + describe "DELETE /projects/:id/merge_requests/:merge_request_iid" do context "when the user is developer" do let(:developer) { create(:user) } @@ -409,25 +427,37 @@ describe API::MergeRequests, api: true do end it "denies the deletion of the merge request" do - delete api("/projects/#{project.id}/merge_requests/#{merge_request.id}", developer) + delete api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", developer) expect(response).to have_http_status(403) end end context "when the user is project owner" do it "destroys the merge request owners can destroy" do - delete api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user) + delete api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user) expect(response).to have_http_status(204) end + + it "returns 404 for an invalid merge request IID" do + delete api("/projects/#{project.id}/merge_requests/12345", user) + + expect(response).to have_http_status(404) + end + + it "returns 404 if the merge request id is used instead of iid" do + delete api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user) + + expect(response).to have_http_status(404) + end end end - describe "PUT /projects/:id/merge_requests/:merge_request_id/merge" do + describe "PUT /projects/:id/merge_requests/:merge_request_iid/merge" do let(:pipeline) { create(:ci_pipeline_without_jobs) } it "returns merge_request in case of success" do - put api("/projects/#{project.id}/merge_requests/#{merge_request.id}/merge", user) + put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user) expect(response).to have_http_status(200) end @@ -436,7 +466,7 @@ describe API::MergeRequests, api: true do allow_any_instance_of(MergeRequest). to receive(:can_be_merged?).and_return(false) - put api("/projects/#{project.id}/merge_requests/#{merge_request.id}/merge", user) + put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user) expect(response).to have_http_status(406) expect(json_response['message']).to eq('Branch cannot be merged') @@ -444,14 +474,14 @@ describe API::MergeRequests, api: true do it "returns 405 if merge_request is not open" do merge_request.close - put api("/projects/#{project.id}/merge_requests/#{merge_request.id}/merge", user) + put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user) expect(response).to have_http_status(405) expect(json_response['message']).to eq('405 Method Not Allowed') end it "returns 405 if merge_request is a work in progress" do merge_request.update_attribute(:title, "WIP: #{merge_request.title}") - put api("/projects/#{project.id}/merge_requests/#{merge_request.id}/merge", user) + put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user) expect(response).to have_http_status(405) expect(json_response['message']).to eq('405 Method Not Allowed') end @@ -459,7 +489,7 @@ describe API::MergeRequests, api: true do it 'returns 405 if the build failed for a merge request that requires success' do allow_any_instance_of(MergeRequest).to receive(:mergeable_ci_state?).and_return(false) - put api("/projects/#{project.id}/merge_requests/#{merge_request.id}/merge", user) + put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user) expect(response).to have_http_status(405) expect(json_response['message']).to eq('405 Method Not Allowed') @@ -468,20 +498,20 @@ describe API::MergeRequests, api: true do it "returns 401 if user has no permissions to merge" do user2 = create(:user) project.team << [user2, :reporter] - put api("/projects/#{project.id}/merge_requests/#{merge_request.id}/merge", user2) + put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user2) expect(response).to have_http_status(401) expect(json_response['message']).to eq('401 Unauthorized') end it "returns 409 if the SHA parameter doesn't match" do - put api("/projects/#{project.id}/merge_requests/#{merge_request.id}/merge", user), sha: merge_request.diff_head_sha.reverse + put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user), sha: merge_request.diff_head_sha.reverse expect(response).to have_http_status(409) expect(json_response['message']).to start_with('SHA does not match HEAD of source branch') end it "succeeds if the SHA parameter matches" do - put api("/projects/#{project.id}/merge_requests/#{merge_request.id}/merge", user), sha: merge_request.diff_head_sha + put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user), sha: merge_request.diff_head_sha expect(response).to have_http_status(200) end @@ -490,18 +520,30 @@ describe API::MergeRequests, api: true do allow_any_instance_of(MergeRequest).to receive(:head_pipeline).and_return(pipeline) allow(pipeline).to receive(:active?).and_return(true) - put api("/projects/#{project.id}/merge_requests/#{merge_request.id}/merge", user), merge_when_pipeline_succeeds: true + put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user), merge_when_pipeline_succeeds: true expect(response).to have_http_status(200) expect(json_response['title']).to eq('Test') expect(json_response['merge_when_pipeline_succeeds']).to eq(true) end + + it "returns 404 for an invalid merge request IID" do + put api("/projects/#{project.id}/merge_requests/12345/merge", user) + + expect(response).to have_http_status(404) + end + + it "returns 404 if the merge request id is used instead of iid" do + put api("/projects/#{project.id}/merge_requests/#{merge_request.id}/merge", user) + + expect(response).to have_http_status(404) + end end - describe "PUT /projects/:id/merge_requests/:merge_request_id" do + describe "PUT /projects/:id/merge_requests/:merge_request_iid" do context "to close a MR" do it "returns merge_request" do - put api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user), state_event: "close" + put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user), state_event: "close" expect(response).to have_http_status(200) expect(json_response['state']).to eq('closed') @@ -509,38 +551,38 @@ describe API::MergeRequests, api: true do end it "updates title and returns merge_request" do - put api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user), title: "New title" + put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user), title: "New title" expect(response).to have_http_status(200) expect(json_response['title']).to eq('New title') end it "updates description and returns merge_request" do - put api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user), description: "New description" + put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user), description: "New description" expect(response).to have_http_status(200) expect(json_response['description']).to eq('New description') end it "updates milestone_id and returns merge_request" do - put api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user), milestone_id: milestone.id + put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user), milestone_id: milestone.id expect(response).to have_http_status(200) expect(json_response['milestone']['id']).to eq(milestone.id) end it "returns merge_request with renamed target_branch" do - put api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user), target_branch: "wiki" + put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user), target_branch: "wiki" expect(response).to have_http_status(200) expect(json_response['target_branch']).to eq('wiki') end it "returns merge_request that removes the source branch" do - put api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user), remove_source_branch: true + put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user), remove_source_branch: true expect(response).to have_http_status(200) expect(json_response['force_remove_source_branch']).to be_truthy end it 'allows special label names' do - put api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user), + put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user), title: 'new issue', labels: 'label, label?, label&foo, ?, &' @@ -553,7 +595,7 @@ describe API::MergeRequests, api: true do end it 'does not update state when title is empty' do - put api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user), state_event: 'close', title: nil + put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user), state_event: 'close', title: nil merge_request.reload expect(response).to have_http_status(400) @@ -561,19 +603,31 @@ describe API::MergeRequests, api: true do end it 'does not update state when target_branch is empty' do - put api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user), state_event: 'close', target_branch: nil + put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user), state_event: 'close', target_branch: nil merge_request.reload expect(response).to have_http_status(400) expect(merge_request.state).to eq('opened') end + + it "returns 404 for an invalid merge request IID" do + put api("/projects/#{project.id}/merge_requests/12345", user), state_event: "close" + + expect(response).to have_http_status(404) + end + + it "returns 404 if the merge request id is used instead of iid" do + put api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user), state_event: "close" + + expect(response).to have_http_status(404) + end end - describe "POST /projects/:id/merge_requests/:merge_request_id/comments" do + describe "POST /projects/:id/merge_requests/:merge_request_iid/comments" do it "returns comment" do original_count = merge_request.notes.size - post api("/projects/#{project.id}/merge_requests/#{merge_request.id}/comments", user), note: "My comment" + post api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/comments", user), note: "My comment" expect(response).to have_http_status(201) expect(json_response['note']).to eq('My comment') @@ -583,23 +637,29 @@ describe API::MergeRequests, api: true do end it "returns 400 if note is missing" do - post api("/projects/#{project.id}/merge_requests/#{merge_request.id}/comments", user) + post api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/comments", user) expect(response).to have_http_status(400) end - it "returns 404 if note is attached to non existent merge request" do + it "returns 404 if merge request iid is invalid" do post api("/projects/#{project.id}/merge_requests/404/comments", user), note: 'My comment' expect(response).to have_http_status(404) end + + it "returns 404 if merge request id is used instead of iid" do + post api("/projects/#{project.id}/merge_requests/#{merge_request.id}/comments", user), + note: 'My comment' + expect(response).to have_http_status(404) + end end - describe "GET :id/merge_requests/:merge_request_id/comments" do + describe "GET :id/merge_requests/:merge_request_iid/comments" do let!(:note) { create(:note_on_merge_request, author: user, project: project, noteable: merge_request, note: "a comment on a MR") } let!(:note2) { create(:note_on_merge_request, author: user, project: project, noteable: merge_request, note: "another comment on a MR") } it "returns merge_request comments ordered by created_at" do - get api("/projects/#{project.id}/merge_requests/#{merge_request.id}/comments", user) + get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/comments", user) expect(response).to have_http_status(200) expect(response).to include_pagination_headers @@ -610,20 +670,25 @@ describe API::MergeRequests, api: true do expect(json_response.last['note']).to eq("another comment on a MR") end - it "returns a 404 error if merge_request_id not found" do + it "returns a 404 error if merge_request_iid is invalid" do get api("/projects/#{project.id}/merge_requests/999/comments", user) expect(response).to have_http_status(404) end + + it "returns a 404 error if merge_request id is used instead of iid" do + get api("/projects/#{project.id}/merge_requests/#{merge_request.id}/comments", user) + expect(response).to have_http_status(404) + end end - describe 'GET :id/merge_requests/:merge_request_id/closes_issues' do + describe 'GET :id/merge_requests/:merge_request_iid/closes_issues' do it 'returns the issue that will be closed on merge' do issue = create(:issue, project: project) mr = merge_request.tap do |mr| mr.update_attribute(:description, "Closes #{issue.to_reference(mr.project)}") end - get api("/projects/#{project.id}/merge_requests/#{mr.id}/closes_issues", user) + get api("/projects/#{project.id}/merge_requests/#{mr.iid}/closes_issues", user) expect(response).to have_http_status(200) expect(response).to include_pagination_headers @@ -633,7 +698,7 @@ describe API::MergeRequests, api: true do end it 'returns an empty array when there are no issues to be closed' do - get api("/projects/#{project.id}/merge_requests/#{merge_request.id}/closes_issues", user) + get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/closes_issues", user) expect(response).to have_http_status(200) expect(response).to include_pagination_headers @@ -647,7 +712,7 @@ describe API::MergeRequests, api: true do merge_request = create(:merge_request, :simple, author: user, assignee: user, source_project: jira_project) merge_request.update_attribute(:description, "Closes #{issue.to_reference(jira_project)}") - get api("/projects/#{jira_project.id}/merge_requests/#{merge_request.id}/closes_issues", user) + get api("/projects/#{jira_project.id}/merge_requests/#{merge_request.iid}/closes_issues", user) expect(response).to have_http_status(200) expect(response).to include_pagination_headers @@ -663,22 +728,34 @@ describe API::MergeRequests, api: true do guest = create(:user) project.team << [guest, :guest] - get api("/projects/#{project.id}/merge_requests/#{merge_request.id}/closes_issues", guest) + get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/closes_issues", guest) expect(response).to have_http_status(403) end + + it "returns 404 for an invalid merge request IID" do + get api("/projects/#{project.id}/merge_requests/12345/closes_issues", user) + + expect(response).to have_http_status(404) + end + + it "returns 404 if the merge request id is used instead of iid" do + get api("/projects/#{project.id}/merge_requests/#{merge_request.id}/closes_issues", user) + + expect(response).to have_http_status(404) + end end - describe 'POST :id/merge_requests/:merge_request_id/subscribe' do + describe 'POST :id/merge_requests/:merge_request_iid/subscribe' do it 'subscribes to a merge request' do - post api("/projects/#{project.id}/merge_requests/#{merge_request.id}/subscribe", admin) + post api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/subscribe", admin) expect(response).to have_http_status(201) expect(json_response['subscribed']).to eq(true) end it 'returns 304 if already subscribed' do - post api("/projects/#{project.id}/merge_requests/#{merge_request.id}/subscribe", user) + post api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/subscribe", user) expect(response).to have_http_status(304) end @@ -689,26 +766,32 @@ describe API::MergeRequests, api: true do expect(response).to have_http_status(404) end + it 'returns 404 if the merge request id is used instead of iid' do + post api("/projects/#{project.id}/merge_requests/#{merge_request.id}/subscribe", user) + + expect(response).to have_http_status(404) + end + it 'returns 403 if user has no access to read code' do guest = create(:user) project.team << [guest, :guest] - post api("/projects/#{project.id}/merge_requests/#{merge_request.id}/subscribe", guest) + post api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/subscribe", guest) expect(response).to have_http_status(403) end end - describe 'POST :id/merge_requests/:merge_request_id/unsubscribe' do + describe 'POST :id/merge_requests/:merge_request_iid/unsubscribe' do it 'unsubscribes from a merge request' do - post api("/projects/#{project.id}/merge_requests/#{merge_request.id}/unsubscribe", user) + post api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/unsubscribe", user) expect(response).to have_http_status(201) expect(json_response['subscribed']).to eq(false) end it 'returns 304 if not subscribed' do - post api("/projects/#{project.id}/merge_requests/#{merge_request.id}/unsubscribe", admin) + post api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/unsubscribe", admin) expect(response).to have_http_status(304) end @@ -719,11 +802,17 @@ describe API::MergeRequests, api: true do expect(response).to have_http_status(404) end + it 'returns 404 if the merge request id is used instead of iid' do + post api("/projects/#{project.id}/merge_requests/#{merge_request.id}/unsubscribe", user) + + expect(response).to have_http_status(404) + end + it 'returns 403 if user has no access to read code' do guest = create(:user) project.team << [guest, :guest] - post api("/projects/#{project.id}/merge_requests/#{merge_request.id}/unsubscribe", guest) + post api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/unsubscribe", guest) expect(response).to have_http_status(403) end diff --git a/spec/requests/api/oauth_tokens_spec.rb b/spec/requests/api/oauth_tokens_spec.rb index 7e2cc50e591..367225df717 100644 --- a/spec/requests/api/oauth_tokens_spec.rb +++ b/spec/requests/api/oauth_tokens_spec.rb @@ -29,5 +29,27 @@ describe API::API, api: true do expect(json_response['access_token']).not_to be_nil end end + + context "when user is blocked" do + it "does not create an access token" do + user = create(:user) + user.block + + request_oauth_token(user) + + expect(response).to have_http_status(401) + end + end + + context "when user is ldap_blocked" do + it "does not create an access token" do + user = create(:user) + user.ldap_block + + request_oauth_token(user) + + expect(response).to have_http_status(401) + end + end end end diff --git a/spec/requests/api/runner_spec.rb b/spec/requests/api/runner_spec.rb index e83202e4196..15d458e0795 100644 --- a/spec/requests/api/runner_spec.rb +++ b/spec/requests/api/runner_spec.rb @@ -16,6 +16,7 @@ describe API::Runner do context 'when no token is provided' do it 'returns 400 error' do post api('/runners') + expect(response).to have_http_status 400 end end @@ -23,6 +24,7 @@ describe API::Runner do context 'when invalid token is provided' do it 'returns 403 error' do post api('/runners'), token: 'invalid' + expect(response).to have_http_status 403 end end @@ -108,7 +110,7 @@ describe API::Runner do context "when info parameter '#{param}' info is present" do let(:value) { "#{param}_value" } - it %q(updates provided Runner's parameter) do + it "updates provided Runner's parameter" do post api('/runners'), token: registration_token, info: { param => value } @@ -148,4 +150,874 @@ describe API::Runner do end end end + + describe '/api/v4/jobs' do + let(:project) { create(:empty_project, shared_runners_enabled: false) } + let(:pipeline) { create(:ci_pipeline_without_jobs, project: project, ref: 'master') } + let(:runner) { create(:ci_runner) } + let!(:job) do + create(:ci_build, :artifacts, :extended_options, + pipeline: pipeline, name: 'spinach', stage: 'test', stage_idx: 0, commands: "ls\ndate") + end + + before { project.runners << runner } + + describe 'POST /api/v4/jobs/request' do + let!(:last_update) {} + let!(:new_update) { } + let(:user_agent) { 'gitlab-runner 9.0.0 (9-0-stable; go1.7.4; linux/amd64)' } + + before { stub_container_registry_config(enabled: false) } + + shared_examples 'no jobs available' do + before { request_job } + + context 'when runner sends version in User-Agent' do + context 'for stable version' do + it 'gives 204 and set X-GitLab-Last-Update' do + expect(response).to have_http_status(204) + expect(response.header).to have_key('X-GitLab-Last-Update') + end + end + + context 'when last_update is up-to-date' do + let(:last_update) { runner.ensure_runner_queue_value } + + it 'gives 204 and set the same X-GitLab-Last-Update' do + expect(response).to have_http_status(204) + expect(response.header['X-GitLab-Last-Update']).to eq(last_update) + end + end + + context 'when last_update is outdated' do + let(:last_update) { runner.ensure_runner_queue_value } + let(:new_update) { runner.tick_runner_queue } + + it 'gives 204 and set a new X-GitLab-Last-Update' do + expect(response).to have_http_status(204) + expect(response.header['X-GitLab-Last-Update']).to eq(new_update) + end + end + + context 'when beta version is sent' do + let(:user_agent) { 'gitlab-runner 9.0.0~beta.167.g2b2bacc (master; go1.7.4; linux/amd64)' } + + it { expect(response).to have_http_status(204) } + end + + context 'when pre-9-0 version is sent' do + let(:user_agent) { 'gitlab-ci-multi-runner 1.6.0 (1-6-stable; go1.6.3; linux/amd64)' } + + it { expect(response).to have_http_status(204) } + end + + context 'when pre-9-0 beta version is sent' do + let(:user_agent) { 'gitlab-ci-multi-runner 1.6.0~beta.167.g2b2bacc (master; go1.6.3; linux/amd64)' } + + it { expect(response).to have_http_status(204) } + end + end + + context "when runner doesn't send version in User-Agent" do + let(:user_agent) { 'Go-http-client/1.1' } + + it { expect(response).to have_http_status(404) } + end + + context "when runner doesn't have a User-Agent" do + let(:user_agent) { nil } + + it { expect(response).to have_http_status(404) } + end + end + + context 'when no token is provided' do + it 'returns 400 error' do + post api('/jobs/request') + + expect(response).to have_http_status 400 + end + end + + context 'when invalid token is provided' do + it 'returns 403 error' do + post api('/jobs/request'), token: 'invalid' + + expect(response).to have_http_status 403 + end + end + + context 'when valid token is provided' do + context 'when Runner is not active' do + let(:runner) { create(:ci_runner, :inactive) } + + it 'returns 404 error' do + request_job + + expect(response).to have_http_status 404 + end + end + + context 'when jobs are finished' do + before { job.success } + + it_behaves_like 'no jobs available' + end + + context 'when other projects have pending jobs' do + before do + job.success + create(:ci_build, :pending) + end + + it_behaves_like 'no jobs available' + end + + context 'when shared runner requests job for project without shared_runners_enabled' do + let(:runner) { create(:ci_runner, :shared) } + + it_behaves_like 'no jobs available' + end + + context 'when there is a pending job' do + let(:expected_job_info) do + { 'name' => job.name, + 'stage' => job.stage, + 'project_id' => job.project.id, + 'project_name' => job.project.name } + end + + let(:expected_git_info) do + { 'repo_url' => job.repo_url, + 'ref' => job.ref, + 'sha' => job.sha, + 'before_sha' => job.before_sha, + 'ref_type' => 'branch' } + end + + let(:expected_steps) do + [{ 'name' => 'script', + 'script' => %w(ls date), + 'timeout' => job.timeout, + 'when' => 'on_success', + 'allow_failure' => false }, + { 'name' => 'after_script', + 'script' => %w(ls date), + 'timeout' => job.timeout, + 'when' => 'always', + 'allow_failure' => true }] + end + + let(:expected_variables) do + [{ 'key' => 'CI_BUILD_NAME', 'value' => 'spinach', 'public' => true }, + { 'key' => 'CI_BUILD_STAGE', 'value' => 'test', 'public' => true }, + { 'key' => 'DB_NAME', 'value' => 'postgres', 'public' => true }] + end + + let(:expected_artifacts) do + [{ 'name' => 'artifacts_file', + 'untracked' => false, + 'paths' => %w(out/), + 'when' => 'always', + 'expire_in' => '7d' }] + end + + let(:expected_cache) do + [{ 'key' => 'cache_key', + 'untracked' => false, + 'paths' => ['vendor/*'] }] + end + + it 'picks a job' do + request_job info: { platform: :darwin } + + expect(response).to have_http_status(201) + expect(response.headers).not_to have_key('X-GitLab-Last-Update') + expect(runner.reload.platform).to eq('darwin') + expect(json_response['id']).to eq(job.id) + expect(json_response['token']).to eq(job.token) + expect(json_response['job_info']).to eq(expected_job_info) + expect(json_response['git_info']).to eq(expected_git_info) + expect(json_response['image']).to eq({ 'name' => 'ruby:2.1' }) + expect(json_response['services']).to eq([{ 'name' => 'postgres' }]) + expect(json_response['steps']).to eq(expected_steps) + expect(json_response['artifacts']).to eq(expected_artifacts) + expect(json_response['cache']).to eq(expected_cache) + expect(json_response['variables']).to include(*expected_variables) + end + + context 'when job is made for tag' do + let!(:job) { create(:ci_build_tag, pipeline: pipeline, name: 'spinach', stage: 'test', stage_idx: 0) } + + it 'sets branch as ref_type' do + request_job + + expect(response).to have_http_status(201) + expect(json_response['git_info']['ref_type']).to eq('tag') + end + end + + context 'when job is made for branch' do + it 'sets tag as ref_type' do + request_job + + expect(response).to have_http_status(201) + expect(json_response['git_info']['ref_type']).to eq('branch') + end + end + + it 'updates runner info' do + expect { request_job }.to change { runner.reload.contacted_at } + end + + %w(name version revision platform architecture).each do |param| + context "when info parameter '#{param}' is present" do + let(:value) { "#{param}_value" } + + it "updates provided Runner's parameter" do + request_job info: { param => value } + + expect(response).to have_http_status(201) + expect(runner.reload.read_attribute(param.to_sym)).to eq(value) + end + end + end + + context 'when concurrently updating a job' do + before do + expect_any_instance_of(Ci::Build).to receive(:run!). + and_raise(ActiveRecord::StaleObjectError.new(nil, nil)) + end + + it 'returns a conflict' do + request_job + + expect(response).to have_http_status(409) + expect(response.headers).not_to have_key('X-GitLab-Last-Update') + end + end + + context 'when project and pipeline have multiple jobs' do + let!(:test_job) { create(:ci_build, pipeline: pipeline, name: 'deploy', stage: 'deploy', stage_idx: 1) } + + before { job.success } + + it 'returns dependent jobs' do + request_job + + expect(response).to have_http_status(201) + expect(json_response['id']).to eq(test_job.id) + expect(json_response['dependencies'].count).to eq(1) + expect(json_response['dependencies'][0]).to include('id' => job.id, 'name' => 'spinach') + end + end + + context 'when job has no tags' do + before { job.update(tags: []) } + + context 'when runner is allowed to pick untagged jobs' do + before { runner.update_column(:run_untagged, true) } + + it 'picks job' do + request_job + + expect(response).to have_http_status 201 + end + end + + context 'when runner is not allowed to pick untagged jobs' do + before { runner.update_column(:run_untagged, false) } + + it_behaves_like 'no jobs available' + end + end + + context 'when triggered job is available' do + let(:expected_variables) do + [{ 'key' => 'CI_BUILD_NAME', 'value' => 'spinach', 'public' => true }, + { 'key' => 'CI_BUILD_STAGE', 'value' => 'test', 'public' => true }, + { 'key' => 'CI_BUILD_TRIGGERED', 'value' => 'true', 'public' => true }, + { 'key' => 'DB_NAME', 'value' => 'postgres', 'public' => true }, + { 'key' => 'SECRET_KEY', 'value' => 'secret_value', 'public' => false }, + { 'key' => 'TRIGGER_KEY_1', 'value' => 'TRIGGER_VALUE_1', 'public' => false }] + end + + before do + trigger = create(:ci_trigger, project: project) + create(:ci_trigger_request_with_variables, pipeline: pipeline, builds: [job], trigger: trigger) + project.variables << Ci::Variable.new(key: 'SECRET_KEY', value: 'secret_value') + end + + it 'returns variables for triggers' do + request_job + + expect(response).to have_http_status(201) + expect(json_response['variables']).to include(*expected_variables) + end + end + + describe 'registry credentials support' do + let(:registry_url) { 'registry.example.com:5005' } + let(:registry_credentials) do + { 'type' => 'registry', + 'url' => registry_url, + 'username' => 'gitlab-ci-token', + 'password' => job.token } + end + + context 'when registry is enabled' do + before { stub_container_registry_config(enabled: true, host_port: registry_url) } + + it 'sends registry credentials key' do + request_job + + expect(json_response).to have_key('credentials') + expect(json_response['credentials']).to include(registry_credentials) + end + end + + context 'when registry is disabled' do + before { stub_container_registry_config(enabled: false, host_port: registry_url) } + + it 'does not send registry credentials' do + request_job + + expect(json_response).to have_key('credentials') + expect(json_response['credentials']).not_to include(registry_credentials) + end + end + end + end + + def request_job(token = runner.token, **params) + new_params = params.merge(token: token, last_update: last_update) + post api('/jobs/request'), new_params, { 'User-Agent' => user_agent } + end + end + end + + describe 'PUT /api/v4/jobs/:id' do + let(:job) { create(:ci_build, :pending, :trace, pipeline: pipeline, runner_id: runner.id) } + + before { job.run! } + + context 'when status is given' do + it 'mark job as succeeded' do + update_job(state: 'success') + + expect(job.reload.status).to eq 'success' + end + + it 'mark job as failed' do + update_job(state: 'failed') + + expect(job.reload.status).to eq 'failed' + end + end + + context 'when tace is given' do + it 'updates a running build' do + update_job(trace: 'BUILD TRACE UPDATED') + + expect(response).to have_http_status(200) + expect(job.reload.trace).to eq 'BUILD TRACE UPDATED' + end + end + + context 'when no trace is given' do + it 'does not override trace information' do + update_job + + expect(job.reload.trace).to eq 'BUILD TRACE' + end + end + + context 'when job has been erased' do + let(:job) { create(:ci_build, runner_id: runner.id, erased_at: Time.now) } + + it 'responds with forbidden' do + update_job + + expect(response).to have_http_status(403) + end + end + + def update_job(token = job.token, **params) + new_params = params.merge(token: token) + put api("/jobs/#{job.id}"), new_params + end + end + + describe 'PATCH /api/v4/jobs/:id/trace' do + let(:job) { create(:ci_build, :running, :trace, runner_id: runner.id, pipeline: pipeline) } + let(:headers) { { API::Helpers::Runner::JOB_TOKEN_HEADER => job.token, 'Content-Type' => 'text/plain' } } + let(:headers_with_range) { headers.merge({ 'Content-Range' => '11-20' }) } + let(:update_interval) { 10.seconds.to_i } + + before { initial_patch_the_trace } + + context 'when request is valid' do + it 'gets correct response' do + expect(response.status).to eq 202 + expect(job.reload.trace).to eq 'BUILD TRACE appended' + expect(response.header).to have_key 'Range' + expect(response.header).to have_key 'Job-Status' + end + + context 'when job has been updated recently' do + it { expect{ patch_the_trace }.not_to change { job.updated_at }} + + it "changes the job's trace" do + patch_the_trace + + expect(job.reload.trace).to eq 'BUILD TRACE appended appended' + end + + context 'when Runner makes a force-patch' do + it { expect{ force_patch_the_trace }.not_to change { job.updated_at }} + + it "doesn't change the build.trace" do + force_patch_the_trace + + expect(job.reload.trace).to eq 'BUILD TRACE appended' + end + end + end + + context 'when job was not updated recently' do + let(:update_interval) { 15.minutes.to_i } + + it { expect { patch_the_trace }.to change { job.updated_at } } + + it 'changes the job.trace' do + patch_the_trace + + expect(job.reload.trace).to eq 'BUILD TRACE appended appended' + end + + context 'when Runner makes a force-patch' do + it { expect { force_patch_the_trace }.to change { job.updated_at } } + + it "doesn't change the job.trace" do + force_patch_the_trace + + expect(job.reload.trace).to eq 'BUILD TRACE appended' + end + end + end + + context 'when project for the build has been deleted' do + let(:job) do + create(:ci_build, :running, :trace, runner_id: runner.id, pipeline: pipeline) do |job| + job.project.update(pending_delete: true) + end + end + + it 'responds with forbidden' do + expect(response.status).to eq(403) + end + end + end + + context 'when Runner makes a force-patch' do + before do + force_patch_the_trace + end + + it 'gets correct response' do + expect(response.status).to eq 202 + expect(job.reload.trace).to eq 'BUILD TRACE appended' + expect(response.header).to have_key 'Range' + expect(response.header).to have_key 'Job-Status' + end + end + + context 'when content-range start is too big' do + let(:headers_with_range) { headers.merge({ 'Content-Range' => '15-20' }) } + + it 'gets 416 error response with range headers' do + expect(response.status).to eq 416 + expect(response.header).to have_key 'Range' + expect(response.header['Range']).to eq '0-11' + end + end + + context 'when content-range start is too small' do + let(:headers_with_range) { headers.merge({ 'Content-Range' => '8-20' }) } + + it 'gets 416 error response with range headers' do + expect(response.status).to eq 416 + expect(response.header).to have_key 'Range' + expect(response.header['Range']).to eq '0-11' + end + end + + context 'when Content-Range header is missing' do + let(:headers_with_range) { headers } + + it { expect(response.status).to eq 400 } + end + + context 'when job has been errased' do + let(:job) { create(:ci_build, runner_id: runner.id, erased_at: Time.now) } + + it { expect(response.status).to eq 403 } + end + + def patch_the_trace(content = ' appended', request_headers = nil) + unless request_headers + offset = job.trace_length + limit = offset + content.length - 1 + request_headers = headers.merge({ 'Content-Range' => "#{offset}-#{limit}" }) + end + + Timecop.travel(job.updated_at + update_interval) do + patch api("/jobs/#{job.id}/trace"), content, request_headers + job.reload + end + end + + def initial_patch_the_trace + patch_the_trace(' appended', headers_with_range) + end + + def force_patch_the_trace + 2.times { patch_the_trace('') } + end + end + + describe 'artifacts' do + let(:job) { create(:ci_build, :pending, pipeline: pipeline, runner_id: runner.id) } + let(:jwt_token) { JWT.encode({ 'iss' => 'gitlab-workhorse' }, Gitlab::Workhorse.secret, 'HS256') } + let(:headers) { { 'GitLab-Workhorse' => '1.0', Gitlab::Workhorse::INTERNAL_API_REQUEST_HEADER => jwt_token } } + let(:headers_with_token) { headers.merge(API::Helpers::Runner::JOB_TOKEN_HEADER => job.token) } + let(:file_upload) { fixture_file_upload(Rails.root + 'spec/fixtures/banana_sample.gif', 'image/gif') } + let(:file_upload2) { fixture_file_upload(Rails.root + 'spec/fixtures/dk.png', 'image/gif') } + + before { job.run! } + + describe 'POST /api/v4/jobs/:id/artifacts/authorize' do + context 'when using token as parameter' do + it 'authorizes posting artifacts to running job' do + authorize_artifacts_with_token_in_params + + expect(response).to have_http_status(200) + expect(response.content_type.to_s).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE) + expect(json_response['TempPath']).not_to be_nil + end + + it 'fails to post too large artifact' do + stub_application_setting(max_artifacts_size: 0) + + authorize_artifacts_with_token_in_params(filesize: 100) + + expect(response).to have_http_status(413) + end + end + + context 'when using token as header' do + it 'authorizes posting artifacts to running job' do + authorize_artifacts_with_token_in_headers + + expect(response).to have_http_status(200) + expect(response.content_type.to_s).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE) + expect(json_response['TempPath']).not_to be_nil + end + + it 'fails to post too large artifact' do + stub_application_setting(max_artifacts_size: 0) + + authorize_artifacts_with_token_in_headers(filesize: 100) + + expect(response).to have_http_status(413) + end + end + + context 'when using runners token' do + it 'fails to authorize artifacts posting' do + authorize_artifacts(token: job.project.runners_token) + + expect(response).to have_http_status(403) + end + end + + it 'reject requests that did not go through gitlab-workhorse' do + headers.delete(Gitlab::Workhorse::INTERNAL_API_REQUEST_HEADER) + + authorize_artifacts + + expect(response).to have_http_status(500) + end + + context 'authorization token is invalid' do + it 'responds with forbidden' do + authorize_artifacts(token: 'invalid', filesize: 100 ) + + expect(response).to have_http_status(403) + end + end + + def authorize_artifacts(params = {}, request_headers = headers) + post api("/jobs/#{job.id}/artifacts/authorize"), params, request_headers + end + + def authorize_artifacts_with_token_in_params(params = {}, request_headers = headers) + params = params.merge(token: job.token) + authorize_artifacts(params, request_headers) + end + + def authorize_artifacts_with_token_in_headers(params = {}, request_headers = headers_with_token) + authorize_artifacts(params, request_headers) + end + end + + describe 'POST /api/v4/jobs/:id/artifacts' do + context 'when artifacts are being stored inside of tmp path' do + before do + # by configuring this path we allow to pass temp file from any path + allow(ArtifactUploader).to receive(:artifacts_upload_path).and_return('/') + end + + context 'when job has been erased' do + let(:job) { create(:ci_build, erased_at: Time.now) } + + before do + upload_artifacts(file_upload, headers_with_token) + end + + it 'responds with forbidden' do + upload_artifacts(file_upload, headers_with_token) + + expect(response).to have_http_status(403) + end + end + + context 'when job is running' do + shared_examples 'successful artifacts upload' do + it 'updates successfully' do + expect(response).to have_http_status(201) + end + end + + context 'when uses regular file post' do + before { upload_artifacts(file_upload, headers_with_token, false) } + + it_behaves_like 'successful artifacts upload' + end + + context 'when uses accelerated file post' do + before { upload_artifacts(file_upload, headers_with_token, true) } + + it_behaves_like 'successful artifacts upload' + end + + context 'when updates artifact' do + before do + upload_artifacts(file_upload2, headers_with_token) + upload_artifacts(file_upload, headers_with_token) + end + + it_behaves_like 'successful artifacts upload' + end + + context 'when using runners token' do + it 'responds with forbidden' do + upload_artifacts(file_upload, headers.merge(API::Helpers::Runner::JOB_TOKEN_HEADER => job.project.runners_token)) + + expect(response).to have_http_status(403) + end + end + end + + context 'when artifacts file is too large' do + it 'fails to post too large artifact' do + stub_application_setting(max_artifacts_size: 0) + + upload_artifacts(file_upload, headers_with_token) + + expect(response).to have_http_status(413) + end + end + + context 'when artifacts post request does not contain file' do + it 'fails to post artifacts without file' do + post api("/jobs/#{job.id}/artifacts"), {}, headers_with_token + + expect(response).to have_http_status(400) + end + end + + context 'GitLab Workhorse is not configured' do + it 'fails to post artifacts without GitLab-Workhorse' do + post api("/jobs/#{job.id}/artifacts"), { token: job.token }, {} + + expect(response).to have_http_status(403) + end + end + + context 'when setting an expire date' do + let(:default_artifacts_expire_in) {} + let(:post_data) do + { 'file.path' => file_upload.path, + 'file.name' => file_upload.original_filename, + 'expire_in' => expire_in } + end + + before do + stub_application_setting(default_artifacts_expire_in: default_artifacts_expire_in) + + post(api("/jobs/#{job.id}/artifacts"), post_data, headers_with_token) + end + + context 'when an expire_in is given' do + let(:expire_in) { '7 days' } + + it 'updates when specified' do + expect(response).to have_http_status(201) + expect(job.reload.artifacts_expire_at).to be_within(5.minutes).of(7.days.from_now) + end + end + + context 'when no expire_in is given' do + let(:expire_in) { nil } + + it 'ignores if not specified' do + expect(response).to have_http_status(201) + expect(job.reload.artifacts_expire_at).to be_nil + end + + context 'with application default' do + context 'when default is 5 days' do + let(:default_artifacts_expire_in) { '5 days' } + + it 'sets to application default' do + expect(response).to have_http_status(201) + expect(job.reload.artifacts_expire_at).to be_within(5.minutes).of(5.days.from_now) + end + end + + context 'when default is 0' do + let(:default_artifacts_expire_in) { '0' } + + it 'does not set expire_in' do + expect(response).to have_http_status(201) + expect(job.reload.artifacts_expire_at).to be_nil + end + end + end + end + end + + context 'posts artifacts file and metadata file' do + let!(:artifacts) { file_upload } + let!(:metadata) { file_upload2 } + + let(:stored_artifacts_file) { job.reload.artifacts_file.file } + let(:stored_metadata_file) { job.reload.artifacts_metadata.file } + let(:stored_artifacts_size) { job.reload.artifacts_size } + + before do + post(api("/jobs/#{job.id}/artifacts"), post_data, headers_with_token) + end + + context 'when posts data accelerated by workhorse is correct' do + let(:post_data) do + { 'file.path' => artifacts.path, + 'file.name' => artifacts.original_filename, + 'metadata.path' => metadata.path, + 'metadata.name' => metadata.original_filename } + end + + it 'stores artifacts and artifacts metadata' do + expect(response).to have_http_status(201) + expect(stored_artifacts_file.original_filename).to eq(artifacts.original_filename) + expect(stored_metadata_file.original_filename).to eq(metadata.original_filename) + expect(stored_artifacts_size).to eq(71759) + end + end + + context 'when there is no artifacts file in post data' do + let(:post_data) do + { 'metadata' => metadata } + end + + it 'is expected to respond with bad request' do + expect(response).to have_http_status(400) + end + + it 'does not store metadata' do + expect(stored_metadata_file).to be_nil + end + end + end + end + + context 'when artifacts are being stored outside of tmp path' do + before do + # by configuring this path we allow to pass file from @tmpdir only + # but all temporary files are stored in system tmp directory + @tmpdir = Dir.mktmpdir + allow(ArtifactUploader).to receive(:artifacts_upload_path).and_return(@tmpdir) + end + + after { FileUtils.remove_entry @tmpdir } + + it' "fails to post artifacts for outside of tmp path"' do + upload_artifacts(file_upload, headers_with_token) + + expect(response).to have_http_status(400) + end + end + + def upload_artifacts(file, headers = {}, accelerated = true) + params = if accelerated + { 'file.path' => file.path, 'file.name' => file.original_filename } + else + { 'file' => file } + end + post api("/jobs/#{job.id}/artifacts"), params, headers + end + end + + describe 'GET /api/v4/jobs/:id/artifacts' do + let(:token) { job.token } + + before { download_artifact } + + context 'when job has artifacts' do + let(:job) { create(:ci_build, :artifacts) } + let(:download_headers) do + { 'Content-Transfer-Encoding' => 'binary', + 'Content-Disposition' => 'attachment; filename=ci_build_artifacts.zip' } + end + + context 'when using job token' do + it 'download artifacts' do + expect(response).to have_http_status(200) + expect(response.headers).to include download_headers + end + end + + context 'when using runnners token' do + let(:token) { job.project.runners_token } + + it 'responds with forbidden' do + expect(response).to have_http_status(403) + end + end + end + + context 'when job does not has artifacts' do + it 'responds with not found' do + expect(response).to have_http_status(404) + end + end + + def download_artifact(params = {}, request_headers = headers) + params = params.merge(token: token) + get api("/jobs/#{job.id}/artifacts"), params, request_headers + end + end + end + end end diff --git a/spec/requests/api/session_spec.rb b/spec/requests/api/session_spec.rb index 794e2b5c04d..28fab2011a5 100644 --- a/spec/requests/api/session_spec.rb +++ b/spec/requests/api/session_spec.rb @@ -87,5 +87,23 @@ describe API::Session, api: true do expect(response).to have_http_status(400) end end + + context "when user is blocked" do + it "returns authentication error" do + user.block + post api("/session"), email: user.username, password: user.password + + expect(response).to have_http_status(401) + end + end + + context "when user is ldap_blocked" do + it "returns authentication error" do + user.ldap_block + post api("/session"), email: user.username, password: user.password + + expect(response).to have_http_status(401) + end + end end end diff --git a/spec/requests/api/todos_spec.rb b/spec/requests/api/todos_spec.rb index 1e401935662..b789284fa8d 100644 --- a/spec/requests/api/todos_spec.rb +++ b/spec/requests/api/todos_spec.rb @@ -163,7 +163,7 @@ describe API::Todos, api: true do shared_examples 'an issuable' do |issuable_type| it 'creates a todo on an issuable' do - post api("/projects/#{project_1.id}/#{issuable_type}/#{issuable.id}/todo", john_doe) + post api("/projects/#{project_1.id}/#{issuable_type}/#{issuable.iid}/todo", john_doe) expect(response.status).to eq(201) expect(json_response['project']).to be_a Hash @@ -180,7 +180,7 @@ describe API::Todos, api: true do it 'returns 304 there already exist a todo on that issuable' do create(:todo, project: project_1, author: author_1, user: john_doe, target: issuable) - post api("/projects/#{project_1.id}/#{issuable_type}/#{issuable.id}/todo", john_doe) + post api("/projects/#{project_1.id}/#{issuable_type}/#{issuable.iid}/todo", john_doe) expect(response.status).to eq(304) end @@ -195,7 +195,7 @@ describe API::Todos, api: true do guest = create(:user) project_1.team << [guest, :guest] - post api("/projects/#{project_1.id}/#{issuable_type}/#{issuable.id}/todo", guest) + post api("/projects/#{project_1.id}/#{issuable_type}/#{issuable.iid}/todo", guest) if issuable_type == 'merge_requests' expect(response).to have_http_status(403) diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index 881c48c75e0..04e7837fd7a 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -10,6 +10,8 @@ describe API::Users, api: true do let(:omniauth_user) { create(:omniauth_user) } let(:ldap_user) { create(:omniauth_user, provider: 'ldapmain') } let(:ldap_blocked_user) { create(:omniauth_user, provider: 'ldapmain', state: 'ldap_blocked') } + let(:not_existing_user_id) { (User.maximum('id') || 0 ) + 10 } + let(:not_existing_pat_id) { (PersonalAccessToken.maximum('id') || 0 ) + 10 } describe "GET /users" do context "when unauthenticated" do @@ -1155,4 +1157,187 @@ describe API::Users, api: true do expect(json_response['message']).to eq('404 User Not Found') end end + + describe 'GET /users/:user_id/impersonation_tokens' do + let!(:active_personal_access_token) { create(:personal_access_token, user: user) } + let!(:revoked_personal_access_token) { create(:personal_access_token, :revoked, user: user) } + let!(:expired_personal_access_token) { create(:personal_access_token, :expired, user: user) } + let!(:impersonation_token) { create(:personal_access_token, :impersonation, user: user) } + let!(:revoked_impersonation_token) { create(:personal_access_token, :impersonation, :revoked, user: user) } + + it 'returns a 404 error if user not found' do + get api("/users/#{not_existing_user_id}/impersonation_tokens", admin) + + expect(response).to have_http_status(404) + expect(json_response['message']).to eq('404 User Not Found') + end + + it 'returns a 403 error when authenticated as normal user' do + get api("/users/#{not_existing_user_id}/impersonation_tokens", user) + + expect(response).to have_http_status(403) + expect(json_response['message']).to eq('403 Forbidden') + end + + it 'returns an array of all impersonated tokens' do + get api("/users/#{user.id}/impersonation_tokens", admin) + + expect(response).to have_http_status(200) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + expect(json_response.size).to eq(2) + end + + it 'returns an array of active impersonation tokens if state active' do + get api("/users/#{user.id}/impersonation_tokens?state=active", admin) + + expect(response).to have_http_status(200) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + expect(json_response.size).to eq(1) + expect(json_response).to all(include('active' => true)) + end + + it 'returns an array of inactive personal access tokens if active is set to false' do + get api("/users/#{user.id}/impersonation_tokens?state=inactive", admin) + + expect(response).to have_http_status(200) + expect(json_response).to be_an Array + expect(json_response.size).to eq(1) + expect(json_response).to all(include('active' => false)) + end + end + + describe 'POST /users/:user_id/impersonation_tokens' do + let(:name) { 'my new pat' } + let(:expires_at) { '2016-12-28' } + let(:scopes) { %w(api read_user) } + let(:impersonation) { true } + + it 'returns validation error if impersonation token misses some attributes' do + post api("/users/#{user.id}/impersonation_tokens", admin) + + expect(response).to have_http_status(400) + expect(json_response['error']).to eq('name is missing') + end + + it 'returns a 404 error if user not found' do + post api("/users/#{not_existing_user_id}/impersonation_tokens", admin), + name: name, + expires_at: expires_at + + expect(response).to have_http_status(404) + expect(json_response['message']).to eq('404 User Not Found') + end + + it 'returns a 403 error when authenticated as normal user' do + post api("/users/#{user.id}/impersonation_tokens", user), + name: name, + expires_at: expires_at + + expect(response).to have_http_status(403) + expect(json_response['message']).to eq('403 Forbidden') + end + + it 'creates a impersonation token' do + post api("/users/#{user.id}/impersonation_tokens", admin), + name: name, + expires_at: expires_at, + scopes: scopes, + impersonation: impersonation + + expect(response).to have_http_status(201) + expect(json_response['name']).to eq(name) + expect(json_response['scopes']).to eq(scopes) + expect(json_response['expires_at']).to eq(expires_at) + expect(json_response['id']).to be_present + expect(json_response['created_at']).to be_present + expect(json_response['active']).to be_falsey + expect(json_response['revoked']).to be_falsey + expect(json_response['token']).to be_present + expect(json_response['impersonation']).to eq(impersonation) + end + end + + describe 'GET /users/:user_id/impersonation_tokens/:impersonation_token_id' do + let!(:personal_access_token) { create(:personal_access_token, user: user) } + let!(:impersonation_token) { create(:personal_access_token, :impersonation, user: user) } + + it 'returns 404 error if user not found' do + get api("/users/#{not_existing_user_id}/impersonation_tokens/1", admin) + + expect(response).to have_http_status(404) + expect(json_response['message']).to eq('404 User Not Found') + end + + it 'returns a 404 error if impersonation token not found' do + get api("/users/#{user.id}/impersonation_tokens/#{not_existing_pat_id}", admin) + + expect(response).to have_http_status(404) + expect(json_response['message']).to eq('404 Impersonation Token Not Found') + end + + it 'returns a 404 error if token is not impersonation token' do + get api("/users/#{user.id}/impersonation_tokens/#{personal_access_token.id}", admin) + + expect(response).to have_http_status(404) + expect(json_response['message']).to eq('404 Impersonation Token Not Found') + end + + it 'returns a 403 error when authenticated as normal user' do + get api("/users/#{user.id}/impersonation_tokens/#{impersonation_token.id}", user) + + expect(response).to have_http_status(403) + expect(json_response['message']).to eq('403 Forbidden') + end + + it 'returns a personal access token' do + get api("/users/#{user.id}/impersonation_tokens/#{impersonation_token.id}", admin) + + expect(response).to have_http_status(200) + expect(json_response['token']).to be_present + expect(json_response['impersonation']).to be_truthy + end + end + + describe 'DELETE /users/:user_id/impersonation_tokens/:impersonation_token_id' do + let!(:personal_access_token) { create(:personal_access_token, user: user) } + let!(:impersonation_token) { create(:personal_access_token, :impersonation, user: user) } + + it 'returns a 404 error if user not found' do + delete api("/users/#{not_existing_user_id}/impersonation_tokens/1", admin) + + expect(response).to have_http_status(404) + expect(json_response['message']).to eq('404 User Not Found') + end + + it 'returns a 404 error if impersonation token not found' do + delete api("/users/#{user.id}/impersonation_tokens/#{not_existing_pat_id}", admin) + + expect(response).to have_http_status(404) + expect(json_response['message']).to eq('404 Impersonation Token Not Found') + end + + it 'returns a 404 error if token is not impersonation token' do + delete api("/users/#{user.id}/impersonation_tokens/#{personal_access_token.id}", admin) + + expect(response).to have_http_status(404) + expect(json_response['message']).to eq('404 Impersonation Token Not Found') + end + + it 'returns a 403 error when authenticated as normal user' do + delete api("/users/#{user.id}/impersonation_tokens/#{impersonation_token.id}", user) + + expect(response).to have_http_status(403) + expect(json_response['message']).to eq('403 Forbidden') + end + + it 'revokes a impersonation token' do + delete api("/users/#{user.id}/impersonation_tokens/#{impersonation_token.id}", admin) + + expect(response).to have_http_status(204) + expect(impersonation_token.revoked).to be_falsey + expect(impersonation_token.reload.revoked).to be_truthy + end + end end diff --git a/spec/requests/api/v3/award_emoji_spec.rb b/spec/requests/api/v3/award_emoji_spec.rb index 91145c8e72c..eeb4d128c1b 100644 --- a/spec/requests/api/v3/award_emoji_spec.rb +++ b/spec/requests/api/v3/award_emoji_spec.rb @@ -13,6 +13,231 @@ describe API::V3::AwardEmoji, api: true do before { project.team << [user, :master] } + describe "GET /projects/:id/awardable/:awardable_id/award_emoji" do + context 'on an issue' do + it "returns an array of award_emoji" do + get v3_api("/projects/#{project.id}/issues/#{issue.id}/award_emoji", user) + + expect(response).to have_http_status(200) + expect(json_response).to be_an Array + expect(json_response.first['name']).to eq(award_emoji.name) + end + + it "returns a 404 error when issue id not found" do + get v3_api("/projects/#{project.id}/issues/12345/award_emoji", user) + + expect(response).to have_http_status(404) + end + end + + context 'on a merge request' do + it "returns an array of award_emoji" do + get v3_api("/projects/#{project.id}/merge_requests/#{merge_request.id}/award_emoji", user) + + expect(response).to have_http_status(200) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + expect(json_response.first['name']).to eq(downvote.name) + end + end + + context 'on a snippet' do + let(:snippet) { create(:project_snippet, :public, project: project) } + let!(:award) { create(:award_emoji, awardable: snippet) } + + it 'returns the awarded emoji' do + get v3_api("/projects/#{project.id}/snippets/#{snippet.id}/award_emoji", user) + + expect(response).to have_http_status(200) + expect(json_response).to be_an Array + expect(json_response.first['name']).to eq(award.name) + end + end + + context 'when the user has no access' do + it 'returns a status code 404' do + user1 = create(:user) + + get v3_api("/projects/#{project.id}/merge_requests/#{merge_request.id}/award_emoji", user1) + + expect(response).to have_http_status(404) + end + end + end + + describe 'GET /projects/:id/awardable/:awardable_id/notes/:note_id/award_emoji' do + let!(:rocket) { create(:award_emoji, awardable: note, name: 'rocket') } + + it 'returns an array of award emoji' do + get v3_api("/projects/#{project.id}/issues/#{issue.id}/notes/#{note.id}/award_emoji", user) + + expect(response).to have_http_status(200) + expect(json_response).to be_an Array + expect(json_response.first['name']).to eq(rocket.name) + end + end + + describe "GET /projects/:id/awardable/:awardable_id/award_emoji/:award_id" do + context 'on an issue' do + it "returns the award emoji" do + get v3_api("/projects/#{project.id}/issues/#{issue.id}/award_emoji/#{award_emoji.id}", user) + + expect(response).to have_http_status(200) + expect(json_response['name']).to eq(award_emoji.name) + expect(json_response['awardable_id']).to eq(issue.id) + expect(json_response['awardable_type']).to eq("Issue") + end + + it "returns a 404 error if the award is not found" do + get v3_api("/projects/#{project.id}/issues/#{issue.id}/award_emoji/12345", user) + + expect(response).to have_http_status(404) + end + end + + context 'on a merge request' do + it 'returns the award emoji' do + get v3_api("/projects/#{project.id}/merge_requests/#{merge_request.id}/award_emoji/#{downvote.id}", user) + + expect(response).to have_http_status(200) + expect(json_response['name']).to eq(downvote.name) + expect(json_response['awardable_id']).to eq(merge_request.id) + expect(json_response['awardable_type']).to eq("MergeRequest") + end + end + + context 'on a snippet' do + let(:snippet) { create(:project_snippet, :public, project: project) } + let!(:award) { create(:award_emoji, awardable: snippet) } + + it 'returns the awarded emoji' do + get v3_api("/projects/#{project.id}/snippets/#{snippet.id}/award_emoji/#{award.id}", user) + + expect(response).to have_http_status(200) + expect(json_response['name']).to eq(award.name) + expect(json_response['awardable_id']).to eq(snippet.id) + expect(json_response['awardable_type']).to eq("Snippet") + end + end + + context 'when the user has no access' do + it 'returns a status code 404' do + user1 = create(:user) + + get v3_api("/projects/#{project.id}/merge_requests/#{merge_request.id}/award_emoji/#{downvote.id}", user1) + + expect(response).to have_http_status(404) + end + end + end + + describe 'GET /projects/:id/awardable/:awardable_id/notes/:note_id/award_emoji/:award_id' do + let!(:rocket) { create(:award_emoji, awardable: note, name: 'rocket') } + + it 'returns an award emoji' do + get v3_api("/projects/#{project.id}/issues/#{issue.id}/notes/#{note.id}/award_emoji/#{rocket.id}", user) + + expect(response).to have_http_status(200) + expect(json_response).not_to be_an Array + expect(json_response['name']).to eq(rocket.name) + end + end + + describe "POST /projects/:id/awardable/:awardable_id/award_emoji" do + let(:issue2) { create(:issue, project: project, author: user) } + + context "on an issue" do + it "creates a new award emoji" do + post v3_api("/projects/#{project.id}/issues/#{issue.id}/award_emoji", user), name: 'blowfish' + + expect(response).to have_http_status(201) + expect(json_response['name']).to eq('blowfish') + expect(json_response['user']['username']).to eq(user.username) + end + + it "returns a 400 bad request error if the name is not given" do + post v3_api("/projects/#{project.id}/issues/#{issue.id}/award_emoji", user) + + expect(response).to have_http_status(400) + end + + it "returns a 401 unauthorized error if the user is not authenticated" do + post v3_api("/projects/#{project.id}/issues/#{issue.id}/award_emoji"), name: 'thumbsup' + + expect(response).to have_http_status(401) + end + + it "returns a 404 error if the user authored issue" do + post v3_api("/projects/#{project.id}/issues/#{issue2.id}/award_emoji", user), name: 'thumbsup' + + expect(response).to have_http_status(404) + end + + it "normalizes +1 as thumbsup award" do + post v3_api("/projects/#{project.id}/issues/#{issue.id}/award_emoji", user), name: '+1' + + expect(issue.award_emoji.last.name).to eq("thumbsup") + end + + context 'when the emoji already has been awarded' do + it 'returns a 404 status code' do + post v3_api("/projects/#{project.id}/issues/#{issue.id}/award_emoji", user), name: 'thumbsup' + post v3_api("/projects/#{project.id}/issues/#{issue.id}/award_emoji", user), name: 'thumbsup' + + expect(response).to have_http_status(404) + expect(json_response["message"]).to match("has already been taken") + end + end + end + + context 'on a snippet' do + it 'creates a new award emoji' do + snippet = create(:project_snippet, :public, project: project) + + post v3_api("/projects/#{project.id}/snippets/#{snippet.id}/award_emoji", user), name: 'blowfish' + + expect(response).to have_http_status(201) + expect(json_response['name']).to eq('blowfish') + expect(json_response['user']['username']).to eq(user.username) + end + end + end + + describe "POST /projects/:id/awardable/:awardable_id/notes/:note_id/award_emoji" do + let(:note2) { create(:note, project: project, noteable: issue, author: user) } + + it 'creates a new award emoji' do + expect do + post v3_api("/projects/#{project.id}/issues/#{issue.id}/notes/#{note.id}/award_emoji", user), name: 'rocket' + end.to change { note.award_emoji.count }.from(0).to(1) + + expect(response).to have_http_status(201) + expect(json_response['user']['username']).to eq(user.username) + end + + it "it returns 404 error when user authored note" do + post v3_api("/projects/#{project.id}/issues/#{issue.id}/notes/#{note2.id}/award_emoji", user), name: 'thumbsup' + + expect(response).to have_http_status(404) + end + + it "normalizes +1 as thumbsup award" do + post v3_api("/projects/#{project.id}/issues/#{issue.id}/notes/#{note.id}/award_emoji", user), name: '+1' + + expect(note.award_emoji.last.name).to eq("thumbsup") + end + + context 'when the emoji already has been awarded' do + it 'returns a 404 status code' do + post v3_api("/projects/#{project.id}/issues/#{issue.id}/notes/#{note.id}/award_emoji", user), name: 'rocket' + post v3_api("/projects/#{project.id}/issues/#{issue.id}/notes/#{note.id}/award_emoji", user), name: 'rocket' + + expect(response).to have_http_status(404) + expect(json_response["message"]).to match("has already been taken") + end + end + end + describe 'DELETE /projects/:id/awardable/:awardable_id/award_emoji/:award_id' do context 'when the awardable is an Issue' do it 'deletes the award' do diff --git a/spec/requests/api/v3/issues_spec.rb b/spec/requests/api/v3/issues_spec.rb index 2a8105d5a2b..1941ca0d7d8 100644 --- a/spec/requests/api/v3/issues_spec.rb +++ b/spec/requests/api/v3/issues_spec.rb @@ -1288,6 +1288,6 @@ describe API::V3::Issues, api: true do describe 'time tracking endpoints' do let(:issuable) { issue } - include_examples 'time tracking endpoints', 'issue' + include_examples 'V3 time tracking endpoints', 'issue' end end diff --git a/spec/requests/api/v3/merge_request_diffs_spec.rb b/spec/requests/api/v3/merge_request_diffs_spec.rb index e1887138aab..c53800eef30 100644 --- a/spec/requests/api/v3/merge_request_diffs_spec.rb +++ b/spec/requests/api/v3/merge_request_diffs_spec.rb @@ -1,6 +1,6 @@ require "spec_helper" -describe API::MergeRequestDiffs, 'MergeRequestDiffs', api: true do +describe API::V3::MergeRequestDiffs, 'MergeRequestDiffs', api: true do include ApiHelpers let!(:user) { create(:user) } @@ -15,7 +15,7 @@ describe API::MergeRequestDiffs, 'MergeRequestDiffs', api: true do describe 'GET /projects/:id/merge_requests/:merge_request_id/versions' do it 'returns 200 for a valid merge request' do - get api("/projects/#{project.id}/merge_requests/#{merge_request.id}/versions", user) + get v3_api("/projects/#{project.id}/merge_requests/#{merge_request.id}/versions", user) merge_request_diff = merge_request.merge_request_diffs.first expect(response.status).to eq 200 @@ -25,7 +25,7 @@ describe API::MergeRequestDiffs, 'MergeRequestDiffs', api: true do end it 'returns a 404 when merge_request_id not found' do - get api("/projects/#{project.id}/merge_requests/999/versions", user) + get v3_api("/projects/#{project.id}/merge_requests/999/versions", user) expect(response).to have_http_status(404) end end @@ -33,7 +33,7 @@ describe API::MergeRequestDiffs, 'MergeRequestDiffs', api: true do describe 'GET /projects/:id/merge_requests/:merge_request_id/versions/:version_id' do it 'returns a 200 for a valid merge request' do merge_request_diff = merge_request.merge_request_diffs.first - get api("/projects/#{project.id}/merge_requests/#{merge_request.id}/versions/#{merge_request_diff.id}", user) + get v3_api("/projects/#{project.id}/merge_requests/#{merge_request.id}/versions/#{merge_request_diff.id}", user) expect(response.status).to eq 200 expect(json_response['id']).to eq(merge_request_diff.id) @@ -42,7 +42,8 @@ describe API::MergeRequestDiffs, 'MergeRequestDiffs', api: true do end it 'returns a 404 when merge_request_id not found' do - get api("/projects/#{project.id}/merge_requests/#{merge_request.id}/versions/999", user) + get v3_api("/projects/#{project.id}/merge_requests/#{merge_request.id}/versions/999", user) + expect(response).to have_http_status(404) end end diff --git a/spec/requests/api/v3/merge_requests_spec.rb b/spec/requests/api/v3/merge_requests_spec.rb index b7ed643bc21..d73e9635c9b 100644 --- a/spec/requests/api/v3/merge_requests_spec.rb +++ b/spec/requests/api/v3/merge_requests_spec.rb @@ -712,7 +712,7 @@ describe API::MergeRequests, api: true do describe 'Time tracking' do let(:issuable) { merge_request } - include_examples 'time tracking endpoints', 'merge_request' + include_examples 'V3 time tracking endpoints', 'merge_request' end def mr_with_later_created_and_updated_at_time diff --git a/spec/requests/api/v3/services_spec.rb b/spec/requests/api/v3/services_spec.rb index 7e8c8753d02..3a760a8f25c 100644 --- a/spec/requests/api/v3/services_spec.rb +++ b/spec/requests/api/v3/services_spec.rb @@ -6,7 +6,9 @@ describe API::V3::Services, api: true do let(:user) { create(:user) } let(:project) { create(:empty_project, creator_id: user.id, namespace: user.namespace) } - Service.available_services_names.each do |service| + available_services = Service.available_services_names + available_services.delete('prometheus') + available_services.each do |service| describe "DELETE /projects/:id/services/#{service.dasherize}" do include_context service diff --git a/spec/requests/git_http_spec.rb b/spec/requests/git_http_spec.rb index 87786e85621..006d6a6af1c 100644 --- a/spec/requests/git_http_spec.rb +++ b/spec/requests/git_http_spec.rb @@ -221,12 +221,20 @@ describe 'Git HTTP requests', lib: true do end context "when the user is blocked" do - it "responds with status 404" do + it "responds with status 401" do user.block project.team << [user, :master] download(path, env) do |response| - expect(response).to have_http_status(404) + expect(response).to have_http_status(401) + end + end + + it "responds with status 401 for unknown projects (no project existence information leak)" do + user.block + + download('doesnt/exist.git', env) do |response| + expect(response).to have_http_status(401) end end end diff --git a/spec/requests/openid_connect_spec.rb b/spec/requests/openid_connect_spec.rb new file mode 100644 index 00000000000..5206634bca5 --- /dev/null +++ b/spec/requests/openid_connect_spec.rb @@ -0,0 +1,134 @@ +require 'spec_helper' + +describe 'OpenID Connect requests' do + include ApiHelpers + + let(:user) { create :user } + let(:access_grant) { create :oauth_access_grant, application: application, resource_owner_id: user.id } + let(:access_token) { create :oauth_access_token, application: application, resource_owner_id: user.id } + + def request_access_token + login_as user + + post '/oauth/token', + grant_type: 'authorization_code', + code: access_grant.token, + redirect_uri: application.redirect_uri, + client_id: application.uid, + client_secret: application.secret + end + + def request_user_info + get '/oauth/userinfo', nil, 'Authorization' => "Bearer #{access_token.token}" + end + + def hashed_subject + Digest::SHA256.hexdigest("#{user.id}-#{Rails.application.secrets.secret_key_base}") + end + + context 'Application without OpenID scope' do + let(:application) { create :oauth_application, scopes: 'api' } + + it 'token response does not include an ID token' do + request_access_token + + expect(json_response).to include 'access_token' + expect(json_response).not_to include 'id_token' + end + + it 'userinfo response is unauthorized' do + request_user_info + + expect(response).to have_http_status 403 + expect(response.body).to be_blank + end + end + + context 'Application with OpenID scope' do + let(:application) { create :oauth_application, scopes: 'openid' } + + it 'token response includes an ID token' do + request_access_token + + expect(json_response).to include 'id_token' + end + + context 'UserInfo payload' do + let(:user) do + create( + :user, + name: 'Alice', + username: 'alice', + emails: [private_email, public_email], + email: private_email.email, + public_email: public_email.email, + website_url: 'https://example.com', + avatar: fixture_file_upload(Rails.root + "spec/fixtures/dk.png"), + ) + end + + let(:public_email) { build :email, email: 'public@example.com' } + let(:private_email) { build :email, email: 'private@example.com' } + + it 'includes all user information' do + request_user_info + + expect(json_response).to eq({ + 'sub' => hashed_subject, + 'name' => 'Alice', + 'nickname' => 'alice', + 'email' => 'public@example.com', + 'email_verified' => true, + 'website' => 'https://example.com', + 'profile' => 'http://localhost/alice', + 'picture' => "http://localhost/uploads/user/avatar/#{user.id}/dk.png", + }) + end + end + + context 'ID token payload' do + before do + request_access_token + @payload = JSON::JWT.decode(json_response['id_token'], :skip_verification) + end + + it 'includes the Gitlab root URL' do + expect(@payload['iss']).to eq Gitlab.config.gitlab.url + end + + it 'includes the hashed user ID' do + expect(@payload['sub']).to eq hashed_subject + end + + it 'includes the time of the last authentication' do + expect(@payload['auth_time']).to eq user.current_sign_in_at.to_i + end + + it 'does not include any unknown properties' do + expect(@payload.keys).to eq %w[iss sub aud exp iat auth_time] + end + end + + context 'when user is blocked' do + it 'returns authentication error' do + access_grant + user.block + + expect do + request_access_token + end.to throw_symbol :warden + end + end + + context 'when user is ldap_blocked' do + it 'returns authentication error' do + access_grant + user.ldap_block + + expect do + request_access_token + end.to throw_symbol :warden + end + end + end +end diff --git a/spec/routing/openid_connect_spec.rb b/spec/routing/openid_connect_spec.rb new file mode 100644 index 00000000000..2c3bc08f1a1 --- /dev/null +++ b/spec/routing/openid_connect_spec.rb @@ -0,0 +1,30 @@ +require 'spec_helper' + +# oauth_discovery_keys GET /oauth/discovery/keys(.:format) doorkeeper/openid_connect/discovery#keys +# oauth_discovery_provider GET /.well-known/openid-configuration(.:format) doorkeeper/openid_connect/discovery#provider +# oauth_discovery_webfinger GET /.well-known/webfinger(.:format) doorkeeper/openid_connect/discovery#webfinger +describe Doorkeeper::OpenidConnect::DiscoveryController, 'routing' do + it "to #provider" do + expect(get('/.well-known/openid-configuration')).to route_to('doorkeeper/openid_connect/discovery#provider') + end + + it "to #webfinger" do + expect(get('/.well-known/webfinger')).to route_to('doorkeeper/openid_connect/discovery#webfinger') + end + + it "to #keys" do + expect(get('/oauth/discovery/keys')).to route_to('doorkeeper/openid_connect/discovery#keys') + end +end + +# oauth_userinfo GET /oauth/userinfo(.:format) doorkeeper/openid_connect/userinfo#show +# POST /oauth/userinfo(.:format) doorkeeper/openid_connect/userinfo#show +describe Doorkeeper::OpenidConnect::UserinfoController, 'routing' do + it "to #show" do + expect(get('/oauth/userinfo')).to route_to('doorkeeper/openid_connect/userinfo#show') + end + + it "to #show" do + expect(post('/oauth/userinfo')).to route_to('doorkeeper/openid_connect/userinfo#show') + end +end diff --git a/spec/rubocop/cop/migration/add_concurrent_index_spec.rb b/spec/rubocop/cop/migration/add_concurrent_index_spec.rb new file mode 100644 index 00000000000..19a5718b0b1 --- /dev/null +++ b/spec/rubocop/cop/migration/add_concurrent_index_spec.rb @@ -0,0 +1,41 @@ +require 'spec_helper' + +require 'rubocop' +require 'rubocop/rspec/support' + +require_relative '../../../../rubocop/cop/migration/add_concurrent_index' + +describe RuboCop::Cop::Migration::AddConcurrentIndex do + include CopHelper + + subject(:cop) { described_class.new } + + context 'in migration' do + before do + allow(cop).to receive(:in_migration?).and_return(true) + end + + it 'registers an offense when add_concurrent_index is used inside a change method' do + inspect_source(cop, 'def change; add_concurrent_index :table, :column; end') + + aggregate_failures do + expect(cop.offenses.size).to eq(1) + expect(cop.offenses.map(&:line)).to eq([1]) + end + end + + it 'registers no offense when add_concurrent_index is used inside an up method' do + inspect_source(cop, 'def up; add_concurrent_index :table, :column; end') + + expect(cop.offenses.size).to eq(0) + end + end + + context 'outside of migration' do + it 'registers no offense' do + inspect_source(cop, 'def change; add_concurrent_index :table, :column; end') + + expect(cop.offenses.size).to eq(0) + end + end +end diff --git a/spec/services/ci/register_build_service_spec.rb b/spec/services/ci/register_job_service_spec.rb index cd7dd53025c..62ba0b01339 100644 --- a/spec/services/ci/register_build_service_spec.rb +++ b/spec/services/ci/register_job_service_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' module Ci - describe RegisterBuildService, services: true do + describe RegisterJobService, services: true do let!(:project) { FactoryGirl.create :empty_project, shared_runners_enabled: false } let!(:pipeline) { FactoryGirl.create :ci_pipeline, project: project } let!(:pending_build) { FactoryGirl.create :ci_build, pipeline: pipeline } @@ -181,7 +181,7 @@ module Ci let!(:other_build) { create :ci_build, pipeline: pipeline } before do - allow_any_instance_of(Ci::RegisterBuildService).to receive(:builds_for_specific_runner) + allow_any_instance_of(Ci::RegisterJobService).to receive(:builds_for_specific_runner) .and_return([pending_build, other_build]) end @@ -193,7 +193,7 @@ module Ci context 'when single build is in queue' do before do - allow_any_instance_of(Ci::RegisterBuildService).to receive(:builds_for_specific_runner) + allow_any_instance_of(Ci::RegisterJobService).to receive(:builds_for_specific_runner) .and_return([pending_build]) end @@ -204,7 +204,7 @@ module Ci context 'when there is no build in queue' do before do - allow_any_instance_of(Ci::RegisterBuildService).to receive(:builds_for_specific_runner) + allow_any_instance_of(Ci::RegisterJobService).to receive(:builds_for_specific_runner) .and_return([]) end diff --git a/spec/services/git_push_service_spec.rb b/spec/services/git_push_service_spec.rb index 2a0f00ce937..bd71618e6f4 100644 --- a/spec/services/git_push_service_spec.rb +++ b/spec/services/git_push_service_spec.rb @@ -150,6 +150,13 @@ describe GitPushService, services: true do execute_service(project, user, @blankrev, 'newrev', 'refs/heads/master' ) end end + + context "Sends System Push data" do + it "when pushing on a branch" do + expect(SystemHookPushWorker).to receive(:perform_async).with(@push_data, :push_hooks) + execute_service(project, user, @oldrev, @newrev, @ref ) + end + end end describe "Updates git attributes" do diff --git a/spec/support/api/time_tracking_shared_examples.rb b/spec/support/api/time_tracking_shared_examples.rb index 210cd5817e0..16a3cf06be7 100644 --- a/spec/support/api/time_tracking_shared_examples.rb +++ b/spec/support/api/time_tracking_shared_examples.rb @@ -7,13 +7,13 @@ shared_examples 'time tracking endpoints' do |issuable_name| describe "POST /projects/:id/#{issuable_collection_name}/:#{issuable_name}_id/time_estimate" do context 'with an unauthorized user' do - subject { post(api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.id}/time_estimate", non_member), duration: '1w') } + subject { post(api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.iid}/time_estimate", non_member), duration: '1w') } it_behaves_like 'an unauthorized API user' end it "sets the time estimate for #{issuable_name}" do - post api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.id}/time_estimate", user), duration: '1w' + post api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.iid}/time_estimate", user), duration: '1w' expect(response).to have_http_status(200) expect(json_response['human_time_estimate']).to eq('1w') @@ -21,12 +21,12 @@ shared_examples 'time tracking endpoints' do |issuable_name| describe 'updating the current estimate' do before do - post api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.id}/time_estimate", user), duration: '1w' + post api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.iid}/time_estimate", user), duration: '1w' end context 'when duration has a bad format' do it 'does not modify the original estimate' do - post api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.id}/time_estimate", user), duration: 'foo' + post api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.iid}/time_estimate", user), duration: 'foo' expect(response).to have_http_status(400) expect(issuable.reload.human_time_estimate).to eq('1w') @@ -35,7 +35,7 @@ shared_examples 'time tracking endpoints' do |issuable_name| context 'with a valid duration' do it 'updates the estimate' do - post api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.id}/time_estimate", user), duration: '3w1h' + post api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.iid}/time_estimate", user), duration: '3w1h' expect(response).to have_http_status(200) expect(issuable.reload.human_time_estimate).to eq('3w 1h') @@ -46,13 +46,13 @@ shared_examples 'time tracking endpoints' do |issuable_name| describe "POST /projects/:id/#{issuable_collection_name}/:#{issuable_name}_id/reset_time_estimate" do context 'with an unauthorized user' do - subject { post(api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.id}/reset_time_estimate", non_member)) } + subject { post(api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.iid}/reset_time_estimate", non_member)) } it_behaves_like 'an unauthorized API user' end it "resets the time estimate for #{issuable_name}" do - post api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.id}/reset_time_estimate", user) + post api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.iid}/reset_time_estimate", user) expect(response).to have_http_status(200) expect(json_response['time_estimate']).to eq(0) @@ -62,7 +62,7 @@ shared_examples 'time tracking endpoints' do |issuable_name| describe "POST /projects/:id/#{issuable_collection_name}/:#{issuable_name}_id/add_spent_time" do context 'with an unauthorized user' do subject do - post api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.id}/add_spent_time", non_member), + post api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.iid}/add_spent_time", non_member), duration: '2h' end @@ -70,7 +70,7 @@ shared_examples 'time tracking endpoints' do |issuable_name| end it "add spent time for #{issuable_name}" do - post api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.id}/add_spent_time", user), + post api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.iid}/add_spent_time", user), duration: '2h' expect(response).to have_http_status(201) @@ -81,7 +81,7 @@ shared_examples 'time tracking endpoints' do |issuable_name| it 'subtracts time of the total spent time' do issuable.update_attributes!(spend_time: { duration: 7200, user: user }) - post api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.id}/add_spent_time", user), + post api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.iid}/add_spent_time", user), duration: '-1h' expect(response).to have_http_status(201) @@ -93,7 +93,7 @@ shared_examples 'time tracking endpoints' do |issuable_name| it 'does not modify the total time spent' do issuable.update_attributes!(spend_time: { duration: 7200, user: user }) - post api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.id}/add_spent_time", user), + post api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.iid}/add_spent_time", user), duration: '-1w' expect(response).to have_http_status(400) @@ -104,13 +104,13 @@ shared_examples 'time tracking endpoints' do |issuable_name| describe "POST /projects/:id/#{issuable_collection_name}/:#{issuable_name}_id/reset_spent_time" do context 'with an unauthorized user' do - subject { post(api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.id}/reset_spent_time", non_member)) } + subject { post(api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.iid}/reset_spent_time", non_member)) } it_behaves_like 'an unauthorized API user' end it "resets spent time for #{issuable_name}" do - post api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.id}/reset_spent_time", user) + post api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.iid}/reset_spent_time", user) expect(response).to have_http_status(200) expect(json_response['total_time_spent']).to eq(0) @@ -122,7 +122,7 @@ shared_examples 'time tracking endpoints' do |issuable_name| issuable.update_attributes!(spend_time: { duration: 1800, user: user }, time_estimate: 3600) - get api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.id}/time_stats", user) + get api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.iid}/time_stats", user) expect(response).to have_http_status(200) expect(json_response['total_time_spent']).to eq(1800) diff --git a/spec/support/api/v3/time_tracking_shared_examples.rb b/spec/support/api/v3/time_tracking_shared_examples.rb new file mode 100644 index 00000000000..f982b10d999 --- /dev/null +++ b/spec/support/api/v3/time_tracking_shared_examples.rb @@ -0,0 +1,128 @@ +shared_examples 'V3 time tracking endpoints' do |issuable_name| + issuable_collection_name = issuable_name.pluralize + + describe "POST /projects/:id/#{issuable_collection_name}/:#{issuable_name}_id/time_estimate" do + context 'with an unauthorized user' do + subject { post(v3_api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.id}/time_estimate", non_member), duration: '1w') } + + it_behaves_like 'an unauthorized API user' + end + + it "sets the time estimate for #{issuable_name}" do + post v3_api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.id}/time_estimate", user), duration: '1w' + + expect(response).to have_http_status(200) + expect(json_response['human_time_estimate']).to eq('1w') + end + + describe 'updating the current estimate' do + before do + post v3_api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.id}/time_estimate", user), duration: '1w' + end + + context 'when duration has a bad format' do + it 'does not modify the original estimate' do + post v3_api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.id}/time_estimate", user), duration: 'foo' + + expect(response).to have_http_status(400) + expect(issuable.reload.human_time_estimate).to eq('1w') + end + end + + context 'with a valid duration' do + it 'updates the estimate' do + post v3_api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.id}/time_estimate", user), duration: '3w1h' + + expect(response).to have_http_status(200) + expect(issuable.reload.human_time_estimate).to eq('3w 1h') + end + end + end + end + + describe "POST /projects/:id/#{issuable_collection_name}/:#{issuable_name}_id/reset_time_estimate" do + context 'with an unauthorized user' do + subject { post(v3_api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.id}/reset_time_estimate", non_member)) } + + it_behaves_like 'an unauthorized API user' + end + + it "resets the time estimate for #{issuable_name}" do + post v3_api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.id}/reset_time_estimate", user) + + expect(response).to have_http_status(200) + expect(json_response['time_estimate']).to eq(0) + end + end + + describe "POST /projects/:id/#{issuable_collection_name}/:#{issuable_name}_id/add_spent_time" do + context 'with an unauthorized user' do + subject do + post v3_api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.id}/add_spent_time", non_member), + duration: '2h' + end + + it_behaves_like 'an unauthorized API user' + end + + it "add spent time for #{issuable_name}" do + post v3_api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.id}/add_spent_time", user), + duration: '2h' + + expect(response).to have_http_status(201) + expect(json_response['human_total_time_spent']).to eq('2h') + end + + context 'when subtracting time' do + it 'subtracts time of the total spent time' do + issuable.update_attributes!(spend_time: { duration: 7200, user: user }) + + post v3_api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.id}/add_spent_time", user), + duration: '-1h' + + expect(response).to have_http_status(201) + expect(json_response['total_time_spent']).to eq(3600) + end + end + + context 'when time to subtract is greater than the total spent time' do + it 'does not modify the total time spent' do + issuable.update_attributes!(spend_time: { duration: 7200, user: user }) + + post v3_api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.id}/add_spent_time", user), + duration: '-1w' + + expect(response).to have_http_status(400) + expect(json_response['message']['time_spent'].first).to match(/exceeds the total time spent/) + end + end + end + + describe "POST /projects/:id/#{issuable_collection_name}/:#{issuable_name}_id/reset_spent_time" do + context 'with an unauthorized user' do + subject { post(v3_api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.id}/reset_spent_time", non_member)) } + + it_behaves_like 'an unauthorized API user' + end + + it "resets spent time for #{issuable_name}" do + post v3_api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.id}/reset_spent_time", user) + + expect(response).to have_http_status(200) + expect(json_response['total_time_spent']).to eq(0) + end + end + + describe "GET /projects/:id/#{issuable_collection_name}/:#{issuable_name}_id/time_stats" do + it "returns the time stats for #{issuable_name}" do + issuable.update_attributes!(spend_time: { duration: 1800, user: user }, + time_estimate: 3600) + + get v3_api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.id}/time_stats", user) + + expect(response).to have_http_status(200) + expect(json_response['total_time_spent']).to eq(1800) + expect(json_response['time_estimate']).to eq(3600) + end + end +end diff --git a/spec/support/prometheus_helpers.rb b/spec/support/prometheus_helpers.rb new file mode 100644 index 00000000000..a52d8f37d14 --- /dev/null +++ b/spec/support/prometheus_helpers.rb @@ -0,0 +1,117 @@ +module PrometheusHelpers + def prometheus_memory_query(environment_slug) + %{sum(container_memory_usage_bytes{container_name="app",environment="#{environment_slug}"})/1024/1024} + end + + def prometheus_cpu_query(environment_slug) + %{sum(rate(container_cpu_usage_seconds_total{container_name="app",environment="#{environment_slug}"}[2m]))} + end + + def prometheus_query_url(prometheus_query) + query = { query: prometheus_query }.to_query + + "https://prometheus.example.com/api/v1/query?#{query}" + end + + def prometheus_query_range_url(prometheus_query, start: 8.hours.ago) + query = { + query: prometheus_query, + start: start.to_f, + end: Time.now.utc.to_f, + step: 1.minute.to_i + }.to_query + + "https://prometheus.example.com/api/v1/query_range?#{query}" + end + + def stub_prometheus_request(url, body: {}, status: 200) + WebMock.stub_request(:get, url) + .to_return({ + status: status, + headers: { 'Content-Type' => 'application/json' }, + body: body.to_json + }) + end + + def stub_all_prometheus_requests(environment_slug, body: nil, status: 200) + stub_prometheus_request( + prometheus_query_url(prometheus_memory_query(environment_slug)), + status: status, + body: body || prometheus_value_body + ) + stub_prometheus_request( + prometheus_query_range_url(prometheus_memory_query(environment_slug)), + status: status, + body: body || prometheus_values_body + ) + stub_prometheus_request( + prometheus_query_url(prometheus_cpu_query(environment_slug)), + status: status, + body: body || prometheus_value_body + ) + stub_prometheus_request( + prometheus_query_range_url(prometheus_cpu_query(environment_slug)), + status: status, + body: body || prometheus_values_body + ) + end + + def prometheus_data(last_update: Time.now.utc) + { + success: true, + metrics: { + memory_values: prometheus_values_body('matrix').dig(:data, :result), + memory_current: prometheus_value_body('vector').dig(:data, :result), + cpu_values: prometheus_values_body('matrix').dig(:data, :result), + cpu_current: prometheus_value_body('vector').dig(:data, :result) + }, + last_update: last_update + } + end + + def prometheus_empty_body(type) + { + "status": "success", + "data": { + "resultType": type, + "result": [] + } + } + end + + def prometheus_value_body(type = 'vector') + { + "status": "success", + "data": { + "resultType": type, + "result": [ + { + "metric": {}, + "value": [ + 1488772511.004, + "0.000041021495238095323" + ] + } + ] + } + } + end + + def prometheus_values_body(type = 'matrix') + { + "status": "success", + "data": { + "resultType": type, + "result": [ + { + "metric": {}, + "values": [ + [1488758662.506, "0.00002996364761904785"], + [1488758722.506, "0.00003090239047619091"] + ] + } + ] + } + } + end +end diff --git a/spec/support/test_env.rb b/spec/support/test_env.rb index c3aa3ef44c2..f1d226b6ae3 100644 --- a/spec/support/test_env.rb +++ b/spec/support/test_env.rb @@ -143,7 +143,7 @@ module TestEnv end def repos_path - Gitlab.config.repositories.storages.default + Gitlab.config.repositories.storages.default['path'] end def backup_path diff --git a/spec/tasks/gitlab/backup_rake_spec.rb b/spec/tasks/gitlab/backup_rake_spec.rb index dfbfbd05f43..10458966cb9 100644 --- a/spec/tasks/gitlab/backup_rake_spec.rb +++ b/spec/tasks/gitlab/backup_rake_spec.rb @@ -227,8 +227,8 @@ describe 'gitlab:app namespace rake task' do FileUtils.mkdir('tmp/tests/default_storage') FileUtils.mkdir('tmp/tests/custom_storage') storages = { - 'default' => 'tmp/tests/default_storage', - 'custom' => 'tmp/tests/custom_storage' + 'default' => { 'path' => 'tmp/tests/default_storage' }, + 'custom' => { 'path' => 'tmp/tests/custom_storage' } } allow(Gitlab.config.repositories).to receive(:storages).and_return(storages) diff --git a/spec/views/projects/pipelines/_stage.html.haml_spec.rb b/spec/views/projects/pipelines/_stage.html.haml_spec.rb index d25de8af5d2..65f9d0125e6 100644 --- a/spec/views/projects/pipelines/_stage.html.haml_spec.rb +++ b/spec/views/projects/pipelines/_stage.html.haml_spec.rb @@ -50,4 +50,23 @@ describe 'projects/pipelines/_stage', :view do expect(rendered).to have_text 'test:build', count: 1 end end + + context 'when there are multiple builds' do + before do + HasStatus::AVAILABLE_STATUSES.each do |status| + create_build(status) + end + end + + it 'shows them in order' do + render + + expect(rendered).to have_text(HasStatus::ORDERED_STATUSES.join(" ")) + end + + def create_build(status) + create(:ci_build, name: status, status: status, + pipeline: pipeline, stage: stage.name) + end + end end diff --git a/spec/workers/post_receive_spec.rb b/spec/workers/post_receive_spec.rb index 5919b99a6ed..7bcb5521202 100644 --- a/spec/workers/post_receive_spec.rb +++ b/spec/workers/post_receive_spec.rb @@ -105,6 +105,6 @@ describe PostReceive do end def pwd(project) - File.join(Gitlab.config.repositories.storages.default, project.path_with_namespace) + File.join(Gitlab.config.repositories.storages.default['path'], project.path_with_namespace) end end diff --git a/spec/workers/system_hook_push_worker_spec.rb b/spec/workers/system_hook_push_worker_spec.rb new file mode 100644 index 00000000000..b1d446ed25f --- /dev/null +++ b/spec/workers/system_hook_push_worker_spec.rb @@ -0,0 +1,19 @@ +require 'spec_helper' + +describe SystemHookPushWorker do + include RepoHelpers + + subject { described_class.new } + + describe '#perform' do + it 'executes SystemHooksService with expected values' do + push_data = double('push_data') + system_hook_service = double('system_hook_service') + + expect(SystemHooksService).to receive(:new).and_return(system_hook_service) + expect(system_hook_service).to receive(:execute_hooks).with(push_data, :push_hooks) + + subject.perform(push_data, :push_hooks) + end + end +end diff --git a/spec/workers/update_merge_requests_worker_spec.rb b/spec/workers/update_merge_requests_worker_spec.rb index c78a69eda67..262d6e5a9ab 100644 --- a/spec/workers/update_merge_requests_worker_spec.rb +++ b/spec/workers/update_merge_requests_worker_spec.rb @@ -23,16 +23,5 @@ describe UpdateMergeRequestsWorker do perform end - - it 'executes SystemHooksService with expected values' do - push_data = double('push_data') - expect(Gitlab::DataBuilder::Push).to receive(:build).with(project, user, oldrev, newrev, ref, []).and_return(push_data) - - system_hook_service = double('system_hook_service') - expect(SystemHooksService).to receive(:new).and_return(system_hook_service) - expect(system_hook_service).to receive(:execute_hooks).with(push_data, :push_hooks) - - perform - end end end |