diff options
Diffstat (limited to 'spec')
20 files changed, 381 insertions, 120 deletions
diff --git a/spec/controllers/admin/clusters_controller_spec.rb b/spec/controllers/admin/clusters_controller_spec.rb index 2e0ee671d3f..d2a569a9d48 100644 --- a/spec/controllers/admin/clusters_controller_spec.rb +++ b/spec/controllers/admin/clusters_controller_spec.rb @@ -99,7 +99,9 @@ RSpec.describe Admin::ClustersController do end describe 'GET #new' do - def get_new(provider: 'gcp') + let(:user) { admin } + + def go(provider: 'gcp') get :new, params: { provider: provider } end @@ -112,7 +114,7 @@ RSpec.describe Admin::ClustersController do context 'when selected provider is gke and no valid gcp token exists' do it 'redirects to gcp authorize_url' do - get_new + go expect(response).to redirect_to(assigns(:authorize_url)) end @@ -125,7 +127,7 @@ RSpec.describe Admin::ClustersController do end it 'does not have authorize_url' do - get_new + go expect(assigns(:authorize_url)).to be_nil end @@ -137,7 +139,7 @@ RSpec.describe Admin::ClustersController do end it 'has new object' do - get_new + go expect(assigns(:gcp_cluster)).to be_an_instance_of(Clusters::ClusterPresenter) end @@ -158,16 +160,18 @@ RSpec.describe Admin::ClustersController do describe 'functionality for existing cluster' do it 'has new object' do - get_new + go expect(assigns(:user_cluster)).to be_an_instance_of(Clusters::ClusterPresenter) end end + include_examples 'GET new cluster shared examples' + describe 'security' do - it { expect { get_new }.to be_allowed_for(:admin) } - it { expect { get_new }.to be_denied_for(:user) } - it { expect { get_new }.to be_denied_for(:external) } + it { expect { go }.to be_allowed_for(:admin) } + it { expect { go }.to be_denied_for(:user) } + it { expect { go }.to be_denied_for(:external) } end end @@ -424,14 +428,13 @@ RSpec.describe Admin::ClustersController do end describe 'POST authorize AWS role for EKS cluster' do - let(:role_arn) { 'arn:aws:iam::123456789012:role/role-name' } - let(:role_external_id) { '12345' } + let!(:role) { create(:aws_role, user: admin) } + let(:role_arn) { 'arn:new-role' } let(:params) do { cluster: { - role_arn: role_arn, - role_external_id: role_external_id + role_arn: role_arn } } end @@ -445,28 +448,32 @@ RSpec.describe Admin::ClustersController do .and_return(double(execute: double)) end - it 'creates an Aws::Role record' do - expect { go }.to change { Aws::Role.count } + it 'updates the associated role with the supplied ARN' do + go expect(response).to have_gitlab_http_status(:ok) - - role = Aws::Role.last - expect(role.user).to eq admin - expect(role.role_arn).to eq role_arn - expect(role.role_external_id).to eq role_external_id + expect(role.reload.role_arn).to eq(role_arn) end - context 'role cannot be created' do + context 'supplied role is invalid' do let(:role_arn) { 'invalid-role' } - it 'does not create a record' do - expect { go }.not_to change { Aws::Role.count } + it 'does not update the associated role' do + expect { go }.not_to change { role.role_arn } expect(response).to have_gitlab_http_status(:unprocessable_entity) end end describe 'security' do + before do + allow_next_instance_of(Clusters::Aws::AuthorizeRoleService) do |service| + response = double(status: :ok, body: double) + + allow(service).to receive(:execute).and_return(response) + end + end + it { expect { go }.to be_allowed_for(:admin) } it { expect { go }.to be_denied_for(:user) } it { expect { go }.to be_denied_for(:external) } diff --git a/spec/controllers/groups/clusters_controller_spec.rb b/spec/controllers/groups/clusters_controller_spec.rb index 1593e1290c4..81d5bc7770f 100644 --- a/spec/controllers/groups/clusters_controller_spec.rb +++ b/spec/controllers/groups/clusters_controller_spec.rb @@ -180,6 +180,8 @@ RSpec.describe Groups::ClustersController do end end + include_examples 'GET new cluster shared examples' + describe 'security' do it { expect { go }.to be_allowed_for(:admin) } it { expect { go }.to be_allowed_for(:owner).of(group) } @@ -493,14 +495,13 @@ RSpec.describe Groups::ClustersController do end describe 'POST authorize AWS role for EKS cluster' do - let(:role_arn) { 'arn:aws:iam::123456789012:role/role-name' } - let(:role_external_id) { '12345' } + let!(:role) { create(:aws_role, user: user) } + let(:role_arn) { 'arn:new-role' } let(:params) do { cluster: { - role_arn: role_arn, - role_external_id: role_external_id + role_arn: role_arn } } end @@ -514,28 +515,32 @@ RSpec.describe Groups::ClustersController do .and_return(double(execute: double)) end - it 'creates an Aws::Role record' do - expect { go }.to change { Aws::Role.count } + it 'updates the associated role with the supplied ARN' do + go expect(response).to have_gitlab_http_status(:ok) - - role = Aws::Role.last - expect(role.user).to eq user - expect(role.role_arn).to eq role_arn - expect(role.role_external_id).to eq role_external_id + expect(role.reload.role_arn).to eq(role_arn) end - context 'role cannot be created' do + context 'supplied role is invalid' do let(:role_arn) { 'invalid-role' } - it 'does not create a record' do - expect { go }.not_to change { Aws::Role.count } + it 'does not update the associated role' do + expect { go }.not_to change { role.role_arn } expect(response).to have_gitlab_http_status(:unprocessable_entity) end end describe 'security' do + before do + allow_next_instance_of(Clusters::Aws::AuthorizeRoleService) do |service| + response = double(status: :ok, body: double) + + allow(service).to receive(:execute).and_return(response) + end + end + it { expect { go }.to be_allowed_for(:admin) } it { expect { go }.to be_allowed_for(:owner).of(group) } it { expect { go }.to be_allowed_for(:maintainer).of(group) } diff --git a/spec/controllers/projects/clusters_controller_spec.rb b/spec/controllers/projects/clusters_controller_spec.rb index da4faad2a39..51a451570c5 100644 --- a/spec/controllers/projects/clusters_controller_spec.rb +++ b/spec/controllers/projects/clusters_controller_spec.rb @@ -183,6 +183,8 @@ RSpec.describe Projects::ClustersController do end end + include_examples 'GET new cluster shared examples' + describe 'security' do it 'is allowed for admin when admin mode enabled', :enable_admin_mode do expect { go }.to be_allowed_for(:admin) @@ -521,14 +523,13 @@ RSpec.describe Projects::ClustersController do end describe 'POST authorize AWS role for EKS cluster' do - let(:role_arn) { 'arn:aws:iam::123456789012:role/role-name' } - let(:role_external_id) { '12345' } + let!(:role) { create(:aws_role, user: user) } + let(:role_arn) { 'arn:new-role' } let(:params) do { cluster: { - role_arn: role_arn, - role_external_id: role_external_id + role_arn: role_arn } } end @@ -542,28 +543,32 @@ RSpec.describe Projects::ClustersController do .and_return(double(execute: double)) end - it 'creates an Aws::Role record' do - expect { go }.to change { Aws::Role.count } + it 'updates the associated role with the supplied ARN' do + go expect(response).to have_gitlab_http_status(:ok) - - role = Aws::Role.last - expect(role.user).to eq user - expect(role.role_arn).to eq role_arn - expect(role.role_external_id).to eq role_external_id + expect(role.reload.role_arn).to eq(role_arn) end - context 'role cannot be created' do + context 'supplied role is invalid' do let(:role_arn) { 'invalid-role' } - it 'does not create a record' do - expect { go }.not_to change { Aws::Role.count } + it 'does not update the associated role' do + expect { go }.not_to change { role.role_arn } expect(response).to have_gitlab_http_status(:unprocessable_entity) end end describe 'security' do + before do + allow_next_instance_of(Clusters::Aws::AuthorizeRoleService) do |service| + response = double(status: :ok, body: double) + + allow(service).to receive(:execute).and_return(response) + end + end + it 'is allowed for admin when admin mode enabled', :enable_admin_mode do expect { go }.to be_allowed_for(:admin) end diff --git a/spec/finders/ci/auth_job_finder_spec.rb b/spec/finders/ci/auth_job_finder_spec.rb new file mode 100644 index 00000000000..6cd58f5cd01 --- /dev/null +++ b/spec/finders/ci/auth_job_finder_spec.rb @@ -0,0 +1,64 @@ +# frozen_string_literal: true +require 'spec_helper' + +RSpec.describe Ci::AuthJobFinder do + let_it_be(:job, reload: true) { create(:ci_build, status: :running) } + + let(:token) { job.token } + + subject(:finder) do + described_class.new(token: token) + end + + describe '#execute!' do + subject(:execute) { finder.execute! } + + it { is_expected.to eq(job) } + + it 'raises error if the job is not running' do + job.success! + + expect { execute }.to raise_error described_class::NotRunningJobError, 'Job is not running' + end + + it 'raises error if the job is erased' do + expect(::Ci::Build).to receive(:find_by_token).with(job.token).and_return(job) + expect(job).to receive(:erased?).and_return(true) + + expect { execute }.to raise_error described_class::ErasedJobError, 'Job has been erased!' + end + + it 'raises error if the the project is missing' do + expect(::Ci::Build).to receive(:find_by_token).with(job.token).and_return(job) + expect(job).to receive(:project).and_return(nil) + + expect { execute }.to raise_error described_class::DeletedProjectError, 'Project has been deleted!' + end + + it 'raises error if the the project is being removed' do + project = double(Project) + + expect(::Ci::Build).to receive(:find_by_token).with(job.token).and_return(job) + expect(job).to receive(:project).twice.and_return(project) + expect(project).to receive(:pending_delete?).and_return(true) + + expect { execute }.to raise_error described_class::DeletedProjectError, 'Project has been deleted!' + end + + context 'with wrong job token' do + let(:token) { 'missing' } + + it { is_expected.to be_nil } + end + end + + describe '#execute' do + subject(:execute) { finder.execute } + + before do + job.success! + end + + it { is_expected.to be_nil } + end +end diff --git a/spec/lib/api/helpers/packages_manager_clients_helpers_spec.rb b/spec/lib/api/helpers/packages_manager_clients_helpers_spec.rb index 832f4abe545..73b67f9e61c 100644 --- a/spec/lib/api/helpers/packages_manager_clients_helpers_spec.rb +++ b/spec/lib/api/helpers/packages_manager_clients_helpers_spec.rb @@ -11,7 +11,7 @@ RSpec.describe API::Helpers::PackagesManagerClientsHelpers do describe '#find_job_from_http_basic_auth' do let_it_be(:user) { personal_access_token.user } - let(:job) { create(:ci_build, user: user) } + let(:job) { create(:ci_build, user: user, status: :running) } let(:password) { job.token } let(:headers) { { Authorization: basic_http_auth(username, password) } } @@ -23,6 +23,14 @@ RSpec.describe API::Helpers::PackagesManagerClientsHelpers do context 'with a valid Authorization header' do it { is_expected.to eq job } + + context 'when the job is not running' do + before do + job.update!(status: :failed) + end + + it { is_expected.to be nil } + end end context 'with an invalid Authorization header' do diff --git a/spec/lib/gitlab/auth/auth_finders_spec.rb b/spec/lib/gitlab/auth/auth_finders_spec.rb index a73ac0b34af..1ac8ebe1369 100644 --- a/spec/lib/gitlab/auth/auth_finders_spec.rb +++ b/spec/lib/gitlab/auth/auth_finders_spec.rb @@ -37,11 +37,29 @@ RSpec.describe Gitlab::Auth::AuthFinders do expect { subject }.to raise_error(Gitlab::Auth::UnauthorizedError) end - it "return user if token is valid" do - set_token(job.token) + context 'with a running job' do + before do + job.update!(status: :running) + end + + it 'return user if token is valid' do + set_token(job.token) + + expect(subject).to eq(user) + expect(@current_authenticated_job).to eq job + end + end - expect(subject).to eq(user) - expect(@current_authenticated_job).to eq job + context 'with a job that is not running' do + before do + job.update!(status: :failed) + end + + it 'returns an Unauthorized exception' do + set_token(job.token) + + expect { subject }.to raise_error(Gitlab::Auth::UnauthorizedError) + end end end end @@ -557,7 +575,7 @@ RSpec.describe Gitlab::Auth::AuthFinders do context 'with CI username' do let(:username) { ::Gitlab::Auth::CI_JOB_USER } let(:user) { create(:user) } - let(:build) { create(:ci_build, user: user) } + let(:build) { create(:ci_build, user: user, status: :running) } it 'returns nil without password' do set_basic_auth_header(username, nil) @@ -576,6 +594,13 @@ RSpec.describe Gitlab::Auth::AuthFinders do expect { subject }.to raise_error(Gitlab::Auth::UnauthorizedError) end + + it 'returns exception if the job is not running' do + set_basic_auth_header(username, build.token) + build.success! + + expect { subject }.to raise_error(Gitlab::Auth::UnauthorizedError) + end end end @@ -586,7 +611,7 @@ RSpec.describe Gitlab::Auth::AuthFinders do context 'with a job token' do let(:route_authentication_setting) { { job_token_allowed: true } } - let(:job) { create(:ci_build, user: user) } + let(:job) { create(:ci_build, user: user, status: :running) } before do env['HTTP_AUTHORIZATION'] = "Bearer #{job.token}" @@ -641,7 +666,7 @@ RSpec.describe Gitlab::Auth::AuthFinders do end describe '#find_user_from_job_token' do - let(:job) { create(:ci_build, user: user) } + let(:job) { create(:ci_build, user: user, status: :running) } let(:route_authentication_setting) { { job_token_allowed: true } } subject { find_user_from_job_token } @@ -666,6 +691,13 @@ RSpec.describe Gitlab::Auth::AuthFinders do expect { subject }.to raise_error(Gitlab::Auth::UnauthorizedError) end + it 'returns exception if the job is not running' do + set_header(described_class::JOB_TOKEN_HEADER, job.token) + job.success! + + expect { subject }.to raise_error(Gitlab::Auth::UnauthorizedError) + end + context 'when route is not allowed to be authenticated' do let(:route_authentication_setting) { { job_token_allowed: false } } diff --git a/spec/lib/gitlab/auth/request_authenticator_spec.rb b/spec/lib/gitlab/auth/request_authenticator_spec.rb index ef83321cc0e..b89ceb37076 100644 --- a/spec/lib/gitlab/auth/request_authenticator_spec.rb +++ b/spec/lib/gitlab/auth/request_authenticator_spec.rb @@ -88,7 +88,7 @@ RSpec.describe Gitlab::Auth::RequestAuthenticator do describe '#find_user_from_job_token' do let!(:user) { build(:user) } - let!(:job) { build(:ci_build, user: user) } + let!(:job) { build(:ci_build, user: user, status: :running) } before do env[Gitlab::Auth::AuthFinders::JOB_TOKEN_HEADER] = 'token' @@ -97,13 +97,18 @@ RSpec.describe Gitlab::Auth::RequestAuthenticator do context 'with API requests' do before do env['SCRIPT_NAME'] = '/api/endpoint' + expect(::Ci::Build).to receive(:find_by_token).with('token').and_return(job) end it 'tries to find the user' do - expect(::Ci::Build).to receive(:find_by_token).and_return(job) - expect(subject.find_sessionless_user([:api])).to eq user end + + it 'returns nil if the job is not running' do + job.status = :success + + expect(subject.find_sessionless_user([:api])).to be_blank + end end context 'without API requests' do diff --git a/spec/lib/gitlab/regex_spec.rb b/spec/lib/gitlab/regex_spec.rb index 1a6858858a7..afa930b795a 100644 --- a/spec/lib/gitlab/regex_spec.rb +++ b/spec/lib/gitlab/regex_spec.rb @@ -180,15 +180,6 @@ RSpec.describe Gitlab::Regex do it { is_expected.not_to match('foo/bar') } end - describe '.conan_file_name_regex' do - subject { described_class.conan_file_name_regex } - - it { is_expected.to match('conanfile.py') } - it { is_expected.to match('conan_package.tgz') } - it { is_expected.not_to match('foo.txt') } - it { is_expected.not_to match('!!()()') } - end - describe '.conan_package_reference_regex' do subject { described_class.conan_package_reference_regex } diff --git a/spec/models/aws/role_spec.rb b/spec/models/aws/role_spec.rb index 612868f6eb0..ee93c9d6fad 100644 --- a/spec/models/aws/role_spec.rb +++ b/spec/models/aws/role_spec.rb @@ -29,6 +29,12 @@ RSpec.describe Aws::Role do it { is_expected.to be_truthy } end + + context 'ARN is nil' do + let(:role_arn) { } + + it { is_expected.to be_truthy } + end end end diff --git a/spec/requests/api/badges_spec.rb b/spec/requests/api/badges_spec.rb index 99d224cb8e9..d8a345a79b0 100644 --- a/spec/requests/api/badges_spec.rb +++ b/spec/requests/api/badges_spec.rb @@ -332,10 +332,32 @@ RSpec.describe API::Badges do context 'when deleting a badge' do context 'and the source is a project' do + let(:badge) { project.group.badges.first } + it 'cannot delete badges owned by the project group' do - delete api("/projects/#{project.id}/badges/#{project_group.badges.first.id}", maintainer) + expect do + delete api("/projects/#{project.id}/badges/#{badge.id}", maintainer) + + expect(response).to have_gitlab_http_status(:not_found) + end.not_to change { badge.reload.persisted? } + end + end + end + + context 'when updating a badge' do + context 'and the source is a project' do + let(:badge) { project.group.badges.first } + let(:example_name) { 'BadgeName' } + let(:example_url) { 'http://www.example.com' } + let(:example_url2) { 'http://www.example1.com' } + + it 'cannot update badges owned by the project group' do + expect do + put api("/projects/#{project.id}/badges/#{badge.id}", maintainer), + params: { name: example_name, link_url: example_url, image_url: example_url2 } - expect(response).to have_gitlab_http_status(:forbidden) + expect(response).to have_gitlab_http_status(:not_found) + end.not_to change { badge.reload.updated_at } end end end diff --git a/spec/requests/api/conan_packages_spec.rb b/spec/requests/api/conan_packages_spec.rb index 95798b060f1..7a97743ede1 100644 --- a/spec/requests/api/conan_packages_spec.rb +++ b/spec/requests/api/conan_packages_spec.rb @@ -13,7 +13,7 @@ RSpec.describe API::ConanPackages do let(:base_secret) { SecureRandom.base64(64) } let(:auth_token) { personal_access_token.token } - let(:job) { create(:ci_build, user: user) } + let(:job) { create(:ci_build, user: user, status: :running) } let(:job_token) { job.token } let(:deploy_token) { create(:deploy_token, read_package_registry: true, write_package_registry: true) } let(:project_deploy_token) { create(:project_deploy_token, deploy_token: deploy_token, project: project) } @@ -93,6 +93,14 @@ RSpec.describe API::ConanPackages do expect(response).to have_gitlab_http_status(:unauthorized) end + it 'responds with 401 Unauthorized when the job is not running' do + job.update!(status: :failed) + jwt = build_jwt_from_job(job) + get api('/packages/conan/v1/ping'), headers: build_token_auth_header(jwt.encoded) + + expect(response).to have_gitlab_http_status(:unauthorized) + end + context 'packages feature disabled' do it 'responds with 404 Not Found' do stub_packages_setting(enabled: false) @@ -233,6 +241,18 @@ RSpec.describe API::ConanPackages do end end + shared_examples 'rejects invalid file_name' do |invalid_file_name| + let(:file_name) { invalid_file_name } + + context 'with invalid file_name' do + it 'returns 400' do + subject + + expect(response).to have_gitlab_http_status(:bad_request) + end + end + end + shared_examples 'rejects recipe for invalid project' do context 'with invalid recipe path' do let(:recipe_path) { 'aa/bb/not-existing-project/ccc' } @@ -685,8 +705,6 @@ RSpec.describe API::ConanPackages do context 'without a file from workhorse' do let(:params) { { file: nil } } - it_behaves_like 'package workhorse uploads' - it 'rejects the request' do subject @@ -694,6 +712,10 @@ RSpec.describe API::ConanPackages do end end + context 'with a file' do + it_behaves_like 'package workhorse uploads' + end + context 'without a token' do it 'rejects request without a token' do headers_with_token.delete('HTTP_AUTHORIZATION') @@ -852,16 +874,22 @@ RSpec.describe API::ConanPackages do end describe 'PUT /api/v4/packages/conan/v1/files/:package_name/package_version/:package_username/:package_channel/:recipe_revision/export/:file_name/authorize' do - subject { put api("/packages/conan/v1/files/#{recipe_path}/0/export/conanfile.py/authorize"), headers: headers_with_token } + let(:file_name) { 'conanfile.py' } + + subject { put api("/packages/conan/v1/files/#{recipe_path}/0/export/#{file_name}/authorize"), headers: headers_with_token } it_behaves_like 'rejects invalid recipe' + it_behaves_like 'rejects invalid file_name', 'conanfile.py.git%2fgit-upload-pack' it_behaves_like 'workhorse authorization' end describe 'PUT /api/v4/packages/conan/v1/files/:package_name/package_version/:package_username/:package_channel/:recipe_revision/export/:conan_package_reference/:package_revision/:file_name/authorize' do - subject { put api("/packages/conan/v1/files/#{recipe_path}/0/package/123456789/0/conaninfo.txt/authorize"), headers: headers_with_token } + let(:file_name) { 'conaninfo.txt' } + + subject { put api("/packages/conan/v1/files/#{recipe_path}/0/package/123456789/0/#{file_name}/authorize"), headers: headers_with_token } it_behaves_like 'rejects invalid recipe' + it_behaves_like 'rejects invalid file_name', 'conaninfo.txttest' it_behaves_like 'workhorse authorization' end @@ -875,11 +903,13 @@ RSpec.describe API::ConanPackages do method: :put, file_key: :file, params: params, + send_rewritten_field: true, headers: headers_with_token ) end it_behaves_like 'rejects invalid recipe' + it_behaves_like 'rejects invalid file_name', 'conanfile.py.git%2fgit-upload-pack' it_behaves_like 'uploads a package file' end @@ -893,12 +923,15 @@ RSpec.describe API::ConanPackages do method: :put, file_key: :file, params: params, - headers: headers_with_token + headers: headers_with_token, + send_rewritten_field: true ) end it_behaves_like 'rejects invalid recipe' + it_behaves_like 'rejects invalid file_name', 'conaninfo.txttest' it_behaves_like 'uploads a package file' + context 'tracking the conan_package.tgz upload' do let(:file_name) { ::Packages::Conan::FileMetadatum::PACKAGE_BINARY } diff --git a/spec/requests/api/go_proxy_spec.rb b/spec/requests/api/go_proxy_spec.rb index 2d7e319b0be..9d422ebbce2 100644 --- a/spec/requests/api/go_proxy_spec.rb +++ b/spec/requests/api/go_proxy_spec.rb @@ -11,7 +11,7 @@ RSpec.describe API::GoProxy do let_it_be(:base) { "#{Settings.build_gitlab_go_url}/#{project.full_path}" } let_it_be(:oauth) { create :oauth_access_token, scopes: 'api', resource_owner: user } - let_it_be(:job) { create :ci_build, user: user } + let_it_be(:job) { create :ci_build, user: user, status: :running } let_it_be(:pa_token) { create :personal_access_token, user: user } let_it_be(:modules) do @@ -393,6 +393,13 @@ RSpec.describe API::GoProxy do expect(response).to have_gitlab_http_status(:ok) end + it 'returns unauthorized with a failed job token' do + job.update!(status: :failed) + get_resource(oauth_access_token: job) + + expect(response).to have_gitlab_http_status(:unauthorized) + end + it 'returns unauthorized with no authentication' do get_resource diff --git a/spec/requests/api/maven_packages_spec.rb b/spec/requests/api/maven_packages_spec.rb index b9351308545..b74887762a2 100644 --- a/spec/requests/api/maven_packages_spec.rb +++ b/spec/requests/api/maven_packages_spec.rb @@ -12,7 +12,7 @@ RSpec.describe API::MavenPackages do let_it_be(:package_file) { package.package_files.with_file_name_like('%.xml').first } let_it_be(:jar_file) { package.package_files.with_file_name_like('%.jar').first } let_it_be(:personal_access_token) { create(:personal_access_token, user: user) } - let_it_be(:job) { create(:ci_build, user: user) } + let_it_be(:job, reload: true) { create(:ci_build, user: user, status: :running) } let_it_be(:deploy_token) { create(:deploy_token, read_package_registry: true, write_package_registry: true) } let_it_be(:project_deploy_token) { create(:project_deploy_token, deploy_token: deploy_token, project: project) } @@ -102,11 +102,25 @@ RSpec.describe API::MavenPackages do end shared_examples 'downloads with a job token' do - it 'allows download with job token' do - download_file(package_file.file_name, job_token: job.token) + context 'with a running job' do + it 'allows download with job token' do + download_file(package_file.file_name, job_token: job.token) - expect(response).to have_gitlab_http_status(:ok) - expect(response.media_type).to eq('application/octet-stream') + expect(response).to have_gitlab_http_status(:ok) + expect(response.media_type).to eq('application/octet-stream') + end + end + + context 'with a finished job' do + before do + job.update!(status: :failed) + end + + it 'returns unauthorized error' do + download_file(package_file.file_name, job_token: job.token) + + expect(response).to have_gitlab_http_status(:unauthorized) + end end end @@ -557,13 +571,20 @@ RSpec.describe API::MavenPackages do expect(jar_file.file_name).to eq(file_upload.original_filename) end - it 'allows upload with job token' do + it 'allows upload with running job token' do upload_file(params.merge(job_token: job.token)) expect(response).to have_gitlab_http_status(:ok) expect(project.reload.packages.last.build_info.pipeline).to eq job.pipeline end + it 'rejects upload without running job token' do + job.update!(status: :failed) + upload_file(params.merge(job_token: job.token)) + + expect(response).to have_gitlab_http_status(:unauthorized) + end + it 'allows upload with deploy token' do upload_file(params, headers_with_deploy_token) diff --git a/spec/requests/api/npm_packages_spec.rb b/spec/requests/api/npm_packages_spec.rb index 94647123df0..108ea84b7e6 100644 --- a/spec/requests/api/npm_packages_spec.rb +++ b/spec/requests/api/npm_packages_spec.rb @@ -12,7 +12,7 @@ RSpec.describe API::NpmPackages do let_it_be(:package, reload: true) { create(:npm_package, project: project) } let_it_be(:token) { create(:oauth_access_token, scopes: 'api', resource_owner: user) } let_it_be(:personal_access_token) { create(:personal_access_token, user: user) } - let_it_be(:job) { create(:ci_build, user: user) } + let_it_be(:job, reload: true) { create(:ci_build, user: user, status: :running) } let_it_be(:deploy_token) { create(:deploy_token, read_package_registry: true, write_package_registry: true) } let_it_be(:project_deploy_token) { create(:project_deploy_token, deploy_token: deploy_token, project: project) } @@ -27,12 +27,19 @@ RSpec.describe API::NpmPackages do expect_a_valid_package_response end - it 'returns the package info with job token' do + it 'returns the package info with running job token' do get_package_with_job_token(package) expect_a_valid_package_response end + it 'denies request without running job token' do + job.update!(status: :success) + get_package_with_job_token(package) + + expect(response).to have_gitlab_http_status(:unauthorized) + end + it 'denies request without oauth token' do get_package(package) diff --git a/spec/requests/api/nuget_packages_spec.rb b/spec/requests/api/nuget_packages_spec.rb index ab537a61058..87c62ec41c6 100644 --- a/spec/requests/api/nuget_packages_spec.rb +++ b/spec/requests/api/nuget_packages_spec.rb @@ -80,7 +80,7 @@ RSpec.describe API::NugetPackages do end with_them do - let(:job) { user_token ? create(:ci_build, project: project, user: user) : double(token: 'wrong') } + let(:job) { user_token ? create(:ci_build, project: project, user: user, status: :running) : double(token: 'wrong') } let(:headers) { user_role == :anonymous ? {} : job_basic_auth_header(job) } subject { get api(url), headers: headers } diff --git a/spec/requests/api/releases_spec.rb b/spec/requests/api/releases_spec.rb index a9a92a4d3cd..779ae983886 100644 --- a/spec/requests/api/releases_spec.rb +++ b/spec/requests/api/releases_spec.rb @@ -671,12 +671,20 @@ RSpec.describe API::Releases do end context 'when a valid token is provided' do - it 'creates the release' do + it 'creates the release for a running job' do + job.update!(status: :running) post api("/projects/#{project.id}/releases"), params: params.merge(job_token: job.token) expect(response).to have_gitlab_http_status(:created) expect(project.releases.last.description).to eq('Another nice release') end + + it 'returns an :unauthorized error for a completed job' do + job.success! + post api("/projects/#{project.id}/releases"), params: params.merge(job_token: job.token) + + expect(response).to have_gitlab_http_status(:unauthorized) + end end end diff --git a/spec/requests/api/terraform/state_spec.rb b/spec/requests/api/terraform/state_spec.rb index c6cba39314b..c47a12456c3 100644 --- a/spec/requests/api/terraform/state_spec.rb +++ b/spec/requests/api/terraform/state_spec.rb @@ -72,7 +72,7 @@ RSpec.describe API::Terraform::State do let(:auth_header) { job_basic_auth_header(job) } context 'with maintainer permissions' do - let(:job) { create(:ci_build, project: project, user: maintainer) } + let(:job) { create(:ci_build, status: :running, project: project, user: maintainer) } it 'returns terraform state belonging to a project of given state name' do request @@ -81,6 +81,13 @@ RSpec.describe API::Terraform::State do expect(response.body).to eq(state.file.read) end + it 'returns unauthorized if the the job is not running' do + job.update!(status: :failed) + request + + expect(response).to have_gitlab_http_status(:unauthorized) + end + context 'for a project that does not exist' do let(:project_id) { '0000' } @@ -93,7 +100,7 @@ RSpec.describe API::Terraform::State do end context 'with developer permissions' do - let(:job) { create(:ci_build, project: project, user: developer) } + let(:job) { create(:ci_build, status: :running, project: project, user: developer) } it 'returns terraform state belonging to a project of given state name' do request diff --git a/spec/services/ci/pipeline_trigger_service_spec.rb b/spec/services/ci/pipeline_trigger_service_spec.rb index 18fab9623ec..ac077e3c30e 100644 --- a/spec/services/ci/pipeline_trigger_service_spec.rb +++ b/spec/services/ci/pipeline_trigger_service_spec.rb @@ -106,9 +106,23 @@ RSpec.describe Ci::PipelineTriggerService do let(:params) { { token: job.token, ref: 'master', variables: nil } } let(:job) { create(:ci_build, :success, pipeline: pipeline, user: user) } - it 'does nothing' do + it 'does nothing', :aggregate_failures do + expect { result }.not_to change { Ci::Pipeline.count } + expect(result[:message]).to eq('Job is not running') + expect(result[:http_status]).to eq(401) + end + end + + context 'when job does not have a project' do + let(:params) { { token: job.token, ref: 'master', variables: nil } } + let(:job) { create(:ci_build, status: :running, pipeline: pipeline, user: user) } + + it 'does nothing', :aggregate_failures do + job.update!(project: nil) + expect { result }.not_to change { Ci::Pipeline.count } - expect(result[:message]).to eq('400 Job has to be running') + expect(result[:message]).to eq('Project has been deleted!') + expect(result[:http_status]).to eq(401) end end diff --git a/spec/services/clusters/aws/authorize_role_service_spec.rb b/spec/services/clusters/aws/authorize_role_service_spec.rb index 3d12400a47b..5b47cf0ecde 100644 --- a/spec/services/clusters/aws/authorize_role_service_spec.rb +++ b/spec/services/clusters/aws/authorize_role_service_spec.rb @@ -3,47 +3,34 @@ require 'spec_helper' RSpec.describe Clusters::Aws::AuthorizeRoleService do - let(:user) { create(:user) } + subject { described_class.new(user, params: params).execute } + + let(:role) { create(:aws_role) } + let(:user) { role.user } let(:credentials) { instance_double(Aws::Credentials) } let(:credentials_service) { instance_double(Clusters::Aws::FetchCredentialsService, execute: credentials) } + let(:role_arn) { 'arn:my-role' } let(:params) do params = ActionController::Parameters.new({ cluster: { - role_arn: 'arn:my-role', - role_external_id: 'external-id' + role_arn: role_arn } }) - params.require(:cluster).permit(:role_arn, :role_external_id) + params.require(:cluster).permit(:role_arn) end - subject { described_class.new(user, params: params).execute } - before do allow(Clusters::Aws::FetchCredentialsService).to receive(:new) .with(instance_of(Aws::Role)).and_return(credentials_service) end - context 'role does not exist' do - it 'creates an Aws::Role record and returns a set of credentials' do - expect(user).to receive(:create_aws_role!) - .with(params).and_call_original - - expect(subject.status).to eq(:ok) - expect(subject.body).to eq(credentials) - end - end - - context 'role already exists' do - let(:role) { create(:aws_role, user: user) } - + context 'role exists' do it 'updates the existing Aws::Role record and returns a set of credentials' do - expect(role).to receive(:update!) - .with(params).and_call_original - expect(subject.status).to eq(:ok) expect(subject.body).to eq(credentials) + expect(role.reload.role_arn).to eq(role_arn) end end @@ -61,11 +48,14 @@ RSpec.describe Clusters::Aws::AuthorizeRoleService do end end - context 'cannot create role' do - before do - allow(user).to receive(:create_aws_role!) - .and_raise(ActiveRecord::RecordInvalid.new(user)) - end + context 'role does not exist' do + let(:user) { create(:user) } + + include_examples 'bad request' + end + + context 'supplied ARN is invalid' do + let(:role_arn) { 'invalid' } include_examples 'bad request' end diff --git a/spec/support/shared_examples/controllers/clusters_controller_shared_examples.rb b/spec/support/shared_examples/controllers/clusters_controller_shared_examples.rb new file mode 100644 index 00000000000..aa17e72d08e --- /dev/null +++ b/spec/support/shared_examples/controllers/clusters_controller_shared_examples.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'GET new cluster shared examples' do + describe 'EKS cluster' do + context 'user already has an associated AWS role' do + let!(:role) { create(:aws_role, user: user) } + + it 'does not create an Aws::Role record' do + expect { go(provider: 'aws') }.not_to change { Aws::Role.count } + + expect(response).to have_gitlab_http_status(:ok) + expect(assigns(:aws_role)).to eq(role) + end + end + + context 'user does not have an associated AWS role' do + it 'creates an Aws::Role record' do + expect { go(provider: 'aws') }.to change { Aws::Role.count } + + expect(response).to have_gitlab_http_status(:ok) + + role = assigns(:aws_role) + expect(role.user).to eq(user) + expect(role.role_arn).to be_nil + expect(role.role_external_id).to be_present + end + end + end +end |