Skip to content

Hide user avatar for blocked and unconfirmed users 2.0

Drew Blessingrequested to merge
dblessing_fix_avatar_for_blocked_users into master

What does this MR do and why?

Follow-up to #341325 (closed) and !75032 (merged) (merged). Also, a reimplementation of !75921 (merged) due to production incident gitlab-com/gl-infra/production#6083 (closed)

We should mask the user avatar for blocked or unconfirmed users to avoid it being used for spam. You can see in screenshots below this also masks the Gravatar for a user, so they can't even show spam via that external service when they're blocked in GitLab.

Admin users are able to see the user avatar regardless of status but only on the user's personal page and in the admin user view.

The problem caused by the previous implementation was due to certain uses of the avatar_icon_for_user method by areas of GitLab that don't have access to the warden current_user method - namely in presenters.

Edit: The latest change now explicitly passes in a current_user when requested. If that user can admin all resources, the user will be able to see the avatar despite a user being unconfirmed or unblocked. This is currently only the case for the user profile and admin area views.

NOTE: This catches most but not all places a user avatar is displayed. Some locations may still show a user avatar even when they're blocked - some popups, commit boxes, etc. But the major one we wanted to catch was the profile page.

Screenshots or screen recordings

Before

Screen_Shot_2021-12-03_at_10.31.47_AM

After

Screen_Shot_2021-12-03_at_10.24.48_AM

Screenshots or screen recordings

These are strongly recommended to assist reviewers and reduce the time to merge your change.

How to set up and validate locally

  1. Create a test project
  2. Create one or more commits as a regular user
  3. As an admin, block the user that committed above.
  4. Navigate to the project previously created and view a file in the repository that was part of the commit.
  5. 'Blame' the file and observe there's no error. This step will fail if you test with !75921 (merged)

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Drew Blessing

Merge request reports