diff --git a/lib/api/concerns/virtual_registries/packages/endpoint.rb b/lib/api/concerns/virtual_registries/packages/endpoint.rb index 24560ec7f1581c231eddcf7f8604f5659f21d6d8..79034209102af445dc599619d99e2c79a2bdb8fc 100644 --- a/lib/api/concerns/virtual_registries/packages/endpoint.rb +++ b/lib/api/concerns/virtual_registries/packages/endpoint.rb @@ -11,6 +11,7 @@ module Endpoint MAJOR_BROWSERS = %i[webkit firefox ie edge opera chrome].freeze WEB_BROWSER_ERROR_MESSAGE = 'This endpoint is not meant to be accessed by a web browser.' UPSTREAM_GID_HEADER = 'X-Gitlab-Virtual-Registry-Upstream-Global-Id' + MAX_FILE_SIZE = 5.gigabytes included do helpers do @@ -53,14 +54,24 @@ def workhorse_upload_url(url:, upstream:) upstream.headers, url, response_headers: NO_BROWSER_EXECUTION_RESPONSE_HEADERS, - upload_config: { headers: { UPSTREAM_GID_HEADER => upstream.to_global_id.to_s } }, allow_localhost: allow_localhost, allowed_uris: allowed_uris, - ssrf_filter: true + ssrf_filter: true, + upload_config: { + headers: { UPSTREAM_GID_HEADER => upstream.to_global_id.to_s }, + authorized_upload_response: authorized_upload_response + } ) ) end + def authorized_upload_response + ::VirtualRegistries::CachedResponseUploader.workhorse_authorize( + has_length: true, + maximum_size: MAX_FILE_SIZE + ) + end + def send_workhorse_headers(headers) header(*headers) env['api.format'] = :binary diff --git a/lib/api/virtual_registries/packages/maven.rb b/lib/api/virtual_registries/packages/maven.rb index 4f802b9ec5edb1a2814ef4f2c6eacb4f0ad60b3d..63b3357900bad1c812771bfd8f381679eb992ed5 100644 --- a/lib/api/virtual_registries/packages/maven.rb +++ b/lib/api/virtual_registries/packages/maven.rb @@ -9,8 +9,6 @@ class Maven < ::API::Base feature_category :virtual_registry urgency :low - MAX_FILE_SIZE = 5.gigabytes - authenticate_with do |accept| accept.token_types(:personal_access_token).sent_through(:http_private_token_header) accept.token_types(:deploy_token).sent_through(:http_deploy_token_header) @@ -107,82 +105,52 @@ def registry send_successful_response_from(service_response: service_response) end - namespace 'upload' do - after_validation do - require_gitlab_workhorse! - authorize!(:read_virtual_registry, registry) - end - - desc 'Workhorse authorize upload endpoint of the Maven virtual registry. Only workhorse can access it.' do - detail 'This feature was introduced in GitLab 17.4. \ - This feature is currently in experiment state. \ - This feature is behind the `virtual_registry_maven` feature flag.' - success [ - { code: 200 } - ] - failure [ - { code: 400, message: 'Bad request' }, - { code: 401, message: 'Unauthorized' }, - { code: 403, message: 'Forbidden' }, - { code: 404, message: 'Not Found' } - ] - tags %w[maven_virtual_registries] - hidden true - end - params do - use :id_and_path - end - post 'authorize' do - status 200 - content_type Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE - ::VirtualRegistries::CachedResponseUploader.workhorse_authorize(has_length: true, - maximum_size: MAX_FILE_SIZE) - end + desc 'Workhorse upload endpoint of the Maven virtual registry. Only workhorse can access it.' do + detail 'This feature was introduced in GitLab 17.4. \ + This feature is currently in experiment state. \ + This feature is behind the `virtual_registry_maven` feature flag.' + success [ + { code: 200 } + ] + failure [ + { code: 400, message: 'Bad request' }, + { code: 401, message: 'Unauthorized' }, + { code: 403, message: 'Forbidden' }, + { code: 404, message: 'Not Found' } + ] + tags %w[maven_virtual_registries] + hidden true + end + params do + use :id_and_path + requires :file, + type: ::API::Validations::Types::WorkhorseFile, + desc: 'The file being uploaded', + documentation: { type: 'file' } + end + post 'upload' do + require_gitlab_workhorse! + authorize!(:read_virtual_registry, registry) + + etag, content_type, upstream_gid = request.headers.fetch_values( + 'Etag', + ::Gitlab::Workhorse::SEND_DEPENDENCY_CONTENT_TYPE_HEADER, + UPSTREAM_GID_HEADER + ) { nil } + + # TODO: revisit this part when multiple upstreams are supported + # https://gitlab.com/gitlab-org/gitlab/-/issues/480461 + # coherence check + not_found!('Upstream') unless upstream == GlobalID::Locator.locate(upstream_gid) + + service_response = ::VirtualRegistries::Packages::Maven::CachedResponses::CreateOrUpdateService.new( + upstream: upstream, + current_user: current_user, + params: declared_params.merge(etag: etag, content_type: content_type) + ).execute - desc 'Workhorse upload endpoint of the Maven virtual registry. Only workhorse can access it.' do - detail 'This feature was introduced in GitLab 17.4. \ - This feature is currently in experiment state. \ - This feature is behind the `virtual_registry_maven` feature flag.' - success [ - { code: 200 } - ] - failure [ - { code: 400, message: 'Bad request' }, - { code: 401, message: 'Unauthorized' }, - { code: 403, message: 'Forbidden' }, - { code: 404, message: 'Not Found' } - ] - tags %w[maven_virtual_registries] - hidden true - end - params do - use :id_and_path - requires :file, - type: ::API::Validations::Types::WorkhorseFile, - desc: 'The file being uploaded', - documentation: { type: 'file' } - end - post do - etag, content_type, upstream_gid = request.headers.fetch_values( - 'Etag', - ::Gitlab::Workhorse::SEND_DEPENDENCY_CONTENT_TYPE_HEADER, - UPSTREAM_GID_HEADER - ) { nil } - - # TODO: revisit this part when multiple upstreams are supported - # https://gitlab.com/gitlab-org/gitlab/-/issues/480461 - # coherence check - not_found!('Upstream') unless upstream == GlobalID::Locator.locate(upstream_gid) - - service_response = ::VirtualRegistries::Packages::Maven::CachedResponses::CreateOrUpdateService.new( - upstream: upstream, - current_user: current_user, - params: declared_params.merge(etag: etag, content_type: content_type) - ).execute - - send_error_response_from!(service_response: service_response) if service_response.error? - created! - end + send_error_response_from!(service_response: service_response) if service_response.error? + status :ok end end end diff --git a/lib/gitlab/workhorse.rb b/lib/gitlab/workhorse.rb index 172acb73348be1e50d790801b3763a2ddecf985b..ddb250e6d7aee7a92711bf6e0e0339fd34b9cfd3 100644 --- a/lib/gitlab/workhorse.rb +++ b/lib/gitlab/workhorse.rb @@ -242,7 +242,8 @@ def send_dependency( 'UploadConfig' => { 'Method' => upload_config[:method], 'Url' => upload_config[:url], - 'Headers' => (upload_config[:headers] || {}).transform_values { |v| Array.wrap(v) } + 'Headers' => (upload_config[:headers] || {}).transform_values { |v| Array.wrap(v) }, + 'AuthorizedUploadResponse' => upload_config[:authorized_upload_response] || {} }.compact_blank! } params.compact_blank! diff --git a/spec/lib/gitlab/workhorse_spec.rb b/spec/lib/gitlab/workhorse_spec.rb index 5112287c4f3027813cba10fae49d1281e10c5be9..fb3d96d256baa6fde00b55c32bc26a422af3ea57 100644 --- a/spec/lib/gitlab/workhorse_spec.rb +++ b/spec/lib/gitlab/workhorse_spec.rb @@ -630,7 +630,8 @@ def call_verify(headers) let(:upload_method) { nil } let(:upload_url) { nil } let(:upload_headers) { {} } - let(:upload_config) { { method: upload_method, headers: upload_headers, url: upload_url }.compact_blank! } + let(:authorized_upload_response) { {} } + let(:upload_config) { { method: upload_method, headers: upload_headers, url: upload_url, authorized_upload_response: authorized_upload_response }.compact_blank! } let(:ssrf_filter) { false } let(:allow_localhost) { true } let(:allowed_uris) { [] } @@ -653,7 +654,8 @@ def call_verify(headers) 'UploadConfig' => { 'Method' => upload_method, 'Url' => upload_url, - 'Headers' => upload_headers.transform_values { |v| Array.wrap(v) } + 'Headers' => upload_headers.transform_values { |v| Array.wrap(v) }, + 'AuthorizedUploadResponse' => authorized_upload_response }.compact_blank! } expected_params.compact_blank! @@ -686,6 +688,12 @@ def call_verify(headers) it_behaves_like 'setting the header correctly', ensure_upload_config_field: 'Headers' end + context 'with authorized upload response set' do + let(:authorized_upload_response) { { 'TempPath' => '/dev/null' } } + + it_behaves_like 'setting the header correctly', ensure_upload_config_field: 'AuthorizedUploadResponse' + end + context 'when `ssrf_filter` parameter is set' do let(:ssrf_filter) { true } diff --git a/spec/requests/api/virtual_registries/packages/maven_spec.rb b/spec/requests/api/virtual_registries/packages/maven_spec.rb index 47d5035377bcffcd7e953db5be844f05d516e872..9c37e7c20659c327d218cde3911881aff5b8cc87 100644 --- a/spec/requests/api/virtual_registries/packages/maven_spec.rb +++ b/spec/requests/api/virtual_registries/packages/maven_spec.rb @@ -1257,14 +1257,15 @@ end expected_upload_config = { - 'Headers' => { described_class::UPSTREAM_GID_HEADER => [upstream.to_global_id.to_s] } + 'Headers' => { described_class::UPSTREAM_GID_HEADER => [upstream.to_global_id.to_s] }, + 'AuthorizedUploadResponse' => a_kind_of(Hash) } expect(send_data_type).to eq('send-dependency') expect(send_data['Url']).to be_present expect(send_data['Headers']).to eq(expected_headers) expect(send_data['ResponseHeaders']).to eq(expected_resp_headers) - expect(send_data['UploadConfig']).to eq(expected_upload_config) + expect(send_data['UploadConfig']).to include(expected_upload_config) end end @@ -1357,53 +1358,6 @@ it_behaves_like 'not authenticated user' end - describe 'POST /api/v4/virtual_registries/packages/maven/:id/*path/upload/authorize' do - include_context 'workhorse headers' - - let(:path) { 'com/test/package/1.2.3/package-1.2.3.pom' } - let(:url) { "/virtual_registries/packages/maven/#{registry.id}/#{path}/upload/authorize" } - - subject(:request) do - post api(url), headers: headers - end - - shared_examples 'returning the workhorse authorization response' do - it 'authorizes the upload' do - 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']).not_to be_nil - end - end - - it_behaves_like 'authenticated endpoint', - success_shared_example_name: 'returning the workhorse authorization response' do - let(:headers) { workhorse_headers } - end - - context 'with a valid user' do - let(:headers) { workhorse_headers.merge(token_header(:personal_access_token)) } - - context 'with no workhorse headers' do - let(:headers) { token_header(:personal_access_token) } - - it_behaves_like 'returning response status', :forbidden - end - - context 'with no permissions on registry' do - let_it_be(:user) { create(:user) } - - it_behaves_like 'returning response status', :forbidden - end - - it_behaves_like 'disabled feature flag' - it_behaves_like 'disabled dependency proxy' - end - - it_behaves_like 'not authenticated user' - end - describe 'POST /api/v4/virtual_registries/packages/maven/:id/*path/upload' do include_context 'workhorse headers' @@ -1431,7 +1385,7 @@ it 'accepts the upload', :freeze_time do expect { request }.to change { upstream.cached_responses.count }.by(1) - expect(response).to have_gitlab_http_status(:created) + expect(response).to have_gitlab_http_status(:ok) expect(upstream.cached_responses.last).to have_attributes( relative_path: "/#{path}", downloads_count: 1, diff --git a/workhorse/_support/lint_last_known_acceptable.txt b/workhorse/_support/lint_last_known_acceptable.txt index 35440a12ac17ffe576b5cfc6a8f3208031cf0cbc..78365bec2a4060124d0d3703faa3e734aab79cce 100644 --- a/workhorse/_support/lint_last_known_acceptable.txt +++ b/workhorse/_support/lint_last_known_acceptable.txt @@ -20,7 +20,8 @@ internal/api/channel_settings.go:57:28: G402: TLS MinVersion too low. (gosec) internal/channel/channel.go:128:31: response body must be closed (bodyclose) internal/config/config.go:246:18: G204: Subprocess launched with variable (gosec) internal/config/config.go:328:8: G101: Potential hardcoded credentials (gosec) -internal/dependencyproxy/dependencyproxy_test.go:476: internal/dependencyproxy/dependencyproxy_test.go:476: Line contains TODO/BUG/FIXME/NOTE/OPTIMIZE/HACK: "note that the timeout duration here is s..." (godox) +internal/dependencyproxy/dependencyproxy.go:114: Function 'Inject' is too long (61 > 60) (funlen) +internal/dependencyproxy/dependencyproxy_test.go:510: internal/dependencyproxy/dependencyproxy_test.go:510: Line contains TODO/BUG/FIXME/NOTE/OPTIMIZE/HACK: "note that the timeout duration here is s..." (godox) internal/git/archive.go:35:2: var-naming: struct field CommitId should be CommitID (revive) internal/git/archive.go:43:2: exported: exported var SendArchive should have comment or be unexported (revive) internal/git/archive.go:53: Function 'Inject' has too many statements (47 > 40) (funlen) diff --git a/workhorse/internal/dependencyproxy/dependencyproxy.go b/workhorse/internal/dependencyproxy/dependencyproxy.go index 0c074e8fbff9c7215c6a04a337d8a321257e4e7a..b7d7b7ecae5e698eef60a65181167bea8edcf807 100644 --- a/workhorse/internal/dependencyproxy/dependencyproxy.go +++ b/workhorse/internal/dependencyproxy/dependencyproxy.go @@ -14,9 +14,11 @@ import ( "gitlab.com/gitlab-org/labkit/log" + "gitlab.com/gitlab-org/gitlab/workhorse/internal/api" "gitlab.com/gitlab-org/gitlab/workhorse/internal/helper/fail" "gitlab.com/gitlab-org/gitlab/workhorse/internal/senddata" "gitlab.com/gitlab-org/gitlab/workhorse/internal/transport" + "gitlab.com/gitlab-org/gitlab/workhorse/internal/upload" ) const dialTimeout = 10 * time.Second @@ -36,7 +38,7 @@ var httpClients sync.Map // Injector provides functionality for injecting dependencies type Injector struct { senddata.Prefix - uploadHandler http.Handler + uploadHandler upload.BodyUploadHandler } type entryParams struct { @@ -50,9 +52,33 @@ type entryParams struct { } type uploadConfig struct { - Headers http.Header - Method string - URL string + Headers http.Header + Method string + URL string + AuthorizedUploadResponse authorizeUploadResponse +} + +type authorizeUploadResponse struct { + TempPath string + RemoteObject api.RemoteObject + MaximumSize int64 + UploadHashFunctions []string +} + +func (u *uploadConfig) ExtractUploadAuthorizeFields() *api.Response { + tempPath := u.AuthorizedUploadResponse.TempPath + remoteID := u.AuthorizedUploadResponse.RemoteObject.RemoteTempObjectID + + if tempPath == "" && remoteID == "" { + return nil + } + + return &api.Response{ + TempPath: tempPath, + RemoteObject: u.AuthorizedUploadResponse.RemoteObject, + MaximumSize: u.AuthorizedUploadResponse.MaximumSize, + UploadHashFunctions: u.AuthorizedUploadResponse.UploadHashFunctions, + } } type nullResponseWriter struct { @@ -80,7 +106,7 @@ func NewInjector() *Injector { } // SetUploadHandler sets the upload handler for the Injector -func (p *Injector) SetUploadHandler(uploadHandler http.Handler) { +func (p *Injector) SetUploadHandler(uploadHandler upload.BodyUploadHandler) { p.uploadHandler = uploadHandler } @@ -135,7 +161,12 @@ func (p *Injector) Inject(w http.ResponseWriter, r *http.Request, sendData strin saveFileRequest.ContentLength = dependencyResponse.ContentLength nrw := &nullResponseWriter{header: make(http.Header)} - p.uploadHandler.ServeHTTP(nrw, saveFileRequest) + apiResponse := params.UploadConfig.ExtractUploadAuthorizeFields() + if apiResponse != nil { + p.uploadHandler.ServeHTTPWithAPIResponse(nrw, saveFileRequest, apiResponse) + } else { + p.uploadHandler.ServeHTTP(nrw, saveFileRequest) + } if nrw.status != http.StatusOK { fields := log.Fields{"code": nrw.status} @@ -213,14 +244,14 @@ func (p *Injector) unpackParams(sendData string) (*entryParams, error) { return nil, fmt.Errorf("dependency proxy: unpack sendData: %w", err) } - if err := p.validateParams(params); err != nil { + if err := p.validateParams(¶ms); err != nil { return nil, fmt.Errorf("dependency proxy: invalid params: %w", err) } return ¶ms, nil } -func (p *Injector) validateParams(params entryParams) error { +func (p *Injector) validateParams(params *entryParams) error { var uploadMethod = params.UploadConfig.Method if uploadMethod != "" && uploadMethod != http.MethodPost && uploadMethod != http.MethodPut { return fmt.Errorf("invalid upload method %s", uploadMethod) diff --git a/workhorse/internal/dependencyproxy/dependencyproxy_test.go b/workhorse/internal/dependencyproxy/dependencyproxy_test.go index 707ffa50e180df4bb02bbb01051a8bda4e37cd29..d8aa2629436e0890c4cd0569c943ba96b32fe113 100644 --- a/workhorse/internal/dependencyproxy/dependencyproxy_test.go +++ b/workhorse/internal/dependencyproxy/dependencyproxy_test.go @@ -8,6 +8,7 @@ import ( "io" "net/http" "net/http/httptest" + "os" "strconv" "strings" "sync" @@ -25,10 +26,12 @@ import ( ) type fakeUploadHandler struct { - request *http.Request - body []byte - skipBody bool - handler func(w http.ResponseWriter, r *http.Request) + request *http.Request + body []byte + skipBody bool + handler func(w http.ResponseWriter, r *http.Request) + serveHTTPUsed bool + serveHTTPWithAPIUsed bool } const ( @@ -43,6 +46,18 @@ func (f *fakeUploadHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { f.body, _ = io.ReadAll(r.Body) } + f.serveHTTPUsed = true + f.handler(w, r) +} + +func (f *fakeUploadHandler) ServeHTTPWithAPIResponse(w http.ResponseWriter, r *http.Request, _ *api.Response) { + f.request = r + + if !f.skipBody { + f.body, _ = io.ReadAll(r.Body) + } + + f.serveHTTPWithAPIUsed = true f.handler(w, r) } @@ -183,9 +198,11 @@ func TestValidUploadConfiguration(t *testing.T) { defer originResourceServer.Close() testCases := []struct { - desc string - uploadConfig *uploadConfig - expectedConfig uploadConfig + desc string + uploadConfig *uploadConfig + expectedConfig uploadConfig + serveHTTPUsed bool + serveHTTPWithAPIUsed bool }{ { desc: "with the default values", @@ -193,6 +210,7 @@ func TestValidUploadConfiguration(t *testing.T) { Method: http.MethodPost, URL: "/target/upload", }, + serveHTTPUsed: true, }, { desc: "with overridden method", uploadConfig: &uploadConfig{ @@ -202,6 +220,7 @@ func TestValidUploadConfiguration(t *testing.T) { Method: http.MethodPut, URL: "/target/upload", }, + serveHTTPUsed: true, }, { desc: "with overridden url", uploadConfig: &uploadConfig{ @@ -211,6 +230,7 @@ func TestValidUploadConfiguration(t *testing.T) { Method: http.MethodPost, URL: "http://test.org/overriden/upload", }, + serveHTTPUsed: true, }, { desc: "with overridden headers", uploadConfig: &uploadConfig{ @@ -221,6 +241,17 @@ func TestValidUploadConfiguration(t *testing.T) { Method: http.MethodPost, URL: "/target/upload", }, + serveHTTPUsed: true, + }, { + desc: "with authorized upload response", + uploadConfig: &uploadConfig{ + AuthorizedUploadResponse: authorizeUploadResponse{TempPath: os.TempDir()}, + }, + expectedConfig: uploadConfig{ + Method: http.MethodPost, + URL: "/target/upload", + }, + serveHTTPWithAPIUsed: true, }, } @@ -258,11 +289,14 @@ func TestValidUploadConfiguration(t *testing.T) { response := makeRequest(injector, string(sendDataJSONString)) - // checking the response + // check the response require.Equal(t, 200, response.Code) require.Equal(t, string(content), response.Body.String()) - // checking remote file request + // check remote file request require.Equal(t, "/remote/file", response.Header().Get(testHeader)) + // check upload handler + require.Equal(t, tc.serveHTTPUsed, uploadHandler.serveHTTPUsed) + require.Equal(t, tc.serveHTTPWithAPIUsed, uploadHandler.serveHTTPWithAPIUsed) }) } } diff --git a/workhorse/internal/upload/body_uploader.go b/workhorse/internal/upload/body_uploader.go index 307c32d2b9fea2b804b492b5acf2f8b19f2275c9..dc041949ab2213b1338f3773b2211cf0f3371346 100644 --- a/workhorse/internal/upload/body_uploader.go +++ b/workhorse/internal/upload/body_uploader.go @@ -13,48 +13,74 @@ import ( "gitlab.com/gitlab-org/gitlab/workhorse/internal/upload/destination" ) +// BodyUploadHandler conforms to the http.Handler interface. +// It also provides an addition function to pass an api.Response. +type BodyUploadHandler interface { + http.Handler + ServeHTTPWithAPIResponse(http.ResponseWriter, *http.Request, *api.Response) +} + // RequestBody is a request middleware. It will store the request body to // a location by determined an api.Response value. It then forwards the // request to gitlab-rails without the original request body. -func RequestBody(rails PreAuthorizer, h http.Handler, p Preparer) http.Handler { - return rails.PreAuthorizeHandler(func(w http.ResponseWriter, r *http.Request, a *api.Response) { - opts, err := p.Prepare(a) - if err != nil { - fail.Request(w, r, fmt.Errorf("RequestBody: preparation failed: %v", err)) - return - } - - fh, err := destination.Upload(r.Context(), r.Body, r.ContentLength, "upload", opts) - if err != nil { - fail.Request(w, r, fmt.Errorf("RequestBody: upload failed: %v", err)) - return - } - - data := url.Values{} - fields, err := fh.GitLabFinalizeFields("file") - if err != nil { - fail.Request(w, r, fmt.Errorf("RequestBody: finalize fields failed: %v", err)) - return - } - - for k, v := range fields { - data.Set(k, v) - } - - // Hijack body - body := data.Encode() - r.Body = io.NopCloser(strings.NewReader(body)) - r.ContentLength = int64(len(body)) - r.Header.Set("Content-Type", "application/x-www-form-urlencoded") - - sft := SavedFileTracker{Request: r} - sft.Track("file", fh.LocalPath) - if err := sft.Finalize(r.Context()); err != nil { - fail.Request(w, r, fmt.Errorf("RequestBody: finalize failed: %v", err)) - return - } - - // And proxy the request - h.ServeHTTP(w, r) +func RequestBody(rails PreAuthorizer, h http.Handler, p Preparer) BodyUploadHandler { + preAuthorizeHandler := rails.PreAuthorizeHandler(func(w http.ResponseWriter, r *http.Request, a *api.Response) { + processRequestBody(h, p, w, r, a) }, "/authorize") + return &bodyUploadHandlerImpl{preAuthorizeHandler, h, p} +} + +type bodyUploadHandlerImpl struct { + preAuthorizeHandler http.Handler + httpHandler http.Handler + preparer Preparer +} + +func (handler *bodyUploadHandlerImpl) ServeHTTPWithAPIResponse(w http.ResponseWriter, r *http.Request, a *api.Response) { + processRequestBody(handler.httpHandler, handler.preparer, w, r, a) +} + +func (handler *bodyUploadHandlerImpl) ServeHTTP(h http.ResponseWriter, r *http.Request) { + handler.preAuthorizeHandler.ServeHTTP(h, r) +} + +func processRequestBody(h http.Handler, p Preparer, w http.ResponseWriter, r *http.Request, a *api.Response) { + opts, err := p.Prepare(a) + if err != nil { + fail.Request(w, r, fmt.Errorf("RequestBody: preparation failed: %v", err)) + return + } + + fh, err := destination.Upload(r.Context(), r.Body, r.ContentLength, "upload", opts) + if err != nil { + fail.Request(w, r, fmt.Errorf("RequestBody: upload failed: %v", err)) + return + } + + data := url.Values{} + fields, err := fh.GitLabFinalizeFields("file") + if err != nil { + fail.Request(w, r, fmt.Errorf("RequestBody: finalize fields failed: %v", err)) + return + } + + for k, v := range fields { + data.Set(k, v) + } + + // Hijack body + body := data.Encode() + r.Body = io.NopCloser(strings.NewReader(body)) + r.ContentLength = int64(len(body)) + r.Header.Set("Content-Type", "application/x-www-form-urlencoded") + + sft := SavedFileTracker{Request: r} + sft.Track("file", fh.LocalPath) + if err := sft.Finalize(r.Context()); err != nil { + fail.Request(w, r, fmt.Errorf("RequestBody: finalize failed: %v", err)) + return + } + + // And proxy the request + h.ServeHTTP(w, r) } diff --git a/workhorse/internal/upload/body_uploader_test.go b/workhorse/internal/upload/body_uploader_test.go index 7bae124a3135afdca6c58b8478a947afed39163c..2679063e843a6991d6f3480a65cf92777b8b749f 100644 --- a/workhorse/internal/upload/body_uploader_test.go +++ b/workhorse/internal/upload/body_uploader_test.go @@ -43,6 +43,24 @@ func TestRequestBody(t *testing.T) { require.Equal(t, fileContent, string(uploadEcho)) } +func TestRequestBodyWithAPIResponse(t *testing.T) { + testhelper.ConfigureSecret() + + body := strings.NewReader(fileContent) + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + resp := testUploadWithAPIResponse(ctx, &rails{}, &alwaysLocalPreparer{}, echoProxy(t, fileLen), body, &api.Response{TempPath: os.TempDir()}) + defer resp.Body.Close() + require.Equal(t, http.StatusOK, resp.StatusCode) + + uploadEcho, err := io.ReadAll(resp.Body) + + require.NoError(t, err, "Can't read response body") + require.Equal(t, fileContent, string(uploadEcho)) +} + func TestRequestBodyCustomPreparer(t *testing.T) { body := strings.NewReader(fileContent) @@ -99,6 +117,15 @@ func testUpload(ctx context.Context, auth PreAuthorizer, preparer Preparer, prox return w.Result() } +func testUploadWithAPIResponse(ctx context.Context, auth PreAuthorizer, preparer Preparer, proxy http.Handler, body io.Reader, api *api.Response) *http.Response { + req := httptest.NewRequest("POST", "http://example.com/upload", body).WithContext(ctx) + w := httptest.NewRecorder() + + RequestBody(auth, proxy, preparer).ServeHTTPWithAPIResponse(w, req, api) + + return w.Result() +} + func echoProxy(t *testing.T, expectedBodyLength int) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { err := r.ParseForm()