Welcome to mirror list, hosted at ThFree Co, Russian Federation.

gitlab.com/gitlab-org/gitlab-foss.git - Unnamed repository; edit this file 'description' to name the repository.
summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--app/assets/javascripts/members/components/avatars/user_avatar.vue8
-rw-r--r--app/assets/javascripts/members/utils.js4
-rw-r--r--app/helpers/projects/project_members_helper.rb4
-rw-r--r--changelogs/unreleased/24908-user-2fa-status-should-not-be-publicly-shown.yml5
-rw-r--r--doc/user/permissions.md2
-rw-r--r--spec/features/groups/members/list_members_spec.rb42
-rw-r--r--spec/features/projects/members/list_spec.rb44
-rw-r--r--spec/frontend/members/components/avatars/user_avatar_spec.js26
-rw-r--r--spec/frontend/members/mock_data.js2
-rw-r--r--spec/frontend/members/utils_spec.js37
-rw-r--r--spec/helpers/projects/project_members_helper_spec.rb4
11 files changed, 159 insertions, 19 deletions
diff --git a/app/assets/javascripts/members/components/avatars/user_avatar.vue b/app/assets/javascripts/members/components/avatars/user_avatar.vue
index 79dda3c1379..658fb43cecb 100644
--- a/app/assets/javascripts/members/components/avatars/user_avatar.vue
+++ b/app/assets/javascripts/members/components/avatars/user_avatar.vue
@@ -5,6 +5,7 @@ import {
GlBadge,
GlSafeHtmlDirective as SafeHtml,
} from '@gitlab/ui';
+import { mapState } from 'vuex';
import { generateBadges } from 'ee_else_ce/members/utils';
import { glEmojiTag } from '~/emoji';
import { __ } from '~/locale';
@@ -34,11 +35,16 @@ export default {
},
},
computed: {
+ ...mapState(['canManageMembers']),
user() {
return this.member.user;
},
badges() {
- return generateBadges(this.member, this.isCurrentUser).filter((badge) => badge.show);
+ return generateBadges({
+ member: this.member,
+ isCurrentUser: this.isCurrentUser,
+ canManageMembers: this.canManageMembers,
+ }).filter((badge) => badge.show);
},
statusEmoji() {
return this.user?.status?.emoji;
diff --git a/app/assets/javascripts/members/utils.js b/app/assets/javascripts/members/utils.js
index 4de2dadb490..2bf30dd7b6e 100644
--- a/app/assets/javascripts/members/utils.js
+++ b/app/assets/javascripts/members/utils.js
@@ -13,7 +13,7 @@ import {
GROUP_LINK_ACCESS_LEVEL_PROPERTY_NAME,
} from './constants';
-export const generateBadges = (member, isCurrentUser) => [
+export const generateBadges = ({ member, isCurrentUser, canManageMembers }) => [
{
show: isCurrentUser,
text: __("It's you"),
@@ -25,7 +25,7 @@ export const generateBadges = (member, isCurrentUser) => [
variant: 'danger',
},
{
- show: member.user?.twoFactorEnabled,
+ show: member.user?.twoFactorEnabled && (canManageMembers || isCurrentUser),
text: __('2FA'),
variant: 'info',
},
diff --git a/app/helpers/projects/project_members_helper.rb b/app/helpers/projects/project_members_helper.rb
index 99c1b742da4..662afbcfd25 100644
--- a/app/helpers/projects/project_members_helper.rb
+++ b/app/helpers/projects/project_members_helper.rb
@@ -40,7 +40,7 @@ module Projects::ProjectMembersHelper
members: project_members_data_json(project, members),
member_path: project_project_member_path(project, ':id'),
source_id: project.id,
- can_manage_members: can_manage_project_members?(project)
+ can_manage_members: can_manage_project_members?(project).to_s
}
end
@@ -49,7 +49,7 @@ module Projects::ProjectMembersHelper
members: project_group_links_data_json(group_links),
member_path: project_group_link_path(project, ':id'),
source_id: project.id,
- can_manage_members: can_manage_project_members?(project)
+ can_manage_members: can_manage_project_members?(project).to_s
}
end
end
diff --git a/changelogs/unreleased/24908-user-2fa-status-should-not-be-publicly-shown.yml b/changelogs/unreleased/24908-user-2fa-status-should-not-be-publicly-shown.yml
new file mode 100644
index 00000000000..b47577d3e69
--- /dev/null
+++ b/changelogs/unreleased/24908-user-2fa-status-should-not-be-publicly-shown.yml
@@ -0,0 +1,5 @@
+---
+title: Only show 2FA badge to project maintainers and group owners
+merge_request: 54646
+author:
+type: changed
diff --git a/doc/user/permissions.md b/doc/user/permissions.md
index 67b18340486..6a05d355fcc 100644
--- a/doc/user/permissions.md
+++ b/doc/user/permissions.md
@@ -173,6 +173,7 @@ The following table depicts the various user permission levels in a project.
| View project Audit Events | | | ✓ (*12*) | ✓ | ✓ |
| Manage [push rules](../push_rules/push_rules.md) | | | | ✓ | ✓ |
| Manage [project access tokens](project/settings/project_access_tokens.md) **(FREE SELF)** | | | | ✓ | ✓ |
+| View 2FA status of members | | | | ✓ | ✓ |
| Switch visibility level | | | | | ✓ |
| Transfer project to another namespace | | | | | ✓ |
| Rename project | | | | | ✓ |
@@ -293,6 +294,7 @@ group.
| View Value Stream analytics | ✓ | ✓ | ✓ | ✓ | ✓ |
| View Billing **(FREE SAAS)** | | | | | ✓ (4) |
| View Usage Quotas **(FREE SAAS)** | | | | | ✓ (4) |
+| View 2FA status of members | | | | | ✓ |
| Filter members by 2FA status | | | | | ✓ |
| Administer project compliance frameworks | | | | | ✓ |
diff --git a/spec/features/groups/members/list_members_spec.rb b/spec/features/groups/members/list_members_spec.rb
index b0a896ec8cb..b81949da85d 100644
--- a/spec/features/groups/members/list_members_spec.rb
+++ b/spec/features/groups/members/list_members_spec.rb
@@ -47,4 +47,46 @@ RSpec.describe 'Groups > Members > List members', :js do
expect(first_row).to have_selector('gl-emoji[data-name="smirk"]')
end
end
+
+ describe 'when user has 2FA enabled' do
+ let_it_be(:admin) { create(:admin) }
+ let_it_be(:user_with_2fa) { create(:user, :two_factor_via_otp) }
+
+ before do
+ group.add_guest(user_with_2fa)
+ end
+
+ it 'shows 2FA badge to user with "Owner" access level' do
+ group.add_owner(user1)
+
+ visit group_group_members_path(group)
+
+ expect(find_member_row(user_with_2fa)).to have_content('2FA')
+ end
+
+ it 'shows 2FA badge to admins' do
+ sign_in(admin)
+ gitlab_enable_admin_mode_sign_in(admin)
+
+ visit group_group_members_path(group)
+
+ expect(find_member_row(user_with_2fa)).to have_content('2FA')
+ end
+
+ it 'does not show 2FA badge to users with access level below "Owner"' do
+ group.add_maintainer(user1)
+
+ visit group_group_members_path(group)
+
+ expect(find_member_row(user_with_2fa)).not_to have_content('2FA')
+ end
+
+ it 'shows 2FA badge to themselves' do
+ sign_in(user_with_2fa)
+
+ visit group_group_members_path(group)
+
+ expect(find_member_row(user_with_2fa)).to have_content('2FA')
+ end
+ end
end
diff --git a/spec/features/projects/members/list_spec.rb b/spec/features/projects/members/list_spec.rb
index b0fe5b9c48a..7cace8e08dd 100644
--- a/spec/features/projects/members/list_spec.rb
+++ b/spec/features/projects/members/list_spec.rb
@@ -8,7 +8,7 @@ RSpec.describe 'Project members list' do
let(:user1) { create(:user, name: 'John Doe') }
let(:user2) { create(:user, name: 'Mary Jane') }
let(:group) { create(:group) }
- let(:project) { create(:project, namespace: group) }
+ let(:project) { create(:project, :internal, namespace: group) }
before do
stub_feature_flags(invite_members_group_modal: false)
@@ -117,6 +117,48 @@ RSpec.describe 'Project members list' do
end
end
end
+
+ describe 'when user has 2FA enabled' do
+ let_it_be(:admin) { create(:admin) }
+ let_it_be(:user_with_2fa) { create(:user, :two_factor_via_otp) }
+
+ before do
+ project.add_guest(user_with_2fa)
+ end
+
+ it 'shows 2FA badge to user with "Maintainer" access level' do
+ project.add_maintainer(user1)
+
+ visit_members_page
+
+ expect(find_member_row(user_with_2fa)).to have_content('2FA')
+ end
+
+ it 'shows 2FA badge to admins' do
+ sign_in(admin)
+ gitlab_enable_admin_mode_sign_in(admin)
+
+ visit_members_page
+
+ expect(find_member_row(user_with_2fa)).to have_content('2FA')
+ end
+
+ it 'does not show 2FA badge to users with access level below "Maintainer"' do
+ group.add_developer(user1)
+
+ visit_members_page
+
+ expect(find_member_row(user_with_2fa)).not_to have_content('2FA')
+ end
+
+ it 'shows 2FA badge to themselves' do
+ sign_in(user_with_2fa)
+
+ visit_members_page
+
+ expect(find_member_row(user_with_2fa)).to have_content('2FA')
+ end
+ end
end
context 'when `vue_project_members_list` feature flag is disabled' do
diff --git a/spec/frontend/members/components/avatars/user_avatar_spec.js b/spec/frontend/members/components/avatars/user_avatar_spec.js
index 303c82582a3..3f4d9155c5d 100644
--- a/spec/frontend/members/components/avatars/user_avatar_spec.js
+++ b/spec/frontend/members/components/avatars/user_avatar_spec.js
@@ -1,21 +1,31 @@
import { GlAvatarLink, GlBadge } from '@gitlab/ui';
import { within } from '@testing-library/dom';
import { mount, createWrapper } from '@vue/test-utils';
+import Vue from 'vue';
+import Vuex from 'vuex';
import UserAvatar from '~/members/components/avatars/user_avatar.vue';
-import { member as memberMock, orphanedMember } from '../../mock_data';
+import { member as memberMock, member2faEnabled, orphanedMember } from '../../mock_data';
+
+Vue.use(Vuex);
describe('UserAvatar', () => {
let wrapper;
const { user } = memberMock;
- const createComponent = (propsData = {}) => {
+ const createComponent = (propsData = {}, state = {}) => {
wrapper = mount(UserAvatar, {
propsData: {
member: memberMock,
isCurrentUser: false,
...propsData,
},
+ store: new Vuex.Store({
+ state: {
+ canManageMembers: true,
+ ...state,
+ },
+ }),
});
};
@@ -69,9 +79,9 @@ describe('UserAvatar', () => {
describe('badges', () => {
it.each`
- member | badgeText
- ${{ ...memberMock, user: { ...memberMock.user, blocked: true } }} | ${'Blocked'}
- ${{ ...memberMock, user: { ...memberMock.user, twoFactorEnabled: true } }} | ${'2FA'}
+ member | badgeText
+ ${{ ...memberMock, user: { ...memberMock.user, blocked: true } }} | ${'Blocked'}
+ ${member2faEnabled} | ${'2FA'}
`('renders the "$badgeText" badge', ({ member, badgeText }) => {
createComponent({ member });
@@ -83,6 +93,12 @@ describe('UserAvatar', () => {
expect(getByText("It's you").exists()).toBe(true);
});
+
+ it('does not render 2FA badge when `canManageMembers` is `false`', () => {
+ createComponent({ member: member2faEnabled }, { canManageMembers: false });
+
+ expect(within(wrapper.element).queryByText('2FA')).toBe(null);
+ });
});
describe('user status', () => {
diff --git a/spec/frontend/members/mock_data.js b/spec/frontend/members/mock_data.js
index fa324ce1cf9..6a73b2fcf8c 100644
--- a/spec/frontend/members/mock_data.js
+++ b/spec/frontend/members/mock_data.js
@@ -75,3 +75,5 @@ export const membersJsonString = JSON.stringify(members);
export const directMember = { ...member, isDirectMember: true };
export const inheritedMember = { ...member, isDirectMember: false };
+
+export const member2faEnabled = { ...member, user: { ...member.user, twoFactorEnabled: true } };
diff --git a/spec/frontend/members/utils_spec.js b/spec/frontend/members/utils_spec.js
index f447a4c4ee9..bfb5a4bc7d3 100644
--- a/spec/frontend/members/utils_spec.js
+++ b/spec/frontend/members/utils_spec.js
@@ -17,6 +17,7 @@ import {
member as memberMock,
directMember,
inheritedMember,
+ member2faEnabled,
group,
invite,
membersJsonString,
@@ -30,7 +31,11 @@ const URL_HOST = 'https://localhost/';
describe('Members Utils', () => {
describe('generateBadges', () => {
it('has correct properties for each badge', () => {
- const badges = generateBadges(memberMock, true);
+ const badges = generateBadges({
+ member: memberMock,
+ isCurrentUser: true,
+ canManageMembers: true,
+ });
badges.forEach((badge) => {
expect(badge).toEqual(
@@ -44,12 +49,32 @@ describe('Members Utils', () => {
});
it.each`
- member | expected
- ${memberMock} | ${{ show: true, text: "It's you", variant: 'success' }}
- ${{ ...memberMock, user: { ...memberMock.user, blocked: true } }} | ${{ show: true, text: 'Blocked', variant: 'danger' }}
- ${{ ...memberMock, user: { ...memberMock.user, twoFactorEnabled: true } }} | ${{ show: true, text: '2FA', variant: 'info' }}
+ member | expected
+ ${memberMock} | ${{ show: true, text: "It's you", variant: 'success' }}
+ ${{ ...memberMock, user: { ...memberMock.user, blocked: true } }} | ${{ show: true, text: 'Blocked', variant: 'danger' }}
+ ${member2faEnabled} | ${{ show: true, text: '2FA', variant: 'info' }}
`('returns expected output for "$expected.text" badge', ({ member, expected }) => {
- expect(generateBadges(member, true)).toContainEqual(expect.objectContaining(expected));
+ expect(
+ generateBadges({ member, isCurrentUser: true, canManageMembers: true }),
+ ).toContainEqual(expect.objectContaining(expected));
+ });
+
+ describe('when `canManageMembers` argument is `false`', () => {
+ describe.each`
+ description | memberIsCurrentUser | expectedBadgeToBeShown
+ ${'is not the current user'} | ${false} | ${false}
+ ${'is the current user'} | ${true} | ${true}
+ `('when member is $description', ({ memberIsCurrentUser, expectedBadgeToBeShown }) => {
+ it(`sets 'show' to '${expectedBadgeToBeShown}' for 2FA badge`, () => {
+ const badges = generateBadges({
+ member: member2faEnabled,
+ isCurrentUser: memberIsCurrentUser,
+ canManageMembers: false,
+ });
+
+ expect(badges.find((badge) => badge.text === '2FA').show).toBe(expectedBadgeToBeShown);
+ });
+ });
});
});
diff --git a/spec/helpers/projects/project_members_helper_spec.rb b/spec/helpers/projects/project_members_helper_spec.rb
index 5e0b4df7f7f..1a55840a58a 100644
--- a/spec/helpers/projects/project_members_helper_spec.rb
+++ b/spec/helpers/projects/project_members_helper_spec.rb
@@ -166,7 +166,7 @@ RSpec.describe Projects::ProjectMembersHelper do
members: helper.project_members_data_json(project, present_members(project_members)),
member_path: '/foo-bar/-/project_members/:id',
source_id: project.id,
- can_manage_members: true
+ can_manage_members: 'true'
})
end
end
@@ -193,7 +193,7 @@ RSpec.describe Projects::ProjectMembersHelper do
members: helper.project_group_links_data_json(project_group_links),
member_path: '/foo-bar/-/group_links/:id',
source_id: project.id,
- can_manage_members: true
+ can_manage_members: 'true'
})
end
end
generated by cgit v1.2.3 (git 2.25.1) at 2024-09-06 17:28:09 +0300