From 1de4d245931ea0caadb977fb45c89c1a30cd4a6d Mon Sep 17 00:00:00 2001 From: Jon Koops Date: Mon, 24 Oct 2022 13:43:46 +0200 Subject: [PATCH] Improve flow for testing e-mail server settings (#3619) --- .../e2e/realm_settings_tabs_test.spec.ts | 21 +- .../support/pages/admin_console/Masthead.ts | 22 +- .../cypress/support/util/AdminClient.ts | 11 + .../public/resources/en/realm-settings.json | 9 +- .../src/components/alert/AlertPanel.tsx | 2 +- .../src/realm-settings/AddUserEmailModal.tsx | 107 ---- apps/admin-ui/src/realm-settings/EmailTab.tsx | 582 +++++++++--------- apps/admin-ui/src/utils/useCurrentUser.ts | 18 + 8 files changed, 356 insertions(+), 416 deletions(-) delete mode 100644 apps/admin-ui/src/realm-settings/AddUserEmailModal.tsx create mode 100644 apps/admin-ui/src/utils/useCurrentUser.ts diff --git a/apps/admin-ui/cypress/e2e/realm_settings_tabs_test.spec.ts b/apps/admin-ui/cypress/e2e/realm_settings_tabs_test.spec.ts index ad377c3616..dd3046ead9 100644 --- a/apps/admin-ui/cypress/e2e/realm_settings_tabs_test.spec.ts +++ b/apps/admin-ui/cypress/e2e/realm_settings_tabs_test.spec.ts @@ -78,7 +78,15 @@ describe("Realm settings tabs tests", () => { }); it("Go to email tab", () => { - const msg: string = "Error! Failed to send email."; + // Configure an e-mail address so we can test the connection settings. + cy.wrap(null).then(async () => { + const adminUser = await adminClient.getAdminUser(); + + await adminClient.updateUser(adminUser.id!, { + email: "admin@example.com", + }); + }); + sidebarPage.goToRealmSettings(); cy.findByTestId("rs-email-tab").click(); //required fields not filled in or not filled properly @@ -100,17 +108,10 @@ describe("Realm settings tabs tests", () => { realmSettingsPage.toggleCheck(realmSettingsPage.enableSslCheck); realmSettingsPage.toggleCheck(realmSettingsPage.enableStartTlsCheck); realmSettingsPage.fillHostField("localhost"); - cy.intercept(`/admin/realms/${realmName}/users/*`).as("load"); + cy.findByTestId(realmSettingsPage.testConnectionButton).click(); - cy.wait("@load"); - //ln109-113 cause the tests to fail locally, but is needed for the test to pass on the dashboard. - realmSettingsPage.fillEmailField( - "example" + (Math.random() + 1).toString(36).substring(7) + "@example.com" - ); - cy.findByTestId(realmSettingsPage.modalTestConnectionButton).click(); - - masthead.checkNotificationMessage(msg, true); + masthead.checkNotificationMessage("Error! Failed to send email", true); }); it("Go to themes tab", () => { diff --git a/apps/admin-ui/cypress/support/pages/admin_console/Masthead.ts b/apps/admin-ui/cypress/support/pages/admin_console/Masthead.ts index 2329d3e100..9c5d7f470b 100644 --- a/apps/admin-ui/cypress/support/pages/admin_console/Masthead.ts +++ b/apps/admin-ui/cypress/support/pages/admin_console/Masthead.ts @@ -4,11 +4,16 @@ export default class Masthead extends CommonElements { private helpBtn = "#help"; private closeAlertMessageBtn = ".pf-c-alert__action button"; private closeLastAlertMessageBtn = - ".pf-c-alert-group > li:first-child .pf-c-alert__action button"; + "li:first-child .pf-c-alert__action button"; private alertMessage = ".pf-c-alert__title"; private userDrpDwn = "#user-dropdown"; private userDrpDwnKebab = "#user-dropdown-kebab"; + private globalAlerts = "global-alerts"; + + private getAlertsContainer() { + return cy.findByTestId(this.globalAlerts); + } checkIsAdminConsole() { cy.get(this.logoBtn).should("exist"); @@ -50,10 +55,13 @@ export default class Masthead extends CommonElements { } checkNotificationMessage(message: string, closeNotification = true) { - cy.get(this.alertMessage).should("contain.text", message); + this.getAlertsContainer() + .find(this.alertMessage) + .should("contain.text", message); if (closeNotification) { - cy.get(`button[title="` + message.replaceAll('"', '\\"') + `"]`) + this.getAlertsContainer() + .find(`button[title="` + message.replaceAll('"', '\\"') + `"]`) .last() .click({ force: true }); } @@ -61,14 +69,16 @@ export default class Masthead extends CommonElements { } closeLastAlertMessage() { - cy.get(this.closeLastAlertMessageBtn).click(); + this.getAlertsContainer().find(this.closeLastAlertMessageBtn).click(); return this; } closeAllAlertMessages() { - cy.get(this.closeAlertMessageBtn).each(() => { - cy.get(this.closeAlertMessageBtn).click({ force: true, multiple: true }); + this.getAlertsContainer().find(this.closeAlertMessageBtn).click({ + force: true, + multiple: true, }); + return this; } diff --git a/apps/admin-ui/cypress/support/util/AdminClient.ts b/apps/admin-ui/cypress/support/util/AdminClient.ts index 53ebcef6b1..8b929323dc 100644 --- a/apps/admin-ui/cypress/support/util/AdminClient.ts +++ b/apps/admin-ui/cypress/support/util/AdminClient.ts @@ -98,6 +98,17 @@ class AdminClient { return await this.client.users.create(user); } + async updateUser(id: string, payload: UserRepresentation) { + await this.login(); + return this.client.users.update({ id }, payload); + } + + async getAdminUser() { + await this.login(); + const [user] = await this.client.users.find({ username: "admin" }); + return user; + } + async addUserToGroup(userId: string, groupId: string) { await this.login(); await this.client.users.addToGroup({ id: userId, groupId }); diff --git a/apps/admin-ui/public/resources/en/realm-settings.json b/apps/admin-ui/public/resources/en/realm-settings.json index f123ef5f6a..c9bd64788a 100644 --- a/apps/admin-ui/public/resources/en/realm-settings.json +++ b/apps/admin-ui/public/resources/en/realm-settings.json @@ -114,8 +114,6 @@ "loginWithEmailHelpText": "Allow users to log in with their email address.", "duplicateEmailsAllowed": "Duplicate emails", "duplicateEmailsHelpText": "Allow multiple users to have the same email address. Changing this setting will also clear the user's cache. It is recommended to manually update email constraints of existing users in the database after switching off support for duplicate email addresses.", - "provideEmailTitle": "Provide your email address", - "provideEmail": "To test connection, you should provide your email address first.", "verifyEmail": "Verify email", "verifyEmailHelpText": "Require user to verify their email address after initial login or after address changes are submitted.", "userInfoSettings": "User info settings", @@ -123,8 +121,13 @@ "enableSwitchSuccess": "{{switch}} changed successfully", "enableSwitchError": "Could not enable / disable due to {{error}}", "testConnection": "Test connection", + "testConnectionHint": { + "withEmail": "When testing the connection an e-mail will be sent to the current user ({{email}}).", + "withoutEmail": "To test the connection you must first configure an e-mail address for the current user ({{userName}}).", + "withoutEmailAction": "Configure e-mail address" + }, "testConnectionSuccess": "Success! SMTP connection successful. E-mail was sent!", - "testConnectionError": "Error! Failed to send email.", + "testConnectionError": "Error! {{error}}", "realmId": "Realm ID", "displayName": "Display name", "htmlDisplayName": "HTML Display name", diff --git a/apps/admin-ui/src/components/alert/AlertPanel.tsx b/apps/admin-ui/src/components/alert/AlertPanel.tsx index 190651cd9f..37884f9d62 100644 --- a/apps/admin-ui/src/components/alert/AlertPanel.tsx +++ b/apps/admin-ui/src/components/alert/AlertPanel.tsx @@ -13,7 +13,7 @@ type AlertPanelProps = { export function AlertPanel({ alerts, onCloseAlert }: AlertPanelProps) { return ( - + {alerts.map(({ id, variant, message, description }) => ( { - const { t } = useTranslation("groups"); - const { adminClient } = useAdminClient(); - const { whoAmI } = useWhoAmI(); - const { - register, - handleSubmit, - watch, - formState: { errors }, - } = useForm({ - defaultValues: { email: "" }, - }); - - const watchEmailInput = watch("email", ""); - const cancel = () => callback(false); - const proceed = () => callback(true); - - const save = async (formData: AddUserEmailForm) => { - await adminClient.users.update({ id: whoAmI.getUserId() }, formData); - proceed(); - }; - - return ( - - {t("common:testConnection")} - , - , - ]} - > - - {t("realm-settings:provideEmail")} - -
- - - -
- - ); -}; diff --git a/apps/admin-ui/src/realm-settings/EmailTab.tsx b/apps/admin-ui/src/realm-settings/EmailTab.tsx index ba364aaacd..e1ae6e53bc 100644 --- a/apps/admin-ui/src/realm-settings/EmailTab.tsx +++ b/apps/admin-ui/src/realm-settings/EmailTab.tsx @@ -1,5 +1,9 @@ +import type RealmRepresentation from "@keycloak/keycloak-admin-client/lib/defs/realmRepresentation"; import { ActionGroup, + ActionListItem, + Alert, + AlertActionLink, AlertVariant, Button, Checkbox, @@ -7,29 +11,29 @@ import { PageSection, Switch, } from "@patternfly/react-core"; -import type RealmRepresentation from "@keycloak/keycloak-admin-client/lib/defs/realmRepresentation"; import { useState } from "react"; import { Controller, useForm, useWatch } from "react-hook-form"; import { useTranslation } from "react-i18next"; +import { Link } from "react-router-dom-v5-compat"; + import { useAlerts } from "../components/alert/Alerts"; import { FormAccess } from "../components/form-access/FormAccess"; import { HelpItem } from "../components/help-enabler/HelpItem"; -import { FormPanel } from "../components/scroll-form/FormPanel"; import { KeycloakTextInput } from "../components/keycloak-text-input/KeycloakTextInput"; +import { PasswordInput } from "../components/password-input/PasswordInput"; +import { FormPanel } from "../components/scroll-form/FormPanel"; import { useAdminClient } from "../context/auth/AdminClient"; import { useRealm } from "../context/realm-context/RealmContext"; -import { useWhoAmI } from "../context/whoami/WhoAmI"; +import { toUser } from "../user/routes/User"; import { emailRegexPattern } from "../util"; -import { AddUserEmailModal } from "./AddUserEmailModal"; -import { PasswordInput } from "../components/password-input/PasswordInput"; +import { useCurrentUser } from "../utils/useCurrentUser"; + import "./realm-settings-section.css"; type RealmSettingsEmailTabProps = { realm: RealmRepresentation; }; -export type EmailRegistrationCallback = (registered: boolean) => void; - export const RealmSettingsEmailTab = ({ realm: initialRealm, }: RealmSettingsEmailTabProps) => { @@ -37,10 +41,9 @@ export const RealmSettingsEmailTab = ({ const { adminClient } = useAdminClient(); const { realm: realmName } = useRealm(); const { addAlert, addError } = useAlerts(); - const { whoAmI } = useWhoAmI(); + const currentUser = useCurrentUser(); const [realm, setRealm] = useState(initialRealm); - const [callback, setCallback] = useState(); const { register, control, @@ -63,12 +66,6 @@ export const RealmSettingsEmailTab = ({ const save = async (form: RealmRepresentation) => { try { - const registered = await registerEmailIfNeeded(); - - if (!registered) { - return; - } - const savedRealm = { ...realm, ...form }; // For default value, back end is expecting null instead of empty string @@ -102,12 +99,6 @@ export const RealmSettingsEmailTab = ({ if (serverSettings.port === 0) serverSettings.port = null; try { - const registered = await registerEmailIfNeeded(); - - if (!registered) { - return; - } - await adminClient.realms.testSMTPConnection( { realm: realm.realm! }, serverSettings @@ -118,276 +109,246 @@ export const RealmSettingsEmailTab = ({ } }; - /** - * Triggers the flow to register the user's email if the user does not yet have one configured, if successful resolves true, otherwise false. - */ - const registerEmailIfNeeded = async () => { - const user = await adminClient.users.findOne({ id: whoAmI.getUserId() }); - - // A user should always be found, throw if it is not. - if (!user) { - throw new Error("Unable to find user."); - } - - // User already has an e-mail associated with it, no need to register. - if (user.email) { - return true; - } - - // User needs to register, show modal to do so. - return new Promise((resolve) => { - const callback: EmailRegistrationCallback = (registered) => { - setCallback(undefined); - resolve(registered); - }; - - setCallback(() => callback); - }); - }; - return ( - <> - {callback && } - - - - - - - - } - > - - - - - - - } - > - - - - } - > - - - - - + + - - + + + } + > + + + + + + + } + > + + + + } + > + + + + + + + + - - - - - - - ( - onChange("" + value)} - /> - )} - /> - ( - onChange("" + value)} - /> - )} - /> - - - ( - { - onChange("" + value); - }} - aria-label={t("authentication")} - /> - )} - /> - - {authenticationEnabled === "true" && ( - <> - + + + + + + ( + onChange("" + value)} + /> + )} + /> + ( + onChange("" + value)} + /> + )} + /> + + + ( + { + onChange("" + value); + }} + aria-label={t("authentication")} + /> + )} + /> + + {authenticationEnabled === "true" && ( + <> + + - + + - - + - } - > - - - - )} + ref={register({ required: true })} + /> + + + )} - + + + + + + - - - - - + + + {currentUser && ( + + {currentUser.email ? ( + + ) : ( + ( + + )} + > + {t("testConnectionHint.withoutEmailAction")} + + } + /> + )} + + )} + + + ); }; diff --git a/apps/admin-ui/src/utils/useCurrentUser.ts b/apps/admin-ui/src/utils/useCurrentUser.ts new file mode 100644 index 0000000000..18284d79bd --- /dev/null +++ b/apps/admin-ui/src/utils/useCurrentUser.ts @@ -0,0 +1,18 @@ +import UserRepresentation from "@keycloak/keycloak-admin-client/lib/defs/userRepresentation"; +import { useState } from "react"; +import { useAdminClient, useFetch } from "../context/auth/AdminClient"; +import { useWhoAmI } from "../context/whoami/WhoAmI"; + +export function useCurrentUser() { + const { whoAmI } = useWhoAmI(); + const { adminClient } = useAdminClient(); + const [currentUser, setCurrentUser] = useState(); + + const userId = whoAmI.getUserId(); + + useFetch(() => adminClient.users.findOne({ id: userId }), setCurrentUser, [ + userId, + ]); + + return currentUser; +}