Commit a0eec2bd authored by Aleksei Lipniagov's avatar Aleksei Lipniagov Committed by Kamil Trzciński

Add image resizing FF based on avatar owner

parent 17162a49
...@@ -53,8 +53,10 @@ module SendFileUpload ...@@ -53,8 +53,10 @@ module SendFileUpload
private private
def image_scaling_request?(file_upload) def image_scaling_request?(file_upload)
Feature.enabled?(:dynamic_image_resizing, current_user) && avatar_safe_for_scaling?(file_upload) &&
avatar_safe_for_scaling?(file_upload) && valid_image_scaling_width? && current_user scaling_allowed_by_feature_flags?(file_upload) &&
valid_image_scaling_width? &&
current_user
end end
def avatar_safe_for_scaling?(file_upload) def avatar_safe_for_scaling?(file_upload)
...@@ -68,4 +70,17 @@ module SendFileUpload ...@@ -68,4 +70,17 @@ module SendFileUpload
def valid_image_scaling_width? def valid_image_scaling_width?
Avatarable::ALLOWED_IMAGE_SCALER_WIDTHS.include?(params[:width]&.to_i) Avatarable::ALLOWED_IMAGE_SCALER_WIDTHS.include?(params[:width]&.to_i)
end end
# We use two separate feature gates to allow image resizing.
# The first, `:dynamic_image_resizing_requester`, based on the content requester.
# Enabling it for the user would allow that user to send resizing requests for any avatar.
# The second, `:dynamic_image_resizing_owner`, based on the content owner.
# Enabling it for the user would allow anyone to send resizing requests against the mentioned user avatar only.
# This flag allows us to operate on trusted data only, more in https://gitlab.com/gitlab-org/gitlab/-/issues/241533.
# Because of this, you need to enable BOTH to serve resized image,
# as you would need at least one allowed requester and at least one allowed avatar.
def scaling_allowed_by_feature_flags?(file_upload)
Feature.enabled?(:dynamic_image_resizing_requester, current_user) &&
Feature.enabled?(:dynamic_image_resizing_owner, file_upload.model)
end
end end
---
name: dynamic_image_resizing_owner
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/40606
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/241533
group: group::memory
type: development
default_enabled: false
--- ---
name: dynamic_image_resizing name: dynamic_image_resizing_requester
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/37342 introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/37342
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/233704 rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/233704
group: group::memory group: group::memory
type: development type: development
default_enabled: false default_enabled: false
\ No newline at end of file
...@@ -49,12 +49,13 @@ RSpec.describe SendFileUpload do ...@@ -49,12 +49,13 @@ RSpec.describe SendFileUpload do
end end
shared_examples 'handles image resize requests' do shared_examples 'handles image resize requests' do
let(:headers) { double }
let(:image_requester) { build(:user) }
let(:image_owner) { build(:user) }
let(:params) do let(:params) do
{ attachment: 'avatar.png' } { attachment: 'avatar.png' }
end end
let(:headers) { double }
before do before do
allow(uploader).to receive(:image_safe_for_scaling?).and_return(true) allow(uploader).to receive(:image_safe_for_scaling?).and_return(true)
allow(uploader).to receive(:mounted_as).and_return(:avatar) allow(uploader).to receive(:mounted_as).and_return(:avatar)
...@@ -64,81 +65,110 @@ RSpec.describe SendFileUpload do ...@@ -64,81 +65,110 @@ RSpec.describe SendFileUpload do
# local or remote files # local or remote files
allow(controller).to receive(:send_file) allow(controller).to receive(:send_file)
allow(controller).to receive(:redirect_to) allow(controller).to receive(:redirect_to)
allow(controller).to receive(:current_user).and_return(image_requester)
allow(uploader).to receive(:model).and_return(image_owner)
end end
context 'when feature is enabled for current user' do context 'when boths FFs are enabled' do
let(:user) { build(:user) } before do
stub_feature_flags(dynamic_image_resizing_requester: image_requester)
stub_feature_flags(dynamic_image_resizing_owner: image_owner)
end
it_behaves_like 'handles image resize requests allowed by FFs'
end
context 'when only FF based on content requester is enabled for current user' do
before do before do
stub_feature_flags(dynamic_image_resizing: user) stub_feature_flags(dynamic_image_resizing_requester: image_requester)
allow(controller).to receive(:current_user).and_return(user) stub_feature_flags(dynamic_image_resizing_owner: false)
end end
context 'with valid width parameter' do it_behaves_like 'bypasses image resize requests not allowed by FFs'
it 'renders OK with workhorse command header' do end
expect(controller).not_to receive(:send_file)
expect(controller).to receive(:params).at_least(:once).and_return(width: '64') context 'when only FF based on content owner is enabled for requested avatar owner' do
expect(controller).to receive(:head).with(:ok) before do
expect(Gitlab::Workhorse).to receive(:send_scaled_image).with(a_string_matching('^(/.+|https://.+)'), 64, 'image/png').and_return([ stub_feature_flags(dynamic_image_resizing_requester: false)
Gitlab::Workhorse::SEND_DATA_HEADER, "send-scaled-img:faux" stub_feature_flags(dynamic_image_resizing_owner: image_owner)
])
expect(headers).to receive(:store).with(Gitlab::Workhorse::SEND_DATA_HEADER, "send-scaled-img:faux")
subject
end
end end
context 'with missing width parameter' do it_behaves_like 'bypasses image resize requests not allowed by FFs'
it 'does not write workhorse command header' do end
expect(headers).not_to receive(:store).with(Gitlab::Workhorse::SEND_DATA_HEADER, /^send-scaled-img:/)
subject context 'when both FFs are disabled' do
end before do
stub_feature_flags(dynamic_image_resizing_requester: false)
stub_feature_flags(dynamic_image_resizing_owner: false)
end end
context 'with invalid width parameter' do it_behaves_like 'bypasses image resize requests not allowed by FFs'
it 'does not write workhorse command header' do end
expect(controller).to receive(:params).at_least(:once).and_return(width: 'not a number') end
expect(headers).not_to receive(:store).with(Gitlab::Workhorse::SEND_DATA_HEADER, /^send-scaled-img:/)
shared_examples 'bypasses image resize requests not allowed by FFs' do
it 'does not write workhorse command header' do
expect(headers).not_to receive(:store).with(Gitlab::Workhorse::SEND_DATA_HEADER, /^send-scaled-img:/)
subject subject
end end
end
shared_examples 'handles image resize requests allowed by FFs' do
context 'with valid width parameter' do
it 'renders OK with workhorse command header' do
expect(controller).not_to receive(:send_file)
expect(controller).to receive(:params).at_least(:once).and_return(width: '64')
expect(controller).to receive(:head).with(:ok)
expect(Gitlab::Workhorse).to receive(:send_scaled_image).with(a_string_matching('^(/.+|https://.+)'), 64, 'image/png').and_return([
Gitlab::Workhorse::SEND_DATA_HEADER, "send-scaled-img:faux"
])
expect(headers).to receive(:store).with(Gitlab::Workhorse::SEND_DATA_HEADER, "send-scaled-img:faux")
subject
end end
end
context 'with width that is not allowed' do context 'with missing width parameter' do
it 'does not write workhorse command header' do it 'does not write workhorse command header' do
expect(controller).to receive(:params).at_least(:once).and_return(width: '63') expect(headers).not_to receive(:store).with(Gitlab::Workhorse::SEND_DATA_HEADER, /^send-scaled-img:/)
expect(headers).not_to receive(:store).with(Gitlab::Workhorse::SEND_DATA_HEADER, /^send-scaled-img:/)
subject subject
end
end end
end
context 'when image file is not an avatar' do context 'with invalid width parameter' do
it 'does not write workhorse command header' do it 'does not write workhorse command header' do
expect(uploader).to receive(:mounted_as).and_return(nil) # FileUploader is not mounted expect(controller).to receive(:params).at_least(:once).and_return(width: 'not a number')
expect(headers).not_to receive(:store).with(Gitlab::Workhorse::SEND_DATA_HEADER, /^send-scaled-img:/) expect(headers).not_to receive(:store).with(Gitlab::Workhorse::SEND_DATA_HEADER, /^send-scaled-img:/)
subject subject
end
end end
end
context 'when image file type is not considered safe for scaling' do context 'with width that is not allowed' do
it 'does not write workhorse command header' do it 'does not write workhorse command header' do
expect(uploader).to receive(:image_safe_for_scaling?).and_return(false) expect(controller).to receive(:params).at_least(:once).and_return(width: '63')
expect(headers).not_to receive(:store).with(Gitlab::Workhorse::SEND_DATA_HEADER, /^send-scaled-img:/) expect(headers).not_to receive(:store).with(Gitlab::Workhorse::SEND_DATA_HEADER, /^send-scaled-img:/)
subject subject
end
end end
end end
context 'when feature is disabled' do context 'when image file is not an avatar' do
before do it 'does not write workhorse command header' do
stub_feature_flags(dynamic_image_resizing: false) expect(uploader).to receive(:mounted_as).and_return(nil) # FileUploader is not mounted
expect(headers).not_to receive(:store).with(Gitlab::Workhorse::SEND_DATA_HEADER, /^send-scaled-img:/)
subject
end end
end
context 'when image file type is not considered safe for scaling' do
it 'does not write workhorse command header' do it 'does not write workhorse command header' do
expect(uploader).to receive(:image_safe_for_scaling?).and_return(false)
expect(headers).not_to receive(:store).with(Gitlab::Workhorse::SEND_DATA_HEADER, /^send-scaled-img:/) expect(headers).not_to receive(:store).with(Gitlab::Workhorse::SEND_DATA_HEADER, /^send-scaled-img:/)
subject subject
......
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