Merge branch 'dblessing_unconfirmed_user_profile' into 'master'

Obfuscate user profile for unconfirmed users

See merge request gitlab-org/gitlab!48271

app/helpers/users_helper.rb

@@ -178,6 +178,15 @@ module UsersHelper
     header + list
   end
 
+  def user_display_name(user)
+    return s_('UserProfile|Blocked user') if user.blocked?
+
+    can_read_profile = can?(user, :read_user_profile, current_user)
+    return s_('UserProfile|Unconfirmed user') unless user.confirmed? || can_read_profile
+
+    user.name
+  end
+
   private
 
   def blocked_user_badge(user)

app/policies/user_policy.rb

@@ -13,6 +13,9 @@ class UserPolicy < BasePolicy
   desc "The user is blocked"
   condition(:blocked_user, scope: :subject, score: 0) { @subject.blocked? }
 
+  desc "The user is unconfirmed"
+  condition(:unconfirmed_user, scope: :subject, score: 0) { !@subject.confirmed? }
+
   rule { ~restricted_public_level }.enable :read_user
   rule { ~anonymous }.enable :read_user
 
@@ -25,7 +28,7 @@ class UserPolicy < BasePolicy
   end
 
   rule { default }.enable :read_user_profile
-  rule { (private_profile | blocked_user) & ~(user_is_self | admin) }.prevent :read_user_profile
+  rule { (private_profile | blocked_user | unconfirmed_user) & ~(user_is_self | admin) }.prevent :read_user_profile
   rule { user_is_self | admin }.enable :disable_two_factor
   rule { (user_is_self | admin) & ~blocked }.enable :create_user_personal_access_token
 end

app/views/users/show.html.haml

 - @hide_top_links = true
 - @hide_breadcrumbs = true
 - @no_container = true
-- page_title       @user.blocked? ? s_('UserProfile|Blocked user') : @user.name
+- page_title       user_display_name(@user)
 - page_description @user.bio_html
 - header_title     @user.name, user_path(@user)
 - page_itemtype    'http://schema.org/Person'
@@ -38,10 +38,10 @@
         = link_to avatar_icon_for_user(@user, 400), target: '_blank', rel: 'noopener noreferrer' do
           = image_tag avatar_icon_for_user(@user, 90), class: "avatar s90", alt: '', itemprop: 'image'
 
-      - if @user.blocked?
+      - if @user.blocked? || !@user.confirmed?
         .user-info
           .cover-title
-            = s_('UserProfile|Blocked user')
+            = user_display_name(@user)
         = render "users/profile_basic_info"
       - else
         .user-info

changelogs/unreleased/dblessing_unconfirmed_user_profile.yml

+---
+title: Obfuscate user profile for unconfirmed users
+merge_request: 48271
+author:
+type: added

locale/gitlab.pot

@@ -29835,6 +29835,9 @@ msgstr ""
 msgid "UserProfile|This user is blocked"
 msgstr ""
 
+msgid "UserProfile|Unconfirmed user"
+msgstr ""
+
 msgid "UserProfile|View all"
 msgstr ""
 

spec/features/users/show_spec.rb

@@ -7,7 +7,7 @@ RSpec.describe 'User page' do
 
   let_it_be(:user) { create(:user, bio: '**Lorem** _ipsum_ dolor sit [amet](https://example.com)') }
 
-  subject { visit(user_path(user)) }
+  subject(:visit_profile) { visit(user_path(user)) }
 
   context 'with public profile' do
     it 'shows all the tabs' do
@@ -123,6 +123,32 @@ RSpec.describe 'User page' do
     end
   end
 
+  context 'with unconfirmed user' do
+    let_it_be(:user) { create(:user, :unconfirmed) }
+
+    before do
+      visit_profile
+    end
+
+    it 'shows user name as unconfirmed' do
+      expect(page).to have_css(".cover-title", text: 'Unconfirmed user')
+    end
+
+    it 'shows no tab' do
+      expect(page).to have_css("div.profile-header")
+      expect(page).not_to have_css("ul.nav-links")
+    end
+
+    it 'shows no additional fields' do
+      expect(page).not_to have_css(".profile-user-bio")
+      expect(page).not_to have_css(".profile-link-holder")
+    end
+
+    it 'shows private profile message' do
+      expect(page).to have_content("This user has a private profile")
+    end
+  end
+
   it 'shows the status if there was one' do
     create(:user_status, user: user, message: "Working hard!")
 

spec/helpers/users_helper_spec.rb

@@ -272,4 +272,65 @@ RSpec.describe UsersHelper do
       end
     end
   end
+
+  describe '#user_display_name' do
+    subject { helper.user_display_name(user) }
+
+    before do
+      stub_current_user(nil)
+    end
+
+    context 'for a confirmed user' do
+      let(:user) { create(:user) }
+
+      before do
+        stub_profile_permission_allowed(true)
+      end
+
+      it { is_expected.to eq(user.name) }
+    end
+
+    context 'for an unconfirmed user' do
+      let(:user) { create(:user, :unconfirmed) }
+
+      before do
+        stub_profile_permission_allowed(false)
+      end
+
+      it { is_expected.to eq('Unconfirmed user') }
+
+      context 'when current user is an admin' do
+        before do
+          admin_user = create(:admin)
+          stub_current_user(admin_user)
+          stub_profile_permission_allowed(true, admin_user)
+        end
+
+        it { is_expected.to eq(user.name) }
+      end
+
+      context 'when the current user is self' do
+        before do
+          stub_current_user(user)
+          stub_profile_permission_allowed(true, user)
+        end
+
+        it { is_expected.to eq(user.name) }
+      end
+    end
+
+    context 'for a blocked user' do
+      let(:user) { create(:user, :blocked) }
+
+      it { is_expected.to eq('Blocked user') }
+    end
+
+    def stub_current_user(user)
+      allow(helper).to receive(:current_user).and_return(user)
+    end
+
+    def stub_profile_permission_allowed(allowed, current_user = nil)
+      allow(helper).to receive(:can?).with(user, :read_user_profile, current_user).and_return(allowed)
+    end
+  end
 end

spec/policies/user_policy_spec.rb

@@ -160,4 +160,16 @@ RSpec.describe UserPolicy do
       it { is_expected.not_to be_allowed(:read_group_count) }
     end
   end
+
+  describe ':read_user_profile' do
+    context 'when the user is unconfirmed' do
+      let(:user) { create(:user, :unconfirmed) }
+
+      it { is_expected.not_to be_allowed(:read_user_profile) }
+    end
+
+    context 'when the user is confirmed' do
+      it { is_expected.to be_allowed(:read_user_profile) }
+    end
+  end
 end