ROX-12343: unauthenticated email integration #2984
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,16 @@ | ||
| /* eslint-disable no-void */ | ||
| import React, { ReactElement, useState } from 'react'; | ||
| import { | ||
| Alert, | ||
| AlertVariant, | ||
| Checkbox, | ||
| Form, | ||
| PageSection, | ||
| SelectOption, | ||
| TextInput, | ||
| Popover, | ||
| } from '@patternfly/react-core'; | ||
| import { HelpIcon } from '@patternfly/react-icons'; | ||
| import * as yup from 'yup'; | ||
|
|
||
| import { NotifierIntegrationBase } from 'services/NotifierIntegrationsService'; | ||
|
|
@@ -27,6 +37,7 @@ export type EmailIntegration = { | |
| sender: string; | ||
| disableTLS: boolean; | ||
| startTLSAuthMethod: 'DISABLED' | 'PLAIN' | 'LOGIN'; | ||
| allowUnauthenticatedSmtp: boolean; | ||
| }; | ||
| type: 'email'; | ||
| } & NotifierIntegrationBase; | ||
|
|
@@ -69,19 +80,21 @@ export const validationSchema = yup.object().shape({ | |
| .trim() | ||
| .required('A server address is required') | ||
| .matches(validHostnameRegex, 'Must be a valid server address'), | ||
| allowUnauthenticatedSmtp: yup.boolean(), | ||
| username: yup.string().when('allowUnauthenticatedSmtp', { | ||
| is: false, | ||
| then: (usernameSchema) => usernameSchema.trim().required('A username is required'), | ||
| }), | ||
| password: yup | ||
| .string() | ||
| .test( | ||
| 'password-test', | ||
| 'A password is required', | ||
| (value, context: yup.TestContext) => { | ||
| const requirePasswordField = | ||
| context?.from?.[2].value?.updatePassword || false; | ||
|
|
||
| if (!requirePasswordField || context.parent.allowUnauthenticatedSmtp) { | ||
| return true; | ||
| } | ||
|
|
||
|
|
@@ -116,6 +129,7 @@ export const defaultValues: EmailIntegrationFormValues = { | |
| sender: '', | ||
| disableTLS: false, | ||
| startTLSAuthMethod: 'DISABLED', | ||
| allowUnauthenticatedSmtp: false, | ||
| }, | ||
| labelDefault: '', | ||
| labelKey: '', | ||
|
|
@@ -130,6 +144,7 @@ function EmailIntegrationForm({ | |
| isEditable = false, | ||
| }: IntegrationFormProps<EmailIntegration>): ReactElement { | ||
| const formInitialValues = { ...defaultValues, ...initialValues }; | ||
| const [storedUsername, setStoredUsername] = useState(''); | ||
| if (initialValues) { | ||
| formInitialValues.notifier = { | ||
| ...formInitialValues.notifier, | ||
|
|
@@ -158,6 +173,7 @@ function EmailIntegrationForm({ | |
| validationSchema, | ||
| }); | ||
| const { isCreating } = usePageState(); | ||
| const { allowUnauthenticatedSmtp } = values.notifier.email; | ||
|
|
||
| function onChange(value, event) { | ||
| return setFieldValue(event.target.id, value); | ||
|
|
@@ -170,6 +186,17 @@ function EmailIntegrationForm({ | |
| } | ||
| } | ||
|
|
||
| function onUpdateUnauthenticatedChange(isChecked) { | ||
| if (isChecked) { | ||
| setStoredUsername(values.notifier.email.username); | ||
| setFieldValue('notifier.email.username', ''); | ||
| setFieldValue('notifier.email.password', ''); | ||
| } else { | ||
| setFieldValue('notifier.email.username', storedUsername); | ||
| } | ||
| setFieldValue('notifier.email.allowUnauthenticatedSmtp', isChecked); | ||
| } | ||
|
|
||
| function onUpdateCredentialsChange(value, event) { | ||
| setFieldValue('notifier.email.password', ''); | ||
| return setFieldValue(event.target.id, value); | ||
|
|
@@ -216,25 +243,71 @@ function EmailIntegrationForm({ | |
| isDisabled={!isEditable} | ||
| /> | ||
| </FormLabelGroup> | ||
| <FormLabelGroup | ||
| label="" | ||
| fieldId="notifier.email.unauthenticated" | ||
| errors={errors} | ||
| > | ||
| <> | ||
| <div className="pf-u-display-flex pf-u-align-items-flex-start"> | ||
|
Contributor
There was a problem hiding this comment. Not a blocker to merge: See if the following example supports an equivalent popover via https://www.patternfly.org/v4/components/form#form-group-with-additional-label-info
Contributor
Author
There was a problem hiding this comment. Yeah I wasn't a huge fan of having to do it this way. I originally tried adding the |
||
| <Checkbox | ||
| label="Enable unauthenticated SMTP" | ||
| id="notifier.email.unauthenticated" | ||
| isChecked={allowUnauthenticatedSmtp} | ||
| onChange={onUpdateUnauthenticatedChange} | ||
| onBlur={handleBlur} | ||
| /> | ||
| <Popover | ||
| showClose={false} | ||
| bodyContent="Enable unauthenticated SMTP will allow you to setup an email notifier if you don’t have authenticated email services." | ||
| > | ||
| <button | ||
| type="button" | ||
| aria-label="More info on unauthenticated SMTP field" | ||
| onClick={(e) => e.preventDefault()} | ||
| className="pf-c-form__group-label-help" | ||
| > | ||
| <HelpIcon /> | ||
| </button> | ||
| </Popover> | ||
| </div> | ||
| {allowUnauthenticatedSmtp && ( | ||
| <Alert | ||
| className="pf-u-mt-md" | ||
| title="Security Warning" | ||
| variant={AlertVariant.warning} | ||
| isInline | ||
| > | ||
| <p> | ||
| Unauthenticated SMTP is an insecure configuration and not | ||
| generally recommended. Please proceed with caution when | ||
| enabling this setting. | ||
| </p> | ||
| </Alert> | ||
| )} | ||
| </> | ||
| </FormLabelGroup> | ||
| <FormLabelGroup | ||
| label="Username" | ||
| isRequired={!allowUnauthenticatedSmtp} | ||
| fieldId="notifier.email.username" | ||
| touched={touched} | ||
| errors={errors} | ||
| > | ||
| <TextInput | ||
| isRequired={!allowUnauthenticatedSmtp} | ||
| type="text" | ||
| id="notifier.email.username" | ||
| value={values.notifier.email.username} | ||
| placeholder={ | ||
| allowUnauthenticatedSmtp ? '' : 'example, postmaster@example.com' | ||
| } | ||
| onChange={onChange} | ||
| onBlur={handleBlur} | ||
| isDisabled={!isEditable || allowUnauthenticatedSmtp} | ||
| /> | ||
| </FormLabelGroup> | ||
| {!isCreating && isEditable && !allowUnauthenticatedSmtp && ( | ||
| <FormLabelGroup | ||
| label="" | ||
| fieldId="updatePassword" | ||
|
|
@@ -253,21 +326,23 @@ function EmailIntegrationForm({ | |
| )} | ||
| <FormLabelGroup | ||
| label="Password" | ||
| isRequired={values.updatePassword && !allowUnauthenticatedSmtp} | ||
| fieldId="notifier.email.password" | ||
| touched={touched} | ||
| errors={errors} | ||
| > | ||
| <TextInput | ||
| isRequired={values.updatePassword && !allowUnauthenticatedSmtp} | ||
| type="password" | ||
| id="notifier.email.password" | ||
| value={values.notifier.email.password} | ||
| onChange={onChange} | ||
| onBlur={handleBlur} | ||
| isDisabled={ | ||
| !isEditable || !values.updatePassword || allowUnauthenticatedSmtp | ||
| } | ||
| placeholder={ | ||
| values.updatePassword || allowUnauthenticatedSmtp | ||
| ? '' | ||
| : 'Currently-stored password will be used.' | ||
| } | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@theencee Should we be clearing out the username when switching to unauthenticated, or is a username still required in certain cases?
(More importantly, is it required for the customer who asked for this feature?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unauth has optional username @vjwilson so safe to clear out