From f2a83cd8e4bf2324b81ad517b229a3d63fd55faa Mon Sep 17 00:00:00 2001 From: Drew Blessing <drew@gitlab.com> Date: Fri, 3 Dec 2021 10:25:33 -0600 Subject: [PATCH] Hide user avatar for blocked and unconfirmed users Avatars can sometimes be used to display spam or other undesirable content for unconfirmed or blocked users. This change hides the user's custom avatar in these cases until they become unblocked or confirmed. Changelog: changed --- app/helpers/avatars_helper.rb | 24 ++++++++++++----- spec/features/users/show_spec.rb | 8 ++++++ spec/helpers/avatars_helper_spec.rb | 42 +++++++++++++++++++++++++++-- 3 files changed, 66 insertions(+), 8 deletions(-) diff --git a/app/helpers/avatars_helper.rb b/app/helpers/avatars_helper.rb index dd852a68682dff72..af9b8a0124843e42 100644 --- a/app/helpers/avatars_helper.rb +++ b/app/helpers/avatars_helper.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true module AvatarsHelper + DEFAULT_AVATAR_PATH = 'no_avatar.png' + def project_icon(project, options = {}) source_icon(project, options) end @@ -34,11 +36,11 @@ def avatar_icon_for_email(email = nil, size = nil, scale = 2, only_path: true) end def avatar_icon_for_user(user = nil, size = nil, scale = 2, only_path: true) - if user - user.avatar_url(size: size, only_path: only_path) || default_avatar - else - gravatar_icon(nil, size, scale) - end + return gravatar_icon(nil, size, scale) unless user + return default_avatar if blocked_or_unconfirmed?(user) && !can_admin?(current_user) + + user_avatar = user.avatar_url(size: size, only_path: only_path) + user_avatar || default_avatar end def gravatar_icon(user_email = '', size = nil, scale = 2) @@ -47,7 +49,7 @@ def gravatar_icon(user_email = '', size = nil, scale = 2) end def default_avatar - ActionController::Base.helpers.image_path('no_avatar.png') + ActionController::Base.helpers.image_path(DEFAULT_AVATAR_PATH) end def author_avatar(commit_or_event, options = {}) @@ -157,4 +159,14 @@ def source_identicon(source, options = {}) source.name[0, 1].upcase end end + + def blocked_or_unconfirmed?(user) + user.blocked? || !user.confirmed? + end + + def can_admin?(user) + return false unless user + + user.can_admin_all_resources? + end end diff --git a/spec/features/users/show_spec.rb b/spec/features/users/show_spec.rb index 8edbf639c81bda7c..4d984e15b41a96dd 100644 --- a/spec/features/users/show_spec.rb +++ b/spec/features/users/show_spec.rb @@ -243,6 +243,10 @@ expect(page).to have_content("@#{user.username}") end + it 'shows default avatar' do + expect(page).to have_css('//img[data-src^="/assets/no_avatar"]') + end + it_behaves_like 'default brand title page meta description' end @@ -286,6 +290,10 @@ expect(page).to have_content("This user has a private profile") end + it 'shows default avatar' do + expect(page).to have_css('//img[data-src^="/assets/no_avatar"]') + end + it_behaves_like 'default brand title page meta description' end diff --git a/spec/helpers/avatars_helper_spec.rb b/spec/helpers/avatars_helper_spec.rb index 7190f2fcd4afc651..f1d9e13c0762f423 100644 --- a/spec/helpers/avatars_helper_spec.rb +++ b/spec/helpers/avatars_helper_spec.rb @@ -4,6 +4,7 @@ RSpec.describe AvatarsHelper do include UploadHelpers + include Devise::Test::ControllerHelpers let_it_be(:user) { create(:user) } @@ -145,12 +146,49 @@ describe '#avatar_icon_for_user' do let(:user) { create(:user, avatar: File.open(uploaded_image_temp_path)) } + let(:helper_args) { [user] } + + shared_examples 'blocked or unconfirmed user with avatar' do + it 'returns the default avatar' do + expect(helper.avatar_icon_for_user(user).to_s) + .to match_asset_path(described_class::DEFAULT_AVATAR_PATH) + end + + context 'when the current user is an admin', :enable_admin_mode do + let(:current_user) { create(:user, :admin) } + + before do + allow(helper).to receive(:current_user).and_return(current_user) + end + + it 'returns the user avatar' do + expect(helper.avatar_icon_for_user(user).to_s) + .to eq(user.avatar.url) + end + end + end context 'with a user object passed' do it 'returns a relative URL for the avatar' do expect(helper.avatar_icon_for_user(user).to_s) .to eq(user.avatar.url) end + + context 'when the user is blocked' do + before do + user.block! + end + + it_behaves_like 'blocked or unconfirmed user with avatar' + end + + context 'when the user is unconfirmed' do + before do + user.update!(confirmed_at: nil) + end + + it_behaves_like 'blocked or unconfirmed user with avatar' + end end context 'without a user object passed' do @@ -171,7 +209,7 @@ end it 'returns a generic avatar' do - expect(helper.gravatar_icon(user_email)).to match_asset_path('no_avatar.png') + expect(helper.gravatar_icon(user_email)).to match_asset_path(described_class::DEFAULT_AVATAR_PATH) end end @@ -181,7 +219,7 @@ end it 'returns a generic avatar when email is blank' do - expect(helper.gravatar_icon('')).to match_asset_path('no_avatar.png') + expect(helper.gravatar_icon('')).to match_asset_path(described_class::DEFAULT_AVATAR_PATH) end it 'returns a valid Gravatar URL' do -- GitLab