From d306437ae0f1153f25e284c1ac2dfe86e3bcb33e Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Tue, 1 Dec 2020 03:09:24 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- GITALY_SERVER_VERSION | 2 +- ...t_upload.rb => workhorse_authorization.rb} | 16 +- .../import/gitlab_groups_controller.rb | 6 +- .../import/gitlab_projects_controller.rb | 6 +- app/graphql/mutations/releases/delete.rb | 40 +++++ app/graphql/types/mutation_type.rb | 1 + .../nfriend-add-release-delete-mutation.yml | 5 + .../development/import_requirements_csv.yml | 8 + .../graphql/reference/gitlab_schema.graphql | 41 +++++ doc/api/graphql/reference/gitlab_schema.json | 143 ++++++++++++++++++ doc/api/graphql/reference/index.md | 10 ++ locale/gitlab.pot | 6 + spec/fixtures/csv_uppercase.CSV | 4 + .../graphql/mutations/releases/delete_spec.rb | 92 +++++++++++ .../graphql/mutations/releases/delete_spec.rb | 132 ++++++++++++++++ .../import/gitlab_groups_controller_spec.rb | 66 +------- .../import/gitlab_projects_controller_spec.rb | 52 +------ spec/support/helpers/graphql_helpers.rb | 1 + .../uploads_auhorize_shared_examples.rb | 59 ++++++++ 19 files changed, 571 insertions(+), 119 deletions(-) rename app/controllers/concerns/{workhorse_import_export_upload.rb => workhorse_authorization.rb} (63%) create mode 100644 app/graphql/mutations/releases/delete.rb create mode 100644 changelogs/unreleased/nfriend-add-release-delete-mutation.yml create mode 100644 config/feature_flags/development/import_requirements_csv.yml create mode 100644 spec/fixtures/csv_uppercase.CSV create mode 100644 spec/graphql/mutations/releases/delete_spec.rb create mode 100644 spec/requests/api/graphql/mutations/releases/delete_spec.rb create mode 100644 spec/support/shared_examples/requests/uploads_auhorize_shared_examples.rb diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index 74091286619..857ea235035 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -f085d841e2f5571b260910a454215bc1a7687bf0 +9677be26e1a7f7e4f56bf4f7ba47bc0d16f40e16 diff --git a/app/controllers/concerns/workhorse_import_export_upload.rb b/app/controllers/concerns/workhorse_authorization.rb similarity index 63% rename from app/controllers/concerns/workhorse_import_export_upload.rb rename to app/controllers/concerns/workhorse_authorization.rb index 3c52f4d7adf..c1ed5579712 100644 --- a/app/controllers/concerns/workhorse_import_export_upload.rb +++ b/app/controllers/concerns/workhorse_authorization.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -module WorkhorseImportExportUpload +module WorkhorseAuthorization extend ActiveSupport::Concern include WorkhorseRequest @@ -12,10 +12,9 @@ module WorkhorseImportExportUpload def authorize set_workhorse_internal_api_content_type - authorized = ImportExportUploader.workhorse_authorize( + authorized = uploader_class.workhorse_authorize( has_length: false, - maximum_size: Gitlab::CurrentSettings.max_import_size.megabytes - ) + maximum_size: Gitlab::CurrentSettings.max_attachment_size.megabytes.to_i) render json: authorized rescue SocketError @@ -27,7 +26,14 @@ module WorkhorseImportExportUpload def file_is_valid?(file) return false unless file.is_a?(::UploadedFile) + file_extension_whitelist.include?(File.extname(file.original_filename).downcase.delete('.')) + end + + def uploader_class + raise NotImplementedError + end + + def file_extension_whitelist ImportExportUploader::EXTENSION_WHITELIST - .include?(File.extname(file.original_filename).delete('.')) end end diff --git a/app/controllers/import/gitlab_groups_controller.rb b/app/controllers/import/gitlab_groups_controller.rb index d8118477a80..99e41190912 100644 --- a/app/controllers/import/gitlab_groups_controller.rb +++ b/app/controllers/import/gitlab_groups_controller.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class Import::GitlabGroupsController < ApplicationController - include WorkhorseImportExportUpload + include WorkhorseAuthorization before_action :ensure_group_import_enabled before_action :import_rate_limit, only: %i[create] @@ -64,4 +64,8 @@ class Import::GitlabGroupsController < ApplicationController redirect_to new_group_path end end + + def uploader_class + ImportExportUploader + end end diff --git a/app/controllers/import/gitlab_projects_controller.rb b/app/controllers/import/gitlab_projects_controller.rb index 39d053347f0..a39d1583542 100644 --- a/app/controllers/import/gitlab_projects_controller.rb +++ b/app/controllers/import/gitlab_projects_controller.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class Import::GitlabProjectsController < Import::BaseController - include WorkhorseImportExportUpload + include WorkhorseAuthorization before_action :whitelist_query_limiting, only: [:create] before_action :verify_gitlab_project_import_enabled @@ -45,4 +45,8 @@ class Import::GitlabProjectsController < Import::BaseController def whitelist_query_limiting Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-foss/issues/42437') end + + def uploader_class + ImportExportUploader + end end diff --git a/app/graphql/mutations/releases/delete.rb b/app/graphql/mutations/releases/delete.rb new file mode 100644 index 00000000000..e887b702cce --- /dev/null +++ b/app/graphql/mutations/releases/delete.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +module Mutations + module Releases + class Delete < Base + graphql_name 'ReleaseDelete' + + field :release, + Types::ReleaseType, + null: true, + description: 'The deleted release.' + + argument :tag_name, GraphQL::STRING_TYPE, + required: true, as: :tag, + description: 'Name of the tag associated with the release to delete.' + + authorize :destroy_release + + def resolve(project_path:, tag:) + project = authorized_find!(full_path: project_path) + + params = { tag: tag }.with_indifferent_access + + result = ::Releases::DestroyService.new(project, current_user, params).execute + + if result[:status] == :success + { + release: result[:release], + errors: [] + } + else + { + release: nil, + errors: [result[:message]] + } + end + end + end + end +end diff --git a/app/graphql/types/mutation_type.rb b/app/graphql/types/mutation_type.rb index 18576b4ca34..93c650c0b97 100644 --- a/app/graphql/types/mutation_type.rb +++ b/app/graphql/types/mutation_type.rb @@ -66,6 +66,7 @@ module Types mount_mutation Mutations::Notes::Destroy mount_mutation Mutations::Releases::Create mount_mutation Mutations::Releases::Update + mount_mutation Mutations::Releases::Delete mount_mutation Mutations::Terraform::State::Delete mount_mutation Mutations::Terraform::State::Lock mount_mutation Mutations::Terraform::State::Unlock diff --git a/changelogs/unreleased/nfriend-add-release-delete-mutation.yml b/changelogs/unreleased/nfriend-add-release-delete-mutation.yml new file mode 100644 index 00000000000..273fabcac5f --- /dev/null +++ b/changelogs/unreleased/nfriend-add-release-delete-mutation.yml @@ -0,0 +1,5 @@ +--- +title: Add GraphQL mutation to delete a release +merge_request: 48364 +author: +type: added diff --git a/config/feature_flags/development/import_requirements_csv.yml b/config/feature_flags/development/import_requirements_csv.yml new file mode 100644 index 00000000000..23ee0445418 --- /dev/null +++ b/config/feature_flags/development/import_requirements_csv.yml @@ -0,0 +1,8 @@ +--- +name: import_requirements_csv +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/48060 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/284846 +milestone: '13.7' +type: development +group: group::product planning +default_enabled: false diff --git a/doc/api/graphql/reference/gitlab_schema.graphql b/doc/api/graphql/reference/gitlab_schema.graphql index 0819bb2973b..237d8a4c776 100644 --- a/doc/api/graphql/reference/gitlab_schema.graphql +++ b/doc/api/graphql/reference/gitlab_schema.graphql @@ -14066,6 +14066,7 @@ type Mutation { prometheusIntegrationUpdate(input: PrometheusIntegrationUpdateInput!): PrometheusIntegrationUpdatePayload promoteToEpic(input: PromoteToEpicInput!): PromoteToEpicPayload releaseCreate(input: ReleaseCreateInput!): ReleaseCreatePayload + releaseDelete(input: ReleaseDeleteInput!): ReleaseDeletePayload releaseUpdate(input: ReleaseUpdateInput!): ReleaseUpdatePayload removeAwardEmoji(input: RemoveAwardEmojiInput!): RemoveAwardEmojiPayload @deprecated(reason: "Use awardEmojiRemove. Deprecated in 13.2") removeProjectFromSecurityDashboard(input: RemoveProjectFromSecurityDashboardInput!): RemoveProjectFromSecurityDashboardPayload @@ -18664,6 +18665,46 @@ type ReleaseCreatePayload { release: Release } +""" +Autogenerated input type of ReleaseDelete +""" +input ReleaseDeleteInput { + """ + A unique identifier for the client performing the mutation. + """ + clientMutationId: String + + """ + Full path of the project the release is associated with + """ + projectPath: ID! + + """ + Name of the tag associated with the release to delete. + """ + tagName: String! +} + +""" +Autogenerated return type of ReleaseDelete +""" +type ReleaseDeletePayload { + """ + A unique identifier for the client performing the mutation. + """ + clientMutationId: String + + """ + Errors encountered during execution of the mutation. + """ + errors: [String!]! + + """ + The deleted release. + """ + release: Release +} + """ An edge in a connection. """ diff --git a/doc/api/graphql/reference/gitlab_schema.json b/doc/api/graphql/reference/gitlab_schema.json index 8ed969e512e..9ced0721e91 100644 --- a/doc/api/graphql/reference/gitlab_schema.json +++ b/doc/api/graphql/reference/gitlab_schema.json @@ -41110,6 +41110,33 @@ "isDeprecated": false, "deprecationReason": null }, + { + "name": "releaseDelete", + "description": null, + "args": [ + { + "name": "input", + "description": null, + "type": { + "kind": "NON_NULL", + "name": null, + "ofType": { + "kind": "INPUT_OBJECT", + "name": "ReleaseDeleteInput", + "ofType": null + } + }, + "defaultValue": null + } + ], + "type": { + "kind": "OBJECT", + "name": "ReleaseDeletePayload", + "ofType": null + }, + "isDeprecated": false, + "deprecationReason": null + }, { "name": "releaseUpdate", "description": null, @@ -54087,6 +54114,122 @@ "enumValues": null, "possibleTypes": null }, + { + "kind": "INPUT_OBJECT", + "name": "ReleaseDeleteInput", + "description": "Autogenerated input type of ReleaseDelete", + "fields": null, + "inputFields": [ + { + "name": "projectPath", + "description": "Full path of the project the release is associated with", + "type": { + "kind": "NON_NULL", + "name": null, + "ofType": { + "kind": "SCALAR", + "name": "ID", + "ofType": null + } + }, + "defaultValue": null + }, + { + "name": "tagName", + "description": "Name of the tag associated with the release to delete.", + "type": { + "kind": "NON_NULL", + "name": null, + "ofType": { + "kind": "SCALAR", + "name": "String", + "ofType": null + } + }, + "defaultValue": null + }, + { + "name": "clientMutationId", + "description": "A unique identifier for the client performing the mutation.", + "type": { + "kind": "SCALAR", + "name": "String", + "ofType": null + }, + "defaultValue": null + } + ], + "interfaces": null, + "enumValues": null, + "possibleTypes": null + }, + { + "kind": "OBJECT", + "name": "ReleaseDeletePayload", + "description": "Autogenerated return type of ReleaseDelete", + "fields": [ + { + "name": "clientMutationId", + "description": "A unique identifier for the client performing the mutation.", + "args": [ + + ], + "type": { + "kind": "SCALAR", + "name": "String", + "ofType": null + }, + "isDeprecated": false, + "deprecationReason": null + }, + { + "name": "errors", + "description": "Errors encountered during execution of the mutation.", + "args": [ + + ], + "type": { + "kind": "NON_NULL", + "name": null, + "ofType": { + "kind": "LIST", + "name": null, + "ofType": { + "kind": "NON_NULL", + "name": null, + "ofType": { + "kind": "SCALAR", + "name": "String", + "ofType": null + } + } + } + }, + "isDeprecated": false, + "deprecationReason": null + }, + { + "name": "release", + "description": "The deleted release.", + "args": [ + + ], + "type": { + "kind": "OBJECT", + "name": "Release", + "ofType": null + }, + "isDeprecated": false, + "deprecationReason": null + } + ], + "inputFields": null, + "interfaces": [ + + ], + "enumValues": null, + "possibleTypes": null + }, { "kind": "OBJECT", "name": "ReleaseEdge", diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 5dfda50285e..d6f4cfe0ad1 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -2638,6 +2638,16 @@ Autogenerated return type of ReleaseCreate. | `errors` | String! => Array | Errors encountered during execution of the mutation. | | `release` | Release | The release after mutation | +### ReleaseDeletePayload + +Autogenerated return type of ReleaseDelete. + +| Field | Type | Description | +| ----- | ---- | ----------- | +| `clientMutationId` | String | A unique identifier for the client performing the mutation. | +| `errors` | String! => Array | Errors encountered during execution of the mutation. | +| `release` | Release | The deleted release. | + ### ReleaseEvidence Evidence for a release. diff --git a/locale/gitlab.pot b/locale/gitlab.pot index d93d2dbb8a3..ef4ae3f39ec 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -27254,6 +27254,9 @@ msgstr "" msgid "The update action will time out after %{number_of_minutes} minutes. For big repositories, use a clone/push combination." msgstr "" +msgid "The uploaded file was invalid. Supported file extensions are %{extensions}." +msgstr "" + msgid "The usage ping is disabled, and cannot be configured through this form." msgstr "" @@ -31656,6 +31659,9 @@ msgstr "" msgid "Your request for access has been queued for review." msgstr "" +msgid "Your requirements are being imported. Once finished, you'll receive a confirmation email." +msgstr "" + msgid "Your response has been recorded." msgstr "" diff --git a/spec/fixtures/csv_uppercase.CSV b/spec/fixtures/csv_uppercase.CSV new file mode 100644 index 00000000000..bef22ed9ddf --- /dev/null +++ b/spec/fixtures/csv_uppercase.CSV @@ -0,0 +1,4 @@ +TITLE,DESCRIPTION +Issue in 中文,Test description +"Hello","World" +"Title with quote""",Description diff --git a/spec/graphql/mutations/releases/delete_spec.rb b/spec/graphql/mutations/releases/delete_spec.rb new file mode 100644 index 00000000000..bedb72b002c --- /dev/null +++ b/spec/graphql/mutations/releases/delete_spec.rb @@ -0,0 +1,92 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Mutations::Releases::Delete do + let_it_be(:project) { create(:project, :public, :repository) } + let_it_be(:non_project_member) { create(:user) } + let_it_be(:developer) { create(:user) } + let_it_be(:maintainer) { create(:user) } + let_it_be(:tag) { 'v1.1.0'} + let_it_be(:release) { create(:release, project: project, tag: tag) } + + let(:mutation) { described_class.new(object: nil, context: { current_user: current_user }, field: nil) } + + let(:mutation_arguments) do + { + project_path: project.full_path, + tag: tag + } + end + + before do + project.add_developer(developer) + project.add_maintainer(maintainer) + end + + shared_examples 'unauthorized or not found error' do + it 'raises a Gitlab::Graphql::Errors::ResourceNotAvailable error' do + expect { subject }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable, "The resource that you are attempting to access does not exist or you don't have permission to perform this action") + end + end + + describe '#resolve' do + subject(:resolve) do + mutation.resolve(**mutation_arguments) + end + + context 'when the current user has access to create releases' do + let(:current_user) { maintainer } + + it 'deletes the release' do + expect { subject }.to change { Release.count }.by(-1) + end + + it 'returns the deleted release' do + expect(subject[:release].tag).to eq(tag) + end + + it 'does not remove the Git tag associated with the deleted release' do + expect { subject }.not_to change { Project.find_by_id(project.id).repository.tag_count } + end + + it 'returns no errors' do + expect(subject[:errors]).to eq([]) + end + + context 'validation' do + context 'when the release does not exist' do + let(:mutation_arguments) { super().merge(tag: 'not-a-real-release') } + + it 'returns the release as nil' do + expect(subject[:release]).to be_nil + end + + it 'returns an errors-at-data message' do + expect(subject[:errors]).to eq(['Release does not exist']) + end + end + + context 'when the project does not exist' do + let(:mutation_arguments) { super().merge(project_path: 'not/a/real/path') } + + it_behaves_like 'unauthorized or not found error' + end + end + end + + context "when the current user doesn't have access to update releases" do + context 'when the user is a developer' do + let(:current_user) { developer } + + it_behaves_like 'unauthorized or not found error' + end + + context 'when the user is a non-project member' do + let(:current_user) { non_project_member } + + it_behaves_like 'unauthorized or not found error' + end + end + end +end diff --git a/spec/requests/api/graphql/mutations/releases/delete_spec.rb b/spec/requests/api/graphql/mutations/releases/delete_spec.rb new file mode 100644 index 00000000000..3710f118bf4 --- /dev/null +++ b/spec/requests/api/graphql/mutations/releases/delete_spec.rb @@ -0,0 +1,132 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Deleting a release' do + include GraphqlHelpers + include Presentable + + let_it_be(:public_user) { create(:user) } + let_it_be(:guest) { create(:user) } + let_it_be(:reporter) { create(:user) } + let_it_be(:developer) { create(:user) } + let_it_be(:maintainer) { create(:user) } + let_it_be(:project) { create(:project, :public, :repository) } + let_it_be(:tag_name) { 'v1.1.0' } + let_it_be(:release) { create(:release, project: project, tag: tag_name) } + + let(:mutation_name) { :release_delete } + + let(:project_path) { project.full_path } + let(:mutation_arguments) do + { + projectPath: project_path, + tagName: tag_name + } + end + + let(:mutation) do + graphql_mutation(mutation_name, mutation_arguments, <<~FIELDS) + release { + tagName + } + errors + FIELDS + end + + let(:delete_release) { post_graphql_mutation(mutation, current_user: current_user) } + let(:mutation_response) { graphql_mutation_response(mutation_name)&.with_indifferent_access } + + before do + project.add_guest(guest) + project.add_reporter(reporter) + project.add_developer(developer) + project.add_maintainer(maintainer) + end + + shared_examples 'unauthorized or not found error' do + it 'returns a top-level error with message' do + delete_release + + expect(mutation_response).to be_nil + expect(graphql_errors.count).to eq(1) + expect(graphql_errors.first['message']).to eq("The resource that you are attempting to access does not exist or you don't have permission to perform this action") + end + end + + context 'when the current user has access to update releases' do + let(:current_user) { maintainer } + + it 'deletes the release' do + expect { delete_release }.to change { Release.count }.by(-1) + end + + it 'returns the deleted release' do + delete_release + + expected_release = { tagName: tag_name }.with_indifferent_access + + expect(mutation_response[:release]).to eq(expected_release) + end + + it 'does not remove the Git tag associated with the deleted release' do + expect { delete_release }.not_to change { Project.find_by_id(project.id).repository.tag_count } + end + + it 'returns no errors' do + delete_release + + expect(mutation_response[:errors]).to eq([]) + end + + context 'validation' do + context 'when the release does not exist' do + let_it_be(:tag_name) { 'not-a-real-release' } + + it 'returns the release as null' do + delete_release + + expect(mutation_response[:release]).to be_nil + end + + it 'returns an errors-at-data message' do + delete_release + + expect(mutation_response[:errors]).to eq(['Release does not exist']) + end + end + + context 'when the project does not exist' do + let(:project_path) { 'not/a/real/path' } + + it_behaves_like 'unauthorized or not found error' + end + end + end + + context "when the current user doesn't have access to update releases" do + context 'when the current user is a Developer' do + let(:current_user) { developer } + + it_behaves_like 'unauthorized or not found error' + end + + context 'when the current user is a Reporter' do + let(:current_user) { reporter } + + it_behaves_like 'unauthorized or not found error' + end + + context 'when the current user is a Guest' do + let(:current_user) { guest } + + it_behaves_like 'unauthorized or not found error' + end + + context 'when the current user is a public user' do + let(:current_user) { public_user } + + it_behaves_like 'unauthorized or not found error' + end + end +end diff --git a/spec/requests/import/gitlab_groups_controller_spec.rb b/spec/requests/import/gitlab_groups_controller_spec.rb index 4125c5c7c7a..fe8875f25f8 100644 --- a/spec/requests/import/gitlab_groups_controller_spec.rb +++ b/spec/requests/import/gitlab_groups_controller_spec.rb @@ -5,6 +5,7 @@ require 'spec_helper' RSpec.describe Import::GitlabGroupsController do include WorkhorseHelpers + 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 @@ -28,8 +29,6 @@ RSpec.describe Import::GitlabGroupsController do describe 'POST create' do subject(:import_request) { upload_archive(file_upload, workhorse_headers, request_params) } - let_it_be(:user) { create(:user) } - let(:file) { File.join('spec', %w[fixtures group_export.tar.gz]) } let(:file_upload) { fixture_file_upload(file) } @@ -194,67 +193,10 @@ RSpec.describe Import::GitlabGroupsController do end describe 'POST authorize' do - let_it_be(:user) { create(:user) } + it_behaves_like 'handle uploads authorize request' do + let(:uploader_class) { ImportExportUploader } - before do - login_as(user) - end - - context 'when using a workhorse header' do - subject(:authorize_request) { post authorize_import_gitlab_group_path, headers: workhorse_headers } - - it 'authorizes the request' do - authorize_request - - expect(response).to have_gitlab_http_status :ok - expect(response.media_type).to eq Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE - expect(json_response['TempPath']).to eq ImportExportUploader.workhorse_local_upload_path - end - end - - context 'when the request bypasses gitlab-workhorse' do - subject(:authorize_request) { post authorize_import_gitlab_group_path } - - it 'rejects the request' do - expect { authorize_request }.to raise_error(JWT::DecodeError) - end - end - - context 'when direct upload is enabled' do - subject(:authorize_request) { post authorize_import_gitlab_group_path, headers: workhorse_headers } - - before do - stub_uploads_object_storage(ImportExportUploader, enabled: true, direct_upload: true) - end - - it 'accepts the request and stores the files' do - authorize_request - - expect(response).to have_gitlab_http_status :ok - expect(response.media_type).to eq Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE - expect(json_response).not_to have_key 'TempPath' - - expect(json_response['RemoteObject'].keys) - .to include('ID', 'GetURL', 'StoreURL', 'DeleteURL', 'MultipartUpload') - end - end - - context 'when direct upload is disabled' do - subject(:authorize_request) { post authorize_import_gitlab_group_path, headers: workhorse_headers } - - before do - stub_uploads_object_storage(ImportExportUploader, enabled: true, direct_upload: false) - end - - it 'handles the local file' do - authorize_request - - expect(response).to have_gitlab_http_status :ok - expect(response.media_type).to eq Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE - - expect(json_response['TempPath']).to eq ImportExportUploader.workhorse_local_upload_path - expect(json_response['RemoteObject']).to be_nil - end + subject { post authorize_import_gitlab_group_path, headers: workhorse_headers } end end end diff --git a/spec/requests/import/gitlab_projects_controller_spec.rb b/spec/requests/import/gitlab_projects_controller_spec.rb index c1ac5a9f2c8..0e92410a2de 100644 --- a/spec/requests/import/gitlab_projects_controller_spec.rb +++ b/spec/requests/import/gitlab_projects_controller_spec.rb @@ -84,56 +84,10 @@ RSpec.describe Import::GitlabProjectsController do end describe 'POST authorize' do - subject { post authorize_import_gitlab_project_path, headers: workhorse_headers } + it_behaves_like 'handle uploads authorize request' do + let(:uploader_class) { ImportExportUploader } - it 'authorizes importing project with workhorse header' do - subject - - expect(response).to have_gitlab_http_status(:ok) - expect(response.content_type.to_s).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE) - expect(json_response['TempPath']).to eq(ImportExportUploader.workhorse_local_upload_path) - end - - it 'rejects requests that bypassed gitlab-workhorse' do - workhorse_headers.delete(Gitlab::Workhorse::INTERNAL_API_REQUEST_HEADER) - - expect { subject }.to raise_error(JWT::DecodeError) - end - - context 'when using remote storage' do - context 'when direct upload is enabled' do - before do - stub_uploads_object_storage(ImportExportUploader, enabled: true, direct_upload: true) - end - - it 'responds with status 200, location of file remote store and object details' do - subject - - expect(response).to have_gitlab_http_status(:ok) - expect(response.content_type.to_s).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE) - expect(json_response).not_to have_key('TempPath') - expect(json_response['RemoteObject']).to have_key('ID') - expect(json_response['RemoteObject']).to have_key('GetURL') - expect(json_response['RemoteObject']).to have_key('StoreURL') - expect(json_response['RemoteObject']).to have_key('DeleteURL') - expect(json_response['RemoteObject']).to have_key('MultipartUpload') - end - end - - context 'when direct upload is disabled' do - before do - stub_uploads_object_storage(ImportExportUploader, enabled: true, direct_upload: false) - end - - it 'handles as a local file' do - subject - - expect(response).to have_gitlab_http_status(:ok) - expect(response.content_type.to_s).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE) - expect(json_response['TempPath']).to eq(ImportExportUploader.workhorse_local_upload_path) - expect(json_response['RemoteObject']).to be_nil - end - end + subject { post authorize_import_gitlab_project_path, headers: workhorse_headers } end end end diff --git a/spec/support/helpers/graphql_helpers.rb b/spec/support/helpers/graphql_helpers.rb index 6a1ed6ce4c6..fa48a0d3cf5 100644 --- a/spec/support/helpers/graphql_helpers.rb +++ b/spec/support/helpers/graphql_helpers.rb @@ -31,6 +31,7 @@ module GraphqlHelpers end # TODO: Remove this method entirely when GraphqlHelpers uses real resolve_field + # see: https://gitlab.com/gitlab-org/gitlab/-/issues/287791 def aliased_args(resolver, args) definitions = resolver.arguments diff --git a/spec/support/shared_examples/requests/uploads_auhorize_shared_examples.rb b/spec/support/shared_examples/requests/uploads_auhorize_shared_examples.rb new file mode 100644 index 00000000000..cbd5ccf5b00 --- /dev/null +++ b/spec/support/shared_examples/requests/uploads_auhorize_shared_examples.rb @@ -0,0 +1,59 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'handle uploads authorize request' do + before do + login_as(user) + end + + describe 'POST authorize' do + it 'authorizes workhorse header' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(response.content_type.to_s).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE) + expect(json_response['TempPath']).to eq(uploader_class.workhorse_local_upload_path) + end + + it 'rejects requests that bypassed gitlab-workhorse' do + workhorse_headers.delete(Gitlab::Workhorse::INTERNAL_API_REQUEST_HEADER) + + expect { subject }.to raise_error(JWT::DecodeError) + end + + context 'when using remote storage' do + context 'when direct upload is enabled' do + before do + stub_uploads_object_storage(uploader_class, enabled: true, direct_upload: true) + end + + it 'responds with status 200, location of file remote store and object details' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(response.content_type.to_s).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE) + expect(json_response).not_to have_key('TempPath') + expect(json_response['RemoteObject']).to have_key('ID') + expect(json_response['RemoteObject']).to have_key('GetURL') + expect(json_response['RemoteObject']).to have_key('StoreURL') + expect(json_response['RemoteObject']).to have_key('DeleteURL') + expect(json_response['RemoteObject']).to have_key('MultipartUpload') + end + end + + context 'when direct upload is disabled' do + before do + stub_uploads_object_storage(uploader_class, enabled: true, direct_upload: false) + end + + it 'handles as a local file' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(response.content_type.to_s).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE) + expect(json_response['TempPath']).to eq(uploader_class.workhorse_local_upload_path) + expect(json_response['RemoteObject']).to be_nil + end + end + end + end +end