Skip to content
Snippets Groups Projects

Hide user avatar for blocked and unconfirmed users

Merged Drew Blessing requested to merge dblessing_avatar_for_blocked_users into master
1 unresolved thread

What does this MR do and why?

Follow-up to #341325 (closed) and !75032 (merged) (merged).

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 always able to see the user avatar regardless of status.

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

How to set up and validate locally

Numbered steps to set up and validate the change are strongly suggested.

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

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Albert
  • Albert approved this merge request

    approved this merge request

  • @dblessing I just have 2 minor suggestions. Otherwise looks good to me.

    Feel free to pass to maintainer after addressing them.

  • Albert removed review request for @alberts-gitlab

    removed review request for @alberts-gitlab

  • :wave: @alberts-gitlab, thanks for approving this merge request.

    This is the first time the merge request is approved. To ensure full test coverage, a new pipeline has been started.

    For more info, please refer to the following links:

  • Andrejs Cunskis approved this merge request

    approved this merge request

  • Andrejs Cunskis removed review request for @acunskis

    removed review request for @acunskis

  • Drew Blessing added 1 commit

    added 1 commit

    • 6c995198 - Hide user avatar for blocked and unconfirmed users

    Compare with previous version

  • Drew Blessing requested review from @smcgivern

    requested review from @smcgivern

  • Drew Blessing added 1 commit

    added 1 commit

    • f2a83cd8 - Hide user avatar for blocked and unconfirmed users

    Compare with previous version

  • Sean McGivern
  • Sean McGivern resolved all threads

    resolved all threads

  • @dblessing @alberts-gitlab thanks, this looks good to me. I'll merge once #347498 (closed) is fixed.

  • Sean McGivern approved this merge request

    approved this merge request

  • Sean McGivern enabled an automatic merge when the pipeline for 661aedf4 succeeds

    enabled an automatic merge when the pipeline for 661aedf4 succeeds

  • merged

  • Sean McGivern mentioned in commit 8e8e9cb6

    mentioned in commit 8e8e9cb6

  • Patrick Bair mentioned in merge request !76888 (merged)

    mentioned in merge request !76888 (merged)

  • added workflowcanary label and removed workflowstaging label

  • added workflowproduction label and removed workflowcanary label

  • Sean McGivern mentioned in commit ee53b29d

    mentioned in commit ee53b29d

  • Sean McGivern mentioned in merge request !76935 (merged)

    mentioned in merge request !76935 (merged)

  • mentioned in issue #348698 (closed)

  • :wave: @dblessing can you please take a look at this bug #348698 (closed)? It appears this change introduced 500 error in some Blame requests

  • Drew Blessing mentioned in merge request !77250 (merged)

    mentioned in merge request !77250 (merged)

    • Late to the party... but has it been considered to make this configurable? :thinking: It's a bit sad to just see the grayed-out avatar for a person who has left the company (and therefore is blocked in our GitLab) when looking at list of commits for older branches in our repositories. To be frank, it's a bit disturbing to look at actually since it makes their commits look very different than all others, for little reason.

      I can get the "spam" part though, particularly with Gravatar, but it would be nice to not force this hiding of avatars for all users in all installations unconditionally. I didn't know about this "feature" so I thought the problem was that the person's avatar was missing in GitLab... updated it, still didn't work, so I thought it was a caching issue.

      (Is the problem that we shouldn't block users who have left the company, but disable their accounts in some other way? We use LDAP for authentication.)

      Edited by Per Lundberg
    • Author Maintainer

      @perlun Sure, we can consider it. Can you please create an issue? Then we'll send it to the groupanti-abuse group for consideration.

    • Thanks. Issue created, #424093. Let's continue there @dblessing. I added the groupanti-abuse label to it but feel free to relabel/edit as you please.

    • Please register or sign in to reply
  • Per Lundberg mentioned in issue #424093

    mentioned in issue #424093

  • Please register or sign in to reply
    Loading