Commit 50eaf781 authored by Alessio Caiazza's avatar Alessio Caiazza

Merge branch '214785-support-jwt-param-from-workhorse-for-file-uploads' into 'master'

Support the JWT params set by Workhorse in upload requests

See merge request gitlab-org/gitlab!33277
parents d1d63b17 1a7a1c3c
---
title: Support JWT params set by Workhorse during uploads
merge_request: 33277
author:
type: changed
---
name: upload_middleware_jwt_params_handler
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/33277
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/233895
group: group::package
type: development
default_enabled: false
...@@ -29,6 +29,8 @@ module Gitlab ...@@ -29,6 +29,8 @@ module Gitlab
module Middleware module Middleware
class Multipart class Multipart
RACK_ENV_KEY = 'HTTP_GITLAB_WORKHORSE_MULTIPART_FIELDS' RACK_ENV_KEY = 'HTTP_GITLAB_WORKHORSE_MULTIPART_FIELDS'
JWT_PARAM_SUFFIX = '.gitlab-workhorse-upload'
JWT_PARAM_FIXED_KEY = 'upload'
class Handler class Handler
def initialize(env, message) def initialize(env, message)
...@@ -133,6 +135,79 @@ module Gitlab ...@@ -133,6 +135,79 @@ module Gitlab
end end
end end
# TODO this class is meant to replace Handler when the feature flag
# upload_middleware_jwt_params_handler is removed
class HandlerForJWTParams < Handler
def with_open_files
@rewritten_fields.keys.each do |field|
parsed_field = Rack::Utils.parse_nested_query(field)
raise "unexpected field: #{field.inspect}" unless parsed_field.count == 1
key, value = parsed_field.first
if value.nil? # we have a top level param, eg. field = 'foo' and not 'foo[bar]'
raise "invalid field: #{field.inspect}" if field != key
value = open_file(extract_upload_params_from(@request.params, with_prefix: key))
@open_files << value
else
value = decorate_params_value(value, @request.params[key])
end
update_param(key, value)
end
yield
ensure
@open_files.compact
.each(&:close)
end
# This function calls itself recursively
def decorate_params_value(hash_path, value_hash)
unless hash_path.is_a?(Hash) && hash_path.count == 1
raise "invalid path: #{hash_path.inspect}"
end
path_key, path_value = hash_path.first
unless value_hash.is_a?(Hash) && value_hash[path_key]
raise "invalid value hash: #{value_hash.inspect}"
end
case path_value
when nil
value_hash[path_key] = open_file(extract_upload_params_from(value_hash[path_key]))
@open_files << value_hash[path_key]
value_hash
when Hash
decorate_params_value(path_value, value_hash[path_key])
value_hash
else
raise "unexpected path value: #{path_value.inspect}"
end
end
def open_file(params)
::UploadedFile.from_params_without_field(params, allowed_paths)
end
private
def extract_upload_params_from(params, with_prefix: '')
param_key = "#{with_prefix}#{JWT_PARAM_SUFFIX}"
jwt_token = params[param_key]
raise "Empty JWT param: #{param_key}" if jwt_token.blank?
payload = Gitlab::Workhorse.decode_jwt(jwt_token).first
raise "Invalid JWT payload: not a Hash" unless payload.is_a?(Hash)
upload_params = payload.fetch(JWT_PARAM_FIXED_KEY, {})
raise "Empty params for: #{param_key}" if upload_params.empty?
upload_params
end
end
def initialize(app) def initialize(app)
@app = app @app = app
end end
...@@ -143,12 +218,22 @@ module Gitlab ...@@ -143,12 +218,22 @@ module Gitlab
message = ::Gitlab::Workhorse.decode_jwt(encoded_message)[0] message = ::Gitlab::Workhorse.decode_jwt(encoded_message)[0]
::Gitlab::Middleware::Multipart::Handler.new(env, message).with_open_files do handler_class.new(env, message).with_open_files do
@app.call(env) @app.call(env)
end end
rescue UploadedFile::InvalidPathError => e rescue UploadedFile::InvalidPathError => e
[400, { 'Content-Type' => 'text/plain' }, e.message] [400, { 'Content-Type' => 'text/plain' }, e.message]
end end
private
def handler_class
if Feature.enabled?(:upload_middleware_jwt_params_handler)
::Gitlab::Middleware::Multipart::HandlerForJWTParams
else
::Gitlab::Middleware::Multipart::Handler
end
end
end end
end end
end end
...@@ -42,6 +42,32 @@ class UploadedFile ...@@ -42,6 +42,32 @@ class UploadedFile
@remote_id = remote_id @remote_id = remote_id
end end
def self.from_params_without_field(params, upload_paths)
path = params['path']
remote_id = params['remote_id']
return if path.blank? && remote_id.blank?
# don't use file_path if remote_id is set
if remote_id.present?
file_path = nil
elsif path.present?
file_path = File.realpath(path)
unless self.allowed_path?(file_path, Array(upload_paths).compact)
raise InvalidPathError, "insecure path used '#{file_path}'"
end
end
UploadedFile.new(
file_path,
filename: params['name'],
content_type: params['type'] || 'application/octet-stream',
sha256: params['sha256'],
remote_id: remote_id,
size: params['size']
)
end
def self.from_params(params, field, upload_paths, path_override = nil) def self.from_params(params, field, upload_paths, path_override = nil)
path = path_override || params["#{field}.path"] path = path_override || params["#{field}.path"]
remote_id = params["#{field}.remote_id"] remote_id = params["#{field}.remote_id"]
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Gitlab::Middleware::Multipart::HandlerForJWTParams do
using RSpec::Parameterized::TableSyntax
let_it_be(:env) { Rack::MockRequest.env_for('/', method: 'post', params: {}) }
let_it_be(:message) { { 'rewritten_fields' => {} } }
describe '#allowed_paths' do
let_it_be(:expected_allowed_paths) do
[
Dir.tmpdir,
::FileUploader.root,
::Gitlab.config.uploads.storage_path,
::JobArtifactUploader.workhorse_upload_path,
::LfsObjectUploader.workhorse_upload_path,
File.join(Rails.root, 'public/uploads/tmp')
]
end
let_it_be(:expected_with_packages_path) { expected_allowed_paths + [::Packages::PackageFileUploader.workhorse_upload_path] }
subject { described_class.new(env, message).send(:allowed_paths) }
where(:package_features_enabled, :object_storage_enabled, :direct_upload_enabled, :expected_paths) do
false | false | true | :expected_allowed_paths
false | false | false | :expected_allowed_paths
false | true | true | :expected_allowed_paths
false | true | false | :expected_allowed_paths
true | false | true | :expected_with_packages_path
true | false | false | :expected_with_packages_path
true | true | true | :expected_allowed_paths
true | true | false | :expected_with_packages_path
end
with_them do
before do
stub_config(packages: {
enabled: package_features_enabled,
object_store: {
enabled: object_storage_enabled,
direct_upload: direct_upload_enabled
},
storage_path: '/any/dir'
})
end
it { is_expected.to eq(send(expected_paths)) }
end
end
end
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Gitlab::Middleware::Multipart do
include MultipartHelpers
describe '#call' do
let(:app) { double(:app) }
let(:middleware) { described_class.new(app) }
let(:secret) { Gitlab::Workhorse.secret }
let(:issuer) { 'gitlab-workhorse' }
subject do
env = post_env(
rewritten_fields: rewritten_fields,
params: params,
secret: secret,
issuer: issuer
)
middleware.call(env)
end
before do
stub_feature_flags(upload_middleware_jwt_params_handler: true)
end
context 'remote file mode' do
let(:mode) { :remote }
it_behaves_like 'handling all upload parameters conditions'
context 'and a path set' do
include_context 'with one temporary file for multipart'
let(:rewritten_fields) { rewritten_fields_hash('file' => uploaded_filepath) }
let(:params) { upload_parameters_for(key: 'file', filename: filename, remote_id: remote_id).merge('file.path' => '/should/not/be/read') }
it 'builds an UploadedFile' do
expect_uploaded_files(original_filename: filename, remote_id: remote_id, size: uploaded_file.size, params_path: %w(file))
subject
end
end
end
context 'local file mode' do
let(:mode) { :local }
it_behaves_like 'handling all upload parameters conditions'
context 'when file is' do
include_context 'with one temporary file for multipart'
let(:allowed_paths) { [Dir.tmpdir] }
before do
expect_next_instance_of(::Gitlab::Middleware::Multipart::HandlerForJWTParams) do |handler|
expect(handler).to receive(:allowed_paths).and_return(allowed_paths)
end
end
context 'in allowed paths' do
let(:rewritten_fields) { rewritten_fields_hash('file' => uploaded_filepath) }
let(:params) { upload_parameters_for(filepath: uploaded_filepath, key: 'file', filename: filename) }
it 'builds an UploadedFile' do
expect_uploaded_files(filepath: uploaded_filepath, original_filename: filename, size: uploaded_file.size, params_path: %w(file))
subject
end
end
context 'not in allowed paths' do
let(:allowed_paths) { [] }
let(:rewritten_fields) { rewritten_fields_hash('file' => uploaded_filepath) }
let(:params) { upload_parameters_for(filepath: uploaded_filepath, key: 'file') }
it 'returns an error' do
result = subject
expect(result[0]).to eq(400)
expect(result[2]).to include('insecure path used')
end
end
end
end
context 'with dummy params in remote mode' do
let(:rewritten_fields) { { 'file' => 'should/not/be/read' } }
let(:params) { upload_parameters_for(key: 'file') }
let(:mode) { :remote }
context 'with an invalid secret' do
let(:secret) { 'INVALID_SECRET' }
it { expect { subject }.to raise_error(JWT::VerificationError) }
end
context 'with an invalid issuer' do
let(:issuer) { 'INVALID_ISSUER' }
it { expect { subject }.to raise_error(JWT::InvalidIssuerError) }
end
context 'with invalid rewritten field key' do
invalid_keys = [
'[file]',
';file',
'file]',
';file]',
'file]]',
'file;;'
]
invalid_keys.each do |invalid_key|
context invalid_key do
let(:rewritten_fields) { { invalid_key => 'should/not/be/read' } }
it { expect { subject }.to raise_error(RuntimeError, "invalid field: \"#{invalid_key}\"") }
end
end
end
context 'with invalid key in parameters' do
include_context 'with one temporary file for multipart'
let(:rewritten_fields) { rewritten_fields_hash('file' => uploaded_filepath) }
let(:params) { upload_parameters_for(filepath: uploaded_filepath, key: 'wrong_key', filename: filename, remote_id: remote_id) }
it 'raises an error' do
expect { subject }.to raise_error(RuntimeError, 'Empty JWT param: file.gitlab-workhorse-upload')
end
end
context 'with a modified JWT payload' do
include_context 'with one temporary file for multipart'
let(:rewritten_fields) { rewritten_fields_hash('file' => uploaded_filepath) }
let(:crafted_payload) { Base64.urlsafe_encode64({ 'path' => 'test' }.to_json) }
let(:params) do
upload_parameters_for(filepath: uploaded_filepath, key: 'file', filename: filename, remote_id: remote_id).tap do |params|
header, _, sig = params['file.gitlab-workhorse-upload'].split('.')
params['file.gitlab-workhorse-upload'] = [header, crafted_payload, sig].join('.')
end
end
it 'raises an error' do
expect { subject }.to raise_error(JWT::VerificationError, 'Signature verification raised')
end
end
context 'with a modified JWT sig' do
include_context 'with one temporary file for multipart'
let(:rewritten_fields) { rewritten_fields_hash('file' => uploaded_filepath) }
let(:params) do
upload_parameters_for(filepath: uploaded_filepath, key: 'file', filename: filename, remote_id: remote_id).tap do |params|
header, payload, sig = params['file.gitlab-workhorse-upload'].split('.')
params['file.gitlab-workhorse-upload'] = [header, payload, "#{sig}modified"].join('.')
end
end
it 'raises an error' do
expect { subject }.to raise_error(JWT::VerificationError, 'Signature verification raised')
end
end
end
end
end
...@@ -21,7 +21,11 @@ RSpec.describe Gitlab::Middleware::Multipart do ...@@ -21,7 +21,11 @@ RSpec.describe Gitlab::Middleware::Multipart do
middleware.call(env) middleware.call(env)
end end
context 'with remote file mode params' do before do
stub_feature_flags(upload_middleware_jwt_params_handler: false)
end
context 'remote file mode' do
let(:mode) { :remote } let(:mode) { :remote }
it_behaves_like 'handling all upload parameters conditions' it_behaves_like 'handling all upload parameters conditions'
...@@ -58,10 +62,10 @@ RSpec.describe Gitlab::Middleware::Multipart do ...@@ -58,10 +62,10 @@ RSpec.describe Gitlab::Middleware::Multipart do
context 'in allowed paths' do context 'in allowed paths' do
let(:rewritten_fields) { rewritten_fields_hash('file' => uploaded_filepath) } let(:rewritten_fields) { rewritten_fields_hash('file' => uploaded_filepath) }
let(:params) { upload_parameters_for(filepath: uploaded_filepath, key: 'file', filename: filename, remote_id: remote_id) } let(:params) { upload_parameters_for(filepath: uploaded_filepath, key: 'file', filename: filename) }
it 'builds an UploadedFile' do it 'builds an UploadedFile' do
expect_uploaded_files(filepath: uploaded_filepath, original_filename: filename, remote_id: remote_id, size: uploaded_file.size, params_path: %w(file)) expect_uploaded_files(filepath: uploaded_filepath, original_filename: filename, size: uploaded_file.size, params_path: %w(file))
subject subject
end end
......
...@@ -14,32 +14,175 @@ RSpec.describe UploadedFile do ...@@ -14,32 +14,175 @@ RSpec.describe UploadedFile do
FileUtils.rm_f(temp_file) FileUtils.rm_f(temp_file)
end end
describe ".from_params" do context 'from_params functions' do
RSpec.shared_examples 'using the file path' do |filename:, content_type:, sha256:, path_suffix:|
it { is_expected.not_to be_nil }
it 'sets properly the attributes' do
expect(subject.original_filename).to eq(filename)
expect(subject.content_type).to eq(content_type)
expect(subject.sha256).to eq(sha256)
expect(subject.remote_id).to be_nil
expect(subject.path).to end_with(path_suffix)
end
it 'handles a blank path' do
params['file.path'] = ''
# Not a real file, so can't determine size itself
params['file.size'] = 1.byte
expect { described_class.from_params(params, :file, upload_path) }
.not_to raise_error
end
end
RSpec.shared_examples 'using the remote id' do |filename:, content_type:, sha256:, size:, remote_id:|
it { is_expected.not_to be_nil }
it 'sets properly the attributes' do
expect(subject.original_filename).to eq(filename)
expect(subject.content_type).to eq('application/octet-stream')
expect(subject.sha256).to eq('sha256')
expect(subject.path).to be_nil
expect(subject.size).to eq(123456)
expect(subject.remote_id).to eq('1234567890')
end
end
describe '.from_params_without_field' do
let(:upload_path) { nil } let(:upload_path) { nil }
let(:file_path_override) { nil }
after do after do
FileUtils.rm_r(upload_path) if upload_path FileUtils.rm_r(upload_path) if upload_path
end end
subject do subject do
described_class.from_params(params, :file, [upload_path, Dir.tmpdir], file_path_override) described_class.from_params_without_field(params, [upload_path, Dir.tmpdir])
end end
context 'when valid file is specified' do context 'when valid file is specified' do
context 'only local path is specified' do context 'only local path is specified' do
let(:params) do let(:params) { { 'path' => temp_file.path } }
{ 'file.path' => temp_file.path }
end
it { is_expected.not_to be_nil } it { is_expected.not_to be_nil }
it "generates filename from path" do it 'generates filename from path' do
expect(subject.original_filename).to eq(::File.basename(temp_file.path)) expect(subject.original_filename).to eq(::File.basename(temp_file.path))
end end
end end
context 'all parameters are specified' do context 'all parameters are specified' do
context 'with a filepath' do
let(:params) do
{ 'path' => temp_file.path,
'name' => 'dir/my file&.txt',
'type' => 'my/type',
'sha256' => 'sha256' }
end
it_behaves_like 'using the file path',
filename: 'my_file_.txt',
content_type: 'my/type',
sha256: 'sha256',
path_suffix: 'test'
end
context 'with a remote id' do
let(:params) do
{
'name' => 'dir/my file&.txt',
'sha256' => 'sha256',
'remote_url' => 'http://localhost/file',
'remote_id' => '1234567890',
'etag' => 'etag1234567890',
'size' => '123456'
}
end
it_behaves_like 'using the remote id',
filename: 'my_file_.txt',
content_type: 'application/octet-stream',
sha256: 'sha256',
size: 123456,
remote_id: '1234567890'
end
context 'with a path and a remote id' do
let(:params) do
{
'path' => temp_file.path,
'name' => 'dir/my file&.txt',
'sha256' => 'sha256',
'remote_url' => 'http://localhost/file',
'remote_id' => '1234567890',
'etag' => 'etag1234567890',
'size' => '123456'
}
end
it_behaves_like 'using the remote id',
filename: 'my_file_.txt',
content_type: 'application/octet-stream',
sha256: 'sha256',
size: 123456,
remote_id: '1234567890'
end
end
end
context 'when no params are specified' do
let(:params) { {} }
it 'does not return an object' do
is_expected.to be_nil
end
end
context 'when verifying allowed paths' do
let(:params) { { 'path' => temp_file.path } }
context 'when file is stored in system temporary folder' do
let(:temp_dir) { Dir.tmpdir }
it { is_expected.not_to be_nil }
end
context 'when file is stored in user provided upload path' do
let(:upload_path) { Dir.mktmpdir }
let(:temp_dir) { upload_path }
it { is_expected.not_to be_nil }
end
context 'when file is stored outside of user provided upload path' do
let!(:generated_dir) { Dir.mktmpdir }
let!(:temp_dir) { Dir.mktmpdir }
before do
# We overwrite default temporary path
allow(Dir).to receive(:tmpdir).and_return(generated_dir)
end
it 'raises an error' do
expect { subject }.to raise_error(UploadedFile::InvalidPathError, /insecure path used/)
end
end
end
end
describe '.from_params' do
let(:upload_path) { nil }
let(:file_path_override) { nil }
after do
FileUtils.rm_r(upload_path) if upload_path
end
subject do
described_class.from_params(params, :file, [upload_path, Dir.tmpdir], file_path_override)
end
RSpec.shared_context 'filepath override' do RSpec.shared_context 'filepath override' do
let(:temp_file_override) { Tempfile.new(%w[override override], temp_dir) } let(:temp_file_override) { Tempfile.new(%w[override override], temp_dir) }
let(:file_path_override) { temp_file_override.path } let(:file_path_override) { temp_file_override.path }
...@@ -53,37 +196,20 @@ RSpec.describe UploadedFile do ...@@ -53,37 +196,20 @@ RSpec.describe UploadedFile do
end end
end end
RSpec.shared_examples 'using the file path' do |filename:, content_type:, sha256:, path_suffix:| context 'when valid file is specified' do
it 'sets properly the attributes' do context 'only local path is specified' do
expect(subject.original_filename).to eq(filename) let(:params) do
expect(subject.content_type).to eq(content_type) { 'file.path' => temp_file.path }
expect(subject.sha256).to eq(sha256)
expect(subject.remote_id).to be_nil
expect(subject.path).to end_with(path_suffix)
end end
it 'handles a blank path' do it { is_expected.not_to be_nil }
params['file.path'] = ''
# Not a real file, so can't determine size itself
params['file.size'] = 1.byte
expect { described_class.from_params(params, :file, upload_path) }
.not_to raise_error
end
end
RSpec.shared_examples 'using the remote id' do |filename:, content_type:, sha256:, size:, remote_id:| it "generates filename from path" do
it 'sets properly the attributes' do expect(subject.original_filename).to eq(::File.basename(temp_file.path))
expect(subject.original_filename).to eq(filename)
expect(subject.content_type).to eq('application/octet-stream')
expect(subject.sha256).to eq('sha256')
expect(subject.path).to be_nil
expect(subject.size).to eq(123456)
expect(subject.remote_id).to eq('1234567890')
end end
end end
context 'all parameters are specified' do
context 'with a filepath' do context 'with a filepath' do
let(:params) do let(:params) do
{ 'file.path' => temp_file.path, { 'file.path' => temp_file.path,
...@@ -92,8 +218,6 @@ RSpec.describe UploadedFile do ...@@ -92,8 +218,6 @@ RSpec.describe UploadedFile do
'file.sha256' => 'sha256' } 'file.sha256' => 'sha256' }
end end
it { is_expected.not_to be_nil }
it_behaves_like 'using the file path', it_behaves_like 'using the file path',
filename: 'my_file_.txt', filename: 'my_file_.txt',
content_type: 'my/type', content_type: 'my/type',
...@@ -111,8 +235,6 @@ RSpec.describe UploadedFile do ...@@ -111,8 +235,6 @@ RSpec.describe UploadedFile do
'file.sha256' => 'sha256' } 'file.sha256' => 'sha256' }
end end
it { is_expected.not_to be_nil }
it_behaves_like 'using the file path', it_behaves_like 'using the file path',
filename: 'my_file_.txt', filename: 'my_file_.txt',
content_type: 'my/type', content_type: 'my/type',
...@@ -132,8 +254,6 @@ RSpec.describe UploadedFile do ...@@ -132,8 +254,6 @@ RSpec.describe UploadedFile do
} }
end end
it { is_expected.not_to be_nil }
it_behaves_like 'using the remote id', it_behaves_like 'using the remote id',
filename: 'my_file_.txt', filename: 'my_file_.txt',
content_type: 'application/octet-stream', content_type: 'application/octet-stream',
...@@ -155,8 +275,6 @@ RSpec.describe UploadedFile do ...@@ -155,8 +275,6 @@ RSpec.describe UploadedFile do
} }
end end
it { is_expected.not_to be_nil }
it_behaves_like 'using the remote id', it_behaves_like 'using the remote id',
filename: 'my_file_.txt', filename: 'my_file_.txt',
content_type: 'application/octet-stream', content_type: 'application/octet-stream',
...@@ -179,8 +297,6 @@ RSpec.describe UploadedFile do ...@@ -179,8 +297,6 @@ RSpec.describe UploadedFile do
} }
end end
it { is_expected.not_to be_nil }
it_behaves_like 'using the remote id', it_behaves_like 'using the remote id',
filename: 'my_file_.txt', filename: 'my_file_.txt',
content_type: 'application/octet-stream', content_type: 'application/octet-stream',
...@@ -238,6 +354,7 @@ RSpec.describe UploadedFile do ...@@ -238,6 +354,7 @@ RSpec.describe UploadedFile do
end end
end end
end end
end
describe '.initialize' do describe '.initialize' do
context 'when no size is provided' do context 'when no size is provided' do
......
# frozen_string_literal: true # frozen_string_literal: true
module MultipartHelpers module MultipartHelpers
include WorkhorseHelpers
def post_env(rewritten_fields:, params:, secret:, issuer:) def post_env(rewritten_fields:, params:, secret:, issuer:)
token = JWT.encode({ 'iss' => issuer, 'rewritten_fields' => rewritten_fields }, secret, 'HS256') token = JWT.encode({ 'iss' => issuer, 'rewritten_fields' => rewritten_fields }, secret, 'HS256')
Rack::MockRequest.env_for( Rack::MockRequest.env_for(
...@@ -29,7 +31,14 @@ module MultipartHelpers ...@@ -29,7 +31,14 @@ module MultipartHelpers
raise ArgumentError, "can't handle #{mode} mode" raise ArgumentError, "can't handle #{mode} mode"
end end
result return result if ::Feature.disabled?(:upload_middleware_jwt_params_handler)
# the HandlerForJWTParams expects a jwt token with the upload parameters
# *without* the "#{key}." prefix
result.deep_transform_keys! { |k| k.remove("#{key}.") }
{
"#{key}.gitlab-workhorse-upload" => jwt_token('upload' => result)
}
end end
# This function assumes a `mode` variable to be set # This function assumes a `mode` variable to be set
......
...@@ -3,6 +3,8 @@ ...@@ -3,6 +3,8 @@
module WorkhorseHelpers module WorkhorseHelpers
extend self extend self
UPLOAD_PARAM_NAMES = %w[name size path remote_id sha256 type].freeze
def workhorse_send_data def workhorse_send_data
@_workhorse_send_data ||= begin @_workhorse_send_data ||= begin
header = response.headers[Gitlab::Workhorse::SEND_DATA_HEADER] header = response.headers[Gitlab::Workhorse::SEND_DATA_HEADER]
...@@ -59,6 +61,7 @@ module WorkhorseHelpers ...@@ -59,6 +61,7 @@ module WorkhorseHelpers
file = workhorse_params.delete(key) file = workhorse_params.delete(key)
rewritten_fields[key] = file.path if file rewritten_fields[key] = file.path if file
workhorse_params = workhorse_disk_accelerated_file_params(key, file).merge(workhorse_params) workhorse_params = workhorse_disk_accelerated_file_params(key, file).merge(workhorse_params)
workhorse_params = workhorse_params.merge(jwt_file_upload_param(key: key, params: workhorse_params))
end end
headers = if send_rewritten_field headers = if send_rewritten_field
...@@ -74,8 +77,19 @@ module WorkhorseHelpers ...@@ -74,8 +77,19 @@ module WorkhorseHelpers
private private
def jwt_token(data = {}) def jwt_file_upload_param(key:, params:)
JWT.encode({ 'iss' => 'gitlab-workhorse' }.merge(data), Gitlab::Workhorse.secret, 'HS256') upload_params = UPLOAD_PARAM_NAMES.map do |file_upload_param|
[file_upload_param, params["#{key}.#{file_upload_param}"]]
end
upload_params = upload_params.to_h.compact
return {} if upload_params.empty?
{ "#{key}.gitlab-workhorse-upload" => jwt_token('upload' => upload_params) }
end
def jwt_token(data = {}, issuer: 'gitlab-workhorse', secret: Gitlab::Workhorse.secret, algorithm: 'HS256')
JWT.encode({ 'iss' => issuer }.merge(data), secret, algorithm)
end end
def workhorse_rewritten_fields_header(fields) def workhorse_rewritten_fields_header(fields)
......
...@@ -2,6 +2,28 @@ ...@@ -2,6 +2,28 @@
RSpec.shared_examples 'handling file uploads' do |shared_examples_name| RSpec.shared_examples 'handling file uploads' do |shared_examples_name|
context 'with object storage disabled' do context 'with object storage disabled' do
context 'with upload_middleware_jwt_params_handler disabled' do
before do
stub_feature_flags(upload_middleware_jwt_params_handler: false)
expect_next_instance_of(Gitlab::Middleware::Multipart::Handler) do |handler|
expect(handler).to receive(:with_open_files).and_call_original
end
end
it_behaves_like shared_examples_name
end
context 'with upload_middleware_jwt_params_handler enabled' do
before do
stub_feature_flags(upload_middleware_jwt_params_handler: true)
expect_next_instance_of(Gitlab::Middleware::Multipart::HandlerForJWTParams) do |handler|
expect(handler).to receive(:with_open_files).and_call_original
end
end
it_behaves_like shared_examples_name it_behaves_like shared_examples_name
end end
end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment