From 3b48b32754ee372b86169e5ddbc5f625719f0078 Mon Sep 17 00:00:00 2001 From: Davinder Singh Date: Thu, 19 Oct 2023 13:51:33 +0100 Subject: [PATCH] Revert "Revert "Group SSO - Adding a bug fix for sending emails"" (#15307) * Revert "Revert "Group SSO - Adding a bug fix for sending emails"" * adding conditional rendering of columns and styling fixes for each render mode with some cypress test GitOrigin-RevId: 168011503ffacff61c8f37bee4c4bfb012909c1f --- .../UserMembershipController.js | 7 ++ .../user_membership/group-members-react.pug | 1 + .../web/frontend/extracted-translations.json | 3 + .../managed-users/managed-user-row.tsx | 14 +++- .../managed-users/managed-users-list.tsx | 19 ++++- .../managed-users-select-all-checkbox.tsx | 8 ++- .../managed-users-select-user-checkbox.tsx | 8 ++- .../components/managed-users/sso-status.tsx | 41 +++++++++++ .../stylesheets/components/group-members.less | 34 +++++++++ services/web/locales/en.json | 3 + .../managed-group-members.spec.tsx | 71 +++++++++++++++++-- .../managed-users/managed-users-list.spec.tsx | 42 ++++++++--- .../UserMembershipControllerTests.js | 7 ++ services/web/types/group-management/user.ts | 1 + 14 files changed, 237 insertions(+), 22 deletions(-) create mode 100644 services/web/frontend/js/features/group-management/components/managed-users/sso-status.tsx diff --git a/services/web/app/src/Features/UserMembership/UserMembershipController.js b/services/web/app/src/Features/UserMembership/UserMembershipController.js index f34599bf69..456f14ad13 100644 --- a/services/web/app/src/Features/UserMembership/UserMembershipController.js +++ b/services/web/app/src/Features/UserMembership/UserMembershipController.js @@ -17,9 +17,14 @@ const EmailHelper = require('../Helpers/EmailHelper') const { csvAttachment } = require('../../infrastructure/Response') const { UserIsManagerError } = require('./UserMembershipErrors') const CSVParser = require('json2csv').Parser +const SSOConfigManager = require('../../../../modules/managed-users/app/src/SSOConfigManager') async function manageGroupMembers(req, res, next) { const { entity, entityConfig } = req + + const ssoConfig = await SSOConfigManager.promises.getSSOConfig( + entity.ssoConfig + ) return entity.fetchV1Data(function (error, entity) { if (error != null) { return next(error) @@ -37,12 +42,14 @@ async function manageGroupMembers(req, res, next) { if (entityConfig.fields.name) { entityName = entity[entityConfig.fields.name] } + return res.render('user_membership/group-members-react', { name: entityName, groupId: entityPrimaryKey, users, groupSize: entity.membersLimit, managedUsersActive: entity.groupPolicy != null, + groupSSOActive: ssoConfig?.enabled, }) } ) diff --git a/services/web/app/views/user_membership/group-members-react.pug b/services/web/app/views/user_membership/group-members-react.pug index 5760c0b432..d76382161b 100644 --- a/services/web/app/views/user_membership/group-members-react.pug +++ b/services/web/app/views/user_membership/group-members-react.pug @@ -9,6 +9,7 @@ block append meta meta(name="ol-groupName", data-type="string", content=name) meta(name="ol-groupSize", data-type="json", content=groupSize) meta(name="ol-managedUsersActive", data-type="boolean", content=managedUsersActive) + meta(name="ol-groupSSOActive", data-type="boolean", content=groupSSOActive) block content main.content.content-alt#subscription-manage-group-root diff --git a/services/web/frontend/extracted-translations.json b/services/web/frontend/extracted-translations.json index 624fa7e7b4..1ea6ec8bd7 100644 --- a/services/web/frontend/extracted-translations.json +++ b/services/web/frontend/extracted-translations.json @@ -1058,6 +1058,7 @@ "sort_by_x": "", "source": "", "spell_check": "", + "sso": "", "sso_config_prop_help_certificate": "", "sso_config_prop_help_first_name": "", "sso_config_prop_help_last_name": "", @@ -1075,7 +1076,9 @@ "sso_is_enabled_explanation_1": "", "sso_is_enabled_explanation_2": "", "sso_link_error": "", + "sso_linked": "", "sso_logs": "", + "sso_unlinked": "", "start_a_free_trial": "", "start_by_adding_your_email": "", "start_free_trial": "", diff --git a/services/web/frontend/js/features/group-management/components/managed-users/managed-user-row.tsx b/services/web/frontend/js/features/group-management/components/managed-users/managed-user-row.tsx index 374067278c..d9fcbcc7aa 100644 --- a/services/web/frontend/js/features/group-management/components/managed-users/managed-user-row.tsx +++ b/services/web/frontend/js/features/group-management/components/managed-users/managed-user-row.tsx @@ -6,8 +6,10 @@ import Badge from '../../../../shared/components/badge' import Tooltip from '../../../../shared/components/tooltip' import type { ManagedUserAlert } from '../../utils/types' import ManagedUserStatus from './managed-user-status' +import SSOStatus from './sso-status' import ManagedUserDropdownButton from './managed-user-dropdown-button' import ManagedUsersSelectUserCheckbox from './managed-users-select-user-checkbox' +import getMeta from '@/utils/meta' type ManagedUserRowProps = { user: User @@ -23,6 +25,7 @@ export default function ManagedUserRow({ groupId, }: ManagedUserRowProps) { const { t } = useTranslation() + const groupSSOActive = getMeta('ol-groupSSOActive') return ( - + {user.email} {user.invite ? ( @@ -73,7 +76,14 @@ export default function ManagedUserRow({ ? moment(user.last_active_at).format('Do MMM YYYY') : 'N/A'} - + {groupSSOActive && ( + +
+ +
+ + )} +
diff --git a/services/web/frontend/js/features/group-management/components/managed-users/managed-users-list.tsx b/services/web/frontend/js/features/group-management/components/managed-users/managed-users-list.tsx index 1086dad7b2..d3f9285cd4 100644 --- a/services/web/frontend/js/features/group-management/components/managed-users/managed-users-list.tsx +++ b/services/web/frontend/js/features/group-management/components/managed-users/managed-users-list.tsx @@ -9,6 +9,7 @@ import ManagedUserRow from './managed-user-row' import OffboardManagedUserModal from './offboard-managed-user-modal' import ManagedUsersListAlert from './managed-users-list-alert' import ManagedUsersSelectAllCheckbox from './managed-users-select-all-checkbox' +import getMeta from '@/utils/meta' type ManagedUsersListProps = { groupId: string @@ -22,6 +23,7 @@ export default function ManagedUsersList({ groupId }: ManagedUsersListProps) { const [managedUserAlert, setManagedUserAlert] = useState(undefined) const { users } = useGroupMembersContext() + const groupSSOActive = getMeta('ol-groupSSOActive') return (
@@ -40,7 +42,13 @@ export default function ManagedUsersList({ groupId }: ManagedUsersListProps) { - + {t('email')} @@ -60,8 +68,13 @@ export default function ManagedUsersList({ groupId }: ManagedUsersListProps) { - - {t('security')} + {groupSSOActive && ( + + {t('security')} + + )} + + {t('managed')} diff --git a/services/web/frontend/js/features/group-management/components/managed-users/managed-users-select-all-checkbox.tsx b/services/web/frontend/js/features/group-management/components/managed-users/managed-users-select-all-checkbox.tsx index b4ca6400ed..ad15990bbc 100644 --- a/services/web/frontend/js/features/group-management/components/managed-users/managed-users-select-all-checkbox.tsx +++ b/services/web/frontend/js/features/group-management/components/managed-users/managed-users-select-all-checkbox.tsx @@ -1,9 +1,11 @@ import { useCallback } from 'react' import { useTranslation } from 'react-i18next' import { useGroupMembersContext } from '../../context/group-members-context' +import getMeta from '@/utils/meta' export default function ManagedUsersSelectAllCheckbox() { const { t } = useTranslation() + const groupSSOActive = getMeta('ol-groupSSOActive') const { selectedUsers, users, selectAllNonManagedUsers, unselectAllUsers } = useGroupMembersContext() @@ -28,7 +30,11 @@ export default function ManagedUsersSelectAllCheckbox() { } return ( - + diff --git a/services/web/frontend/js/features/group-management/components/managed-users/managed-users-select-user-checkbox.tsx b/services/web/frontend/js/features/group-management/components/managed-users/managed-users-select-user-checkbox.tsx index 4493929658..d92d4989f3 100644 --- a/services/web/frontend/js/features/group-management/components/managed-users/managed-users-select-user-checkbox.tsx +++ b/services/web/frontend/js/features/group-management/components/managed-users/managed-users-select-user-checkbox.tsx @@ -2,6 +2,7 @@ import { useTranslation } from 'react-i18next' import type { User } from '../../../../../../types/group-management/user' import { useGroupMembersContext } from '../../context/group-members-context' import { useCallback } from 'react' +import getMeta from '@/utils/meta' type ManagedUsersSelectUserCheckboxProps = { user: User @@ -11,6 +12,7 @@ export default function ManagedUsersSelectUserCheckbox({ user, }: ManagedUsersSelectUserCheckboxProps) { const { t } = useTranslation() + const groupSSOActive = getMeta('ol-groupSSOActive') const { users, selectedUsers, selectUser, unselectUser } = useGroupMembersContext() @@ -38,7 +40,11 @@ export default function ManagedUsersSelectUserCheckbox({ const selected = selectedUsers.includes(user) return ( - + {/* the next check will hide the `checkbox` but still show the `td` */} {user.enrollment?.managedBy ? null : ( <> diff --git a/services/web/frontend/js/features/group-management/components/managed-users/sso-status.tsx b/services/web/frontend/js/features/group-management/components/managed-users/sso-status.tsx new file mode 100644 index 0000000000..328f1af3d5 --- /dev/null +++ b/services/web/frontend/js/features/group-management/components/managed-users/sso-status.tsx @@ -0,0 +1,41 @@ +import { useTranslation } from 'react-i18next' +import { User } from '../../../../../../types/group-management/user' +import MaterialIcon from '@/shared/components/material-icon' + +type SSOStatusProps = { + user: User +} +export default function SSOStatus({ user }: SSOStatusProps) { + const { t } = useTranslation() + return ( + + {user.invite ? ( + + +   {t('sso')} + + ) : ( + <> + {user.enrollment?.sso ? ( + + +   {t('sso')} + + ) : ( + + +   {t('sso')} + + )} + + )} + + ) +} diff --git a/services/web/frontend/stylesheets/components/group-members.less b/services/web/frontend/stylesheets/components/group-members.less index 488e74ce9c..8664f731ce 100644 --- a/services/web/frontend/stylesheets/components/group-members.less +++ b/services/web/frontend/stylesheets/components/group-members.less @@ -106,10 +106,16 @@ .cell-checkbox { width: 5%; } + .cell-checkbox-with-sso-col { + width: 2.5%; + } .cell-email { width: 45%; } + .cell-email-with-sso-col { + width: 37%; + } .cell-name { width: 15%; @@ -126,6 +132,13 @@ } .cell-security { + width: 10%; + overflow-x: hidden; + text-overflow: ellipsis; + white-space: nowrap; + } + + .cell-managed { width: 15%; overflow-x: hidden; text-overflow: ellipsis; @@ -142,9 +155,16 @@ width: 5%; } + .cell-checkbox-with-sso-col { + width: 2.5%; + } + .cell-email { width: 34%; } + .cell-email-with-sso-col { + width: 29%; + } .cell-name { width: 20%; @@ -155,6 +175,10 @@ } .cell-security { + width: 12%; + } + + .cell-managed { width: 15%; } @@ -168,10 +192,16 @@ .cell-checkbox { width: 5%; } + .cell-checkbox-with-sso-col { + width: 2.5%; + } .cell-email { width: 43%; } + .cell-email-with-sso-col { + width: 37%; + } .cell-name { width: 20%; @@ -182,6 +212,10 @@ } .cell-security { + width: 10%; + } + + .cell-managed { width: 12%; } diff --git a/services/web/locales/en.json b/services/web/locales/en.json index d39fefb4fb..fb7247cad9 100644 --- a/services/web/locales/en.json +++ b/services/web/locales/en.json @@ -1628,6 +1628,7 @@ "source": "Source", "spell_check": "Spell check", "spread_the_word_and_fill_bar": "Spread the word and fill this bar up", + "sso": "SSO", "sso_account_already_linked": "Account already linked to another __appName__ user", "sso_config_prop_help_certificate": "Base64 encoded certificate without whitespace", "sso_config_prop_help_first_name": "Property in SAML assertion to use for first name", @@ -1648,8 +1649,10 @@ "sso_is_enabled_explanation_1": "Group members will <0>only be able to sign in via SSO", "sso_is_enabled_explanation_2": "If there are any problems with the configuration, only you (as the group administrator) will be able to disable SSO.", "sso_link_error": "Error linking account", + "sso_linked": "SSO linked", "sso_logs": "SSO Logs", "sso_not_linked": "You have not linked your account to __provider__. Please log in to your account another way and link your __provider__ account via your account settings.", + "sso_unlinked": "SSO unlinked", "sso_user_denied_access": "Cannot log in because __appName__ was not granted access to your __provider__ account. Please try again.", "standard": "Standard", "start_a_free_trial": "Start a free trial", diff --git a/services/web/test/frontend/features/group-management/components/managed-users/managed-group-members.spec.tsx b/services/web/test/frontend/features/group-management/components/managed-users/managed-group-members.spec.tsx index c899cadea1..fba765f949 100644 --- a/services/web/test/frontend/features/group-management/components/managed-users/managed-group-members.spec.tsx +++ b/services/web/test/frontend/features/group-management/components/managed-users/managed-group-members.spec.tsx @@ -28,6 +28,10 @@ const CLAIRE_JENNINGS = { enrollment: { managedBy: GROUP_ID, enrolledAt: new Date('2023-01-03'), + sso: { + providerId: '123', + externalId: '123', + }, }, } const PATHS = { @@ -37,6 +41,14 @@ const PATHS = { exportMembers: `/manage/groups/${GROUP_ID}/members/export`, } +function mountGroupMembersProvider() { + cy.mount( + + + + ) +} + describe('group members, with managed users', function () { beforeEach(function () { cy.window().then(win => { @@ -51,12 +63,7 @@ describe('group members, with managed users', function () { win.metaAttributesCache.set('ol-groupSize', 10) win.metaAttributesCache.set('ol-managedUsersActive', true) }) - - cy.mount( - - - - ) + mountGroupMembersProvider() }) it('renders the group members page', function () { @@ -209,3 +216,55 @@ describe('group members, with managed users', function () { cy.get('.alert').contains('Sorry, something went wrong') }) }) + +describe('Group members when group SSO is enabled', function () { + beforeEach(function () { + cy.window().then(win => { + win.metaAttributesCache = new Map() + win.metaAttributesCache.set('ol-users', [ + JOHN_DOE, + BOBBY_LAPOINTE, + CLAIRE_JENNINGS, + ]) + win.metaAttributesCache.set('ol-groupId', GROUP_ID) + win.metaAttributesCache.set('ol-groupName', 'My Awesome Team') + win.metaAttributesCache.set('ol-groupSize', 10) + win.metaAttributesCache.set('ol-managedUsersActive', true) + }) + }) + + it('should not display SSO Column when group sso is not enabled', function () { + cy.window().then(win => { + win.metaAttributesCache.set('ol-groupSSOActive', false) + }) + mountGroupMembersProvider() + cy.get('ul.managed-users-list table > tbody').within(() => { + cy.get('tr:nth-child(2)').within(() => { + cy.contains('bobby.lapointe@test.com') + cy.get('.sr-only').contains('SSO unlinked').should('not.exist') + }) + + cy.get('tr:nth-child(3)').within(() => { + cy.contains('claire.jennings@test.com') + cy.get('.sr-only').contains('SSO linked').should('not.exist') + }) + }) + }) + it('should display SSO Column when group sso is not enabled', function () { + cy.window().then(win => { + win.metaAttributesCache.set('ol-groupSSOActive', true) + }) + mountGroupMembersProvider() + cy.get('ul.managed-users-list table > tbody').within(() => { + cy.get('tr:nth-child(2)').within(() => { + cy.contains('bobby.lapointe@test.com') + cy.get('.sr-only').contains('SSO unlinked') + }) + + cy.get('tr:nth-child(3)').within(() => { + cy.contains('claire.jennings@test.com') + cy.get('.sr-only').contains('SSO linked') + }) + }) + }) +}) diff --git a/services/web/test/frontend/features/group-management/components/managed-users/managed-users-list.spec.tsx b/services/web/test/frontend/features/group-management/components/managed-users/managed-users-list.spec.tsx index 84b604e808..af3918d72f 100644 --- a/services/web/test/frontend/features/group-management/components/managed-users/managed-users-list.spec.tsx +++ b/services/web/test/frontend/features/group-management/components/managed-users/managed-users-list.spec.tsx @@ -1,9 +1,17 @@ import ManagedUsersList from '../../../../../../frontend/js/features/group-management/components/managed-users/managed-users-list' import { GroupMembersProvider } from '../../../../../../frontend/js/features/group-management/context/group-members-context' -describe('ManagedUsersList', function () { - const groupId = 'somegroup' +const groupId = 'somegroup' +function mountManagedUsersList() { + cy.mount( + + + + ) +} + +describe('ManagedUsersList', function () { describe('with users', function () { const users = [ { @@ -32,15 +40,31 @@ describe('ManagedUsersList', function () { cy.window().then(win => { win.metaAttributesCache.set('ol-users', users) }) - - cy.mount( - - - - ) + mountManagedUsersList() }) - it('should render the table headers', function () { + it('should render the table headers but not SSO Column', function () { + cy.window().then(win => { + win.metaAttributesCache.set('ol-groupSSOActive', false) + }) + mountManagedUsersList() + cy.get('#managed-users-list-headers').should('exist') + + // Select-all checkbox + cy.get('#managed-users-list-headers .select-all').should('exist') + + cy.get('#managed-users-list-headers').contains('Email') + cy.get('#managed-users-list-headers').contains('Name') + cy.get('#managed-users-list-headers').contains('Last Active') + cy.get('#managed-users-list-headers') + .contains('Security') + .should('not.exist') + }) + it('should render the table headers with SSO Column', function () { + cy.window().then(win => { + win.metaAttributesCache.set('ol-groupSSOActive', true) + }) + mountManagedUsersList() cy.get('#managed-users-list-headers').should('exist') // Select-all checkbox diff --git a/services/web/test/unit/src/UserMembership/UserMembershipControllerTests.js b/services/web/test/unit/src/UserMembership/UserMembershipControllerTests.js index bcbeeb28b5..0150dcb75f 100644 --- a/services/web/test/unit/src/UserMembership/UserMembershipControllerTests.js +++ b/services/web/test/unit/src/UserMembership/UserMembershipControllerTests.js @@ -69,6 +69,11 @@ describe('UserMembershipController', function () { getSessionUser: sinon.stub().returns(this.user), getLoggedInUserId: sinon.stub().returns(this.user._id), } + this.SSOConfigManager = { + promises: { + getSSOConfig: sinon.stub().resolves({ enabled: true }), + }, + } this.UserMembershipHandler = { getEntity: sinon.stub().yields(null, this.subscription), createEntity: sinon.stub().yields(null, this.institution), @@ -91,6 +96,8 @@ describe('UserMembershipController', function () { '../SplitTests/SplitTestHandler': this.SplitTestHandler, './UserMembershipHandler': this.UserMembershipHandler, '@overleaf/settings': this.Settings, + '../../../../modules/managed-users/app/src/SSOConfigManager': + this.SSOConfigManager, }, } )) diff --git a/services/web/types/group-management/user.ts b/services/web/types/group-management/user.ts index 501044b850..c4cb92c9e9 100644 --- a/services/web/types/group-management/user.ts +++ b/services/web/types/group-management/user.ts @@ -1,6 +1,7 @@ export type UserEnrollment = { managedBy?: string enrolledAt?: Date + sso?: object } export type User = {