summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGitLab Bot <gitlab-bot@gitlab.com>2023-05-01 12:13:04 +0000
committerGitLab Bot <gitlab-bot@gitlab.com>2023-05-01 12:13:30 +0000
commit2655540094e856f3048fb737a19e4316d8264623 (patch)
treea6ca9a421ac08008dc5277ba701ff1906f03be3b
parent53005cb586fcec5d4e7737afb129ca7a86fb954b (diff)
downloadgitlab-ce-2655540094e856f3048fb737a19e4316d8264623.tar.gz
Add latest changes from gitlab-org/security/gitlab@15-10-stable-ee
-rw-r--r--app/helpers/avatars_helper.rb2
-rw-r--r--lib/banzai/filter/asset_proxy_filter.rb44
-rw-r--r--spec/helpers/avatars_helper_spec.rb16
-rw-r--r--spec/lib/banzai/filter/asset_proxy_filter_spec.rb9
-rw-r--r--spec/lib/banzai/filter/commit_trailers_filter_spec.rb2
5 files changed, 66 insertions, 7 deletions
diff --git a/app/helpers/avatars_helper.rb b/app/helpers/avatars_helper.rb
index 0fac2cb5fc5..57075a44d0f 100644
--- a/app/helpers/avatars_helper.rb
+++ b/app/helpers/avatars_helper.rb
@@ -116,7 +116,7 @@ module AvatarsHelper
private
def avatar_icon_by_user_email_or_gravatar(email, size, scale, only_path:)
- user = User.find_by_any_email(email)
+ user = User.with_public_email(email).first
if user
avatar_icon_for_user(user, size, scale, only_path: only_path)
diff --git a/lib/banzai/filter/asset_proxy_filter.rb b/lib/banzai/filter/asset_proxy_filter.rb
index 4c14ee7299b..6371a8f23af 100644
--- a/lib/banzai/filter/asset_proxy_filter.rb
+++ b/lib/banzai/filter/asset_proxy_filter.rb
@@ -6,11 +6,35 @@ module Banzai
# as well as hiding the customer's IP address when requesting images.
# Copies the original img `src` to `data-canonical-src` then replaces the
# `src` with a new url to the proxy server.
- class AssetProxyFilter < HTML::Pipeline::CamoFilter
+ #
+ # Based on https://github.com/gjtorikian/html-pipeline/blob/v2.14.3/lib/html/pipeline/camo_filter.rb
+ class AssetProxyFilter < HTML::Pipeline::Filter
def initialize(text, context = nil, result = nil)
super
end
+ def call
+ return doc unless asset_proxy_enabled?
+
+ doc.search('img').each do |element|
+ original_src = element['src']
+ next unless original_src
+
+ begin
+ uri = URI.parse(original_src)
+ rescue StandardError
+ next
+ end
+
+ next if uri.host.nil? && !original_src.start_with?('///')
+ next if asset_host_allowed?(uri.host)
+
+ element['src'] = asset_proxy_url(original_src)
+ element['data-canonical-src'] = original_src
+ end
+ doc
+ end
+
def validate
needs(:asset_proxy, :asset_proxy_secret_key) if asset_proxy_enabled?
end
@@ -63,6 +87,24 @@ module Banzai
application_settings.try(:asset_proxy_whitelist).presence ||
[Gitlab.config.gitlab.host]
end
+
+ private
+
+ def asset_proxy_enabled?
+ !context[:disable_asset_proxy]
+ end
+
+ def asset_proxy_url(url)
+ "#{context[:asset_proxy]}/#{asset_url_hash(url)}/#{hexencode(url)}"
+ end
+
+ def asset_url_hash(url)
+ OpenSSL::HMAC.hexdigest('sha1', context[:asset_proxy_secret_key], url)
+ end
+
+ def hexencode(str)
+ str.unpack1('H*')
+ end
end
end
end
diff --git a/spec/helpers/avatars_helper_spec.rb b/spec/helpers/avatars_helper_spec.rb
index cef72d24c43..bf23c74c0f0 100644
--- a/spec/helpers/avatars_helper_spec.rb
+++ b/spec/helpers/avatars_helper_spec.rb
@@ -2,7 +2,7 @@
require 'spec_helper'
-RSpec.describe AvatarsHelper do
+RSpec.describe AvatarsHelper, feature_category: :source_code_management do
include UploadHelpers
let_it_be(:user) { create(:user) }
@@ -88,7 +88,7 @@ RSpec.describe AvatarsHelper do
describe '#avatar_icon_for' do
let!(:user) { create(:user, avatar: File.open(uploaded_image_temp_path), email: 'bar@example.com') }
let(:email) { 'foo@example.com' }
- let!(:another_user) { create(:user, avatar: File.open(uploaded_image_temp_path), email: email) }
+ let!(:another_user) { create(:user, :public_email, avatar: File.open(uploaded_image_temp_path), email: email) }
it 'prefers the user to retrieve the avatar_url' do
expect(helper.avatar_icon_for(user, email).to_s)
@@ -102,7 +102,7 @@ RSpec.describe AvatarsHelper do
end
describe '#avatar_icon_for_email', :clean_gitlab_redis_cache do
- let(:user) { create(:user, avatar: File.open(uploaded_image_temp_path)) }
+ let(:user) { create(:user, :public_email, avatar: File.open(uploaded_image_temp_path)) }
subject { helper.avatar_icon_for_email(user.email).to_s }
@@ -114,6 +114,14 @@ RSpec.describe AvatarsHelper do
end
end
+ context 'when a private email is used' do
+ it 'calls gravatar_icon' do
+ expect(helper).to receive(:gravatar_icon).with(user.commit_email, 20, 2)
+
+ helper.avatar_icon_for_email(user.commit_email, 20, 2)
+ end
+ end
+
context 'when no user exists for the email' do
it 'calls gravatar_icon' do
expect(helper).to receive(:gravatar_icon).with('foo@example.com', 20, 2)
@@ -136,7 +144,7 @@ RSpec.describe AvatarsHelper do
it_behaves_like "returns avatar for email"
it "caches the request" do
- expect(User).to receive(:find_by_any_email).once.and_call_original
+ expect(User).to receive(:with_public_email).once.and_call_original
expect(helper.avatar_icon_for_email(user.email).to_s).to eq(user.avatar.url)
expect(helper.avatar_icon_for_email(user.email).to_s).to eq(user.avatar.url)
diff --git a/spec/lib/banzai/filter/asset_proxy_filter_spec.rb b/spec/lib/banzai/filter/asset_proxy_filter_spec.rb
index 004c70c28f1..dc6ac52a8c2 100644
--- a/spec/lib/banzai/filter/asset_proxy_filter_spec.rb
+++ b/spec/lib/banzai/filter/asset_proxy_filter_spec.rb
@@ -80,6 +80,15 @@ RSpec.describe Banzai::Filter::AssetProxyFilter, feature_category: :team_plannin
expect(doc.at_css('img')['data-canonical-src']).to eq src
end
+ it 'replaces invalid URLs' do
+ src = '///example.com/test.png'
+ new_src = 'https://assets.example.com/3368d2c7b9bed775bdd1e811f36a4b80a0dcd8ab/2f2f2f6578616d706c652e636f6d2f746573742e706e67'
+ doc = filter(image(src), @context)
+
+ expect(doc.at_css('img')['src']).to eq new_src
+ expect(doc.at_css('img')['data-canonical-src']).to eq src
+ end
+
it 'skips internal images' do
src = "#{Gitlab.config.gitlab.url}/test.png"
doc = filter(image(src), @context)
diff --git a/spec/lib/banzai/filter/commit_trailers_filter_spec.rb b/spec/lib/banzai/filter/commit_trailers_filter_spec.rb
index 3ebe0798972..896f3beb7c2 100644
--- a/spec/lib/banzai/filter/commit_trailers_filter_spec.rb
+++ b/spec/lib/banzai/filter/commit_trailers_filter_spec.rb
@@ -218,7 +218,7 @@ RSpec.describe Banzai::Filter::CommitTrailersFilter, feature_category: :source_c
# any path-only link will automatically be prefixed
# with the path of its repository.
# See: "build_relative_path" in "lib/banzai/filter/relative_link_filter.rb"
- let(:user_with_avatar) { create(:user, :with_avatar, username: 'foobar') }
+ let(:user_with_avatar) { create(:user, :public_email, :with_avatar, username: 'foobar') }
it 'returns a full path for avatar urls' do
_, message_html = build_commit_message(