diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2021-02-18 10:34:06 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2021-02-18 10:34:06 +0000 |
commit | 859a6fb938bb9ee2a317c46dfa4fcc1af49608f0 (patch) | |
tree | d7f2700abe6b4ffcb2dcfc80631b2d87d0609239 /spec/requests | |
parent | 446d496a6d000c73a304be52587cd9bbc7493136 (diff) | |
download | gitlab-ce-859a6fb938bb9ee2a317c46dfa4fcc1af49608f0.tar.gz |
Add latest changes from gitlab-org/gitlab@13-9-stable-eev13.9.0-rc42
Diffstat (limited to 'spec/requests')
73 files changed, 2403 insertions, 613 deletions
diff --git a/spec/requests/api/api_spec.rb b/spec/requests/api/api_spec.rb index 9fd30213133..8bd6049e6fa 100644 --- a/spec/requests/api/api_spec.rb +++ b/spec/requests/api/api_spec.rb @@ -36,6 +36,12 @@ RSpec.describe API::API do expect(response).to have_gitlab_http_status(:ok) end + it 'does authorize user for head request' do + head api('/groups', personal_access_token: token) + + expect(response).to have_gitlab_http_status(:ok) + end + it 'does not authorize user for revoked token' do revoked = create(:personal_access_token, :revoked, user: user, scopes: [:read_api]) @@ -126,4 +132,34 @@ RSpec.describe API::API do get(api('/users')) end end + + describe 'supported content-types' do + context 'GET /user/:id.txt' do + let_it_be(:user) { create(:user) } + + subject { get api("/users/#{user.id}.txt", user) } + + it 'returns application/json' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(response.media_type).to eq('application/json') + expect(response.body).to include('{"id":') + end + + context 'when api_always_use_application_json is disabled' do + before do + stub_feature_flags(api_always_use_application_json: false) + end + + it 'returns text/plain' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(response.media_type).to eq('text/plain') + expect(response.body).to include('#<API::Entities::User:') + end + end + end + end end diff --git a/spec/requests/api/applications_spec.rb b/spec/requests/api/applications_spec.rb index 63fbf6e32dd..ca09f5524ca 100644 --- a/spec/requests/api/applications_spec.rb +++ b/spec/requests/api/applications_spec.rb @@ -31,7 +31,7 @@ RSpec.describe API::Applications, :api do expect(response).to have_gitlab_http_status(:bad_request) expect(json_response).to be_a Hash - expect(json_response['message']['redirect_uri'][0]).to eq('must be an absolute URI.') + expect(json_response['message']['redirect_uri'][0]).to eq('must be a valid URI.') end it 'does not allow creating an application with a forbidden URI format' do diff --git a/spec/requests/api/ci/pipelines_spec.rb b/spec/requests/api/ci/pipelines_spec.rb index 767b5704851..a9afbd8bd72 100644 --- a/spec/requests/api/ci/pipelines_spec.rb +++ b/spec/requests/api/ci/pipelines_spec.rb @@ -312,7 +312,7 @@ RSpec.describe API::Ci::Pipelines do let(:query) { {} } let(:api_user) { user } let_it_be(:job) do - create(:ci_build, :success, pipeline: pipeline, + create(:ci_build, :success, name: 'build', pipeline: pipeline, artifacts_expire_at: 1.day.since) end @@ -405,6 +405,38 @@ RSpec.describe API::Ci::Pipelines do get api("/projects/#{project.id}/pipelines/#{pipeline.id}/jobs", api_user), params: query end.not_to exceed_all_query_limit(control_count) end + + context 'pipeline has retried jobs' do + before_all do + job.update!(retried: true) + end + + let_it_be(:successor) { create(:ci_build, :success, name: 'build', pipeline: pipeline) } + + it 'does not return retried jobs by default' do + expect(json_response).to be_an Array + expect(json_response.length).to eq(1) + end + + context 'when include_retried is false' do + let(:query) { { include_retried: false } } + + it 'does not return retried jobs' do + expect(json_response).to be_an Array + expect(json_response.length).to eq(1) + end + end + + context 'when include_retried is true' do + let(:query) { { include_retried: true } } + + it 'returns retried jobs' do + expect(json_response).to be_an Array + expect(json_response.length).to eq(2) + expect(json_response[0]['name']).to eq(json_response[1]['name']) + end + end + end end context 'no pipeline is found' do diff --git a/spec/requests/api/debian_group_packages_spec.rb b/spec/requests/api/debian_group_packages_spec.rb index 8a05d20fb33..9d63d675a02 100644 --- a/spec/requests/api/debian_group_packages_spec.rb +++ b/spec/requests/api/debian_group_packages_spec.rb @@ -6,32 +6,32 @@ RSpec.describe API::DebianGroupPackages do include WorkhorseHelpers include_context 'Debian repository shared context', :group do - describe 'GET groups/:id/-/packages/debian/dists/*distribution/Release.gpg' do - let(:url) { "/groups/#{group.id}/-/packages/debian/dists/#{distribution}/Release.gpg" } + describe 'GET groups/:id/packages/debian/dists/*distribution/Release.gpg' do + let(:url) { "/groups/#{group.id}/packages/debian/dists/#{distribution}/Release.gpg" } it_behaves_like 'Debian group repository GET endpoint', :not_found, nil end - describe 'GET groups/:id/-/packages/debian/dists/*distribution/Release' do - let(:url) { "/groups/#{group.id}/-/packages/debian/dists/#{distribution}/Release" } + describe 'GET groups/:id/packages/debian/dists/*distribution/Release' do + let(:url) { "/groups/#{group.id}/packages/debian/dists/#{distribution}/Release" } it_behaves_like 'Debian group repository GET endpoint', :success, 'TODO Release' end - describe 'GET groups/:id/-/packages/debian/dists/*distribution/InRelease' do - let(:url) { "/groups/#{group.id}/-/packages/debian/dists/#{distribution}/InRelease" } + describe 'GET groups/:id/packages/debian/dists/*distribution/InRelease' do + let(:url) { "/groups/#{group.id}/packages/debian/dists/#{distribution}/InRelease" } it_behaves_like 'Debian group repository GET endpoint', :not_found, nil end - describe 'GET groups/:id/-/packages/debian/dists/*distribution/:component/binary-:architecture/Packages' do - let(:url) { "/groups/#{group.id}/-/packages/debian/dists/#{distribution}/#{component}/binary-#{architecture}/Packages" } + describe 'GET groups/:id/packages/debian/dists/*distribution/:component/binary-:architecture/Packages' do + let(:url) { "/groups/#{group.id}/packages/debian/dists/#{distribution}/#{component}/binary-#{architecture}/Packages" } it_behaves_like 'Debian group repository GET endpoint', :success, 'TODO Packages' end - describe 'GET groups/:id/-/packages/debian/pool/:component/:letter/:source_package/:file_name' do - let(:url) { "/groups/#{group.id}/-/packages/debian/pool/#{component}/#{letter}/#{source_package}/#{package_name}_#{package_version}_#{architecture}.deb" } + describe 'GET groups/:id/packages/debian/pool/:component/:letter/:source_package/:file_name' do + let(:url) { "/groups/#{group.id}/packages/debian/pool/#{component}/#{letter}/#{source_package}/#{package_name}_#{package_version}_#{architecture}.deb" } it_behaves_like 'Debian group repository GET endpoint', :success, 'TODO File' end diff --git a/spec/requests/api/debian_project_packages_spec.rb b/spec/requests/api/debian_project_packages_spec.rb index 663b69b1b76..4941f2a77f4 100644 --- a/spec/requests/api/debian_project_packages_spec.rb +++ b/spec/requests/api/debian_project_packages_spec.rb @@ -6,46 +6,46 @@ RSpec.describe API::DebianProjectPackages do include WorkhorseHelpers include_context 'Debian repository shared context', :project do - describe 'GET projects/:id/-/packages/debian/dists/*distribution/Release.gpg' do - let(:url) { "/projects/#{project.id}/-/packages/debian/dists/#{distribution}/Release.gpg" } + describe 'GET projects/:id/packages/debian/dists/*distribution/Release.gpg' do + let(:url) { "/projects/#{project.id}/packages/debian/dists/#{distribution}/Release.gpg" } it_behaves_like 'Debian project repository GET endpoint', :not_found, nil end - describe 'GET projects/:id/-/packages/debian/dists/*distribution/Release' do - let(:url) { "/projects/#{project.id}/-/packages/debian/dists/#{distribution}/Release" } + describe 'GET projects/:id/packages/debian/dists/*distribution/Release' do + let(:url) { "/projects/#{project.id}/packages/debian/dists/#{distribution}/Release" } it_behaves_like 'Debian project repository GET endpoint', :success, 'TODO Release' end - describe 'GET projects/:id/-/packages/debian/dists/*distribution/InRelease' do - let(:url) { "/projects/#{project.id}/-/packages/debian/dists/#{distribution}/InRelease" } + describe 'GET projects/:id/packages/debian/dists/*distribution/InRelease' do + let(:url) { "/projects/#{project.id}/packages/debian/dists/#{distribution}/InRelease" } it_behaves_like 'Debian project repository GET endpoint', :not_found, nil end - describe 'GET projects/:id/-/packages/debian/dists/*distribution/:component/binary-:architecture/Packages' do - let(:url) { "/projects/#{project.id}/-/packages/debian/dists/#{distribution}/#{component}/binary-#{architecture}/Packages" } + describe 'GET projects/:id/packages/debian/dists/*distribution/:component/binary-:architecture/Packages' do + let(:url) { "/projects/#{project.id}/packages/debian/dists/#{distribution}/#{component}/binary-#{architecture}/Packages" } it_behaves_like 'Debian project repository GET endpoint', :success, 'TODO Packages' end - describe 'GET projects/:id/-/packages/debian/pool/:component/:letter/:source_package/:file_name' do - let(:url) { "/projects/#{project.id}/-/packages/debian/pool/#{component}/#{letter}/#{source_package}/#{package_name}_#{package_version}_#{architecture}.deb" } + describe 'GET projects/:id/packages/debian/pool/:component/:letter/:source_package/:file_name' do + let(:url) { "/projects/#{project.id}/packages/debian/pool/#{component}/#{letter}/#{source_package}/#{package_name}_#{package_version}_#{architecture}.deb" } it_behaves_like 'Debian project repository GET endpoint', :success, 'TODO File' end - describe 'PUT projects/:id/-/packages/debian/incoming/:file_name' do + describe 'PUT projects/:id/packages/debian/:file_name' do let(:method) { :put } - let(:url) { "/projects/#{project.id}/-/packages/debian/incoming/#{file_name}" } + let(:url) { "/projects/#{project.id}/packages/debian/#{file_name}" } it_behaves_like 'Debian project repository PUT endpoint', :created, nil end - describe 'PUT projects/:id/-/packages/debian/incoming/:file_name/authorize' do + describe 'PUT projects/:id/packages/debian/:file_name/authorize' do let(:method) { :put } - let(:url) { "/projects/#{project.id}/-/packages/debian/incoming/#{file_name}/authorize" } + let(:url) { "/projects/#{project.id}/packages/debian/#{file_name}/authorize" } it_behaves_like 'Debian project repository PUT endpoint', :created, nil, is_authorize: true end diff --git a/spec/requests/api/deploy_tokens_spec.rb b/spec/requests/api/deploy_tokens_spec.rb index 8ec4f888e2e..7a31ff725c8 100644 --- a/spec/requests/api/deploy_tokens_spec.rb +++ b/spec/requests/api/deploy_tokens_spec.rb @@ -10,24 +10,12 @@ RSpec.describe API::DeployTokens do let!(:deploy_token) { create(:deploy_token, projects: [project]) } let!(:group_deploy_token) { create(:deploy_token, :group, groups: [group]) } - shared_examples 'with feature flag disabled' do - context 'disabled feature flag' do - before do - stub_feature_flags(deploy_tokens_api: false) - end - - it { is_expected.to have_gitlab_http_status(:service_unavailable) } - end - end - describe 'GET /deploy_tokens' do subject do get api('/deploy_tokens', user) response end - it_behaves_like 'with feature flag disabled' - context 'when unauthenticated' do let(:user) { nil } @@ -81,8 +69,6 @@ RSpec.describe API::DeployTokens do project.add_maintainer(user) end - it_behaves_like 'with feature flag disabled' - it { is_expected.to have_gitlab_http_status(:ok) } it 'returns all deploy tokens for the project' do @@ -128,8 +114,6 @@ RSpec.describe API::DeployTokens do group.add_maintainer(user) end - it_behaves_like 'with feature flag disabled' - it { is_expected.to have_gitlab_http_status(:ok) } it 'returns all deploy tokens for the group' do diff --git a/spec/requests/api/events_spec.rb b/spec/requests/api/events_spec.rb index 6a8d5f91abd..110d6e2f99e 100644 --- a/spec/requests/api/events_spec.rb +++ b/spec/requests/api/events_spec.rb @@ -55,6 +55,12 @@ RSpec.describe API::Events do expect(json_response).to be_an Array expect(json_response.size).to eq(1) end + + it 'returns "200" response on head request' do + head api('/events?action=closed&target_type=issue&after=2016-12-1&before=2016-12-31', personal_access_token: token) + + expect(response).to have_gitlab_http_status(:ok) + end end context 'when the requesting token does not have "read_user" or "api" scope' do diff --git a/spec/requests/api/generic_packages_spec.rb b/spec/requests/api/generic_packages_spec.rb index d162d288129..a47be1ead9c 100644 --- a/spec/requests/api/generic_packages_spec.rb +++ b/spec/requests/api/generic_packages_spec.rb @@ -4,6 +4,9 @@ require 'spec_helper' RSpec.describe API::GenericPackages do include HttpBasicAuthHelpers + using RSpec::Parameterized::TableSyntax + + include_context 'workhorse headers' let_it_be(:personal_access_token) { create(:personal_access_token) } let_it_be(:project, reload: true) { create(:project) } @@ -13,8 +16,6 @@ RSpec.describe API::GenericPackages do let_it_be(:project_deploy_token_ro) { create(:project_deploy_token, deploy_token: deploy_token_ro, project: project) } let_it_be(:deploy_token_wo) { create(:deploy_token, read_package_registry: false, write_package_registry: true) } let_it_be(:project_deploy_token_wo) { create(:project_deploy_token, deploy_token: deploy_token_wo, project: project) } - let(:workhorse_token) { JWT.encode({ 'iss' => 'gitlab-workhorse' }, Gitlab::Workhorse.secret, 'HS256') } - let(:workhorse_header) { { 'GitLab-Workhorse' => '1.0', Gitlab::Workhorse::INTERNAL_API_REQUEST_HEADER => workhorse_token } } let(:user) { personal_access_token.user } let(:ci_build) { create(:ci_build, :running, user: user) } @@ -76,8 +77,6 @@ RSpec.describe API::GenericPackages do describe 'PUT /api/v4/projects/:id/packages/generic/:package_name/:package_version/:file_name/authorize' do context 'with valid project' do - using RSpec::Parameterized::TableSyntax - where(:project_visibility, :user_role, :member?, :authenticate_with, :expected_status) do 'PUBLIC' | :developer | true | :personal_access_token | :success 'PUBLIC' | :guest | true | :personal_access_token | :forbidden @@ -130,7 +129,7 @@ RSpec.describe API::GenericPackages do end it "responds with #{params[:expected_status]}" do - authorize_upload_file(workhorse_header.merge(auth_header)) + authorize_upload_file(workhorse_headers.merge(auth_header)) expect(response).to have_gitlab_http_status(expected_status) end @@ -145,7 +144,7 @@ RSpec.describe API::GenericPackages do with_them do it "responds with #{params[:expected_status]}" do - authorize_upload_file(workhorse_header.merge(deploy_token_auth_header)) + authorize_upload_file(workhorse_headers.merge(deploy_token_auth_header)) expect(response).to have_gitlab_http_status(expected_status) end @@ -163,7 +162,7 @@ RSpec.describe API::GenericPackages do end with_them do - subject { authorize_upload_file(workhorse_header.merge(personal_access_token_header), param_name => param_value) } + subject { authorize_upload_file(workhorse_headers.merge(personal_access_token_header), param_name => param_value) } it_behaves_like 'secure endpoint' end @@ -174,7 +173,7 @@ RSpec.describe API::GenericPackages do stub_feature_flags(generic_packages: false) project.add_developer(user) - authorize_upload_file(workhorse_header.merge(personal_access_token_header)) + authorize_upload_file(workhorse_headers.merge(personal_access_token_header)) expect(response).to have_gitlab_http_status(:not_found) end @@ -194,8 +193,6 @@ RSpec.describe API::GenericPackages do let(:params) { { file: file_upload } } context 'authentication' do - using RSpec::Parameterized::TableSyntax - where(:project_visibility, :user_role, :member?, :authenticate_with, :expected_status) do 'PUBLIC' | :guest | true | :personal_access_token | :forbidden 'PUBLIC' | :guest | true | :user_basic_auth | :forbidden @@ -242,7 +239,7 @@ RSpec.describe API::GenericPackages do end it "responds with #{params[:expected_status]}" do - headers = workhorse_header.merge(auth_header) + headers = workhorse_headers.merge(auth_header) upload_file(params, headers) @@ -257,7 +254,7 @@ RSpec.describe API::GenericPackages do with_them do it "responds with #{params[:expected_status]}" do - headers = workhorse_header.merge(deploy_token_auth_header) + headers = workhorse_headers.merge(deploy_token_auth_header) upload_file(params, headers) @@ -273,7 +270,7 @@ RSpec.describe API::GenericPackages do shared_examples 'creates a package and package file' do it 'creates a package and package file' do - headers = workhorse_header.merge(auth_header) + headers = workhorse_headers.merge(auth_header) expect { upload_file(params, headers) } .to change { project.packages.generic.count }.by(1) @@ -284,6 +281,7 @@ RSpec.describe API::GenericPackages do package = project.packages.generic.last expect(package.name).to eq('mypackage') + expect(package.status).to eq('default') expect(package.version).to eq('0.0.1') if should_set_build_info @@ -296,6 +294,39 @@ RSpec.describe API::GenericPackages do expect(package_file.file_name).to eq('myfile.tar.gz') end end + + context 'with a status' do + context 'valid status' do + let(:params) { super().merge(status: 'hidden') } + + it 'assigns the status to the package' do + headers = workhorse_headers.merge(auth_header) + + upload_file(params, headers) + + aggregate_failures do + expect(response).to have_gitlab_http_status(:created) + + package = project.packages.find_by(name: 'mypackage') + expect(package).to be_hidden + end + end + end + + context 'invalid status' do + let(:params) { super().merge(status: 'processing') } + + it 'rejects the package' do + headers = workhorse_headers.merge(auth_header) + + upload_file(params, headers) + + aggregate_failures do + expect(response).to have_gitlab_http_status(:bad_request) + end + end + end + end end context 'when valid personal access token is used' do @@ -327,26 +358,26 @@ RSpec.describe API::GenericPackages do end context 'event tracking' do - subject { upload_file(params, workhorse_header.merge(personal_access_token_header)) } + subject { upload_file(params, workhorse_headers.merge(personal_access_token_header)) } it_behaves_like 'a gitlab tracking event', described_class.name, 'push_package' end it 'rejects request without a file from workhorse' do - headers = workhorse_header.merge(personal_access_token_header) + headers = workhorse_headers.merge(personal_access_token_header) upload_file({}, headers) expect(response).to have_gitlab_http_status(:bad_request) end it 'rejects request without an auth token' do - upload_file(params, workhorse_header) + upload_file(params, workhorse_headers) expect(response).to have_gitlab_http_status(:unauthorized) end it 'rejects request without workhorse rewritten fields' do - headers = workhorse_header.merge(personal_access_token_header) + headers = workhorse_headers.merge(personal_access_token_header) upload_file(params, headers, send_rewritten_field: false) expect(response).to have_gitlab_http_status(:bad_request) @@ -357,7 +388,7 @@ RSpec.describe API::GenericPackages do allow(uploaded_file).to receive(:size).and_return(project.actual_limits.generic_packages_max_file_size + 1) end - headers = workhorse_header.merge(personal_access_token_header) + headers = workhorse_headers.merge(personal_access_token_header) upload_file(params, headers) expect(response).to have_gitlab_http_status(:bad_request) @@ -373,8 +404,6 @@ RSpec.describe API::GenericPackages do end context 'application security' do - using RSpec::Parameterized::TableSyntax - where(:param_name, :param_value) do :package_name | 'my-package/../' :package_name | 'my-package%2f%2e%2e%2f' @@ -383,7 +412,7 @@ RSpec.describe API::GenericPackages do end with_them do - subject { upload_file(params, workhorse_header.merge(personal_access_token_header), param_name => param_value) } + subject { upload_file(params, workhorse_headers.merge(personal_access_token_header), param_name => param_value) } it_behaves_like 'secure endpoint' end @@ -404,8 +433,6 @@ RSpec.describe API::GenericPackages do end describe 'GET /api/v4/projects/:id/packages/generic/:package_name/:package_version/:file_name' do - using RSpec::Parameterized::TableSyntax - let_it_be(:package) { create(:generic_package, project: project) } let_it_be(:package_file) { create(:package_file, :generic, package: package) } @@ -527,8 +554,6 @@ RSpec.describe API::GenericPackages do end context 'application security' do - using RSpec::Parameterized::TableSyntax - where(:param_name, :param_value) do :package_name | 'my-package/../' :package_name | 'my-package%2f%2e%2e%2f' diff --git a/spec/requests/api/graphql/ci/application_setting_spec.rb b/spec/requests/api/graphql/ci/application_setting_spec.rb new file mode 100644 index 00000000000..156ee550f16 --- /dev/null +++ b/spec/requests/api/graphql/ci/application_setting_spec.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'getting Application Settings' do + include GraphqlHelpers + + let(:fields) do + <<~QUERY + #{all_graphql_fields_for('CiApplicationSettings', max_depth: 1)} + QUERY + end + + let(:query) do + graphql_query_for( + 'ciApplicationSettings', + fields + ) + end + + let(:settings_data) { graphql_data['ciApplicationSettings'] } + + context 'without admin permissions' do + let(:user) { create(:user) } + + before do + post_graphql(query, current_user: user) + end + + it_behaves_like 'a working graphql query' + + specify { expect(settings_data).to be nil } + end + + context 'with admin permissions' do + let(:user) { create(:user, :admin) } + + before do + post_graphql(query, current_user: user) + end + + it_behaves_like 'a working graphql query' + + it 'fetches the settings data' do + # assert against hash to ensure no additional fields are exposed + expect(settings_data).to match({ 'keepLatestArtifact' => Gitlab::CurrentSettings.current_application_settings.keep_latest_artifact }) + end + end +end diff --git a/spec/requests/api/graphql/ci/ci_cd_setting_spec.rb b/spec/requests/api/graphql/ci/ci_cd_setting_spec.rb index db8a412e45c..99647d0fa3a 100644 --- a/spec/requests/api/graphql/ci/ci_cd_setting_spec.rb +++ b/spec/requests/api/graphql/ci/ci_cd_setting_spec.rb @@ -46,7 +46,7 @@ RSpec.describe 'Getting Ci Cd Setting' do it 'fetches the settings data' do expect(settings_data['mergePipelinesEnabled']).to eql project.ci_cd_settings.merge_pipelines_enabled? expect(settings_data['mergeTrainsEnabled']).to eql project.ci_cd_settings.merge_trains_enabled? - expect(settings_data['keepLatestArtifact']).to eql project.ci_keep_latest_artifact? + expect(settings_data['keepLatestArtifact']).to eql project.keep_latest_artifacts_available? end end end diff --git a/spec/requests/api/graphql/issue/issue_spec.rb b/spec/requests/api/graphql/issue/issue_spec.rb index 42c8e0cc9c0..09e89f65882 100644 --- a/spec/requests/api/graphql/issue/issue_spec.rb +++ b/spec/requests/api/graphql/issue/issue_spec.rb @@ -24,6 +24,20 @@ RSpec.describe 'Query.issue(id)' do end end + it_behaves_like 'a noteable graphql type we can query' do + let(:noteable) { issue } + let(:project) { issue.project } + let(:path_to_noteable) { [:issue] } + + before do + project.add_guest(current_user) + end + + def query(fields) + graphql_query_for('issue', issue_params, fields) + end + end + context 'when the user does not have access to the issue' do it 'returns nil' do project.project_feature.update!(issues_access_level: ProjectFeature::PRIVATE) diff --git a/spec/requests/api/graphql/mutations/alert_management/http_integration/update_spec.rb b/spec/requests/api/graphql/mutations/alert_management/http_integration/update_spec.rb index bf7eb3d980c..18cbb7d8b00 100644 --- a/spec/requests/api/graphql/mutations/alert_management/http_integration/update_spec.rb +++ b/spec/requests/api/graphql/mutations/alert_management/http_integration/update_spec.rb @@ -5,7 +5,7 @@ require 'spec_helper' RSpec.describe 'Updating an existing HTTP Integration' do include GraphqlHelpers - let_it_be(:user) { create(:user) } + let_it_be(:current_user) { create(:user) } let_it_be(:project) { create(:project) } let_it_be(:integration) { create(:alert_management_http_integration, project: project) } @@ -32,18 +32,8 @@ RSpec.describe 'Updating an existing HTTP Integration' do let(:mutation_response) { graphql_mutation_response(:http_integration_update) } before do - project.add_maintainer(user) + project.add_maintainer(current_user) end - it 'updates the integration' do - post_graphql_mutation(mutation, current_user: user) - - integration_response = mutation_response['integration'] - - expect(response).to have_gitlab_http_status(:success) - expect(integration_response['id']).to eq(GitlabSchema.id_from_object(integration).to_s) - expect(integration_response['name']).to eq('Modified Name') - expect(integration_response['active']).to be_falsey - expect(integration_response['url']).to include('modified-name') - end + it_behaves_like 'updating an existing HTTP integration' end diff --git a/spec/requests/api/graphql/mutations/boards/lists/create_spec.rb b/spec/requests/api/graphql/mutations/boards/lists/create_spec.rb index 328f4fb7b6e..fec9a8c6307 100644 --- a/spec/requests/api/graphql/mutations/boards/lists/create_spec.rb +++ b/spec/requests/api/graphql/mutations/boards/lists/create_spec.rb @@ -3,52 +3,10 @@ require 'spec_helper' RSpec.describe 'Create a label or backlog board list' do - include GraphqlHelpers - let_it_be(:group) { create(:group, :private) } let_it_be(:board) { create(:board, group: group) } - let_it_be(:user) { create(:user) } - let_it_be(:dev_label) do - create(:group_label, title: 'Development', color: '#FFAABB', group: group) - end - - let(:current_user) { user } - let(:mutation) { graphql_mutation(:board_list_create, input) } - let(:mutation_response) { graphql_mutation_response(:board_list_create) } - - context 'the user is not allowed to read board lists' do - let(:input) { { board_id: board.to_global_id.to_s, backlog: true } } - - it_behaves_like 'a mutation that returns a top-level access error' - end - - context 'when user has permissions to admin board lists' do - before do - group.add_reporter(current_user) - end - - describe 'backlog list' do - let(:input) { { board_id: board.to_global_id.to_s, backlog: true } } - - it 'creates the list' do - post_graphql_mutation(mutation, current_user: current_user) - - expect(response).to have_gitlab_http_status(:success) - expect(mutation_response['list']) - .to include('position' => nil, 'listType' => 'backlog') - end - end - - describe 'label list' do - let(:input) { { board_id: board.to_global_id.to_s, label_id: dev_label.to_global_id.to_s } } - - it 'creates the list' do - post_graphql_mutation(mutation, current_user: current_user) - expect(response).to have_gitlab_http_status(:success) - expect(mutation_response['list']) - .to include('position' => 0, 'listType' => 'label', 'label' => include('title' => 'Development')) - end - end + it_behaves_like 'board lists create request' do + let(:mutation_name) { :board_list_create } end end diff --git a/spec/requests/api/graphql/mutations/ci/ci_cd_settings_update_spec.rb b/spec/requests/api/graphql/mutations/ci/ci_cd_settings_update_spec.rb index 283badeaf33..0dcae28ac5d 100644 --- a/spec/requests/api/graphql/mutations/ci/ci_cd_settings_update_spec.rb +++ b/spec/requests/api/graphql/mutations/ci/ci_cd_settings_update_spec.rb @@ -5,7 +5,7 @@ require 'spec_helper' RSpec.describe 'CiCdSettingsUpdate' do include GraphqlHelpers - let_it_be(:project) { create(:project, ci_keep_latest_artifact: true) } + let_it_be(:project) { create(:project, keep_latest_artifact: true) } let(:variables) { { full_path: project.full_path, keep_latest_artifact: false } } let(:mutation) { graphql_mutation(:ci_cd_settings_update, variables) } @@ -42,7 +42,7 @@ RSpec.describe 'CiCdSettingsUpdate' do project.reload expect(response).to have_gitlab_http_status(:success) - expect(project.ci_keep_latest_artifact).to eq(false) + expect(project.keep_latest_artifact).to eq(false) end context 'when bad arguments are provided' do diff --git a/spec/requests/api/graphql/mutations/merge_requests/reviewer_rereview_spec.rb b/spec/requests/api/graphql/mutations/merge_requests/reviewer_rereview_spec.rb new file mode 100644 index 00000000000..2e4f35cbcde --- /dev/null +++ b/spec/requests/api/graphql/mutations/merge_requests/reviewer_rereview_spec.rb @@ -0,0 +1,65 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Setting assignees of a merge request' do + include GraphqlHelpers + + let(:current_user) { create(:user) } + let(:merge_request) { create(:merge_request, reviewers: [user]) } + let(:project) { merge_request.project } + let(:user) { create(:user) } + let(:input) { { user_id: global_id_of(user) } } + + let(:mutation) do + variables = { + project_path: project.full_path, + iid: merge_request.iid.to_s + } + graphql_mutation(:merge_request_reviewer_rereview, variables.merge(input), + <<-QL.strip_heredoc + clientMutationId + errors + QL + ) + end + + def mutation_response + graphql_mutation_response(:merge_request_reviewer_rereview) + end + + def mutation_errors + mutation_response['errors'] + end + + before do + project.add_developer(current_user) + project.add_developer(user) + end + + it 'returns an error if the user is not allowed to update the merge request' do + post_graphql_mutation(mutation, current_user: create(:user)) + + expect(graphql_errors).not_to be_empty + end + + describe 'reviewer does not exist' do + let(:input) { { user_id: global_id_of(create(:user)) } } + + it 'returns an error' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(response).to have_gitlab_http_status(:success) + expect(mutation_errors).not_to be_empty + end + end + + describe 'reviewer exists' do + it 'does not return an error' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(response).to have_gitlab_http_status(:success) + expect(mutation_errors).to be_empty + end + end +end diff --git a/spec/requests/api/graphql/mutations/notes/create/diff_note_spec.rb b/spec/requests/api/graphql/mutations/notes/create/diff_note_spec.rb index 21da1332465..7dd897f6466 100644 --- a/spec/requests/api/graphql/mutations/notes/create/diff_note_spec.rb +++ b/spec/requests/api/graphql/mutations/notes/create/diff_note_spec.rb @@ -43,6 +43,8 @@ RSpec.describe 'Adding a DiffNote' do it_behaves_like 'a Note mutation when there are active record validation errors', model: DiffNote + it_behaves_like 'a Note mutation when there are rate limit validation errors' + context do let(:diff_refs) { build(:commit).diff_refs } # Allow fake diff refs so arguments are valid diff --git a/spec/requests/api/graphql/mutations/notes/create/image_diff_note_spec.rb b/spec/requests/api/graphql/mutations/notes/create/image_diff_note_spec.rb index 8bc68e6017c..0e5744fb64f 100644 --- a/spec/requests/api/graphql/mutations/notes/create/image_diff_note_spec.rb +++ b/spec/requests/api/graphql/mutations/notes/create/image_diff_note_spec.rb @@ -46,6 +46,8 @@ RSpec.describe 'Adding an image DiffNote' do it_behaves_like 'a Note mutation when there are active record validation errors', model: DiffNote + it_behaves_like 'a Note mutation when there are rate limit validation errors' + context do let(:diff_refs) { build(:commit).diff_refs } # Allow fake diff refs so arguments are valid diff --git a/spec/requests/api/graphql/mutations/notes/create/note_spec.rb b/spec/requests/api/graphql/mutations/notes/create/note_spec.rb index 6d761eb0a54..1eed1c8e2ae 100644 --- a/spec/requests/api/graphql/mutations/notes/create/note_spec.rb +++ b/spec/requests/api/graphql/mutations/notes/create/note_spec.rb @@ -37,6 +37,8 @@ RSpec.describe 'Adding a Note' do it_behaves_like 'a Note mutation when the given resource id is not for a Noteable' + it_behaves_like 'a Note mutation when there are rate limit validation errors' + it 'returns the note' do post_graphql_mutation(mutation, current_user: current_user) diff --git a/spec/requests/api/graphql/mutations/notes/update/image_diff_note_spec.rb b/spec/requests/api/graphql/mutations/notes/update/image_diff_note_spec.rb index 713b26a6a9b..1ce09881fde 100644 --- a/spec/requests/api/graphql/mutations/notes/update/image_diff_note_spec.rb +++ b/spec/requests/api/graphql/mutations/notes/update/image_diff_note_spec.rb @@ -6,7 +6,7 @@ RSpec.describe 'Updating an image DiffNote' do include GraphqlHelpers using RSpec::Parameterized::TableSyntax - let_it_be(:noteable) { create(:merge_request, :with_diffs) } + let_it_be(:noteable) { create(:merge_request) } let_it_be(:original_body) { 'Original body' } let_it_be(:original_position) do Gitlab::Diff::Position.new( diff --git a/spec/requests/api/graphql/mutations/releases/create_spec.rb b/spec/requests/api/graphql/mutations/releases/create_spec.rb index 79bdcec7944..a4918cd560c 100644 --- a/spec/requests/api/graphql/mutations/releases/create_spec.rb +++ b/spec/requests/api/graphql/mutations/releases/create_spec.rb @@ -309,10 +309,7 @@ RSpec.describe 'Creation of a new release' do let(:asset_link_2) { { name: 'My link', url: 'https://example.com/2' } } let(:assets) { { links: [asset_link_1, asset_link_2] } } - # Right now the raw Postgres error message is sent to the user as the validation message. - # We should catch this validation error and return a nicer message: - # https://gitlab.com/gitlab-org/gitlab/-/issues/277087 - it_behaves_like 'errors-as-data with message', 'PG::UniqueViolation' + it_behaves_like 'errors-as-data with message', %r{Validation failed: Links have duplicate values \(My link\)} end context 'when two release assets share the same URL' do @@ -320,8 +317,7 @@ RSpec.describe 'Creation of a new release' do let(:asset_link_2) { { name: 'My second link', url: 'https://example.com' } } let(:assets) { { links: [asset_link_1, asset_link_2] } } - # Same note as above about the ugly error message - it_behaves_like 'errors-as-data with message', 'PG::UniqueViolation' + it_behaves_like 'errors-as-data with message', %r{Validation failed: Links have duplicate values \(https://example.com\)} end context 'when the provided tag name is HEAD' do diff --git a/spec/requests/api/graphql/mutations/snippets/create_spec.rb b/spec/requests/api/graphql/mutations/snippets/create_spec.rb index fd0dc98a8d3..1c2260070ec 100644 --- a/spec/requests/api/graphql/mutations/snippets/create_spec.rb +++ b/spec/requests/api/graphql/mutations/snippets/create_spec.rb @@ -17,6 +17,7 @@ RSpec.describe 'Creating a Snippet' do let(:actions) { [{ action: action }.merge(file_1), { action: action }.merge(file_2)] } let(:project_path) { nil } let(:uploaded_files) { nil } + let(:spam_mutation_vars) { {} } let(:mutation_vars) do { description: description, @@ -25,7 +26,7 @@ RSpec.describe 'Creating a Snippet' do project_path: project_path, uploaded_files: uploaded_files, blob_actions: actions - } + }.merge(spam_mutation_vars) end let(:mutation) do @@ -77,7 +78,20 @@ RSpec.describe 'Creating a Snippet' do expect(mutation_response['snippet']).to be_nil end - it_behaves_like 'spam flag is present' + context 'when snippet_spam flag is disabled' do + before do + stub_feature_flags(snippet_spam: false) + end + + it 'passes disable_spam_action_service param to service' do + expect(::Snippets::CreateService) + .to receive(:new) + .with(anything, anything, hash_including(disable_spam_action_service: true)) + .and_call_original + + subject + end + end end shared_examples 'creates snippet' do @@ -98,15 +112,24 @@ RSpec.describe 'Creating a Snippet' do end context 'when action is invalid' do - let(:file_1) { { filePath: 'example_file1' }} + let(:file_1) { { filePath: 'example_file1' } } it_behaves_like 'a mutation that returns errors in the response', errors: ['Snippet actions have invalid data'] it_behaves_like 'does not create snippet' end it_behaves_like 'snippet edit usage data counters' - it_behaves_like 'spam flag is present' - it_behaves_like 'can raise spam flag' do + + it_behaves_like 'a mutation which can mutate a spammable' do + let(:captcha_response) { 'abc123' } + let(:spam_log_id) { 1234 } + let(:spam_mutation_vars) do + { + captcha_response: captcha_response, + spam_log_id: spam_log_id + } + end + let(:service) { Snippets::CreateService } end end @@ -148,9 +171,6 @@ RSpec.describe 'Creating a Snippet' do it_behaves_like 'a mutation that returns errors in the response', errors: ["Title can't be blank"] it_behaves_like 'does not create snippet' - it_behaves_like 'can raise spam flag' do - let(:service) { Snippets::CreateService } - end end context 'when there non ActiveRecord errors' do diff --git a/spec/requests/api/graphql/mutations/snippets/update_spec.rb b/spec/requests/api/graphql/mutations/snippets/update_spec.rb index 21d403c6f73..43dc8d8bc44 100644 --- a/spec/requests/api/graphql/mutations/snippets/update_spec.rb +++ b/spec/requests/api/graphql/mutations/snippets/update_spec.rb @@ -16,6 +16,7 @@ RSpec.describe 'Updating a Snippet' do let(:updated_file) { 'CHANGELOG' } let(:deleted_file) { 'README' } let(:snippet_gid) { GitlabSchema.id_from_object(snippet).to_s } + let(:spam_mutation_vars) { {} } let(:mutation_vars) do { id: snippet_gid, @@ -26,7 +27,7 @@ RSpec.describe 'Updating a Snippet' do { action: :update, filePath: updated_file, content: updated_content }, { action: :delete, filePath: deleted_file } ] - } + }.merge(spam_mutation_vars) end let(:mutation) do @@ -81,11 +82,20 @@ RSpec.describe 'Updating a Snippet' do end end - it_behaves_like 'can raise spam flag' do - let(:service) { Snippets::UpdateService } - end + context 'when snippet_spam flag is disabled' do + before do + stub_feature_flags(snippet_spam: false) + end - it_behaves_like 'spam flag is present' + it 'passes disable_spam_action_service param to service' do + expect(::Snippets::UpdateService) + .to receive(:new) + .with(anything, anything, hash_including(disable_spam_action_service: true)) + .and_call_original + + subject + end + end context 'when there are ActiveRecord validation errors' do let(:updated_title) { '' } @@ -112,11 +122,19 @@ RSpec.describe 'Updating a Snippet' do expect(mutation_response['snippet']['visibilityLevel']).to eq('private') end end + end - it_behaves_like 'spam flag is present' - it_behaves_like 'can raise spam flag' do - let(:service) { Snippets::UpdateService } + it_behaves_like 'a mutation which can mutate a spammable' do + let(:captcha_response) { 'abc123' } + let(:spam_log_id) { 1234 } + let(:spam_mutation_vars) do + { + captcha_response: captcha_response, + spam_log_id: spam_log_id + } end + + let(:service) { Snippets::UpdateService } end def blob_at(filename) diff --git a/spec/requests/api/graphql/packages/package_composer_details_spec.rb b/spec/requests/api/graphql/packages/package_composer_details_spec.rb deleted file mode 100644 index 1a2cf4a972a..00000000000 --- a/spec/requests/api/graphql/packages/package_composer_details_spec.rb +++ /dev/null @@ -1,39 +0,0 @@ -# frozen_string_literal: true -require 'spec_helper' - -RSpec.describe 'package composer details' do - using RSpec::Parameterized::TableSyntax - include GraphqlHelpers - - let_it_be(:project) { create(:project) } - let_it_be(:package) { create(:composer_package, project: project) } - let_it_be(:composer_metadatum) do - # we are forced to manually create the metadatum, without using the factory to force the sha to be a string - # and avoid an error where gitaly can't find the repository - create(:composer_metadatum, package: package, target_sha: 'foo_sha', composer_json: { name: 'name', type: 'type', license: 'license', version: 1 }) - end - - let(:query) do - graphql_query_for( - 'packageComposerDetails', - { id: package_global_id }, - all_graphql_fields_for('PackageComposerDetails', max_depth: 2) - ) - end - - let(:user) { project.owner } - let(:package_global_id) { package.to_global_id.to_s } - let(:package_composer_details_response) { graphql_data.dig('packageComposerDetails') } - - subject { post_graphql(query, current_user: user) } - - it_behaves_like 'a working graphql query' do - before do - subject - end - - it 'matches the JSON schema' do - expect(package_composer_details_response).to match_schema('graphql/packages/package_composer_details') - end - end -end diff --git a/spec/requests/api/graphql/packages/package_spec.rb b/spec/requests/api/graphql/packages/package_spec.rb new file mode 100644 index 00000000000..bb3ceb81f16 --- /dev/null +++ b/spec/requests/api/graphql/packages/package_spec.rb @@ -0,0 +1,80 @@ +# frozen_string_literal: true +require 'spec_helper' + +RSpec.describe 'package details' do + using RSpec::Parameterized::TableSyntax + include GraphqlHelpers + + let_it_be(:project) { create(:project) } + let_it_be(:package) { create(:composer_package, project: project) } + let_it_be(:composer_json) { { name: 'name', type: 'type', license: 'license', version: 1 } } + let_it_be(:composer_metadatum) do + # we are forced to manually create the metadatum, without using the factory to force the sha to be a string + # and avoid an error where gitaly can't find the repository + create(:composer_metadatum, package: package, target_sha: 'foo_sha', composer_json: composer_json) + end + + let(:depth) { 3 } + let(:excluded) { %w[metadata apiFuzzingCiConfiguration] } + + let(:query) do + graphql_query_for(:package, { id: package_global_id }, <<~FIELDS) + #{all_graphql_fields_for('Package', max_depth: depth, excluded: excluded)} + metadata { + #{query_graphql_fragment('ComposerMetadata')} + } + FIELDS + end + + let(:user) { project.owner } + let(:package_global_id) { global_id_of(package) } + let(:package_details) { graphql_data_at(:package) } + + subject { post_graphql(query, current_user: user) } + + it_behaves_like 'a working graphql query' do + before do + subject + end + + it 'matches the JSON schema' do + expect(package_details).to match_schema('graphql/packages/package_details') + end + + it 'includes the fields of the correct package' do + expect(package_details).to include( + 'id' => package_global_id, + 'metadata' => { + 'targetSha' => 'foo_sha', + 'composerJson' => composer_json.transform_keys(&:to_s).transform_values(&:to_s) + } + ) + end + end + + context 'there are other versions of this package' do + let(:depth) { 3 } + let(:excluded) { %w[metadata project tags pipelines] } # to limit the query complexity + + let_it_be(:siblings) { create_list(:composer_package, 2, project: project, name: package.name) } + + it 'includes the sibling versions' do + subject + + expect(graphql_data_at(:package, :versions, :nodes)).to match_array( + siblings.map { |p| a_hash_including('id' => global_id_of(p)) } + ) + end + + context 'going deeper' do + let(:depth) { 6 } + + it 'does not create a cycle of versions' do + subject + + expect(graphql_data_at(:package, :versions, :nodes, :version)).to be_present + expect(graphql_data_at(:package, :versions, :nodes, :versions)).not_to be_present + end + end + end +end diff --git a/spec/requests/api/graphql/project/container_repositories_spec.rb b/spec/requests/api/graphql/project/container_repositories_spec.rb index 6b1c8689515..2087d8c2cc3 100644 --- a/spec/requests/api/graphql/project/container_repositories_spec.rb +++ b/spec/requests/api/graphql/project/container_repositories_spec.rb @@ -156,4 +156,51 @@ RSpec.describe 'getting container repositories in a project' do expect(container_repositories_count_response).to eq(container_repositories.size) end + + describe 'sorting and pagination' do + let_it_be(:data_path) { [:project, :container_repositories] } + let_it_be(:sort_project) { create(:project, :public) } + let_it_be(:current_user) { create(:user) } + let_it_be(:container_repository1) { create(:container_repository, name: 'b', project: sort_project) } + let_it_be(:container_repository2) { create(:container_repository, name: 'a', project: sort_project) } + let_it_be(:container_repository3) { create(:container_repository, name: 'd', project: sort_project) } + let_it_be(:container_repository4) { create(:container_repository, name: 'c', project: sort_project) } + let_it_be(:container_repository5) { create(:container_repository, name: 'e', project: sort_project) } + + before do + stub_container_registry_tags(repository: container_repository1.path, tags: %w(tag1 tag1 tag3), with_manifest: false) + stub_container_registry_tags(repository: container_repository2.path, tags: %w(tag4 tag5 tag6), with_manifest: false) + stub_container_registry_tags(repository: container_repository3.path, tags: %w(tag7 tag8), with_manifest: false) + stub_container_registry_tags(repository: container_repository4.path, tags: %w(tag9), with_manifest: false) + stub_container_registry_tags(repository: container_repository5.path, tags: %w(tag10 tag11), with_manifest: false) + end + + def pagination_query(params) + graphql_query_for(:project, { full_path: sort_project.full_path }, + query_nodes(:container_repositories, :name, include_pagination_info: true, args: params) + ) + end + + def pagination_results_data(data) + data.map { |container_repository| container_repository.dig('name') } + end + + context 'when sorting by name' do + context 'when ascending' do + it_behaves_like 'sorted paginated query' do + let(:sort_param) { :NAME_ASC } + let(:first_param) { 2 } + let(:expected_results) { [container_repository2.name, container_repository1.name, container_repository4.name, container_repository3.name, container_repository5.name] } + end + end + + context 'when descending' do + it_behaves_like 'sorted paginated query' do + let(:sort_param) { :NAME_DESC } + let(:first_param) { 2 } + let(:expected_results) { [container_repository5.name, container_repository3.name, container_repository4.name, container_repository1.name, container_repository2.name] } + end + end + end + end end diff --git a/spec/requests/api/graphql/project/issue/designs/notes_spec.rb b/spec/requests/api/graphql/project/issue/designs/notes_spec.rb index a671ddc7ab1..7148750b6cb 100644 --- a/spec/requests/api/graphql/project/issue/designs/notes_spec.rb +++ b/spec/requests/api/graphql/project/issue/designs/notes_spec.rb @@ -22,6 +22,23 @@ RSpec.describe 'Getting designs related to an issue' do end end + it_behaves_like 'a noteable graphql type we can query' do + let(:noteable) { design } + let(:note_factory) { :diff_note_on_design } + let(:discussion_factory) { :diff_note_on_design } + let(:path_to_noteable) { [:issue, :design_collection, :designs, :nodes, 0] } + + before do + project.add_developer(current_user) + end + + def query(fields) + graphql_query_for(:issue, { id: global_id_of(issue) }, <<~FIELDS) + designCollection { designs { nodes { #{fields} } } } + FIELDS + end + end + it 'is not too deep for anonymous users' do note_fields = <<~FIELDS id @@ -37,7 +54,7 @@ RSpec.describe 'Getting designs related to an issue' do expect(note_data['id']).to eq(note.to_global_id.to_s) end - def query(note_fields = all_graphql_fields_for(Note)) + def query(note_fields = all_graphql_fields_for(Note, max_depth: 1)) design_node = <<~NODE designs { nodes { diff --git a/spec/requests/api/graphql/project/merge_request/pipelines_spec.rb b/spec/requests/api/graphql/project/merge_request/pipelines_spec.rb index ac0b18a37d6..70c5bda35e1 100644 --- a/spec/requests/api/graphql/project/merge_request/pipelines_spec.rb +++ b/spec/requests/api/graphql/project/merge_request/pipelines_spec.rb @@ -8,9 +8,11 @@ RSpec.describe 'Query.project.mergeRequests.pipelines' do let_it_be(:project) { create(:project, :public, :repository) } let_it_be(:author) { create(:user) } let_it_be(:merge_requests) do - %i[with_diffs with_image_diffs conflict].map do |trait| - create(:merge_request, trait, author: author, source_project: project) - end + [ + create(:merge_request, author: author, source_project: project), + create(:merge_request, :with_image_diffs, author: author, source_project: project), + create(:merge_request, :conflict, author: author, source_project: project) + ] end describe '.count' do diff --git a/spec/requests/api/graphql/project/merge_request_spec.rb b/spec/requests/api/graphql/project/merge_request_spec.rb index e1b867ad097..a4e8d0bc35e 100644 --- a/spec/requests/api/graphql/project/merge_request_spec.rb +++ b/spec/requests/api/graphql/project/merge_request_spec.rb @@ -66,14 +66,6 @@ RSpec.describe 'getting merge request information nested in a project' do expect(graphql_data_at(:project, :merge_request, :reviewers, :nodes)).to match_array(expected) expect(graphql_data_at(:project, :merge_request, :participants, :nodes)).to include(*expected) end - - it 'suppresses reviewers if reviewers are not allowed' do - stub_feature_flags(merge_request_reviewers: false) - - post_graphql(query, current_user: current_user) - - expect(graphql_data_at(:project, :merge_request, :reviewers)).to be_nil - end end it 'includes diff stats' do diff --git a/spec/requests/api/graphql/project/merge_requests_spec.rb b/spec/requests/api/graphql/project/merge_requests_spec.rb index c85cb8b2ffe..d684be91dc9 100644 --- a/spec/requests/api/graphql/project/merge_requests_spec.rb +++ b/spec/requests/api/graphql/project/merge_requests_spec.rb @@ -196,17 +196,18 @@ RSpec.describe 'getting merge request listings nested in a project' do end context 'when requesting `commit_count`' do - let(:requested_fields) { [:commit_count] } + let(:merge_request_with_commits) { create(:merge_request, source_project: project) } + let(:search_params) { { iids: [merge_request_a.iid.to_s, merge_request_with_commits.iid.to_s] } } + let(:requested_fields) { [:iid, :commit_count] } it 'exposes `commit_count`' do - merge_request_a.metrics.update!(commits_count: 5) - execute_query - expect(results).to include(a_hash_including('commitCount' => 5)) + expect(results).to match_array([ + { "iid" => merge_request_a.iid.to_s, "commitCount" => 0 }, + { "iid" => merge_request_with_commits.iid.to_s, "commitCount" => 29 } + ]) end - - include_examples 'N+1 query check' end context 'when requesting `merged_at`' do @@ -264,18 +265,6 @@ RSpec.describe 'getting merge request listings nested in a project' do }) end - context 'the feature flag is disabled' do - before do - stub_feature_flags(merge_request_reviewers: false) - end - - it 'does not return reviewers' do - execute_query - - expect(results).to all(match a_hash_including('reviewers' => be_nil)) - end - end - include_examples 'N+1 query check' end end @@ -396,4 +385,87 @@ RSpec.describe 'getting merge request listings nested in a project' do end end end + + context 'when only the count is requested' do + context 'when merged at filter is present' do + let_it_be(:merge_request) do + create(:merge_request, :unique_branches, source_project: project).tap do |mr| + mr.metrics.update!(merged_at: Time.new(2020, 1, 3)) + end + end + + let(:query) do + # Note: __typename meta field is always requested by the FE + graphql_query_for(:project, { full_path: project.full_path }, + <<~QUERY + mergeRequests(mergedAfter: "2020-01-01", mergedBefore: "2020-01-05", first: 0, sourceBranches: null, labels: null) { + count + __typename + } + QUERY + ) + end + + shared_examples 'count examples' do + it 'returns the correct count' do + post_graphql(query, current_user: current_user) + + count = graphql_data.dig('project', 'mergeRequests', 'count') + expect(count).to eq(1) + end + end + + context 'when "optimized_merge_request_count_with_merged_at_filter" feature flag is enabled' do + before do + stub_feature_flags(optimized_merge_request_count_with_merged_at_filter: true) + end + + it 'does not query the merge requests table for the count' do + query_recorder = ActiveRecord::QueryRecorder.new { post_graphql(query, current_user: current_user) } + + queries = query_recorder.data.each_value.first[:occurrences] + expect(queries).not_to include(match(/SELECT COUNT\(\*\) FROM "merge_requests"/)) + expect(queries).to include(match(/SELECT COUNT\(\*\) FROM "merge_request_metrics"/)) + end + + context 'when total_time_to_merge and count is queried' do + let(:query) do + graphql_query_for(:project, { full_path: project.full_path }, + <<~QUERY + mergeRequests(mergedAfter: "2020-01-01", mergedBefore: "2020-01-05", first: 0) { + totalTimeToMerge + count + } + QUERY + ) + end + + it 'does not query the merge requests table for the total_time_to_merge' do + query_recorder = ActiveRecord::QueryRecorder.new { post_graphql(query, current_user: current_user) } + + queries = query_recorder.data.each_value.first[:occurrences] + expect(queries).to include(match(/SELECT.+SUM.+FROM "merge_request_metrics" WHERE/)) + end + end + + it_behaves_like 'count examples' + + context 'when "optimized_merge_request_count_with_merged_at_filter" feature flag is disabled' do + before do + stub_feature_flags(optimized_merge_request_count_with_merged_at_filter: false) + end + + it 'queries the merge requests table for the count' do + query_recorder = ActiveRecord::QueryRecorder.new { post_graphql(query, current_user: current_user) } + + queries = query_recorder.data.each_value.first[:occurrences] + expect(queries).to include(match(/SELECT COUNT\(\*\) FROM "merge_requests"/)) + expect(queries).not_to include(match(/SELECT COUNT\(\*\) FROM "merge_request_metrics"/)) + end + + it_behaves_like 'count examples' + end + end + end + end end diff --git a/spec/requests/api/graphql/project/packages_spec.rb b/spec/requests/api/graphql/project/packages_spec.rb index 5df98ed1e6b..b20c96d54c8 100644 --- a/spec/requests/api/graphql/project/packages_spec.rb +++ b/spec/requests/api/graphql/project/packages_spec.rb @@ -5,16 +5,27 @@ require 'spec_helper' RSpec.describe 'getting a package list for a project' do include GraphqlHelpers - let_it_be(:project) { create(:project) } + let_it_be(:project) { create(:project, :repository) } let_it_be(:current_user) { create(:user) } + let_it_be(:package) { create(:package, project: project) } - let(:packages_data) { graphql_data['project']['packages']['edges'] } + let_it_be(:maven_package) { create(:maven_package, project: project) } + let_it_be(:debian_package) { create(:debian_package, project: project) } + let_it_be(:composer_package) { create(:composer_package, project: project) } + let_it_be(:composer_metadatum) do + create(:composer_metadatum, package: composer_package, + target_sha: 'afdeh', + composer_json: { name: 'x', type: 'y', license: 'z', version: 1 }) + end + + let(:package_names) { graphql_data_at(:project, :packages, :edges, :node, :name) } let(:fields) do <<~QUERY edges { node { - #{all_graphql_fields_for('packages'.classify)} + #{all_graphql_fields_for('packages'.classify, excluded: ['project'])} + metadata { #{query_graphql_fragment('ComposerMetadata')} } } } QUERY @@ -37,7 +48,17 @@ RSpec.describe 'getting a package list for a project' do it_behaves_like 'a working graphql query' it 'returns packages successfully' do - expect(packages_data[0]['node']['name']).to eq package.name + expect(package_names).to contain_exactly( + package.name, + maven_package.name, + debian_package.name, + composer_package.name + ) + end + + it 'deals with metadata' do + target_shas = graphql_data_at(:project, :packages, :edges, :node, :metadata, :target_sha) + expect(target_shas).to contain_exactly(composer_metadatum.target_sha) end end @@ -53,7 +74,7 @@ RSpec.describe 'getting a package list for a project' do end end - context 'when the user is not autenthicated' do + context 'when the user is not authenticated' do before do post_graphql(query) end diff --git a/spec/requests/api/graphql/project/release_spec.rb b/spec/requests/api/graphql/project/release_spec.rb index ccc2825da25..72197f00df4 100644 --- a/spec/requests/api/graphql/project/release_spec.rb +++ b/spec/requests/api/graphql/project/release_spec.rb @@ -283,7 +283,7 @@ RSpec.describe 'Query.project(fullPath).release(tagName)' do let_it_be(:project) { create(:project, :repository, :private) } let_it_be(:milestone_1) { create(:milestone, project: project) } let_it_be(:milestone_2) { create(:milestone, project: project) } - let_it_be(:release) { create(:release, :with_evidence, project: project, milestones: [milestone_1, milestone_2], released_at: released_at) } + let_it_be(:release, reload: true) { create(:release, :with_evidence, project: project, milestones: [milestone_1, milestone_2], released_at: released_at) } let_it_be(:release_link_1) { create(:release_link, release: release) } let_it_be(:release_link_2) { create(:release_link, release: release, filepath: link_filepath) } @@ -324,7 +324,7 @@ RSpec.describe 'Query.project(fullPath).release(tagName)' do let_it_be(:project) { create(:project, :repository, :public) } let_it_be(:milestone_1) { create(:milestone, project: project) } let_it_be(:milestone_2) { create(:milestone, project: project) } - let_it_be(:release) { create(:release, :with_evidence, project: project, milestones: [milestone_1, milestone_2], released_at: released_at) } + let_it_be(:release, reload: true) { create(:release, :with_evidence, project: project, milestones: [milestone_1, milestone_2], released_at: released_at) } let_it_be(:release_link_1) { create(:release_link, release: release) } let_it_be(:release_link_2) { create(:release_link, release: release, filepath: link_filepath) } @@ -435,13 +435,13 @@ RSpec.describe 'Query.project(fullPath).release(tagName)' do let_it_be_with_reload(:release) { create(:release, project: project) } let(:release_fields) do - query_graphql_field(%{ + %{ milestones { nodes { title } } - }) + } end let(:actual_milestone_title_order) do diff --git a/spec/requests/api/graphql/project/terraform/state_spec.rb b/spec/requests/api/graphql/project/terraform/state_spec.rb new file mode 100644 index 00000000000..9f1d9ab204a --- /dev/null +++ b/spec/requests/api/graphql/project/terraform/state_spec.rb @@ -0,0 +1,83 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'query a single terraform state' do + include GraphqlHelpers + include ::API::Helpers::RelatedResourcesHelpers + + let_it_be(:terraform_state) { create(:terraform_state, :with_version, :locked) } + + let(:latest_version) { terraform_state.latest_version } + let(:project) { terraform_state.project } + let(:current_user) { project.creator } + let(:data) { graphql_data.dig('project', 'terraformState') } + + let(:query) do + graphql_query_for( + :project, + { fullPath: project.full_path }, + query_graphql_field( + :terraformState, + { name: terraform_state.name }, + %{ + id + name + lockedAt + createdAt + updatedAt + + latestVersion { + id + serial + createdAt + updatedAt + + createdByUser { + id + } + + job { + name + } + } + + lockedByUser { + id + } + } + ) + ) + end + + before do + post_graphql(query, current_user: current_user) + end + + it_behaves_like 'a working graphql query' + + it 'returns terraform state data' do + expect(data).to match(a_hash_including({ + 'id' => global_id_of(terraform_state), + 'name' => terraform_state.name, + 'lockedAt' => terraform_state.locked_at.iso8601, + 'createdAt' => terraform_state.created_at.iso8601, + 'updatedAt' => terraform_state.updated_at.iso8601, + 'lockedByUser' => { 'id' => global_id_of(terraform_state.locked_by_user) }, + 'latestVersion' => { + 'id' => eq(global_id_of(latest_version)), + 'serial' => eq(latest_version.version), + 'createdAt' => eq(latest_version.created_at.iso8601), + 'updatedAt' => eq(latest_version.updated_at.iso8601), + 'createdByUser' => { 'id' => eq(global_id_of(latest_version.created_by_user)) }, + 'job' => { 'name' => eq(latest_version.build.name) } + } + })) + end + + context 'unauthorized users' do + let(:current_user) { nil } + + it { expect(data).to be_nil } + end +end diff --git a/spec/requests/api/group_import_spec.rb b/spec/requests/api/group_import_spec.rb index d8e945baf6a..bb7436502ed 100644 --- a/spec/requests/api/group_import_spec.rb +++ b/spec/requests/api/group_import_spec.rb @@ -5,13 +5,13 @@ require 'spec_helper' RSpec.describe API::GroupImport do include WorkhorseHelpers + include_context 'workhorse headers' + let_it_be(:user) { create(:user) } let_it_be(:group) { create(:group) } let(:path) { '/groups/import' } let(:file) { File.join('spec', 'fixtures', 'group_export.tar.gz') } let(:export_path) { "#{Dir.tmpdir}/group_export_spec" } - let(:workhorse_token) { JWT.encode({ 'iss' => 'gitlab-workhorse' }, Gitlab::Workhorse.secret, 'HS256') } - let(:workhorse_headers) { { 'GitLab-Workhorse' => '1.0', Gitlab::Workhorse::INTERNAL_API_REQUEST_HEADER => workhorse_token } } before do allow_next_instance_of(Gitlab::ImportExport) do |import_export| diff --git a/spec/requests/api/group_labels_spec.rb b/spec/requests/api/group_labels_spec.rb index 72621e2ce5e..c677e68b285 100644 --- a/spec/requests/api/group_labels_spec.rb +++ b/spec/requests/api/group_labels_spec.rb @@ -3,13 +3,19 @@ require 'spec_helper' RSpec.describe API::GroupLabels do + let_it_be(:valid_group_label_title_1) { 'Label foo & bar:subgroup::v.1' } + let_it_be(:valid_group_label_title_1_esc) { ERB::Util.url_encode(valid_group_label_title_1) } + let_it_be(:valid_group_label_title_2) { 'Bar & foo:subgroup::v.2' } + let_it_be(:valid_subgroup_label_title_1) { 'Support label foobar:sub::v.1' } + let_it_be(:valid_new_label_title) { 'New & foo:feature::v.3' } + let(:user) { create(:user) } let(:group) { create(:group) } let(:subgroup) { create(:group, parent: group) } let!(:group_member) { create(:group_member, group: group, user: user) } - let!(:group_label1) { create(:group_label, title: 'feature-label', group: group) } - let!(:group_label2) { create(:group_label, title: 'bug', group: group) } - let!(:subgroup_label) { create(:group_label, title: 'support-label', group: subgroup) } + let!(:group_label1) { create(:group_label, title: valid_group_label_title_1, group: group) } + let!(:group_label2) { create(:group_label, title: valid_group_label_title_2, group: group) } + let!(:subgroup_label) { create(:group_label, title: valid_subgroup_label_title_1, group: subgroup) } describe 'GET :id/labels' do context 'get current group labels' do @@ -104,7 +110,7 @@ RSpec.describe API::GroupLabels do describe 'GET :id/labels/:label_id' do it 'returns a single label for the group' do - get api("/groups/#{group.id}/labels/#{group_label1.name}", user) + get api("/groups/#{group.id}/labels/#{valid_group_label_title_1_esc}", user) expect(response).to have_gitlab_http_status(:ok) expect(json_response['name']).to eq(group_label1.name) @@ -117,13 +123,13 @@ RSpec.describe API::GroupLabels do it 'returns created label when all params are given' do post api("/groups/#{group.id}/labels", user), params: { - name: 'Foo', + name: valid_new_label_title, color: '#FFAABB', description: 'test' } expect(response).to have_gitlab_http_status(:created) - expect(json_response['name']).to eq('Foo') + expect(json_response['name']).to eq(valid_new_label_title) expect(json_response['color']).to eq('#FFAABB') expect(json_response['description']).to eq('test') end @@ -131,12 +137,12 @@ RSpec.describe API::GroupLabels do it 'returns created label when only required params are given' do post api("/groups/#{group.id}/labels", user), params: { - name: 'Foo & Bar', + name: valid_new_label_title, color: '#FFAABB' } expect(response).to have_gitlab_http_status(:created) - expect(json_response['name']).to eq('Foo & Bar') + expect(json_response['name']).to eq(valid_new_label_title) expect(json_response['color']).to eq('#FFAABB') expect(json_response['description']).to be_nil end @@ -204,7 +210,7 @@ RSpec.describe API::GroupLabels do describe 'DELETE /groups/:id/labels/:label_id' do it 'returns 204 for existing label' do - delete api("/groups/#{group.id}/labels/#{group_label1.name}", user) + delete api("/groups/#{group.id}/labels/#{valid_group_label_title_1_esc}", user) expect(response).to have_gitlab_http_status(:no_content) end @@ -228,7 +234,7 @@ RSpec.describe API::GroupLabels do end it_behaves_like '412 response' do - let(:request) { api("/groups/#{group.id}/labels/#{group_label1.name}", user) } + let(:request) { api("/groups/#{group.id}/labels/#{valid_group_label_title_1_esc}", user) } end end @@ -237,13 +243,13 @@ RSpec.describe API::GroupLabels do put api("/groups/#{group.id}/labels", user), params: { name: group_label1.name, - new_name: 'New Label', + new_name: valid_new_label_title, color: '#FFFFFF', description: 'test' } expect(response).to have_gitlab_http_status(:ok) - expect(json_response['name']).to eq('New Label') + expect(json_response['name']).to eq(valid_new_label_title) expect(json_response['color']).to eq('#FFFFFF') expect(json_response['description']).to eq('test') end @@ -255,11 +261,11 @@ RSpec.describe API::GroupLabels do put api("/groups/#{subgroup.id}/labels", user), params: { name: subgroup_label.name, - new_name: 'New Label' + new_name: valid_new_label_title } expect(response).to have_gitlab_http_status(:ok) - expect(subgroup.labels[0].name).to eq('New Label') + expect(subgroup.labels[0].name).to eq(valid_new_label_title) expect(group_label1.name).to eq(group_label1.title) end @@ -267,7 +273,7 @@ RSpec.describe API::GroupLabels do put api("/groups/#{group.id}/labels", user), params: { name: 'not_exists', - new_name: 'label3' + new_name: valid_new_label_title } expect(response).to have_gitlab_http_status(:not_found) @@ -291,15 +297,15 @@ RSpec.describe API::GroupLabels do describe 'PUT /groups/:id/labels/:label_id' do it 'returns 200 if name and colors and description are changed' do - put api("/groups/#{group.id}/labels/#{group_label1.name}", user), + put api("/groups/#{group.id}/labels/#{valid_group_label_title_1_esc}", user), params: { - new_name: 'New Label', + new_name: valid_new_label_title, color: '#FFFFFF', description: 'test' } expect(response).to have_gitlab_http_status(:ok) - expect(json_response['name']).to eq('New Label') + expect(json_response['name']).to eq(valid_new_label_title) expect(json_response['color']).to eq('#FFFFFF') expect(json_response['description']).to eq('test') end @@ -310,25 +316,25 @@ RSpec.describe API::GroupLabels do put api("/groups/#{subgroup.id}/labels/#{subgroup_label.name}", user), params: { - new_name: 'New Label' + new_name: valid_new_label_title } expect(response).to have_gitlab_http_status(:ok) - expect(subgroup.labels[0].name).to eq('New Label') + expect(subgroup.labels[0].name).to eq(valid_new_label_title) expect(group_label1.name).to eq(group_label1.title) end it 'returns 404 if label does not exist' do put api("/groups/#{group.id}/labels/not_exists", user), params: { - new_name: 'label3' + new_name: valid_new_label_title } expect(response).to have_gitlab_http_status(:not_found) end it 'returns 400 if no new parameters given' do - put api("/groups/#{group.id}/labels/#{group_label1.name}", user) + put api("/groups/#{group.id}/labels/#{valid_group_label_title_1_esc}", user) expect(response).to have_gitlab_http_status(:bad_request) expect(json_response['error']).to eq('new_name, color, description are missing, '\ @@ -339,7 +345,7 @@ RSpec.describe API::GroupLabels do describe 'POST /groups/:id/labels/:label_id/subscribe' do context 'when label_id is a label title' do it 'subscribes to the label' do - post api("/groups/#{group.id}/labels/#{group_label1.title}/subscribe", user) + post api("/groups/#{group.id}/labels/#{valid_group_label_title_1_esc}/subscribe", user) expect(response).to have_gitlab_http_status(:created) expect(json_response['name']).to eq(group_label1.title) @@ -385,7 +391,7 @@ RSpec.describe API::GroupLabels do context 'when label_id is a label title' do it 'unsubscribes from the label' do - post api("/groups/#{group.id}/labels/#{group_label1.title}/unsubscribe", user) + post api("/groups/#{group.id}/labels/#{valid_group_label_title_1_esc}/unsubscribe", user) expect(response).to have_gitlab_http_status(:created) expect(json_response['name']).to eq(group_label1.title) diff --git a/spec/requests/api/group_packages_spec.rb b/spec/requests/api/group_packages_spec.rb index 26895e473de..792aa2c1f20 100644 --- a/spec/requests/api/group_packages_spec.rb +++ b/spec/requests/api/group_packages_spec.rb @@ -144,6 +144,7 @@ RSpec.describe API::GroupPackages do end it_behaves_like 'with versionless packages' + it_behaves_like 'with status param' it_behaves_like 'does not cause n^2 queries' end end diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb index c7756a4fae5..1c359b6e50f 100644 --- a/spec/requests/api/groups_spec.rb +++ b/spec/requests/api/groups_spec.rb @@ -54,7 +54,7 @@ RSpec.describe API::Groups do it_behaves_like 'invalid file upload request' end - context 'when file format is not supported' do + context 'when file is too large' do let(:file_path) { 'spec/fixtures/big-image.png' } let(:message) { 'is too big' } @@ -661,6 +661,7 @@ RSpec.describe API::Groups do describe 'PUT /groups/:id' do let(:new_group_name) { 'New Group'} + let(:file_path) { 'spec/fixtures/dk.png' } it_behaves_like 'group avatar upload' do def make_upload_request @@ -678,7 +679,8 @@ RSpec.describe API::Groups do request_access_enabled: true, project_creation_level: "noone", subgroup_creation_level: "maintainer", - default_branch_protection: ::Gitlab::Access::MAINTAINER_PROJECT_ACCESS + default_branch_protection: ::Gitlab::Access::MAINTAINER_PROJECT_ACCESS, + avatar: fixture_file_upload(file_path) } expect(response).to have_gitlab_http_status(:ok) @@ -701,6 +703,7 @@ RSpec.describe API::Groups do expect(json_response['shared_projects']).to be_an Array expect(json_response['shared_projects'].length).to eq(0) expect(json_response['default_branch_protection']).to eq(::Gitlab::Access::MAINTAINER_PROJECT_ACCESS) + expect(json_response['avatar_url']).to end_with('dk.png') end context 'updating the `default_branch_protection` attribute' do diff --git a/spec/requests/api/internal/base_spec.rb b/spec/requests/api/internal/base_spec.rb index e04f63befd0..86999c4adaa 100644 --- a/spec/requests/api/internal/base_spec.rb +++ b/spec/requests/api/internal/base_spec.rb @@ -50,41 +50,6 @@ RSpec.describe API::Internal::Base do end end - shared_examples 'actor key validations' do - context 'key id is not provided' do - let(:key_id) { nil } - - it 'returns an error message' do - subject - - expect(json_response['success']).to be_falsey - expect(json_response['message']).to eq('Could not find a user without a key') - end - end - - context 'key does not exist' do - let(:key_id) { non_existing_record_id } - - it 'returns an error message' do - subject - - expect(json_response['success']).to be_falsey - expect(json_response['message']).to eq('Could not find the given key') - end - end - - context 'key without user' do - let(:key_id) { create(:key, user: nil).id } - - it 'returns an error message' do - subject - - expect(json_response['success']).to be_falsey - expect(json_response['message']).to eq('Could not find a user for the given key') - end - end - end - describe 'GET /internal/two_factor_recovery_codes' do let(:key_id) { key.id } @@ -578,25 +543,51 @@ RSpec.describe API::Internal::Base do end context "git pull" do - before do - stub_feature_flags(gitaly_mep_mep: true) + context "with a feature flag enabled globally" do + before do + stub_feature_flags(gitaly_mep_mep: true) + end + + it "has the correct payload" do + pull(key, project) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response["status"]).to be_truthy + expect(json_response["gl_repository"]).to eq("project-#{project.id}") + expect(json_response["gl_project_path"]).to eq(project.full_path) + 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(json_response["gitaly"]["features"]).to eq('gitaly-feature-mep-mep' => 'true') + expect(user.reload.last_activity_on).to eql(Date.today) + end end - it "has the correct payload" do - pull(key, project) + context "with a feature flag enabled for a project" do + before do + stub_feature_flags(gitaly_mep_mep: project) + end - expect(response).to have_gitlab_http_status(:ok) - expect(json_response["status"]).to be_truthy - expect(json_response["gl_repository"]).to eq("project-#{project.id}") - expect(json_response["gl_project_path"]).to eq(project.full_path) - 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(json_response["gitaly"]["features"]).to eq('gitaly-feature-mep-mep' => 'true') - expect(user.reload.last_activity_on).to eql(Date.today) + it "has the flag set to true for that project" do + pull(key, project) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response["gl_repository"]).to eq("project-#{project.id}") + expect(json_response["gitaly"]["features"]).to eq('gitaly-feature-mep-mep' => 'true') + end + + it "has the flag set to false for other projects" do + other_project = create(:project, :public, :repository) + + pull(key, other_project) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response["gl_repository"]).to eq("project-#{other_project.id}") + expect(json_response["gitaly"]["features"]).to eq('gitaly-feature-mep-mep' => 'false') + end end end @@ -1094,6 +1085,104 @@ RSpec.describe API::Internal::Base do expect(response).to have_gitlab_http_status(:unauthorized) end end + + context 'admin mode' do + shared_examples 'pushes succeed for ssh and http' do + it 'accepts the SSH push' do + push(key, project) + + expect(response).to have_gitlab_http_status(:ok) + end + + it 'accepts the HTTP push' do + push(key, project, 'http') + + expect(response).to have_gitlab_http_status(:ok) + end + end + + shared_examples 'pushes fail for ssh and http' do + it 'rejects the SSH push' do + push(key, project) + + expect(response).to have_gitlab_http_status(:not_found) + end + + it 'rejects the HTTP push' do + push(key, project, 'http') + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'feature flag :user_mode_in_session is enabled' do + context 'with an admin user' do + let(:user) { create(:admin) } + + context 'is member of the project' do + before do + project.add_developer(user) + end + + it_behaves_like 'pushes succeed for ssh and http' + end + + context 'is not member of the project' do + it_behaves_like 'pushes succeed for ssh and http' + end + end + + context 'with a regular user' do + context 'is member of the project' do + before do + project.add_developer(user) + end + + it_behaves_like 'pushes succeed for ssh and http' + end + + context 'is not member of the project' do + it_behaves_like 'pushes fail for ssh and http' + end + end + end + + context 'feature flag :user_mode_in_session is disabled' do + before do + stub_feature_flags(user_mode_in_session: false) + end + + context 'with an admin user' do + let(:user) { create(:admin) } + + context 'is member of the project' do + before do + project.add_developer(user) + end + + it_behaves_like 'pushes succeed for ssh and http' + end + + context 'is not member of the project' do + it_behaves_like 'pushes succeed for ssh and http' + end + end + + context 'with a regular user' do + context 'is member of the project' do + before do + project.add_developer(user) + end + + it_behaves_like 'pushes succeed for ssh and http' + end + + context 'is not member of the project' do + it_behaves_like 'pushes fail for ssh and http' + end + end + end + end end describe 'POST /internal/post_receive', :clean_gitlab_redis_shared_state do @@ -1308,10 +1397,6 @@ RSpec.describe API::Internal::Base do let(:key_id) { key.id } let(:otp) { '123456'} - before do - stub_feature_flags(two_factor_for_cli: true) - end - subject do post api('/internal/two_factor_otp_check'), params: { @@ -1321,76 +1406,10 @@ RSpec.describe API::Internal::Base do } end - it_behaves_like 'actor key validations' - - context 'when the key is a deploy key' do - let(:key_id) { create(:deploy_key).id } - - it 'returns an error message' do - subject - - expect(json_response['success']).to be_falsey - expect(json_response['message']).to eq('Deploy keys cannot be used for Two Factor') - end - end - - context 'when the two factor is enabled' do - before do - allow_any_instance_of(User).to receive(:two_factor_enabled?).and_return(true) - end - - context 'when the OTP is valid' do - it 'registers a new OTP session and returns success' do - allow_any_instance_of(Users::ValidateOtpService).to receive(:execute).with(otp).and_return(status: :success) - - expect_next_instance_of(::Gitlab::Auth::Otp::SessionEnforcer) do |session_enforcer| - expect(session_enforcer).to receive(:update_session).once - end - - subject - - expect(json_response['success']).to be_truthy - end - end - - context 'when the OTP is invalid' do - it 'is not success' do - allow_any_instance_of(Users::ValidateOtpService).to receive(:execute).with(otp).and_return(status: :error) - - subject - - expect(json_response['success']).to be_falsey - end - end - end - - context 'when the two factor is disabled' do - before do - allow_any_instance_of(User).to receive(:two_factor_enabled?).and_return(false) - end + it 'is not available' do + subject - it 'returns an error message' do - subject - - expect(json_response['success']).to be_falsey - expect(json_response['message']).to eq 'Two-factor authentication is not enabled for this user' - end - end - - context 'two_factor_for_cli feature is disabled' do - before do - stub_feature_flags(two_factor_for_cli: false) - end - - context 'when two-factor is enabled for the user' do - it 'returns user two factor config' do - allow_any_instance_of(User).to receive(:two_factor_enabled?).and_return(true) - - subject - - expect(json_response['success']).to be_falsey - end - end + expect(json_response['success']).to be_falsey end end diff --git a/spec/requests/api/internal/kubernetes_spec.rb b/spec/requests/api/internal/kubernetes_spec.rb index afff3647b91..2e13016a0a6 100644 --- a/spec/requests/api/internal/kubernetes_spec.rb +++ b/spec/requests/api/internal/kubernetes_spec.rb @@ -62,25 +62,25 @@ RSpec.describe API::Internal::Kubernetes do let!(:agent_token) { create(:cluster_agent_token) } it 'returns no_content for valid gitops_sync_count' do - send_request(params: { gitops_sync_count: 10 }, headers: { 'Authorization' => "Bearer #{agent_token.token}" }) + send_request(params: { gitops_sync_count: 10 }) expect(response).to have_gitlab_http_status(:no_content) end it 'returns no_content 0 gitops_sync_count' do - send_request(params: { gitops_sync_count: 0 }, headers: { 'Authorization' => "Bearer #{agent_token.token}" }) + send_request(params: { gitops_sync_count: 0 }) expect(response).to have_gitlab_http_status(:no_content) end it 'returns 400 for non number' do - send_request(params: { gitops_sync_count: 'string' }, headers: { 'Authorization' => "Bearer #{agent_token.token}" }) + send_request(params: { gitops_sync_count: 'string' }) expect(response).to have_gitlab_http_status(:bad_request) end it 'returns 400 for negative number' do - send_request(params: { gitops_sync_count: '-1' }, headers: { 'Authorization' => "Bearer #{agent_token.token}" }) + send_request(params: { gitops_sync_count: '-1' }) expect(response).to have_gitlab_http_status(:bad_request) end @@ -125,6 +125,36 @@ RSpec.describe API::Internal::Kubernetes do ) ) end + + context 'on GitLab.com' do + before do + allow(::Gitlab).to receive(:com?).and_return(true) + end + + context 'kubernetes_agent_on_gitlab_com feature flag disabled' do + before do + stub_feature_flags(kubernetes_agent_on_gitlab_com: false) + end + + it 'returns 403' do + send_request(headers: { 'Authorization' => "Bearer #{agent_token.token}" }) + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'kubernetes_agent_on_gitlab_com feature flag enabled' do + before do + stub_feature_flags(kubernetes_agent_on_gitlab_com: agent_token.agent.project) + end + + it 'returns success' do + send_request(headers: { 'Authorization' => "Bearer #{agent_token.token}" }) + + expect(response).to have_gitlab_http_status(:success) + end + end + end end end @@ -174,6 +204,36 @@ RSpec.describe API::Internal::Kubernetes do expect(response).to have_gitlab_http_status(:not_found) end end + + context 'on GitLab.com' do + before do + allow(::Gitlab).to receive(:com?).and_return(true) + end + + context 'kubernetes_agent_on_gitlab_com feature flag disabled' do + before do + stub_feature_flags(kubernetes_agent_on_gitlab_com: false) + end + + it 'returns 403' do + send_request(params: { id: project.id }, headers: { 'Authorization' => "Bearer #{agent_token.token}" }) + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'kubernetes_agent_on_gitlab_com feature flag enabled' do + before do + stub_feature_flags(kubernetes_agent_on_gitlab_com: agent_token.agent.project) + end + + it 'returns success' do + send_request(params: { id: project.id }, headers: { 'Authorization' => "Bearer #{agent_token.token}" }) + + expect(response).to have_gitlab_http_status(:success) + end + end + end end context 'project is private' do diff --git a/spec/requests/api/jobs_spec.rb b/spec/requests/api/jobs_spec.rb index 6f854a28cec..1c43ef25f14 100644 --- a/spec/requests/api/jobs_spec.rb +++ b/spec/requests/api/jobs_spec.rb @@ -17,7 +17,7 @@ RSpec.describe API::Jobs do end let!(:job) do - create(:ci_build, :success, pipeline: pipeline, + create(:ci_build, :success, :tags, pipeline: pipeline, artifacts_expire_at: 1.day.since) end @@ -50,6 +50,7 @@ RSpec.describe API::Jobs do expect(json_response).not_to be_empty expect(json_response.first['commit']['id']).to eq project.commit.id expect(Time.parse(json_response.first['artifacts_expire_at'])).to be_like_time(job.artifacts_expire_at) + expect(json_response.first['tag_list'].sort).to eq job.tag_list.sort end context 'without artifacts and trace' do diff --git a/spec/requests/api/labels_spec.rb b/spec/requests/api/labels_spec.rb index 6db6de4b533..e3fffd3e3fd 100644 --- a/spec/requests/api/labels_spec.rb +++ b/spec/requests/api/labels_spec.rb @@ -10,14 +10,19 @@ RSpec.describe API::Labels do else label_id = spec_params[:name] || spec_params[:label_id] - put api("/projects/#{project.id}/labels/#{label_id}", user), + put api("/projects/#{project.id}/labels/#{ERB::Util.url_encode(label_id)}", user), params: request_params.merge(spec_params.except(:name, :id)) end end + let_it_be(:valid_label_title_1) { 'Label foo & bar:subgroup::v.1' } + let_it_be(:valid_label_title_1_esc) { ERB::Util.url_encode(valid_label_title_1) } + let_it_be(:valid_label_title_2) { 'Label bar & foo:subgroup::v.2' } + let_it_be(:valid_group_label_title_1) { 'Group label foobar:sub::v.1' } + let(:user) { create(:user) } let(:project) { create(:project, creator_id: user.id, namespace: user.namespace) } - let!(:label1) { create(:label, description: 'the best label', title: 'label1', project: project) } + let!(:label1) { create(:label, description: 'the best label v.1', title: valid_label_title_1, project: project) } let!(:priority_label) { create(:label, title: 'bug', project: project, priority: 3) } route_types = [:deprecated, :rest] @@ -25,10 +30,10 @@ RSpec.describe API::Labels do shared_examples 'label update API' do route_types.each do |route_type| it "returns 200 if name is changed (#{route_type} route)" do - put_labels_api(route_type, user, spec_params, new_name: 'New Label') + put_labels_api(route_type, user, spec_params, new_name: valid_label_title_2) expect(response).to have_gitlab_http_status(:ok) - expect(json_response['name']).to eq('New Label') + expect(json_response['name']).to eq(valid_label_title_2) expect(json_response['color']).to eq(label1.color) end @@ -77,10 +82,10 @@ RSpec.describe API::Labels do end it "returns 200 if name and colors and description are changed (#{route_type} route)" do - put_labels_api(route_type, user, spec_params, new_name: 'New Label', color: '#FFFFFF', description: 'test') + put_labels_api(route_type, user, spec_params, new_name: valid_label_title_2, color: '#FFFFFF', description: 'test') expect(response).to have_gitlab_http_status(:ok) - expect(json_response['name']).to eq('New Label') + expect(json_response['name']).to eq(valid_label_title_2) expect(json_response['color']).to eq('#FFFFFF') expect(json_response['description']).to eq('test') end @@ -141,7 +146,7 @@ RSpec.describe API::Labels do priority: nil }.merge(spec_params.except(:name, :id)) - put api("/projects/#{project.id}/labels/#{label_id}", user), + put api("/projects/#{project.id}/labels/#{ERB::Util.url_encode(label_id)}", user), params: request_params expect(response).to have_gitlab_http_status(:ok) @@ -167,7 +172,7 @@ RSpec.describe API::Labels do it 'returns 204 for existing label (rest route)' do label_id = spec_params[:name] || spec_params[:label_id] - delete api("/projects/#{project.id}/labels/#{label_id}", user), params: spec_params.except(:name, :label_id) + delete api("/projects/#{project.id}/labels/#{ERB::Util.url_encode(label_id)}", user), params: spec_params.except(:name, :label_id) expect(response).to have_gitlab_http_status(:no_content) end @@ -179,7 +184,7 @@ RSpec.describe API::Labels do describe 'GET /projects/:id/labels' do let_it_be(:group) { create(:group) } - let_it_be(:group_label) { create(:group_label, title: 'feature label', group: group) } + let_it_be(:group_label) { create(:group_label, title: valid_group_label_title_1, group: group) } before do project.update!(group: group) @@ -219,7 +224,7 @@ RSpec.describe API::Labels do 'closed_issues_count' => 1, 'open_merge_requests_count' => 0, 'name' => label1.name, - 'description' => 'the best label', + 'description' => label1.description, 'color' => a_string_matching(/^#\h{6}$/), 'text_color' => a_string_matching(/^#\h{6}$/), 'priority' => nil, @@ -293,14 +298,14 @@ RSpec.describe API::Labels do it 'returns created label when all params' do post api("/projects/#{project.id}/labels", user), params: { - name: 'Foo', + name: valid_label_title_2, color: '#FFAABB', description: 'test', priority: 2 } expect(response).to have_gitlab_http_status(:created) - expect(json_response['name']).to eq('Foo') + expect(json_response['name']).to eq(valid_label_title_2) expect(json_response['color']).to eq('#FFAABB') expect(json_response['description']).to eq('test') expect(json_response['priority']).to eq(2) @@ -309,12 +314,12 @@ RSpec.describe API::Labels do it 'returns created label when only required params' do post api("/projects/#{project.id}/labels", user), params: { - name: 'Foo & Bar', + name: valid_label_title_2, color: '#FFAABB' } expect(response).to have_gitlab_http_status(:created) - expect(json_response['name']).to eq('Foo & Bar') + expect(json_response['name']).to eq(valid_label_title_2) expect(json_response['color']).to eq('#FFAABB') expect(json_response['description']).to be_nil expect(json_response['priority']).to be_nil @@ -323,13 +328,13 @@ RSpec.describe API::Labels do it 'creates a prioritized label' do post api("/projects/#{project.id}/labels", user), params: { - name: 'Foo & Bar', + name: valid_label_title_2, color: '#FFAABB', priority: 3 } expect(response).to have_gitlab_http_status(:created) - expect(json_response['name']).to eq('Foo & Bar') + expect(json_response['name']).to eq(valid_label_title_2) expect(json_response['color']).to eq('#FFAABB') expect(json_response['description']).to be_nil expect(json_response['priority']).to eq(3) @@ -348,7 +353,7 @@ RSpec.describe API::Labels do it 'returns 400 for invalid color' do post api("/projects/#{project.id}/labels", user), params: { - name: 'Foo', + name: valid_label_title_2, color: '#FFAA' } expect(response).to have_gitlab_http_status(:bad_request) @@ -358,7 +363,7 @@ RSpec.describe API::Labels do it 'returns 400 for too long color code' do post api("/projects/#{project.id}/labels", user), params: { - name: 'Foo', + name: valid_label_title_2, color: '#FFAAFFFF' } expect(response).to have_gitlab_http_status(:bad_request) @@ -393,7 +398,7 @@ RSpec.describe API::Labels do it 'returns 400 for invalid priority' do post api("/projects/#{project.id}/labels", user), params: { - name: 'Foo', + name: valid_label_title_2, color: '#FFAAFFFF', priority: 'foo' } @@ -404,7 +409,7 @@ RSpec.describe API::Labels do it 'returns 409 if label already exists in project' do post api("/projects/#{project.id}/labels", user), params: { - name: 'label1', + name: valid_label_title_1, color: '#FFAABB' } expect(response).to have_gitlab_http_status(:conflict) @@ -414,7 +419,7 @@ RSpec.describe API::Labels do describe 'DELETE /projects/:id/labels' do it_behaves_like 'label delete API' do - let(:spec_params) { { name: 'label1' } } + let(:spec_params) { { name: valid_label_title_1 } } end it_behaves_like 'label delete API' do @@ -422,7 +427,7 @@ RSpec.describe API::Labels do end it 'returns 404 for non existing label' do - delete api("/projects/#{project.id}/labels", user), params: { name: 'label2' } + delete api("/projects/#{project.id}/labels", user), params: { name: 'unknown' } expect(response).to have_gitlab_http_status(:not_found) expect(json_response['message']).to eq('404 Label Not Found') @@ -446,14 +451,14 @@ RSpec.describe API::Labels do it_behaves_like '412 response' do let(:request) { api("/projects/#{project.id}/labels", user) } - let(:params) { { name: 'label1' } } + let(:params) { { name: valid_label_title_1 } } end end describe 'PUT /projects/:id/labels' do context 'when using name' do it_behaves_like 'label update API' do - let(:spec_params) { { name: 'label1' } } + let(:spec_params) { { name: valid_label_title_1 } } let(:expected_response_label_id) { label1.id } end end @@ -468,7 +473,7 @@ RSpec.describe API::Labels do it 'returns 404 if label does not exist' do put api("/projects/#{project.id}/labels", user), params: { - name: 'label2', + name: valid_label_title_2, new_name: 'label3' } @@ -571,7 +576,7 @@ RSpec.describe API::Labels do describe "POST /projects/:id/labels/:label_id/subscribe" do context "when label_id is a label title" do it "subscribes to the label" do - post api("/projects/#{project.id}/labels/#{label1.title}/subscribe", user) + post api("/projects/#{project.id}/labels/#{valid_label_title_1_esc}/subscribe", user) expect(response).to have_gitlab_http_status(:created) expect(json_response["name"]).to eq(label1.title) @@ -617,7 +622,7 @@ RSpec.describe API::Labels do context "when label_id is a label title" do it "unsubscribes from the label" do - post api("/projects/#{project.id}/labels/#{label1.title}/unsubscribe", user) + post api("/projects/#{project.id}/labels/#{valid_label_title_1_esc}/unsubscribe", user) expect(response).to have_gitlab_http_status(:created) expect(json_response["name"]).to eq(label1.title) diff --git a/spec/requests/api/maven_packages_spec.rb b/spec/requests/api/maven_packages_spec.rb index e5d11fb1218..7f0e4f18e3b 100644 --- a/spec/requests/api/maven_packages_spec.rb +++ b/spec/requests/api/maven_packages_spec.rb @@ -4,6 +4,8 @@ require 'spec_helper' RSpec.describe API::MavenPackages do include WorkhorseHelpers + include_context 'workhorse headers' + let_it_be_with_refind(:package_settings) { create(:namespace_package_setting, :group) } let_it_be(:group) { package_settings.namespace } let_it_be(:user) { create(:user) } @@ -20,8 +22,7 @@ RSpec.describe API::MavenPackages do let_it_be(:group_deploy_token) { create(:group_deploy_token, deploy_token: deploy_token_for_group, group: group) } let(:package_name) { 'com/example/my-app' } - let(:workhorse_token) { JWT.encode({ 'iss' => 'gitlab-workhorse' }, Gitlab::Workhorse.secret, 'HS256') } - let(:headers) { { 'GitLab-Workhorse' => '1.0', Gitlab::Workhorse::INTERNAL_API_REQUEST_HEADER => workhorse_token } } + let(:headers) { workhorse_headers } let(:headers_with_token) { headers.merge('Private-Token' => personal_access_token.token) } let(:group_deploy_token_headers) { { Gitlab::Auth::AuthFinders::DEPLOY_TOKEN_HEADER => deploy_token_for_group.token } } @@ -32,6 +33,7 @@ RSpec.describe API::MavenPackages do end let(:version) { '1.0-SNAPSHOT' } + let(:param_path) { "#{package_name}/#{version}"} before do project.add_developer(user) @@ -547,8 +549,8 @@ RSpec.describe API::MavenPackages do end describe 'PUT /api/v4/projects/:id/packages/maven/*path/:file_name' do - let(:workhorse_token) { JWT.encode({ 'iss' => 'gitlab-workhorse' }, Gitlab::Workhorse.secret, 'HS256') } - let(:workhorse_header) { { 'GitLab-Workhorse' => '1.0', Gitlab::Workhorse::INTERNAL_API_REQUEST_HEADER => workhorse_token } } + include_context 'workhorse headers' + let(:send_rewritten_field) { true } let(:file_upload) { fixture_file_upload('spec/fixtures/packages/maven/my-app-1.0-20180724.124855-1.jar') } @@ -601,7 +603,7 @@ RSpec.describe API::MavenPackages do end context 'without workhorse header' do - let(:workhorse_header) { {} } + let(:workhorse_headers) { {} } subject { upload_file_with_token(params: params) } @@ -695,6 +697,14 @@ RSpec.describe API::MavenPackages do expect(json_response['message']).to include('Duplicate package is not allowed') end + context 'when uploading to the versionless package which contains metadata about all versions' do + let(:version) { nil } + let(:param_path) { package_name } + let!(:package) { create(:maven_package, project: project, version: version, name: project.full_path) } + + it_behaves_like 'storing the package file' + end + context 'when uploading different non-duplicate files to the same package' do let!(:package) { create(:maven_package, project: project, name: project.full_path) } @@ -744,7 +754,7 @@ RSpec.describe API::MavenPackages do end def upload_file(params: {}, request_headers: headers, file_extension: 'jar') - url = "/projects/#{project.id}/packages/maven/#{package_name}/#{version}/my-app-1.0-20180724.124855-1.#{file_extension}" + url = "/projects/#{project.id}/packages/maven/#{param_path}/my-app-1.0-20180724.124855-1.#{file_extension}" workhorse_finalize( api(url), method: :put, diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index a04867658e8..ad8e21bf4c1 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -1322,7 +1322,16 @@ RSpec.describe API::MergeRequests do end context 'Work in Progress' do - let!(:merge_request_wip) { create(:merge_request, author: user, assignees: [user], source_project: project, target_project: project, title: "WIP: Test", created_at: base_time + 1.second) } + let!(:merge_request_wip) do + create(:merge_request, + author: user, + assignees: [user], + source_project: project, + target_project: project, + title: "WIP: Test", + created_at: base_time + 1.second + ) + end it "returns merge request" do get api("/projects/#{project.id}/merge_requests/#{merge_request_wip.iid}", user) @@ -1566,9 +1575,9 @@ RSpec.describe API::MergeRequests do end end - context 'when access_raw_diffs is passed as an option' do + context 'when access_raw_diffs is true' do it_behaves_like 'accesses diffs via raw_diffs' do - let(:params) { { access_raw_diffs: true } } + let(:params) { { access_raw_diffs: "true" } } end end end @@ -1766,6 +1775,36 @@ RSpec.describe API::MergeRequests do end end + context 'accepts reviewer_ids' do + let(:params) do + { + title: 'Test merge request', + source_branch: 'feature_conflict', + target_branch: 'master', + author_id: user.id, + reviewer_ids: [user2.id] + } + end + + it 'creates a new merge request with a reviewer' do + post api("/projects/#{project.id}/merge_requests", user), params: params + + expect(response).to have_gitlab_http_status(:created) + expect(json_response['title']).to eq('Test merge request') + expect(json_response['reviewers'].first['name']).to eq(user2.name) + end + + it 'creates a new merge request with no reviewer' do + params[:reviewer_ids] = [] + + post api("/projects/#{project.id}/merge_requests", user), params: params + + expect(response).to have_gitlab_http_status(:created) + expect(json_response['title']).to eq('Test merge request') + expect(json_response['reviewers']).to be_empty + end + end + context 'between branches projects' do context 'different labels' do let(:params) do @@ -2111,6 +2150,34 @@ RSpec.describe API::MergeRequests do it_behaves_like 'issuable update endpoint' do let(:entity) { merge_request } end + + context 'accepts reviewer_ids' do + let(:params) do + { + title: 'Updated merge request', + reviewer_ids: [user2.id] + } + end + + it 'adds a reviewer to the existing merge request' do + put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user), params: params + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['title']).to eq('Updated merge request') + expect(json_response['reviewers'].first['name']).to eq(user2.name) + end + + it 'removes a reviewer from the existing merge request' do + merge_request.reviewers = [user2] + params[:reviewer_ids] = [] + + put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user), params: params + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['title']).to eq('Updated merge request') + expect(json_response['reviewers']).to be_empty + end + end end describe "POST /projects/:id/merge_requests/:merge_request_iid/context_commits" do diff --git a/spec/requests/api/npm_instance_packages_spec.rb b/spec/requests/api/npm_instance_packages_spec.rb index 8299717b5c7..70c76067a6e 100644 --- a/spec/requests/api/npm_instance_packages_spec.rb +++ b/spec/requests/api/npm_instance_packages_spec.rb @@ -6,7 +6,7 @@ RSpec.describe API::NpmInstancePackages do include_context 'npm api setup' describe 'GET /api/v4/packages/npm/*package_name' do - it_behaves_like 'handling get metadata requests' do + it_behaves_like 'handling get metadata requests', scope: :instance do let(:url) { api("/packages/npm/#{package_name}") } end end diff --git a/spec/requests/api/npm_project_packages_spec.rb b/spec/requests/api/npm_project_packages_spec.rb index 1421f20ac28..7ea238c0607 100644 --- a/spec/requests/api/npm_project_packages_spec.rb +++ b/spec/requests/api/npm_project_packages_spec.rb @@ -6,25 +6,25 @@ RSpec.describe API::NpmProjectPackages do include_context 'npm api setup' describe 'GET /api/v4/projects/:id/packages/npm/*package_name' do - it_behaves_like 'handling get metadata requests' do + it_behaves_like 'handling get metadata requests', scope: :project do let(:url) { api("/projects/#{project.id}/packages/npm/#{package_name}") } end end describe 'GET /api/v4/projects/:id/packages/npm/-/package/*package_name/dist-tags' do - it_behaves_like 'handling get dist tags requests' do + it_behaves_like 'handling get dist tags requests', scope: :project do let(:url) { api("/projects/#{project.id}/packages/npm/-/package/#{package_name}/dist-tags") } end end describe 'PUT /api/v4/projects/:id/packages/npm/-/package/*package_name/dist-tags/:tag' do - it_behaves_like 'handling create dist tag requests' do + it_behaves_like 'handling create dist tag requests', scope: :project do let(:url) { api("/projects/#{project.id}/packages/npm/-/package/#{package_name}/dist-tags/#{tag_name}") } end end describe 'DELETE /api/v4/projects/:id/packages/npm/-/package/*package_name/dist-tags/:tag' do - it_behaves_like 'handling delete dist tag requests' do + it_behaves_like 'handling delete dist tag requests', scope: :project do let(:url) { api("/projects/#{project.id}/packages/npm/-/package/#{package_name}/dist-tags/#{tag_name}") } end end @@ -32,10 +32,14 @@ RSpec.describe API::NpmProjectPackages do describe 'GET /api/v4/projects/:id/packages/npm/*package_name/-/*file_name' do let_it_be(:package_file) { package.package_files.first } - let(:params) { {} } - let(:url) { api("/projects/#{project.id}/packages/npm/#{package_file.package.name}/-/#{package_file.file_name}") } + let(:headers) { {} } + let(:url) { api("/projects/#{project.id}/packages/npm/#{package.name}/-/#{package_file.file_name}") } - subject { get(url, params: params) } + subject { get(url, headers: headers) } + + before do + project.add_developer(user) + end shared_examples 'a package file that requires auth' do it 'denies download with no token' do @@ -45,7 +49,7 @@ RSpec.describe API::NpmProjectPackages do end context 'with access token' do - let(:params) { { access_token: token.token } } + let(:headers) { build_token_auth_header(token.token) } it 'returns the file' do subject @@ -56,7 +60,7 @@ RSpec.describe API::NpmProjectPackages do end context 'with job token' do - let(:params) { { job_token: job.token } } + let(:headers) { build_token_auth_header(job.token) } it 'returns the file' do subject @@ -86,7 +90,7 @@ RSpec.describe API::NpmProjectPackages do it_behaves_like 'a package file that requires auth' context 'with guest' do - let(:params) { { access_token: token.token } } + let(:headers) { build_token_auth_header(token.token) } it 'denies download when not enough permissions' do project.add_guest(user) @@ -108,7 +112,11 @@ RSpec.describe API::NpmProjectPackages do end describe 'PUT /api/v4/projects/:id/packages/npm/:package_name' do - RSpec.shared_examples 'handling invalid record with 400 error' do + before do + project.add_developer(user) + end + + shared_examples 'handling invalid record with 400 error' do it 'handles an ActiveRecord::RecordInvalid exception with 400 error' do expect { upload_package_with_token(package_name, params) } .not_to change { project.packages.count } @@ -261,7 +269,9 @@ RSpec.describe API::NpmProjectPackages do end def upload_package(package_name, params = {}) - put api("/projects/#{project.id}/packages/npm/#{package_name.sub('/', '%2f')}"), params: params + token = params.delete(:access_token) || params.delete(:job_token) + headers = build_token_auth_header(token) + put api("/projects/#{project.id}/packages/npm/#{package_name.sub('/', '%2f')}"), params: params, headers: headers end def upload_package_with_token(package_name, params = {}) diff --git a/spec/requests/api/nuget_project_packages_spec.rb b/spec/requests/api/nuget_project_packages_spec.rb index 813ebc35ede..0277aa73220 100644 --- a/spec/requests/api/nuget_project_packages_spec.rb +++ b/spec/requests/api/nuget_project_packages_spec.rb @@ -144,8 +144,8 @@ RSpec.describe API::NugetProjectPackages do end describe 'PUT /api/v4/projects/:id/packages/nuget/authorize' do - let_it_be(:workhorse_token) { JWT.encode({ 'iss' => 'gitlab-workhorse' }, Gitlab::Workhorse.secret, 'HS256') } - let_it_be(:workhorse_header) { { 'GitLab-Workhorse' => '1.0', Gitlab::Workhorse::INTERNAL_API_REQUEST_HEADER => workhorse_token } } + include_context 'workhorse headers' + let(:url) { "/projects/#{target.id}/packages/nuget/authorize" } let(:headers) { {} } @@ -176,7 +176,7 @@ RSpec.describe API::NugetProjectPackages do with_them do let(:token) { user_token ? personal_access_token.token : 'wrong' } let(:user_headers) { user_role == :anonymous ? {} : basic_auth_header(user.username, token) } - let(:headers) { user_headers.merge(workhorse_header) } + let(:headers) { user_headers.merge(workhorse_headers) } before do update_visibility_to(Gitlab::VisibilityLevel.const_get(visibility_level, false)) @@ -194,8 +194,8 @@ RSpec.describe API::NugetProjectPackages do end describe 'PUT /api/v4/projects/:id/packages/nuget' do - let(:workhorse_token) { JWT.encode({ 'iss' => 'gitlab-workhorse' }, Gitlab::Workhorse.secret, 'HS256') } - let(:workhorse_header) { { 'GitLab-Workhorse' => '1.0', Gitlab::Workhorse::INTERNAL_API_REQUEST_HEADER => workhorse_token } } + include_context 'workhorse headers' + let_it_be(:file_name) { 'package.nupkg' } let(:url) { "/projects/#{target.id}/packages/nuget" } let(:headers) { {} } @@ -239,7 +239,7 @@ RSpec.describe API::NugetProjectPackages do with_them do let(:token) { user_token ? personal_access_token.token : 'wrong' } let(:user_headers) { user_role == :anonymous ? {} : basic_auth_header(user.username, token) } - let(:headers) { user_headers.merge(workhorse_header) } + let(:headers) { user_headers.merge(workhorse_headers) } before do update_visibility_to(Gitlab::VisibilityLevel.const_get(visibility_level, false)) @@ -256,7 +256,7 @@ RSpec.describe API::NugetProjectPackages do it_behaves_like 'rejects nuget access with invalid target id' context 'file size above maximum limit' do - let(:headers) { basic_auth_header(deploy_token.username, deploy_token.token).merge(workhorse_header) } + let(:headers) { basic_auth_header(deploy_token.username, deploy_token.token).merge(workhorse_headers) } before do allow_next_instance_of(UploadedFile) do |uploaded_file| diff --git a/spec/requests/api/oauth_tokens_spec.rb b/spec/requests/api/oauth_tokens_spec.rb index 23d5df873d4..52c7408545f 100644 --- a/spec/requests/api/oauth_tokens_spec.rb +++ b/spec/requests/api/oauth_tokens_spec.rb @@ -26,17 +26,14 @@ RSpec.describe 'OAuth tokens' do end context 'when user does not have 2FA enabled' do - # NOTE: using ROPS grant flow without client credentials will be deprecated - # and removed in the next version of Doorkeeper. - # See https://gitlab.com/gitlab-org/gitlab/-/issues/219137 context 'when no client credentials provided' do - it 'creates an access token' do + it 'does not create an access token' do user = create(:user) request_oauth_token(user) - expect(response).to have_gitlab_http_status(:ok) - expect(json_response['access_token']).not_to be_nil + expect(response).to have_gitlab_http_status(:unauthorized) + expect(json_response['access_token']).to be_nil end end @@ -54,15 +51,11 @@ RSpec.describe 'OAuth tokens' do context 'with invalid credentials' do it 'does not create an access token' do - # NOTE: remove this after update to Doorkeeper 5.5 or newer, see - # https://gitlab.com/gitlab-org/gitlab/-/issues/219137 - pending 'Enable this example after upgrading Doorkeeper to 5.5 or newer' - user = create(:user) request_oauth_token(user, basic_auth_header(client.uid, 'invalid secret')) - expect(response).to have_gitlab_http_status(:bad_request) + expect(response).to have_gitlab_http_status(:unauthorized) expect(json_response['error']).to eq('invalid_client') end end diff --git a/spec/requests/api/project_attributes.yml b/spec/requests/api/project_attributes.yml new file mode 100644 index 00000000000..181fcafd577 --- /dev/null +++ b/spec/requests/api/project_attributes.yml @@ -0,0 +1,149 @@ +--- +itself: # project + unexposed_attributes: + - bfg_object_map + - delete_error + - detected_repository_languages + - disable_overriding_approvers_per_merge_request + - external_authorization_classification_label + - external_webhook_token + - has_external_issue_tracker + - has_external_wiki + - import_source + - import_type + - import_url + - issues_template + - jobs_cache_index + - last_repository_check_at + - last_repository_check_failed + - last_repository_updated_at + - marked_for_deletion_at + - marked_for_deletion_by_user_id + - max_artifacts_size + - max_pages_size + - merge_requests_author_approval + - merge_requests_disable_committers_approval + - merge_requests_rebase_enabled + - merge_requests_template + - mirror_last_successful_update_at + - mirror_last_update_at + - mirror_overwrites_diverged_branches + - mirror_trigger_builds + - mirror_user_id + - only_mirror_protected_branches + - pages_https_only + - pending_delete + - pool_repository_id + - pull_mirror_available_overridden + - pull_mirror_branch_prefix + - remote_mirror_available_overridden + - repository_read_only + - repository_size_limit + - require_password_to_approve + - reset_approvals_on_push + - runners_token_encrypted + - storage_version + - updated_at + remapped_attributes: + avatar: avatar_url + build_allow_git_fetch: build_git_strategy + merge_requests_ff_only_enabled: merge_method + namespace_id: namespace + public_builds: public_jobs + visibility_level: visibility + computed_attributes: + - _links + - can_create_merge_request_in + - compliance_frameworks + - container_expiration_policy + - default_branch + - empty_repo + - forks_count + - http_url_to_repo + - name_with_namespace + - open_issues_count + - owner + - path_with_namespace + - permissions + - readme_url + - shared_with_groups + - ssh_url_to_repo + - web_url + +build_auto_devops: # auto_devops + unexposed_attributes: + - id + - project_id + - created_at + - updated_at + remapped_attributes: + enabled: auto_devops_enabled + deploy_strategy: auto_devops_deploy_strategy + +ci_cd_settings: + unexposed_attributes: + - id + - project_id + - group_runners_enabled + - keep_latest_artifact + - merge_pipelines_enabled + - merge_trains_enabled + - auto_rollback_enabled + remapped_attributes: + default_git_depth: ci_default_git_depth + forward_deployment_enabled: ci_forward_deployment_enabled + +build_import_state: # import_state + unexposed_attributes: + - id + - project_id + - retry_count + - last_update_started_at + - last_update_scheduled_at + - next_execution_timestamp + - jid + - last_update_at + - last_successful_update_at + - correlation_id_value + remapped_attributes: + status: import_status + last_error: import_error + +project_feature: + unexposed_attributes: + - id + - created_at + - metrics_dashboard_access_level + - project_id + - requirements_access_level + - security_and_compliance_access_level + - updated_at + computed_attributes: + - issues_enabled + - jobs_enabled + - merge_requests_enabled + - requirements_enabled + - security_and_compliance_enabled + - snippets_enabled + - wiki_enabled + +project_setting: + unexposed_attributes: + - allow_editing_commit_messages + - created_at + - has_confluence + - has_vulnerabilities + - prevent_merge_without_jira_issue + - project_id + - push_rule_id + - show_default_award_emojis + - squash_option + - updated_at + +build_service_desk_setting: # service_desk_setting + unexposed_attributes: + - project_id + - issue_template_key + - outgoing_name + remapped_attributes: + project_key: service_desk_address diff --git a/spec/requests/api/project_import_spec.rb b/spec/requests/api/project_import_spec.rb index 8e99d37c84f..a049d7d7515 100644 --- a/spec/requests/api/project_import_spec.rb +++ b/spec/requests/api/project_import_spec.rb @@ -5,13 +5,12 @@ require 'spec_helper' RSpec.describe API::ProjectImport do include WorkhorseHelpers + include_context 'workhorse headers' + let(:user) { create(:user) } let(:file) { File.join('spec', 'features', 'projects', 'import_export', 'test_project_export.tar.gz') } let(:namespace) { create(:group) } - let(:workhorse_token) { JWT.encode({ 'iss' => 'gitlab-workhorse' }, Gitlab::Workhorse.secret, 'HS256') } - let(:workhorse_headers) { { 'GitLab-Workhorse' => '1.0', Gitlab::Workhorse::INTERNAL_API_REQUEST_HEADER => workhorse_token } } - before do namespace.add_owner(user) end diff --git a/spec/requests/api/project_packages_spec.rb b/spec/requests/api/project_packages_spec.rb index eb86df36dbb..1f3887cab8a 100644 --- a/spec/requests/api/project_packages_spec.rb +++ b/spec/requests/api/project_packages_spec.rb @@ -120,6 +120,7 @@ RSpec.describe API::ProjectPackages do end it_behaves_like 'with versionless packages' + it_behaves_like 'with status param' it_behaves_like 'does not cause n^2 queries' end end diff --git a/spec/requests/api/project_templates_spec.rb b/spec/requests/api/project_templates_spec.rb index fc1035fc17d..a424bc62014 100644 --- a/spec/requests/api/project_templates_spec.rb +++ b/spec/requests/api/project_templates_spec.rb @@ -131,7 +131,7 @@ RSpec.describe API::ProjectTemplates do end end - describe 'GET /projects/:id/templates/:type/:key' do + describe 'GET /projects/:id/templates/:type/:name' do it 'returns a specific dockerfile' do get api("/projects/#{public_project.id}/templates/dockerfiles/Binary") diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 8a4a7880ab4..ad36777184a 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -1140,7 +1140,7 @@ RSpec.describe API::Projects do let!(:public_project) { create(:project, :public, name: 'public_project', creator_id: user4.id, namespace: user4.namespace) } it 'returns error when user not found' do - get api('/users/0/projects/') + get api("/users/#{non_existing_record_id}/projects/") expect(response).to have_gitlab_http_status(:not_found) expect(json_response['message']).to eq('404 User Not Found') @@ -1540,6 +1540,35 @@ RSpec.describe API::Projects do end context 'when authenticated as an admin' do + let(:project_attributes_file) { 'spec/requests/api/project_attributes.yml' } + let(:project_attributes) { YAML.load_file(project_attributes_file) } + + let(:expected_keys) do + keys = project_attributes.map do |relation, relation_config| + begin + actual_keys = project.send(relation).attributes.keys + rescue NoMethodError + actual_keys = ["#{relation} is nil"] + end + unexposed_attributes = relation_config['unexposed_attributes'] || [] + remapped_attributes = relation_config['remapped_attributes'] || {} + computed_attributes = relation_config['computed_attributes'] || [] + actual_keys - unexposed_attributes - remapped_attributes.keys + remapped_attributes.values + computed_attributes + end.flatten + + unless Gitlab.ee? + keys -= %w[ + approvals_before_merge + compliance_frameworks + mirror + requirements_enabled + security_and_compliance_enabled + ] + end + + keys + end + it 'returns a project by id' do project project_member @@ -1588,6 +1617,27 @@ RSpec.describe API::Projects do expect(json_response['only_allow_merge_if_all_discussions_are_resolved']).to eq(project.only_allow_merge_if_all_discussions_are_resolved) expect(json_response['operations_access_level']).to be_present end + + it 'exposes all necessary attributes' do + create(:project_group_link, project: project) + + get api("/projects/#{project.id}", admin) + + diff = Set.new(json_response.keys) ^ Set.new(expected_keys) + + expect(diff).to be_empty, failure_message(diff) + end + + def failure_message(diff) + <<~MSG + It looks like project's set of exposed attributes is different from the expected set. + + The following attributes are missing or newly added: + #{diff.to_a.to_sentence} + + Please update #{project_attributes_file} file" + MSG + end end context 'when authenticated as a regular user' do @@ -2155,7 +2205,7 @@ RSpec.describe API::Projects do end it 'fails if forked_from project which does not exist' do - post api("/projects/#{project_fork_target.id}/fork/0", admin) + post api("/projects/#{project_fork_target.id}/fork/#{non_existing_record_id}", admin) expect(response).to have_gitlab_http_status(:not_found) end @@ -2399,7 +2449,7 @@ RSpec.describe API::Projects do end it 'returns a 404 error when project does not exist' do - delete api("/projects/123/share/#{non_existing_record_id}", user) + delete api("/projects/#{non_existing_record_id}/share/#{non_existing_record_id}", user) expect(response).to have_gitlab_http_status(:not_found) end @@ -2956,7 +3006,7 @@ RSpec.describe API::Projects do end it 'returns the proper security headers' do - get api('/projects/1/starrers', current_user) + get api("/projects/#{public_project.id}/starrers", current_user) expect(response).to include_security_headers end @@ -3029,7 +3079,7 @@ RSpec.describe API::Projects do end it 'returns not_found(404) for not existing project' do - get api("/projects/0/languages", user) + get api("/projects/#{non_existing_record_id}/languages", user) expect(response).to have_gitlab_http_status(:not_found) end @@ -3080,7 +3130,7 @@ RSpec.describe API::Projects do end it 'does not remove a non existing project' do - delete api('/projects/1328', user) + delete api("/projects/#{non_existing_record_id}", user) expect(response).to have_gitlab_http_status(:not_found) end @@ -3099,7 +3149,7 @@ RSpec.describe API::Projects do end it 'does not remove a non existing project' do - delete api('/projects/1328', admin) + delete api("/projects/#{non_existing_record_id}", admin) expect(response).to have_gitlab_http_status(:not_found) end @@ -3329,8 +3379,8 @@ RSpec.describe API::Projects do expect(json_response['message']['path']).to eq(['has already been taken']) end - it 'accepts a name for the target project' do - post api("/projects/#{project.id}/fork", user2), params: { name: 'My Random Project' } + it 'accepts custom parameters for the target project' do + post api("/projects/#{project.id}/fork", user2), params: { name: 'My Random Project', description: 'A description', visibility: 'private' } expect(response).to have_gitlab_http_status(:created) expect(json_response['name']).to eq('My Random Project') @@ -3338,6 +3388,8 @@ RSpec.describe API::Projects do expect(json_response['owner']['id']).to eq(user2.id) expect(json_response['namespace']['id']).to eq(user2.namespace.id) expect(json_response['forked_from_project']['id']).to eq(project.id) + expect(json_response['description']).to eq('A description') + expect(json_response['visibility']).to eq('private') expect(json_response['import_status']).to eq('scheduled') expect(json_response).to include("import_error") end @@ -3369,6 +3421,13 @@ RSpec.describe API::Projects do expect(json_response['message']['path']).to eq(['has already been taken']) expect(json_response['message']['name']).to eq(['has already been taken']) end + + it 'fails to fork with an unknown visibility level' do + post api("/projects/#{project.id}/fork", user2), params: { visibility: 'something' } + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['error']).to eq('visibility does not have a valid value') + end end context 'when unauthenticated' do diff --git a/spec/requests/api/pypi_packages_spec.rb b/spec/requests/api/pypi_packages_spec.rb index 72a470dca4b..ae5b132f409 100644 --- a/spec/requests/api/pypi_packages_spec.rb +++ b/spec/requests/api/pypi_packages_spec.rb @@ -5,6 +5,7 @@ RSpec.describe API::PypiPackages do include WorkhorseHelpers include PackagesManagerApiSpecHelpers include HttpBasicAuthHelpers + using RSpec::Parameterized::TableSyntax let_it_be(:user) { create(:user) } let_it_be(:project, reload: true) { create(:project, :public) } @@ -20,8 +21,6 @@ RSpec.describe API::PypiPackages do subject { get api(url) } context 'with valid project' do - using RSpec::Parameterized::TableSyntax - where(:project_visibility_level, :user_role, :member, :user_token, :shared_examples_name, :expected_status) do 'PUBLIC' | :developer | true | true | 'PyPI package versions' | :success 'PUBLIC' | :guest | true | true | 'PyPI package versions' | :success @@ -75,16 +74,14 @@ RSpec.describe API::PypiPackages do end describe 'POST /api/v4/projects/:id/packages/pypi/authorize' do - let_it_be(:workhorse_token) { JWT.encode({ 'iss' => 'gitlab-workhorse' }, Gitlab::Workhorse.secret, 'HS256') } - let_it_be(:workhorse_header) { { 'GitLab-Workhorse' => '1.0', Gitlab::Workhorse::INTERNAL_API_REQUEST_HEADER => workhorse_token } } + include_context 'workhorse headers' + let(:url) { "/projects/#{project.id}/packages/pypi/authorize" } let(:headers) { {} } subject { post api(url), headers: headers } context 'with valid project' do - using RSpec::Parameterized::TableSyntax - where(:project_visibility_level, :user_role, :member, :user_token, :shared_examples_name, :expected_status) do 'PUBLIC' | :developer | true | true | 'process PyPI api request' | :success 'PUBLIC' | :guest | true | true | 'process PyPI api request' | :forbidden @@ -109,7 +106,7 @@ RSpec.describe API::PypiPackages do with_them do let(:token) { user_token ? personal_access_token.token : 'wrong' } let(:user_headers) { user_role == :anonymous ? {} : basic_auth_header(user.username, token) } - let(:headers) { user_headers.merge(workhorse_header) } + let(:headers) { user_headers.merge(workhorse_headers) } before do project.update!(visibility_level: Gitlab::VisibilityLevel.const_get(project_visibility_level, false)) @@ -127,8 +124,8 @@ RSpec.describe API::PypiPackages do end describe 'POST /api/v4/projects/:id/packages/pypi' do - let(:workhorse_token) { JWT.encode({ 'iss' => 'gitlab-workhorse' }, Gitlab::Workhorse.secret, 'HS256') } - let(:workhorse_header) { { 'GitLab-Workhorse' => '1.0', Gitlab::Workhorse::INTERNAL_API_REQUEST_HEADER => workhorse_token } } + include_context 'workhorse headers' + let_it_be(:file_name) { 'package.whl' } let(:url) { "/projects/#{project.id}/packages/pypi" } let(:headers) { {} } @@ -149,8 +146,6 @@ RSpec.describe API::PypiPackages do end context 'with valid project' do - using RSpec::Parameterized::TableSyntax - where(:project_visibility_level, :user_role, :member, :user_token, :shared_examples_name, :expected_status) do 'PUBLIC' | :developer | true | true | 'PyPI package creation' | :created 'PUBLIC' | :guest | true | true | 'process PyPI api request' | :forbidden @@ -175,7 +170,7 @@ RSpec.describe API::PypiPackages do with_them do let(:token) { user_token ? personal_access_token.token : 'wrong' } let(:user_headers) { user_role == :anonymous ? {} : basic_auth_header(user.username, token) } - let(:headers) { user_headers.merge(workhorse_header) } + let(:headers) { user_headers.merge(workhorse_headers) } before do project.update!(visibility_level: Gitlab::VisibilityLevel.const_get(project_visibility_level, false)) @@ -189,7 +184,7 @@ RSpec.describe API::PypiPackages do let(:requires_python) { 'x' * 256 } let(:token) { personal_access_token.token } let(:user_headers) { basic_auth_header(user.username, token) } - let(:headers) { user_headers.merge(workhorse_header) } + let(:headers) { user_headers.merge(workhorse_headers) } before do project.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE) @@ -201,7 +196,7 @@ RSpec.describe API::PypiPackages do context 'with an invalid package' do let(:token) { personal_access_token.token } let(:user_headers) { basic_auth_header(user.username, token) } - let(:headers) { user_headers.merge(workhorse_header) } + let(:headers) { user_headers.merge(workhorse_headers) } before do params[:name] = '.$/@!^*' @@ -218,7 +213,7 @@ RSpec.describe API::PypiPackages do it_behaves_like 'rejects PyPI access with unknown project id' context 'file size above maximum limit' do - let(:headers) { basic_auth_header(deploy_token.username, deploy_token.token).merge(workhorse_header) } + let(:headers) { basic_auth_header(deploy_token.username, deploy_token.token).merge(workhorse_headers) } before do allow_next_instance_of(UploadedFile) do |uploaded_file| @@ -239,8 +234,6 @@ RSpec.describe API::PypiPackages do subject { get api(url) } context 'with valid project' do - using RSpec::Parameterized::TableSyntax - where(:project_visibility_level, :user_role, :member, :user_token, :shared_examples_name, :expected_status) do 'PUBLIC' | :developer | true | true | 'PyPI package download' | :success 'PUBLIC' | :guest | true | true | 'PyPI package download' | :success diff --git a/spec/requests/api/repositories_spec.rb b/spec/requests/api/repositories_spec.rb index 45bce8c8a5c..ace73e49c7c 100644 --- a/spec/requests/api/repositories_spec.rb +++ b/spec/requests/api/repositories_spec.rb @@ -610,4 +610,85 @@ RSpec.describe API::Repositories do end end end + + describe 'POST /projects/:id/repository/changelog' do + it 'generates the changelog for a version' do + spy = instance_spy(Repositories::ChangelogService) + + allow(Repositories::ChangelogService) + .to receive(:new) + .with( + project, + user, + version: '1.0.0', + from: 'foo', + to: 'bar', + date: DateTime.new(2020, 1, 1), + branch: 'kittens', + trailer: 'Foo', + file: 'FOO.md', + message: 'Commit message' + ) + .and_return(spy) + + allow(spy).to receive(:execute) + + post( + api("/projects/#{project.id}/repository/changelog", user), + params: { + version: '1.0.0', + from: 'foo', + to: 'bar', + date: '2020-01-01', + branch: 'kittens', + trailer: 'Foo', + file: 'FOO.md', + message: 'Commit message' + } + ) + + expect(response).to have_gitlab_http_status(:ok) + end + + it 'produces an error when generating the changelog fails' do + spy = instance_spy(Repositories::ChangelogService) + + allow(Repositories::ChangelogService) + .to receive(:new) + .with( + project, + user, + version: '1.0.0', + from: 'foo', + to: 'bar', + date: DateTime.new(2020, 1, 1), + branch: 'kittens', + trailer: 'Foo', + file: 'FOO.md', + message: 'Commit message' + ) + .and_return(spy) + + allow(spy) + .to receive(:execute) + .and_raise(Gitlab::Changelog::Error.new('oops')) + + post( + api("/projects/#{project.id}/repository/changelog", user), + params: { + version: '1.0.0', + from: 'foo', + to: 'bar', + date: '2020-01-01', + branch: 'kittens', + trailer: 'Foo', + file: 'FOO.md', + message: 'Commit message' + } + ) + + expect(response).to have_gitlab_http_status(:unprocessable_entity) + expect(json_response['message']).to eq('Failed to generate the changelog: oops') + end + end end diff --git a/spec/requests/api/resource_access_tokens_spec.rb b/spec/requests/api/resource_access_tokens_spec.rb new file mode 100644 index 00000000000..9fd7eb2177d --- /dev/null +++ b/spec/requests/api/resource_access_tokens_spec.rb @@ -0,0 +1,293 @@ +# frozen_string_literal: true + +require "spec_helper" + +RSpec.describe API::ResourceAccessTokens do + context "when the resource is a project" do + let_it_be(:project) { create(:project) } + let_it_be(:other_project) { create(:project) } + let_it_be(:user) { create(:user) } + + describe "GET projects/:id/access_tokens" do + subject(:get_tokens) { get api("/projects/#{project_id}/access_tokens", user) } + + context "when the user has maintainer permissions" do + let_it_be(:project_bot) { create(:user, :project_bot) } + let_it_be(:access_tokens) { create_list(:personal_access_token, 3, user: project_bot) } + let_it_be(:project_id) { project.id } + + before do + project.add_maintainer(user) + project.add_maintainer(project_bot) + end + + it "gets a list of access tokens for the specified project" do + get_tokens + + token_ids = json_response.map { |token| token['id'] } + + expect(response).to have_gitlab_http_status(:ok) + expect(token_ids).to match_array(access_tokens.pluck(:id)) + end + + context "when using a project access token to GET other project access tokens" do + let_it_be(:token) { access_tokens.first } + + it "gets a list of access tokens for the specified project" do + get api("/projects/#{project_id}/access_tokens", personal_access_token: token) + + token_ids = json_response.map { |token| token['id'] } + + expect(response).to have_gitlab_http_status(:ok) + expect(token_ids).to match_array(access_tokens.pluck(:id)) + end + end + + context "when tokens belong to a different project" do + let_it_be(:bot) { create(:user, :project_bot) } + let_it_be(:token) { create(:personal_access_token, user: bot) } + + before do + other_project.add_maintainer(bot) + other_project.add_maintainer(user) + end + + it "does not return tokens from a different project" do + get_tokens + + token_ids = json_response.map { |token| token['id'] } + + expect(token_ids).not_to include(token.id) + end + end + + context "when the project has no access tokens" do + let(:project_id) { other_project.id } + + before do + other_project.add_maintainer(user) + end + + it 'returns an empty array' do + get_tokens + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to eq([]) + end + end + + context "when trying to get the tokens of a different project" do + let_it_be(:project_id) { other_project.id } + + it "returns 404" do + get_tokens + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context "when the project does not exist" do + let(:project_id) { non_existing_record_id } + + it "returns 404" do + get_tokens + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + + context "when the user does not have valid permissions" do + let_it_be(:project_bot) { create(:user, :project_bot) } + let_it_be(:access_tokens) { create_list(:personal_access_token, 3, user: project_bot) } + let_it_be(:project_id) { project.id } + + before do + project.add_developer(user) + project.add_maintainer(project_bot) + end + + it "returns 401" do + get_tokens + + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + end + + describe "DELETE projects/:id/access_tokens/:token_id", :sidekiq_inline do + subject(:delete_token) { delete api("/projects/#{project_id}/access_tokens/#{token_id}", user) } + + let_it_be(:project_bot) { create(:user, :project_bot) } + let_it_be(:token) { create(:personal_access_token, user: project_bot) } + let_it_be(:project_id) { project.id } + let_it_be(:token_id) { token.id } + + before do + project.add_maintainer(project_bot) + end + + context "when the user has maintainer permissions" do + before do + project.add_maintainer(user) + end + + it "deletes the project access token from the project" do + delete_token + + expect(response).to have_gitlab_http_status(:no_content) + expect(User.exists?(project_bot.id)).to be_falsy + end + + context "when attempting to delete a non-existent project access token" do + let_it_be(:token_id) { non_existing_record_id } + + it "does not delete the token, and returns 404" do + delete_token + + expect(response).to have_gitlab_http_status(:not_found) + expect(response.body).to include("Could not find project access token with token_id: #{token_id}") + end + end + + context "when attempting to delete a token that does not belong to the specified project" do + let_it_be(:project_id) { other_project.id } + + before do + other_project.add_maintainer(user) + end + + it "does not delete the token, and returns 404" do + delete_token + + expect(response).to have_gitlab_http_status(:not_found) + expect(response.body).to include("Could not find project access token with token_id: #{token_id}") + end + end + end + + context "when the user does not have valid permissions" do + before do + project.add_developer(user) + end + + it "does not delete the token, and returns 400", :aggregate_failures do + delete_token + + expect(response).to have_gitlab_http_status(:bad_request) + expect(User.exists?(project_bot.id)).to be_truthy + expect(response.body).to include("#{user.name} cannot delete #{token.user.name}") + end + end + end + + describe "POST projects/:id/access_tokens" do + let_it_be(:params) { { name: "test", scopes: ["api"], expires_at: Date.today + 1.month } } + + subject(:create_token) { post api("/projects/#{project_id}/access_tokens", user), params: params } + + context "when the user has maintainer permissions" do + let_it_be(:project_id) { project.id } + let_it_be(:expires_at) { 1.month.from_now } + + before do + project.add_maintainer(user) + end + + context "with valid params" do + context "with full params" do + it "creates a project access token with the params", :aggregate_failures do + create_token + + expect(response).to have_gitlab_http_status(:created) + expect(json_response["name"]).to eq("test") + expect(json_response["scopes"]).to eq(["api"]) + expect(json_response["expires_at"]).to eq(expires_at.to_date.iso8601) + end + end + + context "when 'expires_at' is not set" do + let_it_be(:params) { { name: "test", scopes: ["api"] } } + + it "creates a project access token with the params", :aggregate_failures do + create_token + + expect(response).to have_gitlab_http_status(:created) + expect(json_response["name"]).to eq("test") + expect(json_response["scopes"]).to eq(["api"]) + expect(json_response["expires_at"]).to eq(nil) + end + end + end + + context "with invalid params" do + context "when missing the 'name' param" do + let_it_be(:params) { { scopes: ["api"], expires_at: 5.days.from_now } } + + it "does not create a project access token without 'name'" do + create_token + + expect(response).to have_gitlab_http_status(:bad_request) + expect(response.body).to include("name is missing") + end + end + + context "when missing the 'scopes' param" do + let_it_be(:params) { { name: "test", expires_at: 5.days.from_now } } + + it "does not create a project access token without 'scopes'" do + create_token + + expect(response).to have_gitlab_http_status(:bad_request) + expect(response.body).to include("scopes is missing") + end + end + end + + context "when trying to create a token in a different project" do + let_it_be(:project_id) { other_project.id } + + it "does not create the token, and returns the project not found error" do + create_token + + expect(response).to have_gitlab_http_status(:not_found) + expect(response.body).to include("Project Not Found") + end + end + end + + context "when the user does not have valid permissions" do + let_it_be(:project_id) { project.id } + + context "when the user is a developer" do + before do + project.add_developer(user) + end + + it "does not create the token, and returns the permission error" do + create_token + + expect(response).to have_gitlab_http_status(:bad_request) + expect(response.body).to include("User does not have permission to create project access token") + end + end + + context "when a project access token tries to create another project access token" do + let_it_be(:project_bot) { create(:user, :project_bot) } + let_it_be(:user) { project_bot } + + before do + project.add_maintainer(user) + end + + it "does not allow a project access token to create another project access token" do + create_token + + expect(response).to have_gitlab_http_status(:bad_request) + expect(response.body).to include("User does not have permission to create project access token") + end + end + end + end + end +end diff --git a/spec/requests/api/rubygem_packages_spec.rb b/spec/requests/api/rubygem_packages_spec.rb new file mode 100644 index 00000000000..5dd68bf9b10 --- /dev/null +++ b/spec/requests/api/rubygem_packages_spec.rb @@ -0,0 +1,139 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe API::RubygemPackages do + using RSpec::Parameterized::TableSyntax + + let_it_be(:project) { create(:project) } + let_it_be(:personal_access_token) { create(:personal_access_token) } + let_it_be(:user) { personal_access_token.user } + let_it_be(:job) { create(:ci_build, :running, user: user) } + 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) } + let_it_be(:headers) { {} } + + shared_examples 'when feature flag is disabled' do + let(:headers) do + { 'HTTP_AUTHORIZATION' => personal_access_token.token } + end + + before do + stub_feature_flags(rubygem_packages: false) + end + + it_behaves_like 'returning response status', :not_found + end + + shared_examples 'when package feature is disabled' do + before do + stub_config(packages: { enabled: false }) + end + + it_behaves_like 'returning response status', :not_found + end + + shared_examples 'without authentication' do + it_behaves_like 'returning response status', :unauthorized + end + + shared_examples 'with authentication' do + let(:headers) do + { 'HTTP_AUTHORIZATION' => token } + end + + let(:tokens) do + { + personal_access_token: personal_access_token.token, + deploy_token: deploy_token.token, + job_token: job.token + } + end + + where(:user_role, :token_type, :valid_token, :status) do + :guest | :personal_access_token | true | :not_found + :guest | :personal_access_token | false | :unauthorized + :guest | :deploy_token | true | :not_found + :guest | :deploy_token | false | :unauthorized + :guest | :job_token | true | :not_found + :guest | :job_token | false | :unauthorized + :reporter | :personal_access_token | true | :not_found + :reporter | :personal_access_token | false | :unauthorized + :reporter | :deploy_token | true | :not_found + :reporter | :deploy_token | false | :unauthorized + :reporter | :job_token | true | :not_found + :reporter | :job_token | false | :unauthorized + :developer | :personal_access_token | true | :not_found + :developer | :personal_access_token | false | :unauthorized + :developer | :deploy_token | true | :not_found + :developer | :deploy_token | false | :unauthorized + :developer | :job_token | true | :not_found + :developer | :job_token | false | :unauthorized + end + + with_them do + before do + project.send("add_#{user_role}", user) unless user_role == :anonymous + end + + let(:token) { valid_token ? tokens[token_type] : 'invalid-token123' } + + it_behaves_like 'returning response status', params[:status] + end + end + + shared_examples 'an unimplemented route' do + it_behaves_like 'without authentication' + it_behaves_like 'with authentication' + it_behaves_like 'when feature flag is disabled' + it_behaves_like 'when package feature is disabled' + end + + describe 'GET /api/v4/projects/:project_id/packages/rubygems/:filename' do + let(:url) { api("/projects/#{project.id}/packages/rubygems/specs.4.8.gz") } + + subject { get(url, headers: headers) } + + it_behaves_like 'an unimplemented route' + end + + describe 'GET /api/v4/projects/:project_id/packages/rubygems/quick/Marshal.4.8/:file_name' do + let(:url) { api("/projects/#{project.id}/packages/rubygems/quick/Marshal.4.8/my_gem-1.0.0.gemspec.rz") } + + subject { get(url, headers: headers) } + + it_behaves_like 'an unimplemented route' + end + + describe 'GET /api/v4/projects/:project_id/packages/rubygems/gems/:file_name' do + let(:url) { api("/projects/#{project.id}/packages/rubygems/gems/my_gem-1.0.0.gem") } + + subject { get(url, headers: headers) } + + it_behaves_like 'an unimplemented route' + end + + describe 'POST /api/v4/projects/:project_id/packages/rubygems/api/v1/gems/authorize' do + let(:url) { api("/projects/#{project.id}/packages/rubygems/api/v1/gems/authorize") } + + subject { post(url, headers: headers) } + + it_behaves_like 'an unimplemented route' + end + + describe 'POST /api/v4/projects/:project_id/packages/rubygems/api/v1/gems' do + let(:url) { api("/projects/#{project.id}/packages/rubygems/api/v1/gems") } + + subject { post(url, headers: headers) } + + it_behaves_like 'an unimplemented route' + end + + describe 'GET /api/v4/projects/:project_id/packages/rubygems/api/v1/dependencies' do + let(:url) { api("/projects/#{project.id}/packages/rubygems/api/v1/dependencies") } + + subject { get(url, headers: headers) } + + it_behaves_like 'an unimplemented route' + end +end diff --git a/spec/requests/api/settings_spec.rb b/spec/requests/api/settings_spec.rb index 8fb0f8fc51a..3b84c812010 100644 --- a/spec/requests/api/settings_spec.rb +++ b/spec/requests/api/settings_spec.rb @@ -199,6 +199,14 @@ RSpec.describe API::Settings, 'Settings' do expect(json_response['allow_local_requests_from_hooks_and_services']).to eq(true) end + it 'supports legacy asset_proxy_whitelist' do + put api("/application/settings", admin), + params: { asset_proxy_whitelist: ['example.com', '*.example.com'] } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['asset_proxy_allowlist']).to eq(['example.com', '*.example.com', 'localhost']) + end + it 'disables ability to switch to legacy storage' do put api("/application/settings", admin), params: { hashed_storage_enabled: false } @@ -362,24 +370,24 @@ RSpec.describe API::Settings, 'Settings' do asset_proxy_enabled: true, asset_proxy_url: 'http://assets.example.com', asset_proxy_secret_key: 'shared secret', - asset_proxy_whitelist: ['example.com', '*.example.com'] + asset_proxy_allowlist: ['example.com', '*.example.com'] } expect(response).to have_gitlab_http_status(:ok) expect(json_response['asset_proxy_enabled']).to be(true) expect(json_response['asset_proxy_url']).to eq('http://assets.example.com') expect(json_response['asset_proxy_secret_key']).to be_nil - expect(json_response['asset_proxy_whitelist']).to eq(['example.com', '*.example.com', 'localhost']) + expect(json_response['asset_proxy_allowlist']).to eq(['example.com', '*.example.com', 'localhost']) end - it 'allows a string for asset_proxy_whitelist' do + it 'allows a string for asset_proxy_allowlist' do put api('/application/settings', admin), params: { - asset_proxy_whitelist: 'example.com, *.example.com' + asset_proxy_allowlist: 'example.com, *.example.com' } expect(response).to have_gitlab_http_status(:ok) - expect(json_response['asset_proxy_whitelist']).to eq(['example.com', '*.example.com', 'localhost']) + expect(json_response['asset_proxy_allowlist']).to eq(['example.com', '*.example.com', 'localhost']) end end diff --git a/spec/requests/api/suggestions_spec.rb b/spec/requests/api/suggestions_spec.rb index 78a2688ac5e..7f53d379af5 100644 --- a/spec/requests/api/suggestions_spec.rb +++ b/spec/requests/api/suggestions_spec.rb @@ -65,6 +65,19 @@ RSpec.describe API::Suggestions do end end + context 'when a custom commit message is included' do + it 'renders an ok response and returns json content' do + project.add_maintainer(user) + + message = "cool custom commit message!" + + put api(url, user), params: { commit_message: message } + + expect(response).to have_gitlab_http_status(:ok) + expect(project.repository.commit.message).to eq(message) + end + end + context 'when not able to apply patch' do let(:url) { "/suggestions/#{unappliable_suggestion.id}/apply" } @@ -113,9 +126,11 @@ RSpec.describe API::Suggestions do let(:url) { "/suggestions/batch_apply" } context 'when successfully applies multiple patches as a batch' do - it 'renders an ok response and returns json content' do + before do project.add_maintainer(user) + end + it 'renders an ok response and returns json content' do put api(url, user), params: { ids: [suggestion.id, suggestion2.id] } expect(response).to have_gitlab_http_status(:ok) @@ -123,6 +138,16 @@ RSpec.describe API::Suggestions do 'appliable', 'applied', 'from_content', 'to_content')) end + + it 'provides a custom commit message' do + message = "cool custom commit message!" + + put api(url, user), params: { ids: [suggestion.id, suggestion2.id], + commit_message: message } + + expect(response).to have_gitlab_http_status(:ok) + expect(project.repository.commit.message).to eq(message) + end end context 'when not able to apply one or more of the patches' do diff --git a/spec/requests/api/templates_spec.rb b/spec/requests/api/templates_spec.rb index e1c5bfd82c4..adb37c62dc3 100644 --- a/spec/requests/api/templates_spec.rb +++ b/spec/requests/api/templates_spec.rb @@ -65,7 +65,9 @@ RSpec.describe API::Templates do expect(json_response['nickname']).to be_nil expect(json_response['popular']).to be true expect(json_response['html_url']).to eq('http://choosealicense.com/licenses/mit/') - expect(json_response['source_url']).to eq('https://opensource.org/licenses/MIT') + # This was dropped: + # https://github.com/github/choosealicense.com/commit/325806b42aa3d5b78e84120327ec877bc936dbdd#diff-66df8f1997786f7052d29010f2cbb4c66391d60d24ca624c356acc0ab986f139 + expect(json_response['source_url']).to be_nil expect(json_response['description']).to include('A short and simple permissive license with conditions') expect(json_response['conditions']).to eq(%w[include-copyright]) expect(json_response['permissions']).to eq(%w[commercial-use modifications distribution private-use]) @@ -81,7 +83,7 @@ RSpec.describe API::Templates do expect(response).to have_gitlab_http_status(:ok) expect(response).to include_pagination_headers expect(json_response).to be_an Array - expect(json_response.size).to eq(12) + expect(json_response.size).to eq(13) expect(json_response.map { |l| l['key'] }).to include('agpl-3.0') end diff --git a/spec/requests/api/terraform/state_spec.rb b/spec/requests/api/terraform/state_spec.rb index bfdb5458fd1..2cb3c8e9ab5 100644 --- a/spec/requests/api/terraform/state_spec.rb +++ b/spec/requests/api/terraform/state_spec.rb @@ -39,7 +39,7 @@ RSpec.describe API::Terraform::State do context 'with maintainer permissions' do let(:current_user) { maintainer } - it_behaves_like 'tracking unique hll events', :usage_data_p_terraform_state_api_unique_users do + it_behaves_like 'tracking unique hll events' do let(:target_id) { 'p_terraform_state_api_unique_users' } let(:expected_type) { instance_of(Integer) } end diff --git a/spec/requests/api/triggers_spec.rb b/spec/requests/api/triggers_spec.rb index c51358bf659..55d17fabc9a 100644 --- a/spec/requests/api/triggers_spec.rb +++ b/spec/requests/api/triggers_spec.rb @@ -54,6 +54,15 @@ RSpec.describe API::Triggers do expect(pipeline.builds.size).to eq(5) end + it 'stores payload as a variable' do + post api("/projects/#{project.id}/trigger/pipeline"), params: options.merge(ref: 'master') + + expect(response).to have_gitlab_http_status(:created) + expect(pipeline.variables.find { |v| v.key == 'TRIGGER_PAYLOAD' }.value).to eq( + "{\"ref\":\"master\",\"id\":\"#{project.id}\",\"variables\":{}}" + ) + end + it 'returns bad request with no pipeline created if there\'s no commit for that ref' do post api("/projects/#{project.id}/trigger/pipeline"), params: options.merge(ref: 'other-branch') @@ -84,7 +93,7 @@ RSpec.describe API::Triggers do post api("/projects/#{project.id}/trigger/pipeline"), params: options.merge(variables: variables, ref: 'master') expect(response).to have_gitlab_http_status(:created) - expect(pipeline.variables.map { |v| { v.key => v.value } }.last).to eq(variables) + expect(pipeline.variables.find { |v| v.key == 'TRIGGER_KEY' }.value).to eq('TRIGGER_VALUE') end end end diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index 94fba451860..d70a8bd692d 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -652,6 +652,34 @@ RSpec.describe API::Users do expect(response).to match_response_schema('public_api/v4/user/basic') expect(json_response.keys).not_to include 'created_at' end + + it "returns the `followers` field for public users" do + get api("/users/#{user.id}") + + expect(response).to match_response_schema('public_api/v4/user/basic') + expect(json_response.keys).to include 'followers' + end + + it "does not return the `followers` field for private users" do + get api("/users/#{private_user.id}") + + expect(response).to match_response_schema('public_api/v4/user/basic') + expect(json_response.keys).not_to include 'followers' + end + + it "returns the `following` field for public users" do + get api("/users/#{user.id}") + + expect(response).to match_response_schema('public_api/v4/user/basic') + expect(json_response.keys).to include 'following' + end + + it "does not return the `following` field for private users" do + get api("/users/#{private_user.id}") + + expect(response).to match_response_schema('public_api/v4/user/basic') + expect(json_response.keys).not_to include 'following' + end end it "returns a 404 error if user id not found" do @@ -688,6 +716,128 @@ RSpec.describe API::Users do end end + describe 'POST /users/:id/follow' do + let(:followee) { create(:user) } + + context 'on an unfollowed user' do + it 'follows the user' do + post api("/users/#{followee.id}/follow", user) + + expect(user.followees).to contain_exactly(followee) + expect(response).to have_gitlab_http_status(:created) + end + end + + context 'on a followed user' do + before do + user.follow(followee) + end + + it 'does not change following' do + post api("/users/#{followee.id}/follow", user) + + expect(user.followees).to contain_exactly(followee) + expect(response).to have_gitlab_http_status(:not_modified) + end + end + end + + describe 'POST /users/:id/unfollow' do + let(:followee) { create(:user) } + + context 'on a followed user' do + before do + user.follow(followee) + end + + it 'unfollow the user' do + post api("/users/#{followee.id}/unfollow", user) + + expect(user.followees).to be_empty + expect(response).to have_gitlab_http_status(:created) + end + end + + context 'on an unfollowed user' do + it 'does not change following' do + post api("/users/#{followee.id}/unfollow", user) + + expect(user.followees).to be_empty + expect(response).to have_gitlab_http_status(:not_modified) + end + end + end + + describe 'GET /users/:id/followers' do + let(:follower) { create(:user) } + + context 'user has followers' do + it 'lists followers' do + follower.follow(user) + + get api("/users/#{user.id}/followers", user) + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + end + + it 'do not lists followers if profile is private' do + follower.follow(private_user) + + get api("/users/#{private_user.id}/followers", user) + + expect(response).to have_gitlab_http_status(:not_found) + expect(json_response['message']).to eq('404 User Not Found') + end + end + + context 'user does not have any follower' do + it 'does list nothing' do + get api("/users/#{user.id}/followers", user) + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_pagination_headers + expect(json_response).to be_empty + end + end + end + + describe 'GET /users/:id/following' do + let(:followee) { create(:user) } + + context 'user has followers' do + it 'lists following user' do + user.follow(followee) + + get api("/users/#{user.id}/following", user) + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + end + + it 'do not lists following user if profile is private' do + user.follow(private_user) + + get api("/users/#{private_user.id}/following", user) + + expect(response).to have_gitlab_http_status(:not_found) + expect(json_response['message']).to eq('404 User Not Found') + end + end + + context 'user does not have any follower' do + it 'does list nothing' do + get api("/users/#{user.id}/following", user) + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_pagination_headers + expect(json_response).to be_empty + end + end + end + describe "POST /users" do it "creates user" do expect do @@ -2565,7 +2715,7 @@ RSpec.describe API::Users do it 'does not approve a deactivated user' do expect { approve }.not_to change { deactivated_user.reload.state } expect(response).to have_gitlab_http_status(:conflict) - expect(json_response['message']).to eq('The user you are trying to approve is not pending an approval') + expect(json_response['message']).to eq('The user you are trying to approve is not pending approval') end end @@ -2585,7 +2735,7 @@ RSpec.describe API::Users do it 'returns 201' do expect { approve }.not_to change { user.reload.state } expect(response).to have_gitlab_http_status(:conflict) - expect(json_response['message']).to eq('The user you are trying to approve is not pending an approval') + expect(json_response['message']).to eq('The user you are trying to approve is not pending approval') end end @@ -2595,7 +2745,7 @@ RSpec.describe API::Users do it 'returns 403' do expect { approve }.not_to change { blocked_user.reload.state } expect(response).to have_gitlab_http_status(:conflict) - expect(json_response['message']).to eq('The user you are trying to approve is not pending an approval') + expect(json_response['message']).to eq('The user you are trying to approve is not pending approval') end end @@ -2605,7 +2755,7 @@ RSpec.describe API::Users do it 'returns 403' do expect { approve }.not_to change { ldap_blocked_user.reload.state } expect(response).to have_gitlab_http_status(:conflict) - expect(json_response['message']).to eq('The user you are trying to approve is not pending an approval') + expect(json_response['message']).to eq('The user you are trying to approve is not pending approval') end end @@ -2866,6 +3016,47 @@ RSpec.describe API::Users do expect(response).to have_gitlab_http_status(:success) expect(user.reload.status).to be_nil end + + context 'when clear_status_after is given' do + it 'sets the clear_status_at column' do + freeze_time do + expected_clear_status_at = 3.hours.from_now + + put api('/user/status', user), params: { emoji: 'smirk', message: 'hello world', clear_status_after: '3_hours' } + + expect(response).to have_gitlab_http_status(:success) + expect(user.status.reload.clear_status_at).to be_within(1.minute).of(expected_clear_status_at) + expect(Time.parse(json_response["clear_status_at"])).to be_within(1.minute).of(expected_clear_status_at) + end + end + + it 'unsets the clear_status_at column' do + user.create_status!(clear_status_at: 5.hours.ago) + + put api('/user/status', user), params: { emoji: 'smirk', message: 'hello world', clear_status_after: nil } + + expect(response).to have_gitlab_http_status(:success) + expect(user.status.reload.clear_status_at).to be_nil + end + + it 'raises error when unknown status value is given' do + put api('/user/status', user), params: { emoji: 'smirk', message: 'hello world', clear_status_after: 'wrong' } + + expect(response).to have_gitlab_http_status(:bad_request) + end + + context 'when the clear_status_with_quick_options feature flag is disabled' do + before do + stub_feature_flags(clear_status_with_quick_options: false) + end + + it 'does not persist clear_status_at' do + put api('/user/status', user), params: { emoji: 'smirk', message: 'hello world', clear_status_after: '3_hours' } + + expect(user.status.reload.clear_status_at).to be_nil + end + end + end end describe 'POST /users/:user_id/personal_access_tokens' do diff --git a/spec/requests/api/version_spec.rb b/spec/requests/api/version_spec.rb index a0a0f66c8d1..7abbaf4f9ec 100644 --- a/spec/requests/api/version_spec.rb +++ b/spec/requests/api/version_spec.rb @@ -33,6 +33,12 @@ RSpec.describe API::Version do expect_version end + + it 'returns "200" response on head requests' do + head api('/version', personal_access_token: personal_access_token) + + expect(response).to have_gitlab_http_status(:ok) + end end context 'with read_user scope' do @@ -43,6 +49,12 @@ RSpec.describe API::Version do expect_version end + + it 'returns "200" response on head requests' do + head api('/version', personal_access_token: personal_access_token) + + expect(response).to have_gitlab_http_status(:ok) + end end context 'with neither api nor read_user scope' do diff --git a/spec/requests/git_http_spec.rb b/spec/requests/git_http_spec.rb index 1ee3e36be8b..a1e28c18769 100644 --- a/spec/requests/git_http_spec.rb +++ b/spec/requests/git_http_spec.rb @@ -382,6 +382,14 @@ RSpec.describe 'Git HTTP requests' do end end end + + context 'but the service parameter is missing' do + it 'rejects clones with 403 Forbidden' do + get("/#{path}/info/refs", headers: auth_env(*env.values_at(:user, :password), nil)) + + expect(response).to have_gitlab_http_status(:forbidden) + end + end end context 'and not a member of the team' do @@ -409,6 +417,14 @@ RSpec.describe 'Git HTTP requests' do it_behaves_like 'pushes are allowed' end + + context 'but the service parameter is missing' do + it 'rejects clones with 401 Unauthorized' do + get("/#{path}/info/refs") + + expect(response).to have_gitlab_http_status(:unauthorized) + end + end end end diff --git a/spec/requests/groups/email_campaigns_controller_spec.rb b/spec/requests/groups/email_campaigns_controller_spec.rb new file mode 100644 index 00000000000..930e645f6c0 --- /dev/null +++ b/spec/requests/groups/email_campaigns_controller_spec.rb @@ -0,0 +1,86 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Groups::EmailCampaignsController do + include InProductMarketingHelper + using RSpec::Parameterized::TableSyntax + + describe 'GET #index', :snowplow do + let_it_be(:group) { create(:group) } + let_it_be(:project) { create(:project, group: group) } + let_it_be(:user) { create(:user) } + let(:track) { 'create' } + let(:series) { '0' } + let(:schema) { described_class::EMAIL_CAMPAIGNS_SCHEMA_URL } + let(:data) do + { + namespace_id: group.id, + track: track.to_sym, + series: series.to_i, + subject_line: subject_line(track.to_sym, series.to_i) + } + end + + before do + sign_in(user) + group.add_developer(user) + allow(Gitlab::Tracking).to receive(:self_describing_event) + end + + subject do + get group_email_campaigns_url(group, track: track, series: series) + response + end + + shared_examples 'track and redirect' do + it do + is_expected.to track_self_describing_event(schema, data) + is_expected.to have_gitlab_http_status(:redirect) + end + end + + shared_examples 'no track and 404' do + it do + is_expected.not_to track_self_describing_event + is_expected.to have_gitlab_http_status(:not_found) + end + end + + describe 'track parameter' do + context 'when valid' do + where(track: Namespaces::InProductMarketingEmailsService::TRACKS.keys) + + with_them do + it_behaves_like 'track and redirect' + end + end + + context 'when invalid' do + where(track: [nil, 'xxxx']) + + with_them do + it_behaves_like 'no track and 404' + end + end + end + + describe 'series parameter' do + context 'when valid' do + where(series: (0..Namespaces::InProductMarketingEmailsService::INTERVAL_DAYS.length - 1).to_a) + + with_them do + it_behaves_like 'track and redirect' + end + end + + context 'when invalid' do + where(series: [-1, nil, Namespaces::InProductMarketingEmailsService::INTERVAL_DAYS.length]) + + with_them do + it_behaves_like 'no track and 404' + end + end + end + end +end diff --git a/spec/requests/health_controller_spec.rb b/spec/requests/health_controller_spec.rb index 592a57bc637..f70faf5bb9c 100644 --- a/spec/requests/health_controller_spec.rb +++ b/spec/requests/health_controller_spec.rb @@ -77,91 +77,129 @@ RSpec.describe HealthController do shared_context 'endpoint responding with readiness data' do context 'when requesting instance-checks' do - it 'responds with readiness checks data' do - expect(Gitlab::HealthChecks::MasterCheck).to receive(:check) { true } - - subject - - expect(json_response).to include({ 'status' => 'ok' }) - expect(json_response['master_check']).to contain_exactly({ 'status' => 'ok' }) - end + context 'when Puma runs in Clustered mode' do + before do + allow(Gitlab::Runtime).to receive(:puma_in_clustered_mode?).and_return(true) + end - it 'responds with readiness checks data when a failure happens' do - expect(Gitlab::HealthChecks::MasterCheck).to receive(:check) { false } + it 'responds with readiness checks data' do + expect(Gitlab::HealthChecks::MasterCheck).to receive(:check) { true } - subject + subject - expect(json_response).to include({ 'status' => 'failed' }) - expect(json_response['master_check']).to contain_exactly( - { 'status' => 'failed', 'message' => 'unexpected Master check result: false' }) + expect(json_response).to include({ 'status' => 'ok' }) + expect(json_response['master_check']).to contain_exactly({ 'status' => 'ok' }) + end - expect(response).to have_gitlab_http_status(:service_unavailable) - expect(response.headers['X-GitLab-Custom-Error']).to eq(1) - end - end + it 'responds with readiness checks data when a failure happens' do + expect(Gitlab::HealthChecks::MasterCheck).to receive(:check) { false } - context 'when requesting all checks' do - before do - params.merge!(all: true) - end + subject - it 'responds with readiness checks data' do - subject + expect(json_response).to include({ 'status' => 'failed' }) + expect(json_response['master_check']).to contain_exactly( + { 'status' => 'failed', 'message' => 'unexpected Master check result: false' }) - expect(json_response['db_check']).to contain_exactly({ 'status' => 'ok' }) - expect(json_response['cache_check']).to contain_exactly({ 'status' => 'ok' }) - expect(json_response['queues_check']).to contain_exactly({ 'status' => 'ok' }) - expect(json_response['shared_state_check']).to contain_exactly({ 'status' => 'ok' }) - expect(json_response['gitaly_check']).to contain_exactly( - { 'status' => 'ok', 'labels' => { 'shard' => 'default' } }) + expect(response).to have_gitlab_http_status(:service_unavailable) + expect(response.headers['X-GitLab-Custom-Error']).to eq(1) + end end - it 'responds with readiness checks data when a failure happens' do - allow(Gitlab::HealthChecks::Redis::RedisCheck).to receive(:readiness).and_return( - Gitlab::HealthChecks::Result.new('redis_check', false, "check error")) + context 'when Puma runs in Single mode' do + before do + allow(Gitlab::Runtime).to receive(:puma_in_clustered_mode?).and_return(false) + end - subject + it 'does not invoke MasterCheck, succeedes' do + expect(Gitlab::HealthChecks::MasterCheck).not_to receive(:check) { true } - expect(json_response['cache_check']).to contain_exactly({ 'status' => 'ok' }) - expect(json_response['redis_check']).to contain_exactly( - { 'status' => 'failed', 'message' => 'check error' }) + subject - expect(response).to have_gitlab_http_status(:service_unavailable) - expect(response.headers['X-GitLab-Custom-Error']).to eq(1) + expect(json_response).to eq('status' => 'ok') + end end + end - context 'when DB is not accessible and connection raises an exception' do + context 'when requesting all checks' do + shared_context 'endpoint responding with readiness data for all checks' do before do - expect(Gitlab::HealthChecks::DbCheck) - .to receive(:readiness) - .and_raise(PG::ConnectionBad, 'could not connect to server') + params.merge!(all: true) end - it 'responds with 500 including the exception info' do + it 'responds with readiness checks data' do subject - expect(response).to have_gitlab_http_status(:internal_server_error) + expect(json_response['db_check']).to contain_exactly({ 'status' => 'ok' }) + expect(json_response['cache_check']).to contain_exactly({ 'status' => 'ok' }) + expect(json_response['queues_check']).to contain_exactly({ 'status' => 'ok' }) + expect(json_response['shared_state_check']).to contain_exactly({ 'status' => 'ok' }) + expect(json_response['gitaly_check']).to contain_exactly( + { 'status' => 'ok', 'labels' => { 'shard' => 'default' } }) + end + + it 'responds with readiness checks data when a failure happens' do + allow(Gitlab::HealthChecks::Redis::RedisCheck).to receive(:readiness).and_return( + Gitlab::HealthChecks::Result.new('redis_check', false, "check error")) + + subject + + expect(json_response['cache_check']).to contain_exactly({ 'status' => 'ok' }) + expect(json_response['redis_check']).to contain_exactly( + { 'status' => 'failed', 'message' => 'check error' }) + + expect(response).to have_gitlab_http_status(:service_unavailable) expect(response.headers['X-GitLab-Custom-Error']).to eq(1) - expect(json_response).to eq( - { 'status' => 'failed', 'message' => 'PG::ConnectionBad : could not connect to server' }) + end + + context 'when DB is not accessible and connection raises an exception' do + before do + expect(Gitlab::HealthChecks::DbCheck) + .to receive(:readiness) + .and_raise(PG::ConnectionBad, 'could not connect to server') + end + + it 'responds with 500 including the exception info' do + subject + + expect(response).to have_gitlab_http_status(:internal_server_error) + expect(response.headers['X-GitLab-Custom-Error']).to eq(1) + expect(json_response).to eq( + { 'status' => 'failed', 'message' => 'PG::ConnectionBad : could not connect to server' }) + end + end + + context 'when any exception happens during the probing' do + before do + expect(Gitlab::HealthChecks::Redis::RedisCheck) + .to receive(:readiness) + .and_raise(::Redis::CannotConnectError, 'Redis down') + end + + it 'responds with 500 including the exception info' do + subject + + expect(response).to have_gitlab_http_status(:internal_server_error) + expect(response.headers['X-GitLab-Custom-Error']).to eq(1) + expect(json_response).to eq( + { 'status' => 'failed', 'message' => 'Redis::CannotConnectError : Redis down' }) + end end end - context 'when any exception happens during the probing' do + context 'when Puma runs in Clustered mode' do before do - expect(Gitlab::HealthChecks::Redis::RedisCheck) - .to receive(:readiness) - .and_raise(::Redis::CannotConnectError, 'Redis down') + allow(Gitlab::Runtime).to receive(:puma_in_clustered_mode?).and_return(true) end - it 'responds with 500 including the exception info' do - subject + it_behaves_like 'endpoint responding with readiness data for all checks' + end - expect(response).to have_gitlab_http_status(:internal_server_error) - expect(response.headers['X-GitLab-Custom-Error']).to eq(1) - expect(json_response).to eq( - { 'status' => 'failed', 'message' => 'Redis::CannotConnectError : Redis down' }) + context 'when Puma runs in Single mode' do + before do + allow(Gitlab::Runtime).to receive(:puma_in_clustered_mode?).and_return(false) end + + it_behaves_like 'endpoint responding with readiness data for all checks' end end end diff --git a/spec/requests/import/gitlab_groups_controller_spec.rb b/spec/requests/import/gitlab_groups_controller_spec.rb index 51f1363cf1c..c65caf2ebf0 100644 --- a/spec/requests/import/gitlab_groups_controller_spec.rb +++ b/spec/requests/import/gitlab_groups_controller_spec.rb @@ -5,12 +5,10 @@ require 'spec_helper' RSpec.describe Import::GitlabGroupsController do include WorkhorseHelpers + include_context 'workhorse headers' + let_it_be(:user) { create(:user) } let(:import_path) { "#{Dir.tmpdir}/gitlab_groups_controller_spec" } - let(:workhorse_token) { JWT.encode({ 'iss' => 'gitlab-workhorse' }, Gitlab::Workhorse.secret, 'HS256') } - let(:workhorse_headers) do - { 'GitLab-Workhorse' => '1.0', Gitlab::Workhorse::INTERNAL_API_REQUEST_HEADER => workhorse_token } - end before do allow_next_instance_of(Gitlab::ImportExport) do |import_export| diff --git a/spec/requests/import/gitlab_projects_controller_spec.rb b/spec/requests/import/gitlab_projects_controller_spec.rb index d7d4de21a33..58843a7fec4 100644 --- a/spec/requests/import/gitlab_projects_controller_spec.rb +++ b/spec/requests/import/gitlab_projects_controller_spec.rb @@ -5,8 +5,7 @@ require 'spec_helper' RSpec.describe Import::GitlabProjectsController do include WorkhorseHelpers - let(:workhorse_token) { JWT.encode({ 'iss' => 'gitlab-workhorse' }, Gitlab::Workhorse.secret, 'HS256') } - let(:workhorse_headers) { { 'GitLab-Workhorse' => '1.0', Gitlab::Workhorse::INTERNAL_API_REQUEST_HEADER => workhorse_token } } + include_context 'workhorse headers' let_it_be(:namespace) { create(:namespace) } let_it_be(:user) { namespace.owner } diff --git a/spec/requests/oauth/tokens_controller_spec.rb b/spec/requests/oauth/tokens_controller_spec.rb new file mode 100644 index 00000000000..c3cdae2cd21 --- /dev/null +++ b/spec/requests/oauth/tokens_controller_spec.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Oauth::TokensController do + it 'allows cross-origin POST requests' do + post '/oauth/token', headers: { 'Origin' => 'http://notgitlab.com' } + + expect(response.headers['Access-Control-Allow-Origin']).to eq '*' + expect(response.headers['Access-Control-Allow-Methods']).to eq 'POST' + expect(response.headers['Access-Control-Allow-Headers']).to be_nil + expect(response.headers['Access-Control-Allow-Credentials']).to be_nil + end +end diff --git a/spec/requests/projects/ci/promeheus_metrics/histograms_controller_spec.rb b/spec/requests/projects/ci/promeheus_metrics/histograms_controller_spec.rb new file mode 100644 index 00000000000..5d2f3e98bb4 --- /dev/null +++ b/spec/requests/projects/ci/promeheus_metrics/histograms_controller_spec.rb @@ -0,0 +1,46 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Projects::Ci::PrometheusMetrics::HistogramsController' do + let_it_be(:project) { create(:project, :public) } + + describe 'POST /*namespace_id/:project_id/-/ci/prometheus_metrics/histograms' do + context 'with known histograms' do + it 'returns 201 Created' do + post histograms_route(histograms: [ + { name: :pipeline_graph_link_calculation_duration_seconds, value: 1 }, + { name: :pipeline_graph_links_total, value: 10 } + ]) + + expect(response).to have_gitlab_http_status(:created) + end + end + + context 'with unknown histograms' do + it 'returns 404 Not Found' do + post histograms_route(histograms: [{ name: :chunky_bacon, value: 5 }]) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'with the feature flag disabled' do + before do + stub_feature_flags(ci_accept_frontend_prometheus_metrics: false) + end + + it 'returns 202 Accepted' do + post histograms_route(histograms: [ + { name: :pipeline_graph_link_calculation_duration_seconds, value: 1 } + ]) + + expect(response).to have_gitlab_http_status(:accepted) + end + end + end + + def histograms_route(params = {}) + namespace_project_ci_prometheus_metrics_histograms_path(namespace_id: project.namespace, project_id: project, **params) + end +end diff --git a/spec/requests/projects/noteable_notes_spec.rb b/spec/requests/projects/noteable_notes_spec.rb index 2bf1ffb2edc..5ae2aadaa84 100644 --- a/spec/requests/projects/noteable_notes_spec.rb +++ b/spec/requests/projects/noteable_notes_spec.rb @@ -18,9 +18,7 @@ RSpec.describe 'Project noteable notes' do login_as(user) end - it 'does not set a Gitlab::EtagCaching ETag if there is a note' do - create(:note_on_merge_request, noteable: merge_request, project: merge_request.project) - + it 'does not set a Gitlab::EtagCaching ETag' do get notes_path expect(response).to have_gitlab_http_status(:ok) @@ -29,12 +27,5 @@ RSpec.describe 'Project noteable notes' do # interfere with notes pagination expect(response_etag).not_to eq(stored_etag) end - - it 'sets a Gitlab::EtagCaching ETag if there is no note' do - get notes_path - - expect(response).to have_gitlab_http_status(:ok) - expect(response_etag).to eq(stored_etag) - end end end diff --git a/spec/requests/rack_attack_global_spec.rb b/spec/requests/rack_attack_global_spec.rb index 34f34c0b850..1bb260b5ea1 100644 --- a/spec/requests/rack_attack_global_spec.rb +++ b/spec/requests/rack_attack_global_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'Rack Attack global throttles' do +RSpec.describe 'Rack Attack global throttles', :use_clean_rails_memory_store_caching do include RackAttackSpecHelpers let(:settings) { Gitlab::CurrentSettings.current_application_settings } @@ -149,14 +149,14 @@ RSpec.describe 'Rack Attack global throttles' do expect(response).to have_gitlab_http_status(:ok) end - arguments = { + arguments = a_hash_including({ message: 'Rack_Attack', env: :throttle, remote_ip: '127.0.0.1', request_method: 'GET', path: '/users/sign_in', matched: 'throttle_unauthenticated' - } + }) expect(Gitlab::AuthLogger).to receive(:error).with(arguments) diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index b224ef87229..f3ddcbef1c2 100644 --- a/spec/requests/users_controller_spec.rb +++ b/spec/requests/users_controller_spec.rb @@ -268,6 +268,14 @@ RSpec.describe UsersController do end it_behaves_like 'renders all public keys' + + context 'when public visibility is restricted' do + before do + stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::PUBLIC]) + end + + it_behaves_like 'renders all public keys' + end end end end diff --git a/spec/requests/whats_new_controller_spec.rb b/spec/requests/whats_new_controller_spec.rb index 8005d38dbb0..ba7b5d4c000 100644 --- a/spec/requests/whats_new_controller_spec.rb +++ b/spec/requests/whats_new_controller_spec.rb @@ -2,64 +2,48 @@ require 'spec_helper' -RSpec.describe WhatsNewController do +RSpec.describe WhatsNewController, :clean_gitlab_redis_cache do + after do + ReleaseHighlight.instance_variable_set(:@file_paths, nil) + end + describe 'whats_new_path' do let(:item) { double(:item) } let(:highlights) { double(:highlight, items: [item], map: [item].map, next_page: 2) } - context 'with whats_new_drawer feature enabled' do - before do - stub_feature_flags(whats_new_drawer: true) - end - - context 'with no page param' do - it 'responds with paginated data and headers' do - allow(ReleaseHighlight).to receive(:paginated).with(page: 1).and_return(highlights) - allow(Gitlab::WhatsNew::ItemPresenter).to receive(:present).with(item).and_return(item) + context 'with no page param' do + it 'responds with paginated data and headers' do + allow(ReleaseHighlight).to receive(:paginated).with(page: 1).and_return(highlights) - get whats_new_path, xhr: true + get whats_new_path, xhr: true - expect(response.body).to eq(highlights.items.to_json) - expect(response.headers['X-Next-Page']).to eq(2) - end + expect(response.body).to eq(highlights.items.to_json) + expect(response.headers['X-Next-Page']).to eq(2) end + end - context 'with page param' do - it 'passes the page parameter' do - expect(ReleaseHighlight).to receive(:paginated).with(page: 2).and_call_original - - get whats_new_path(page: 2), xhr: true - end - - it 'returns a 404 if page param is negative' do - get whats_new_path(page: -1), xhr: true + context 'with page param' do + it 'passes the page parameter' do + expect(ReleaseHighlight).to receive(:paginated).with(page: 2).and_call_original - expect(response).to have_gitlab_http_status(:not_found) - end + get whats_new_path(page: 2), xhr: true end - context 'with version param' do - it 'returns items without pagination headers' do - allow(ReleaseHighlight).to receive(:for_version).with(version: '42').and_return(highlights) - allow(Gitlab::WhatsNew::ItemPresenter).to receive(:present).with(item).and_return(item) - - get whats_new_path(version: 42), xhr: true + it 'returns a 404 if page param is negative' do + get whats_new_path(page: -1), xhr: true - expect(response.body).to eq(highlights.items.to_json) - expect(response.headers['X-Next-Page']).to be_nil - end + expect(response).to have_gitlab_http_status(:not_found) end end - context 'with whats_new_drawer feature disabled' do - before do - stub_feature_flags(whats_new_drawer: false) - end + context 'with version param' do + it 'returns items without pagination headers' do + allow(ReleaseHighlight).to receive(:for_version).with(version: '42').and_return(highlights) - it 'returns a 404' do - get whats_new_path, xhr: true + get whats_new_path(version: 42), xhr: true - expect(response).to have_gitlab_http_status(:not_found) + expect(response.body).to eq(highlights.items.to_json) + expect(response.headers['X-Next-Page']).to be_nil end end end |