diff options
author | Kamil Trzciński <ayufan@ayufan.eu> | 2018-02-28 23:11:53 +0300 |
---|---|---|
committer | Kamil Trzciński <ayufan@ayufan.eu> | 2018-02-28 23:11:53 +0300 |
commit | 729391fbfce4dea58478b65c684a24a1bfd125a2 (patch) | |
tree | f9f0d9c391744fed388f99d44e96c908ed7aa1f1 /spec/requests | |
parent | 999118f0ec6edabc9e13c089381ad664970a080a (diff) | |
parent | 8af23def1d6450420d06b8de54d23311a978de20 (diff) |
Merge commit '8af23def1d6' into object-storage-ee-to-ce-backport
Diffstat (limited to 'spec/requests')
24 files changed, 558 insertions, 115 deletions
diff --git a/spec/requests/api/applications_spec.rb b/spec/requests/api/applications_spec.rb new file mode 100644 index 00000000000..f56bc932f40 --- /dev/null +++ b/spec/requests/api/applications_spec.rb @@ -0,0 +1,86 @@ +require 'spec_helper' + +describe API::Applications, :api do + include ApiHelpers + + let(:admin_user) { create(:user, admin: true) } + let(:user) { create(:user, admin: false) } + + describe 'POST /applications' do + context 'authenticated and authorized user' do + it 'creates and returns an OAuth application' do + expect do + post api('/applications', admin_user), name: 'application_name', redirect_uri: 'http://application.url', scopes: '' + end.to change { Doorkeeper::Application.count }.by 1 + + application = Doorkeeper::Application.find_by(name: 'application_name', redirect_uri: 'http://application.url') + + expect(response).to have_http_status 201 + expect(json_response).to be_a Hash + expect(json_response['application_id']).to eq application.uid + expect(json_response['secret']).to eq application.secret + expect(json_response['callback_url']).to eq application.redirect_uri + end + + it 'does not allow creating an application with the wrong redirect_uri format' do + expect do + post api('/applications', admin_user), name: 'application_name', redirect_uri: 'wrong_url_format', scopes: '' + end.not_to change { Doorkeeper::Application.count } + + expect(response).to have_http_status 400 + expect(json_response).to be_a Hash + expect(json_response['message']['redirect_uri'][0]).to eq('must be an absolute URI.') + end + + it 'does not allow creating an application without a name' do + expect do + post api('/applications', admin_user), redirect_uri: 'http://application.url', scopes: '' + end.not_to change { Doorkeeper::Application.count } + + expect(response).to have_http_status 400 + expect(json_response).to be_a Hash + expect(json_response['error']).to eq('name is missing') + end + + it 'does not allow creating an application without a redirect_uri' do + expect do + post api('/applications', admin_user), name: 'application_name', scopes: '' + end.not_to change { Doorkeeper::Application.count } + + expect(response).to have_http_status 400 + expect(json_response).to be_a Hash + expect(json_response['error']).to eq('redirect_uri is missing') + end + + it 'does not allow creating an application without scopes' do + expect do + post api('/applications', admin_user), name: 'application_name', redirect_uri: 'http://application.url' + end.not_to change { Doorkeeper::Application.count } + + expect(response).to have_http_status 400 + expect(json_response).to be_a Hash + expect(json_response['error']).to eq('scopes is missing') + end + end + + context 'authorized user without authorization' do + it 'does not create application' do + expect do + post api('/applications', user), name: 'application_name', redirect_uri: 'http://application.url', scopes: '' + end.not_to change { Doorkeeper::Application.count } + + expect(response).to have_http_status 403 + end + end + + context 'non-authenticated user' do + it 'does not create application' do + expect do + post api('/applications'), name: 'application_name', redirect_uri: 'http://application.url' + end.not_to change { Doorkeeper::Application.count } + + expect(response).to have_http_status 401 + end + end + end +end diff --git a/spec/requests/api/commit_statuses_spec.rb b/spec/requests/api/commit_statuses_spec.rb index ffa17d296e8..f246bb79ab7 100644 --- a/spec/requests/api/commit_statuses_spec.rb +++ b/spec/requests/api/commit_statuses_spec.rb @@ -142,6 +142,7 @@ describe API::CommitStatuses do expect(json_response['ref']).not_to be_empty expect(json_response['target_url']).to be_nil expect(json_response['description']).to be_nil + if status == 'failed' expect(CommitStatus.find(json_response['id'])).to be_api_failure end diff --git a/spec/requests/api/commits_spec.rb b/spec/requests/api/commits_spec.rb index 0d2bd3207c0..ff5f207487b 100644 --- a/spec/requests/api/commits_spec.rb +++ b/spec/requests/api/commits_spec.rb @@ -62,7 +62,7 @@ describe API::Commits do context "since optional parameter" do it "returns project commits since provided parameter" do - commits = project.repository.commits("master") + commits = project.repository.commits("master", limit: 2) after = commits.second.created_at get api("/projects/#{project_id}/repository/commits?since=#{after.utc.iso8601}", user) @@ -73,7 +73,7 @@ describe API::Commits do end it 'include correct pagination headers' do - commits = project.repository.commits("master") + commits = project.repository.commits("master", limit: 2) after = commits.second.created_at commit_count = project.repository.count_commits(ref: 'master', after: after).to_s @@ -87,12 +87,12 @@ describe API::Commits do context "until optional parameter" do it "returns project commits until provided parameter" do - commits = project.repository.commits("master") + commits = project.repository.commits("master", limit: 20) before = commits.second.created_at get api("/projects/#{project_id}/repository/commits?until=#{before.utc.iso8601}", user) - if commits.size >= 20 + if commits.size == 20 expect(json_response.size).to eq(20) else expect(json_response.size).to eq(commits.size - 1) @@ -103,7 +103,7 @@ describe API::Commits do end it 'include correct pagination headers' do - commits = project.repository.commits("master") + commits = project.repository.commits("master", limit: 2) before = commits.second.created_at commit_count = project.repository.count_commits(ref: 'master', before: before).to_s @@ -181,7 +181,7 @@ describe API::Commits do let(:page) { 3 } it 'returns the third 5 commits' do - commit = project.repository.commits('HEAD', offset: (page - 1) * per_page).first + commit = project.repository.commits('HEAD', limit: per_page, offset: (page - 1) * per_page).first expect(json_response.size).to eq(per_page) expect(json_response.first['id']).to eq(commit.id) @@ -512,6 +512,31 @@ describe API::Commits do end end + context 'when stat param' do + let(:route) { "/projects/#{project_id}/repository/commits/#{commit_id}" } + + it 'is not present return stats by default' do + get api(route, user) + + expect(response).to have_gitlab_http_status(200) + expect(json_response).to include 'stats' + end + + it "is false it does not include stats" do + get api(route, user), stats: false + + expect(response).to have_gitlab_http_status(200) + expect(json_response).not_to include 'stats' + end + + it "is true it includes stats" do + get api(route, user), stats: true + + expect(response).to have_gitlab_http_status(200) + expect(json_response).to include 'stats' + end + end + context 'when unauthenticated', 'and project is public' do let(:project) { create(:project, :public, :repository) } diff --git a/spec/requests/api/deploy_keys_spec.rb b/spec/requests/api/deploy_keys_spec.rb index 1f1e6ea17e4..0772b3f2e64 100644 --- a/spec/requests/api/deploy_keys_spec.rb +++ b/spec/requests/api/deploy_keys_spec.rb @@ -110,7 +110,7 @@ describe API::DeployKeys do end it 'accepts can_push parameter' do - key_attrs = attributes_for :write_access_key + key_attrs = attributes_for(:another_key).merge(can_push: true) post api("/projects/#{project.id}/deploy_keys", admin), key_attrs @@ -160,16 +160,6 @@ describe API::DeployKeys do expect(json_response['title']).to eq('new title') expect(json_response['can_push']).to eq(true) end - - it 'updates a private ssh key from projects user has access with correct attributes' do - create(:deploy_keys_project, project: project2, deploy_key: private_deploy_key) - - put api("/projects/#{project.id}/deploy_keys/#{private_deploy_key.id}", admin), { title: 'new title', can_push: true } - - expect(json_response['id']).to eq(private_deploy_key.id) - expect(json_response['title']).to eq('new title') - expect(json_response['can_push']).to eq(true) - end end describe 'DELETE /projects/:id/deploy_keys/:key_id' do diff --git a/spec/requests/api/deployments_spec.rb b/spec/requests/api/deployments_spec.rb index 6732c99e329..51b70fda148 100644 --- a/spec/requests/api/deployments_spec.rb +++ b/spec/requests/api/deployments_spec.rb @@ -3,24 +3,65 @@ require 'spec_helper' describe API::Deployments do let(:user) { create(:user) } let(:non_member) { create(:user) } - let(:project) { deployment.environment.project } - let!(:deployment) { create(:deployment) } before do project.add_master(user) end describe 'GET /projects/:id/deployments' do + let(:project) { create(:project) } + let!(:deployment_1) { create(:deployment, project: project, iid: 11, ref: 'master', created_at: Time.now) } + let!(:deployment_2) { create(:deployment, project: project, iid: 12, ref: 'feature', created_at: 1.day.ago) } + let!(:deployment_3) { create(:deployment, project: project, iid: 8, ref: 'feature', created_at: 2.days.ago) } + context 'as member of the project' do - it 'returns projects deployments' do + it 'returns projects deployments sorted by id asc' do get api("/projects/#{project.id}/deployments", user) expect(response).to have_gitlab_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.first['iid']).to eq(deployment.iid) + expect(json_response.size).to eq(3) + expect(json_response.first['iid']).to eq(deployment_1.iid) expect(json_response.first['sha']).to match /\A\h{40}\z/ + expect(json_response.second['iid']).to eq(deployment_2.iid) + expect(json_response.last['iid']).to eq(deployment_3.iid) + end + + describe 'ordering' do + using RSpec::Parameterized::TableSyntax + + let(:order_by) { nil } + let(:sort) { nil } + + subject { get api("/projects/#{project.id}/deployments?order_by=#{order_by}&sort=#{sort}", user) } + + def expect_deployments(ordered_deployments) + json_response.each_with_index do |deployment_json, index| + expect(deployment_json['id']).to eq(public_send(ordered_deployments[index]).id) + end + end + + before do + subject + end + + where(:order_by, :sort, :ordered_deployments) do + 'created_at' | 'asc' | [:deployment_3, :deployment_2, :deployment_1] + 'created_at' | 'desc' | [:deployment_1, :deployment_2, :deployment_3] + 'id' | 'asc' | [:deployment_1, :deployment_2, :deployment_3] + 'id' | 'desc' | [:deployment_3, :deployment_2, :deployment_1] + 'iid' | 'asc' | [:deployment_3, :deployment_1, :deployment_2] + 'iid' | 'desc' | [:deployment_2, :deployment_1, :deployment_3] + 'ref' | 'asc' | [:deployment_2, :deployment_3, :deployment_1] + 'ref' | 'desc' | [:deployment_1, :deployment_2, :deployment_3] + end + + with_them do + it 'returns the deployments ordered' do + expect_deployments(ordered_deployments) + end + end end end @@ -34,6 +75,9 @@ describe API::Deployments do end describe 'GET /projects/:id/deployments/:deployment_id' do + let(:project) { deployment.environment.project } + let!(:deployment) { create(:deployment) } + context 'as a member of the project' do it 'returns the projects deployment' do get api("/projects/#{project.id}/deployments/#{deployment.id}", user) diff --git a/spec/requests/api/helpers_spec.rb b/spec/requests/api/helpers_spec.rb index 0462f494e15..837389451e8 100644 --- a/spec/requests/api/helpers_spec.rb +++ b/spec/requests/api/helpers_spec.rb @@ -68,6 +68,12 @@ describe API::Helpers do end it { is_expected.to eq(user) } + + it 'sets the environment with data of the current user' do + subject + + expect(env[API::Helpers::API_USER_ENV]).to eq({ user_id: subject.id, username: subject.username }) + end end context "HEAD request" do diff --git a/spec/requests/api/internal_spec.rb b/spec/requests/api/internal_spec.rb index 7b25047ea8f..884a258fd12 100644 --- a/spec/requests/api/internal_spec.rb +++ b/spec/requests/api/internal_spec.rb @@ -192,6 +192,54 @@ describe API::Internal do end end + describe "GET /internal/authorized_keys" do + context "using an existing key's fingerprint" do + it "finds the key" do + get(api('/internal/authorized_keys'), fingerprint: key.fingerprint, secret_token: secret_token) + + expect(response.status).to eq(200) + expect(json_response["key"]).to eq(key.key) + end + end + + context "non existing key's fingerprint" do + it "returns 404" do + get(api('/internal/authorized_keys'), fingerprint: "no:t-:va:li:d0", secret_token: secret_token) + + expect(response.status).to eq(404) + end + end + + context "using a partial fingerprint" do + it "returns 404" do + get(api('/internal/authorized_keys'), fingerprint: "#{key.fingerprint[0..5]}%", secret_token: secret_token) + + expect(response.status).to eq(404) + end + end + + context "sending the key" do + it "finds the key" do + get(api('/internal/authorized_keys'), key: key.key.split[1], secret_token: secret_token) + + expect(response.status).to eq(200) + expect(json_response["key"]).to eq(key.key) + end + + it "returns 404 with a partial key" do + get(api('/internal/authorized_keys'), key: key.key.split[1][0...-3], secret_token: secret_token) + + expect(response.status).to eq(404) + end + + it "returns 404 with an not valid base64 string" do + get(api('/internal/authorized_keys'), key: "whatever!", secret_token: secret_token) + + expect(response.status).to eq(404) + end + end + end + describe "POST /internal/allowed", :clean_gitlab_redis_shared_state do context "access granted" do around do |example| @@ -269,35 +317,20 @@ describe API::Internal do end context "git pull" do - context "gitaly disabled", :disable_gitaly do - it "has the correct payload" do - pull(key, project) - - expect(response).to have_gitlab_http_status(200) - expect(json_response["status"]).to be_truthy - expect(json_response["repository_path"]).to eq(project.repository.path_to_repo) - expect(json_response["gl_repository"]).to eq("project-#{project.id}") - expect(json_response["gitaly"]).to be_nil - expect(user).to have_an_activity_record - end - end - - context "gitaly enabled" do - it "has the correct payload" do - pull(key, project) + it "has the correct payload" do + pull(key, project) - expect(response).to have_gitlab_http_status(200) - expect(json_response["status"]).to be_truthy - expect(json_response["repository_path"]).to eq(project.repository.path_to_repo) - expect(json_response["gl_repository"]).to eq("project-#{project.id}") - expect(json_response["gitaly"]).not_to be_nil - expect(json_response["gitaly"]["repository"]).not_to be_nil - expect(json_response["gitaly"]["repository"]["storage_name"]).to eq(project.repository.gitaly_repository.storage_name) - expect(json_response["gitaly"]["repository"]["relative_path"]).to eq(project.repository.gitaly_repository.relative_path) - expect(json_response["gitaly"]["address"]).to eq(Gitlab::GitalyClient.address(project.repository_storage)) - expect(json_response["gitaly"]["token"]).to eq(Gitlab::GitalyClient.token(project.repository_storage)) - expect(user).to have_an_activity_record - end + expect(response).to have_gitlab_http_status(200) + expect(json_response["status"]).to be_truthy + expect(json_response["repository_path"]).to eq(project.repository.path_to_repo) + expect(json_response["gl_repository"]).to eq("project-#{project.id}") + expect(json_response["gitaly"]).not_to be_nil + expect(json_response["gitaly"]["repository"]).not_to be_nil + expect(json_response["gitaly"]["repository"]["storage_name"]).to eq(project.repository.gitaly_repository.storage_name) + expect(json_response["gitaly"]["repository"]["relative_path"]).to eq(project.repository.gitaly_repository.relative_path) + expect(json_response["gitaly"]["address"]).to eq(Gitlab::GitalyClient.address(project.repository_storage)) + expect(json_response["gitaly"]["token"]).to eq(Gitlab::GitalyClient.token(project.repository_storage)) + expect(user).to have_an_activity_record end end diff --git a/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb index 320217f2032..13db40d21a5 100644 --- a/spec/requests/api/issues_spec.rb +++ b/spec/requests/api/issues_spec.rb @@ -847,6 +847,15 @@ describe API::Issues, :mailer do expect(json_response['assignee']['name']).to eq(user2.name) expect(json_response['assignees'].first['name']).to eq(user2.name) end + + it 'creates a new project issue when assignee_id is empty' do + post api("/projects/#{project.id}/issues", user), + title: 'new issue', assignee_id: '' + + expect(response).to have_gitlab_http_status(201) + expect(json_response['title']).to eq('new issue') + expect(json_response['assignee']).to be_nil + end end context 'single assignee restrictions' do @@ -1432,7 +1441,7 @@ describe API::Issues, :mailer 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.iid}/move", user), + post api("/projects/12345/issues/#{issue.iid}/move", user), to_project_id: target_project.id expect(response).to have_gitlab_http_status(404) @@ -1443,7 +1452,7 @@ describe API::Issues, :mailer 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.iid}/move", user), - to_project_id: 123 + to_project_id: 12345 expect(response).to have_gitlab_http_status(404) end diff --git a/spec/requests/api/jobs_spec.rb b/spec/requests/api/jobs_spec.rb index 366bfeb4d72..983bef6cdd0 100644 --- a/spec/requests/api/jobs_spec.rb +++ b/spec/requests/api/jobs_spec.rb @@ -31,9 +31,10 @@ describe API::Jobs do describe 'GET /projects/:id/jobs' do let(:query) { Hash.new } - before do - job - get api("/projects/#{project.id}/jobs", api_user), query + before do |example| + unless example.metadata[:skip_before_request] + get api("/projects/#{project.id}/jobs", api_user), query + end end context 'authorized user' do @@ -58,6 +59,23 @@ describe API::Jobs do expect(json_job['pipeline']['status']).to eq job.pipeline.status end + it 'avoids N+1 queries', :skip_before_request do + first_build = create(:ci_build, :artifacts, pipeline: pipeline) + first_build.runner = create(:ci_runner) + first_build.user = create(:user) + first_build.save + + control_count = ActiveRecord::QueryRecorder.new { go }.count + + second_pipeline = create(:ci_empty_pipeline, project: project, sha: project.commit.id, ref: project.default_branch) + second_build = create(:ci_build, :artifacts, pipeline: second_pipeline) + second_build.runner = create(:ci_runner) + second_build.user = create(:user) + second_build.save + + expect { go }.not_to exceed_query_limit(control_count) + end + context 'filter project with one scope element' do let(:query) { { 'scope' => 'pending' } } @@ -90,6 +108,10 @@ describe API::Jobs do expect(response).to have_gitlab_http_status(401) end end + + def go + get api("/projects/#{project.id}/jobs", api_user), query + end end describe 'GET /projects/:id/pipelines/:pipeline_id/jobs' do @@ -336,37 +358,11 @@ describe API::Jobs do end end end - - context 'authorized by job_token' do - let(:job) { create(:ci_build, :artifacts, pipeline: pipeline, user: api_user) } - - before do - get api("/projects/#{project.id}/jobs/#{job.id}/artifacts"), job_token: job.token - end - - context 'user is developer' do - let(:api_user) { user } - - it_behaves_like 'downloads artifact' - end - - context 'when artifacts are stored remotely' do - let(:job) { create(:ci_build, :artifacts, :remote_store, pipeline: pipeline) } - - it 'returns location redirect' do - expect(response).to have_http_status(302) - end - end - end - - it 'does not return job artifacts if not uploaded' do - expect(response).to have_gitlab_http_status(404) - end end describe 'GET /projects/:id/artifacts/:ref_name/download?job=name' do let(:api_user) { reporter } - let(:job) { create(:ci_build, :artifacts, pipeline: pipeline) } + let(:job) { create(:ci_build, :artifacts, pipeline: pipeline, user: api_user) } before do stub_artifacts_object_storage(licensed: :skip) diff --git a/spec/requests/api/members_spec.rb b/spec/requests/api/members_spec.rb index 5d4f81e07a6..ec500838eb2 100644 --- a/spec/requests/api/members_spec.rb +++ b/spec/requests/api/members_spec.rb @@ -44,6 +44,21 @@ describe API::Members do end end + it 'avoids N+1 queries' do + # Establish baseline + get api("/#{source_type.pluralize}/#{source.id}/members", master) + + control = ActiveRecord::QueryRecorder.new do + get api("/#{source_type.pluralize}/#{source.id}/members", master) + end + + project.add_developer(create(:user)) + + expect do + get api("/#{source_type.pluralize}/#{source.id}/members", master) + end.not_to exceed_query_limit(control) + end + it 'does not return invitees' do create(:"#{source_type}_member", invite_token: '123', invite_email: 'test@abc.com', source: source, user: nil) @@ -65,6 +80,16 @@ describe API::Members do expect(json_response.count).to eq(1) expect(json_response.first['username']).to eq(master.username) end + + it 'finds all members with no query specified' do + get api("/#{source_type.pluralize}/#{source.id}/members", developer), query: '' + + expect(response).to have_gitlab_http_status(200) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + expect(json_response.count).to eq(2) + expect(json_response.map { |u| u['id'] }).to match_array [master.id, developer.id] + end end end diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index 0c9fbb1f187..14dd9da119d 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -198,6 +198,8 @@ describe API::MergeRequests do create(:merge_request, state: 'closed', milestone: milestone1, author: user, assignee: user, source_project: project, target_project: project, title: "Test", created_at: base_time) + create(:merge_request, milestone: milestone1, author: user, assignee: user, source_project: project, target_project: project, title: "Test", created_at: base_time) + expect do get api("/projects/#{project.id}/merge_requests", user) end.not_to exceed_query_limit(control) @@ -551,6 +553,49 @@ describe API::MergeRequests do end end + describe 'GET /projects/:id/merge_requests/:merge_request_iid/pipelines' do + context 'when authorized' do + let!(:pipeline) { create(:ci_empty_pipeline, project: project, user: user, ref: merge_request.source_branch, sha: merge_request.diff_head_sha) } + let!(:pipeline2) { create(:ci_empty_pipeline, project: project) } + + it 'returns a paginated array of corresponding pipelines' do + get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/pipelines") + + expect(response).to have_gitlab_http_status(200) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + expect(json_response.count).to eq(1) + expect(json_response.first['id']).to eq(pipeline.id) + end + + it 'exposes basic attributes' do + get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/pipelines") + + expect(response).to have_gitlab_http_status(200) + expect(response).to match_response_schema('public_api/v4/pipelines') + end + + it 'returns 404 if MR does not exist' do + get api("/projects/#{project.id}/merge_requests/777/pipelines") + + expect(response).to have_gitlab_http_status(404) + end + end + + context 'when unauthorized' do + it 'returns 403' do + project = create(:project, public_builds: false) + merge_request = create(:merge_request, :simple, source_project: project) + guest = create(:user) + project.add_guest(guest) + + get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/pipelines", guest) + + expect(response).to have_gitlab_http_status(403) + end + end + end + describe "POST /projects/:id/merge_requests" do context 'between branches projects' do it "returns merge_request" do @@ -711,16 +756,28 @@ describe API::MergeRequests do expect(response).to have_gitlab_http_status(400) end - context 'when target_branch is specified' do + context 'when target_branch and target_project_id is specified' do + let(:params) do + { title: 'Test merge_request', + target_branch: 'master', + source_branch: 'markdown', + author: user2, + target_project_id: unrelated_project.id } + end + it 'returns 422 if targeting a different fork' do - post api("/projects/#{forked_project.id}/merge_requests", user2), - title: 'Test merge_request', - target_branch: 'master', - source_branch: 'markdown', - author: user2, - target_project_id: unrelated_project.id + unrelated_project.add_developer(user2) + + post api("/projects/#{forked_project.id}/merge_requests", user2), params + expect(response).to have_gitlab_http_status(422) end + + it 'returns 403 if targeting a different fork which user can not access' do + post api("/projects/#{forked_project.id}/merge_requests", user2), params + + expect(response).to have_gitlab_http_status(403) + end end it "returns 201 when target_branch is specified and for the same project" do diff --git a/spec/requests/api/project_milestones_spec.rb b/spec/requests/api/project_milestones_spec.rb index 08ea7314bb3..6c05c166bd6 100644 --- a/spec/requests/api/project_milestones_spec.rb +++ b/spec/requests/api/project_milestones_spec.rb @@ -14,6 +14,46 @@ describe API::ProjectMilestones do let(:route) { "/projects/#{project.id}/milestones" } end + describe 'DELETE /projects/:id/milestones/:milestone_id' do + let(:guest) { create(:user) } + let(:reporter) { create(:user) } + + before do + project.add_reporter(reporter) + end + + it 'returns 404 response when the project does not exists' do + delete api("/projects/999/milestones/#{milestone.id}", user) + + expect(response).to have_gitlab_http_status(404) + end + + it 'returns 404 response when the milestone does not exists' do + delete api("/projects/#{project.id}/milestones/999", user) + + expect(response).to have_gitlab_http_status(404) + end + + it "returns 404 from guest user deleting a milestone" do + delete api("/projects/#{project.id}/milestones/#{milestone.id}", guest) + + expect(response).to have_gitlab_http_status(404) + end + + it "rejects a member with reporter access from deleting a milestone" do + delete api("/projects/#{project.id}/milestones/#{milestone.id}", reporter) + + expect(response).to have_gitlab_http_status(403) + end + + it 'deletes the milestone when the user has developer access to the project' do + delete api("/projects/#{project.id}/milestones/#{milestone.id}", user) + + expect(project.milestones.find_by_id(milestone.id)).to be_nil + expect(response).to have_gitlab_http_status(204) + end + end + describe 'PUT /projects/:id/milestones/:milestone_id to test observer on close' do it 'creates an activity event when an milestone is closed' do expect(Event).to receive(:create!) diff --git a/spec/requests/api/project_snippets_spec.rb b/spec/requests/api/project_snippets_spec.rb index e741ac4b7bd..4a2289ca137 100644 --- a/spec/requests/api/project_snippets_spec.rb +++ b/spec/requests/api/project_snippets_spec.rb @@ -1,9 +1,9 @@ require 'rails_helper' describe API::ProjectSnippets do - let(:project) { create(:project, :public) } - let(:user) { create(:user) } - let(:admin) { create(:admin) } + set(:project) { create(:project, :public) } + set(:user) { create(:user) } + set(:admin) { create(:admin) } describe "GET /projects/:project_id/snippets/:id/user_agent_detail" do let(:snippet) { create(:project_snippet, :public, project: project) } @@ -18,6 +18,13 @@ describe API::ProjectSnippets do expect(json_response['akismet_submitted']).to eq(user_agent_detail.submitted) end + it 'respects project scoping' do + other_project = create(:project) + + get api("/projects/#{other_project.id}/snippets/#{snippet.id}/user_agent_detail", admin) + expect(response).to have_gitlab_http_status(404) + end + it "returns unautorized for non-admin users" do get api("/projects/#{snippet.project.id}/snippets/#{snippet.id}/user_agent_detail", user) diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index de1763015fa..97e7ffcd38e 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -150,6 +150,19 @@ describe API::Projects do expect(json_response.find { |hash| hash['id'] == project.id }.keys).not_to include('open_issues_count') end + context 'and with_issues_enabled=true' do + it 'only returns projects with issues enabled' do + project.project_feature.update_attribute(:issues_access_level, ProjectFeature::DISABLED) + + get api('/projects?with_issues_enabled=true', user) + + expect(response.status).to eq 200 + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + expect(json_response.map { |p| p['id'] }).not_to include(project.id) + end + end + it "does not include statistics by default" do get api('/projects', user) @@ -352,6 +365,19 @@ describe API::Projects do let(:current_user) { user2 } let(:projects) { [public_project] } end + + context 'and with_issues_enabled=true' do + it 'does not return private issue projects' do + project.project_feature.update_attribute(:issues_access_level, ProjectFeature::PRIVATE) + + get api('/projects?with_issues_enabled=true', user2) + + expect(response.status).to eq 200 + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + expect(json_response.map { |p| p['id'] }).not_to include(project.id) + end + end end context 'when authenticated as admin' do diff --git a/spec/requests/api/protected_branches_spec.rb b/spec/requests/api/protected_branches_spec.rb index 10e6a3c07c8..1d23e023bb6 100644 --- a/spec/requests/api/protected_branches_spec.rb +++ b/spec/requests/api/protected_branches_spec.rb @@ -80,6 +80,12 @@ describe API::ProtectedBranches do it_behaves_like 'protected branch' end + + context 'when protected branch contains a period' do + let(:protected_name) { 'my.feature' } + + it_behaves_like 'protected branch' + end end context 'when authenticated as a guest' do diff --git a/spec/requests/api/runner_spec.rb b/spec/requests/api/runner_spec.rb index d68bba0082c..42631499ca2 100644 --- a/spec/requests/api/runner_spec.rb +++ b/spec/requests/api/runner_spec.rb @@ -1150,6 +1150,7 @@ describe API::Runner do else { 'file' => file } end + post api("/jobs/#{job.id}/artifacts"), params, headers end end diff --git a/spec/requests/api/services_spec.rb b/spec/requests/api/services_spec.rb index 26d56c04862..236f8d7faf5 100644 --- a/spec/requests/api/services_spec.rb +++ b/spec/requests/api/services_spec.rb @@ -83,14 +83,14 @@ describe API::Services do get api("/projects/#{project.id}/services/#{dashed_service}", admin) expect(response).to have_gitlab_http_status(200) - expect(json_response['properties'].keys.map(&:to_sym)).to match_array(service_attrs_list.map) + expect(json_response['properties'].keys).to match_array(service_instance.api_field_names) end it "returns properties of service #{service} other than passwords when authenticated as project owner" do get api("/projects/#{project.id}/services/#{dashed_service}", user) expect(response).to have_gitlab_http_status(200) - expect(json_response['properties'].keys.map(&:to_sym)).to match_array(service_attrs_list_without_passwords) + expect(json_response['properties'].keys).to match_array(service_instance.api_field_names) end it "returns error when authenticated but not a project owner" do diff --git a/spec/requests/api/system_hooks_spec.rb b/spec/requests/api/system_hooks_spec.rb index c7a009e999e..6c57d443cbf 100644 --- a/spec/requests/api/system_hooks_spec.rb +++ b/spec/requests/api/system_hooks_spec.rb @@ -36,6 +36,7 @@ describe API::SystemHooks do expect(json_response.first['url']).to eq(hook.url) expect(json_response.first['push_events']).to be false expect(json_response.first['tag_push_events']).to be false + expect(json_response.first['merge_requests_events']).to be false expect(json_response.first['repository_update_events']).to be true end end @@ -67,11 +68,28 @@ describe API::SystemHooks do end it 'sets default values for events' do - post api('/hooks', admin), url: 'http://mep.mep', enable_ssl_verification: true + post api('/hooks', admin), url: 'http://mep.mep' expect(response).to have_gitlab_http_status(201) expect(json_response['enable_ssl_verification']).to be true + expect(json_response['push_events']).to be false expect(json_response['tag_push_events']).to be false + expect(json_response['merge_requests_events']).to be false + end + + it 'sets explicit values for events' do + post api('/hooks', admin), + url: 'http://mep.mep', + enable_ssl_verification: false, + push_events: true, + tag_push_events: true, + merge_requests_events: true + + expect(response).to have_http_status(201) + expect(json_response['enable_ssl_verification']).to be false + expect(json_response['push_events']).to be true + expect(json_response['tag_push_events']).to be true + expect(json_response['merge_requests_events']).to be true end end diff --git a/spec/requests/api/v3/builds_spec.rb b/spec/requests/api/v3/builds_spec.rb index c6094c5ca5c..00f067889a0 100644 --- a/spec/requests/api/v3/builds_spec.rb +++ b/spec/requests/api/v3/builds_spec.rb @@ -4,20 +4,23 @@ describe API::V3::Builds do set(:user) { create(:user) } let(:api_user) { user } set(:project) { create(:project, :repository, creator: user, public_builds: false) } - set(:developer) { create(:project_member, :developer, user: user, project: project) } - set(:reporter) { create(:project_member, :reporter, project: project) } - set(:guest) { create(:project_member, :guest, project: project) } - set(:pipeline) { create(:ci_empty_pipeline, project: project, sha: project.commit.id, ref: project.default_branch) } - let!(:build) { create(:ci_build, pipeline: pipeline) } + let!(:developer) { create(:project_member, :developer, user: user, project: project) } + let(:reporter) { create(:project_member, :reporter, project: project) } + let(:guest) { create(:project_member, :guest, project: project) } + let(:pipeline) { create(:ci_empty_pipeline, project: project, sha: project.commit.id, ref: project.default_branch) } + let(:build) { create(:ci_build, pipeline: pipeline) } describe 'GET /projects/:id/builds ' do let(:query) { '' } - before do + before do |example| build + create(:ci_build, :skipped, pipeline: pipeline) - get v3_api("/projects/#{project.id}/builds?#{query}", api_user) + unless example.metadata[:skip_before_request] + get v3_api("/projects/#{project.id}/builds?#{query}", api_user) + end end context 'authorized user' do @@ -41,6 +44,23 @@ describe API::V3::Builds do expect(json_build['pipeline']['status']).to eq build.pipeline.status end + it 'avoids N+1 queries', :skip_before_request do + first_build = create(:ci_build, :artifacts, pipeline: pipeline) + first_build.runner = create(:ci_runner) + first_build.user = create(:user) + first_build.save + + control_count = ActiveRecord::QueryRecorder.new { go }.count + + second_pipeline = create(:ci_empty_pipeline, project: project, sha: project.commit.id, ref: project.default_branch) + second_build = create(:ci_build, :artifacts, pipeline: second_pipeline) + second_build.runner = create(:ci_runner) + second_build.user = create(:user) + second_build.save + + expect { go }.not_to exceed_query_limit(control_count) + end + context 'filter project with one scope element' do let(:query) { 'scope=pending' } @@ -85,6 +105,10 @@ describe API::V3::Builds do expect(response).to have_gitlab_http_status(401) end end + + def go + get v3_api("/projects/#{project.id}/builds?#{query}", api_user) + end end describe 'GET /projects/:id/repository/commits/:sha/builds' do diff --git a/spec/requests/api/v3/commits_spec.rb b/spec/requests/api/v3/commits_spec.rb index 8b115e01f47..9ef3b859001 100644 --- a/spec/requests/api/v3/commits_spec.rb +++ b/spec/requests/api/v3/commits_spec.rb @@ -36,7 +36,7 @@ describe API::V3::Commits do context "since optional parameter" do it "returns project commits since provided parameter" do - commits = project.repository.commits("master") + commits = project.repository.commits("master", limit: 2) since = commits.second.created_at get v3_api("/projects/#{project.id}/repository/commits?since=#{since.utc.iso8601}", user) @@ -49,12 +49,12 @@ describe API::V3::Commits do context "until optional parameter" do it "returns project commits until provided parameter" do - commits = project.repository.commits("master") + commits = project.repository.commits("master", limit: 20) before = commits.second.created_at get v3_api("/projects/#{project.id}/repository/commits?until=#{before.utc.iso8601}", user) - if commits.size >= 20 + if commits.size == 20 expect(json_response.size).to eq(20) else expect(json_response.size).to eq(commits.size - 1) @@ -403,6 +403,33 @@ describe API::V3::Commits do expect(response).to have_gitlab_http_status(200) expect(json_response['status']).to eq("created") end + + context 'when stat param' do + let(:project_id) { project.id } + let(:commit_id) { project.repository.commit.id } + let(:route) { "/projects/#{project_id}/repository/commits/#{commit_id}" } + + it 'is not present return stats by default' do + get v3_api(route, user) + + expect(response).to have_gitlab_http_status(200) + expect(json_response).to include 'stats' + end + + it "is false it does not include stats" do + get v3_api(route, user), stats: false + + expect(response).to have_gitlab_http_status(200) + expect(json_response).not_to include 'stats' + end + + it "is true it includes stats" do + get v3_api(route, user), stats: true + + expect(response).to have_gitlab_http_status(200) + expect(json_response).to include 'stats' + end + end end context "unauthorized user" do diff --git a/spec/requests/api/v3/deploy_keys_spec.rb b/spec/requests/api/v3/deploy_keys_spec.rb index 785bc1eb4ba..501af587ad4 100644 --- a/spec/requests/api/v3/deploy_keys_spec.rb +++ b/spec/requests/api/v3/deploy_keys_spec.rb @@ -107,7 +107,7 @@ describe API::V3::DeployKeys do end it 'accepts can_push parameter' do - key_attrs = attributes_for :write_access_key + key_attrs = attributes_for(:another_key).merge(can_push: true) post v3_api("/projects/#{project.id}/#{path}", admin), key_attrs diff --git a/spec/requests/api/v3/members_spec.rb b/spec/requests/api/v3/members_spec.rb index b91782ae511..de4339ecb8b 100644 --- a/spec/requests/api/v3/members_spec.rb +++ b/spec/requests/api/v3/members_spec.rb @@ -58,6 +58,16 @@ describe API::V3::Members do expect(json_response.count).to eq(1) expect(json_response.first['username']).to eq(master.username) end + + it 'finds all members with no query specified' do + get v3_api("/#{source_type.pluralize}/#{source.id}/members", developer), query: '' + + expect(response).to have_gitlab_http_status(200) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + expect(json_response.count).to eq(2) + expect(json_response.map { |u| u['id'] }).to match_array [master.id, developer.id] + end end end diff --git a/spec/requests/api/v3/merge_requests_spec.rb b/spec/requests/api/v3/merge_requests_spec.rb index b8b7d9d1c40..6b748369f0d 100644 --- a/spec/requests/api/v3/merge_requests_spec.rb +++ b/spec/requests/api/v3/merge_requests_spec.rb @@ -371,16 +371,28 @@ describe API::MergeRequests do expect(response).to have_gitlab_http_status(400) end - context 'when target_branch is specified' do + context 'when target_branch and target_project_id is specified' do + let(:params) do + { title: 'Test merge_request', + target_branch: 'master', + source_branch: 'markdown', + author: user2, + target_project_id: unrelated_project.id } + end + it 'returns 422 if targeting a different fork' do - post v3_api("/projects/#{forked_project.id}/merge_requests", user2), - title: 'Test merge_request', - target_branch: 'master', - source_branch: 'markdown', - author: user2, - target_project_id: unrelated_project.id + unrelated_project.add_developer(user2) + + post v3_api("/projects/#{forked_project.id}/merge_requests", user2), params + expect(response).to have_gitlab_http_status(422) end + + it 'returns 403 if targeting a different fork which user can not access' do + post v3_api("/projects/#{forked_project.id}/merge_requests", user2), params + + expect(response).to have_gitlab_http_status(403) + end end it "returns 201 when target_branch is specified and for the same project" do diff --git a/spec/requests/lfs_http_spec.rb b/spec/requests/lfs_http_spec.rb index 0a8788fd57e..e26fe39ce7f 100644 --- a/spec/requests/lfs_http_spec.rb +++ b/spec/requests/lfs_http_spec.rb @@ -798,11 +798,11 @@ describe 'Git LFS API and storage' do end context 'when deploy key has project push access' do - let(:key) { create(:deploy_key, can_push: true) } + let(:key) { create(:deploy_key) } let(:authorization) { authorize_deploy_key } let(:update_user_permissions) do - project.deploy_keys << key + project.deploy_keys_projects.create(deploy_key: key, can_push: true) end it_behaves_like 'pushes new LFS objects' |