From e9d37e11aa76b5b76c6e5ce8fa1752c96ac1013a Mon Sep 17 00:00:00 2001
From: Alexander Polyankin <alexander.polyankin@metabase.com>
Date: Tue, 12 Oct 2021 13:21:23 +0300
Subject: [PATCH] Add approved domains FE validation (#18327)
---
.../AuditNotificationEditModal.jsx | 17 +-
.../AuditNotificationEditModal.styled.jsx | 6 +
.../AuditAlertEditModal.jsx | 3 +
.../AuditSubscriptionEditModal.jsx | 3 +
.../src/metabase/components/TokenField.jsx | 18 +-
.../metabase/components/TokenField.styled.jsx | 23 +++
.../components/UserPicker/UserPicker.jsx | 7 +-
.../metabase/dashboard/components/Sidebar.jsx | 12 +-
.../dashboard/components/Sidebar.styled.jsx | 11 --
frontend/src/metabase/lib/alert.js | 21 +++
frontend/src/metabase/lib/pulse.js | 67 +++++---
frontend/src/metabase/lib/settings.js | 5 +
frontend/src/metabase/lib/utils.js | 10 +-
.../metabase/pulse/components/PulseEdit.jsx | 3 +
.../pulse/components/PulseEditChannels.jsx | 2 +
.../pulse/components/RecipientPicker.jsx | 102 ++++++-----
.../components/RecipientPicker.styled.jsx | 7 +
.../query_builder/components/AlertModals.jsx | 67 +++-----
.../components/AlertModals.styled.jsx | 5 -
.../sharing/components/AddEditSidebar.jsx | 21 +--
.../sharing/components/SharingSidebar.jsx | 9 +-
frontend/test/metabase/lib/pulse.unit.spec.js | 159 +++++++++++-------
.../sharing/approved-domains.cy.spec.js | 81 ++++++---
23 files changed, 406 insertions(+), 253 deletions(-)
create mode 100644 frontend/src/metabase/components/TokenField.styled.jsx
delete mode 100644 frontend/src/metabase/dashboard/components/Sidebar.styled.jsx
create mode 100644 frontend/src/metabase/lib/alert.js
create mode 100644 frontend/src/metabase/pulse/components/RecipientPicker.styled.jsx
diff --git a/enterprise/frontend/src/metabase-enterprise/audit_app/components/AuditNotificationEditModal/AuditNotificationEditModal.jsx b/enterprise/frontend/src/metabase-enterprise/audit_app/components/AuditNotificationEditModal/AuditNotificationEditModal.jsx
index e53fc697a4c..d3b0e02c642 100644
--- a/enterprise/frontend/src/metabase-enterprise/audit_app/components/AuditNotificationEditModal/AuditNotificationEditModal.jsx
+++ b/enterprise/frontend/src/metabase-enterprise/audit_app/components/AuditNotificationEditModal/AuditNotificationEditModal.jsx
@@ -1,15 +1,19 @@
import React, { useState } from "react";
import PropTypes from "prop-types";
import { t } from "ttag";
+import MetabaseSettings from "metabase/lib/settings";
+import { recipientIsValid } from "metabase/lib/pulse";
import Button from "metabase/components/Button";
import FormMessage from "metabase/components/form/FormMessage";
import ModalContent from "metabase/components/ModalContent";
import UserPicker from "metabase/components/UserPicker";
+import { ModalErrorMessage } from "metabase-enterprise/audit_app/components/AuditNotificationEditModal/AuditNotificationEditModal.styled";
const propTypes = {
item: PropTypes.object.isRequired,
type: PropTypes.oneOf(["alert", "pulse"]).isRequired,
users: PropTypes.array.isRequired,
+ invalidRecipientText: PropTypes.func.isRequired,
onUpdate: PropTypes.func,
onDelete: PropTypes.func,
onClose: PropTypes.func,
@@ -19,13 +23,18 @@ const AuditNotificationEditModal = ({
item,
type,
users,
+ invalidRecipientText,
onUpdate,
onDelete,
onClose,
}) => {
const [channels, setChannels] = useState(item.channels);
const [error, setError] = useState();
- const hasRecipients = channels.some(c => c.recipients.length > 0);
+ const domains = MetabaseSettings.subscriptionAllowedDomains().join(", ");
+ const recipients = channels.flatMap(c => c.recipients);
+ const hasRecipients = recipients.length > 0;
+ const hasValidDomains = recipients.every(recipientIsValid);
+ const hasValidRecipients = hasRecipients && hasValidDomains;
const handleRecipientsChange = (recipients, index) => {
const newChannels = [...channels];
@@ -59,7 +68,7 @@ const AuditNotificationEditModal = ({
<Button
key="update"
primary
- disabled={!hasRecipients}
+ disabled={!hasValidRecipients}
onClick={handleUpdateClick}
>
{t`Update`}
@@ -76,10 +85,14 @@ const AuditNotificationEditModal = ({
<UserPicker
key={index}
value={channel.recipients}
+ validateValue={recipientIsValid}
users={users}
onChange={recipients => handleRecipientsChange(recipients, index)}
/>
))}
+ {!hasValidDomains && (
+ <ModalErrorMessage>{invalidRecipientText(domains)}</ModalErrorMessage>
+ )}
</ModalContent>
);
};
diff --git a/enterprise/frontend/src/metabase-enterprise/audit_app/components/AuditNotificationEditModal/AuditNotificationEditModal.styled.jsx b/enterprise/frontend/src/metabase-enterprise/audit_app/components/AuditNotificationEditModal/AuditNotificationEditModal.styled.jsx
index d1e323f6d07..d3390528bec 100644
--- a/enterprise/frontend/src/metabase-enterprise/audit_app/components/AuditNotificationEditModal/AuditNotificationEditModal.styled.jsx
+++ b/enterprise/frontend/src/metabase-enterprise/audit_app/components/AuditNotificationEditModal/AuditNotificationEditModal.styled.jsx
@@ -1,5 +1,6 @@
import styled from "styled-components";
import Button from "metabase/components/Button";
+import { color } from "metabase/lib/colors";
import { space } from "metabase/styled-components/theme";
export const ModalButton = styled(Button)`
@@ -9,3 +10,8 @@ export const ModalButton = styled(Button)`
margin-left: ${space(2)};
}
`;
+
+export const ModalErrorMessage = styled.div`
+ color: ${color("error")};
+ margin-top: 0.5rem;
+`;
diff --git a/enterprise/frontend/src/metabase-enterprise/audit_app/containers/AuditAlertEditModal/AuditAlertEditModal.jsx b/enterprise/frontend/src/metabase-enterprise/audit_app/containers/AuditAlertEditModal/AuditAlertEditModal.jsx
index 11bf8424aa7..984cebbe715 100644
--- a/enterprise/frontend/src/metabase-enterprise/audit_app/containers/AuditAlertEditModal/AuditAlertEditModal.jsx
+++ b/enterprise/frontend/src/metabase-enterprise/audit_app/containers/AuditAlertEditModal/AuditAlertEditModal.jsx
@@ -1,6 +1,7 @@
import { connect } from "react-redux";
import { push } from "react-router-redux";
import _ from "underscore";
+import { t } from "ttag";
import Alerts from "metabase/entities/alerts";
import Users from "metabase/entities/users";
import AuditNotificationEditModal from "../../components/AuditNotificationEditModal";
@@ -8,6 +9,8 @@ import AuditNotificationEditModal from "../../components/AuditNotificationEditMo
const mapStateToProps = (state, { alert }) => ({
item: alert,
type: "alert",
+ invalidRecipientText: domains =>
+ t`You're only allowed to email alerts to addresses ending in ${domains}`,
});
const mapDispatchToProps = {
diff --git a/enterprise/frontend/src/metabase-enterprise/audit_app/containers/AuditSubscriptionEditModal/AuditSubscriptionEditModal.jsx b/enterprise/frontend/src/metabase-enterprise/audit_app/containers/AuditSubscriptionEditModal/AuditSubscriptionEditModal.jsx
index 9598a0963c8..a162504f4ca 100644
--- a/enterprise/frontend/src/metabase-enterprise/audit_app/containers/AuditSubscriptionEditModal/AuditSubscriptionEditModal.jsx
+++ b/enterprise/frontend/src/metabase-enterprise/audit_app/containers/AuditSubscriptionEditModal/AuditSubscriptionEditModal.jsx
@@ -1,6 +1,7 @@
import { connect } from "react-redux";
import { push } from "react-router-redux";
import _ from "underscore";
+import { t } from "ttag";
import Pulses from "metabase/entities/pulses";
import Users from "metabase/entities/users";
import AuditNotificationEditModal from "../../components/AuditNotificationEditModal";
@@ -8,6 +9,8 @@ import AuditNotificationEditModal from "../../components/AuditNotificationEditMo
const mapStateToProps = (state, { pulse }) => ({
item: pulse,
type: "pulse",
+ invalidRecipientText: domains =>
+ t`You're only allowed to email subscriptions to addresses ending in ${domains}`,
});
const mapDispatchToProps = {
diff --git a/frontend/src/metabase/components/TokenField.jsx b/frontend/src/metabase/components/TokenField.jsx
index 873baf80eb0..3f308e23d9b 100644
--- a/frontend/src/metabase/components/TokenField.jsx
+++ b/frontend/src/metabase/components/TokenField.jsx
@@ -8,6 +8,7 @@ import cx from "classnames";
import OnClickOutsideWrapper from "metabase/components/OnClickOutsideWrapper";
import Icon from "metabase/components/Icon";
import Popover from "metabase/components/Popover";
+import { TokenFieldAddon, TokenFieldItem } from "./TokenField.styled";
import {
KEYCODE_ESCAPE,
@@ -62,6 +63,7 @@ type Props = {
onFocus?: () => void,
onBlur?: () => void,
+ validateValue: (value: Value) => boolean,
updateOnInputChange?: boolean,
updateOnInputBlur?: boolean,
// if provided, parseFreeformValue parses the input string into a value,
@@ -121,6 +123,7 @@ export default class TokenField extends Component {
valueRenderer: value => <span>{value}</span>,
optionRenderer: option => <span>{option}</span>,
layoutRenderer: props => <DefaultTokenFieldLayout {...props} />,
+ validateValue: () => true,
color: "brand",
@@ -503,6 +506,7 @@ export default class TokenField extends Component {
placeholder,
multi,
+ validateValue,
parseFreeformValue,
updateOnInputChange,
@@ -568,11 +572,7 @@ export default class TokenField extends Component {
onMouseDownCapture={this.onMouseDownCapture}
>
{value.map((v, index) => (
- <li
- key={index}
- className={cx("flex align-center mr1 mb1 px1 rounded bg-medium")}
- style={{ paddingTop: "12px", paddingBottom: "12px" }}
- >
+ <TokenFieldItem key={index} isValid={validateValue(v)}>
<span
style={{ ...defaultStyleValue, ...valueStyle }}
className={multi ? "pl1 pr0" : "px1"}
@@ -580,8 +580,8 @@ export default class TokenField extends Component {
{valueRenderer(v)}
</span>
{multi && (
- <a
- className="text-medium flex align-center text-error-hover px1"
+ <TokenFieldAddon
+ isValid={validateValue(v)}
onClick={e => {
e.preventDefault();
this.removeValue(v);
@@ -589,9 +589,9 @@ export default class TokenField extends Component {
onMouseDown={e => e.preventDefault()}
>
<Icon name="close" className="flex align-center" size={12} />
- </a>
+ </TokenFieldAddon>
)}
- </li>
+ </TokenFieldItem>
))}
<li className={cx("flex-full flex align-center mr1 mb1 p1")}>
<input
diff --git a/frontend/src/metabase/components/TokenField.styled.jsx b/frontend/src/metabase/components/TokenField.styled.jsx
new file mode 100644
index 00000000000..18f3f8b073e
--- /dev/null
+++ b/frontend/src/metabase/components/TokenField.styled.jsx
@@ -0,0 +1,23 @@
+import styled from "styled-components";
+import { color } from "metabase/lib/colors";
+
+export const TokenFieldItem = styled.li`
+ display: flex;
+ align-items: center;
+ margin: 0 0.25rem 0.25rem 0;
+ padding: 0.75rem 0.5rem;
+ border-radius: 0.5rem;
+ color: ${({ isValid }) => (isValid ? "" : color("error"))};
+ background-color: ${color("bg-medium")};
+`;
+
+export const TokenFieldAddon = styled.a`
+ display: flex;
+ align-items: center;
+ padding: 0 0.5rem;
+ color: ${({ isValid }) => (isValid ? "" : color("error"))};
+
+ &:hover {
+ color: ${color("error")};
+ }
+`;
diff --git a/frontend/src/metabase/components/UserPicker/UserPicker.jsx b/frontend/src/metabase/components/UserPicker/UserPicker.jsx
index 1681ba4f3e4..9920e8ecd51 100644
--- a/frontend/src/metabase/components/UserPicker/UserPicker.jsx
+++ b/frontend/src/metabase/components/UserPicker/UserPicker.jsx
@@ -12,11 +12,12 @@ import {
const propTypes = {
value: PropTypes.array.isRequired,
+ validateValue: PropTypes.func,
users: PropTypes.array.isRequired,
onChange: PropTypes.func,
};
-const UserPicker = ({ value, users, onChange }) => {
+const UserPicker = ({ value, validateValue, users, onChange }) => {
const placeholder = !value.length
? t`Enter user names or email addresses`
: null;
@@ -60,13 +61,15 @@ const UserPicker = ({ value, users, onChange }) => {
<TokenField
idKey={idKey}
value={value}
+ validateValue={validateValue}
valueRenderer={valueRenderer}
options={options}
optionRenderer={optionRenderer}
filterOption={filterOption}
parseFreeformValue={parseFreeformValue}
placeholder={placeholder}
- multi={true}
+ multi
+ updateOnInputBlur
onChange={onChange}
/>
</UserPickerRoot>
diff --git a/frontend/src/metabase/dashboard/components/Sidebar.jsx b/frontend/src/metabase/dashboard/components/Sidebar.jsx
index 0779f400586..a16c053d39e 100644
--- a/frontend/src/metabase/dashboard/components/Sidebar.jsx
+++ b/frontend/src/metabase/dashboard/components/Sidebar.jsx
@@ -1,23 +1,18 @@
-/* eslint-disable react/prop-types */
import React from "react";
import PropTypes from "prop-types";
import { t } from "ttag";
-
import Button from "metabase/components/Button";
-import { getErrorMessage } from "metabase/components/form/FormMessage";
-import { SidebarError, SidebarFooter } from "./Sidebar.styled";
const WIDTH = 384;
const propTypes = {
closeIsDisabled: PropTypes.bool,
- formError: PropTypes.object,
children: PropTypes.node,
onClose: PropTypes.func,
onCancel: PropTypes.func,
};
-function Sidebar({ closeIsDisabled, formError, children, onClose, onCancel }) {
+function Sidebar({ closeIsDisabled, children, onClose, onCancel }) {
return (
<aside
style={{ width: WIDTH, minWidth: WIDTH }}
@@ -50,11 +45,6 @@ function Sidebar({ closeIsDisabled, formError, children, onClose, onCancel }) {
)}
</div>
)}
- {formError && (
- <SidebarFooter>
- <SidebarError>{getErrorMessage(formError)}</SidebarError>
- </SidebarFooter>
- )}
</aside>
);
}
diff --git a/frontend/src/metabase/dashboard/components/Sidebar.styled.jsx b/frontend/src/metabase/dashboard/components/Sidebar.styled.jsx
deleted file mode 100644
index 82233ec8db6..00000000000
--- a/frontend/src/metabase/dashboard/components/Sidebar.styled.jsx
+++ /dev/null
@@ -1,11 +0,0 @@
-import styled from "styled-components";
-import { color } from "metabase/lib/colors";
-import { space } from "metabase/styled-components/theme";
-
-export const SidebarFooter = styled.div`
- padding: 0 ${space(3)} ${space(2)};
-`;
-
-export const SidebarError = styled.div`
- color: ${color("error")};
-`;
diff --git a/frontend/src/metabase/lib/alert.js b/frontend/src/metabase/lib/alert.js
new file mode 100644
index 00000000000..56e31f35e53
--- /dev/null
+++ b/frontend/src/metabase/lib/alert.js
@@ -0,0 +1,21 @@
+import { recipientIsValid, scheduleIsValid } from "metabase/lib/pulse";
+
+export function channelIsValid(channel) {
+ switch (channel.channel_type) {
+ case "email":
+ return (
+ channel.recipients &&
+ channel.recipients.length > 0 &&
+ channel.recipients.every(recipientIsValid) &&
+ scheduleIsValid(channel)
+ );
+ case "slack":
+ return channel.details && scheduleIsValid(channel);
+ default:
+ return false;
+ }
+}
+
+export function alertIsValid(alert) {
+ return alert.channels.length > 0 && alert.channels.every(channelIsValid);
+}
diff --git a/frontend/src/metabase/lib/pulse.js b/frontend/src/metabase/lib/pulse.js
index 71c8e2a28f0..a331891e4f1 100644
--- a/frontend/src/metabase/lib/pulse.js
+++ b/frontend/src/metabase/lib/pulse.js
@@ -1,4 +1,6 @@
import _ from "underscore";
+import MetabaseSettings from "metabase/lib/settings";
+import MetabaseUtils from "metabase/lib/utils";
import {
hasDefaultParameterValue,
hasParameterValue,
@@ -14,31 +16,28 @@ export const NEW_PULSE_TEMPLATE = {
};
export function channelIsValid(channel, channelSpec) {
- if (!channelSpec) {
- return false;
- }
-
- if (channelSpec.recipients) {
- // default from formInput is an empty array, not a null array
- // check for both
- if (!channel.recipients || channel.recipients.length < 1) {
- return false;
- }
- }
-
- if (channelSpec.fields) {
- for (const field of channelSpec.fields) {
- if (
- field.required &&
+ switch (channel.channel_type) {
+ case "email":
+ return (
+ channel.recipients &&
+ channel.recipients.length > 0 &&
+ channel.recipients.every(recipientIsValid) &&
+ fieldsAreValid(channel, channelSpec) &&
+ scheduleIsValid(channel)
+ );
+ case "slack":
+ return (
channel.details &&
- (channel.details[field.name] == null ||
- channel.details[field.name] === "")
- ) {
- return false;
- }
- }
+ channel.details.channel &&
+ fieldsAreValid(channel, channelSpec) &&
+ scheduleIsValid(channel)
+ );
+ default:
+ return false;
}
+}
+export function scheduleIsValid(channel) {
switch (channel.schedule_type) {
case "monthly":
if (channel.schedule_frame != null && channel.schedule_hour != null) {
@@ -65,6 +64,20 @@ export function channelIsValid(channel, channelSpec) {
return true;
}
+export function fieldsAreValid(channel, channelSpec) {
+ if (!channelSpec) {
+ return false;
+ }
+
+ if (!channelSpec.fields) {
+ return true;
+ }
+
+ return channelSpec.fields
+ .filter(field => field.required)
+ .every(field => Boolean(channel.details?.[field.name]));
+}
+
function pulseChannelsAreValid(pulse, channelSpecs) {
return (
pulse.channels.filter(c =>
@@ -73,6 +86,16 @@ function pulseChannelsAreValid(pulse, channelSpecs) {
);
}
+export function recipientIsValid(recipient) {
+ if (recipient.id) {
+ return true;
+ }
+
+ const recipientDomain = MetabaseUtils.getEmailDomain(recipient.email);
+ const allowedDomains = MetabaseSettings.subscriptionAllowedDomains();
+ return _.isEmpty(allowedDomains) || allowedDomains.includes(recipientDomain);
+}
+
export function pulseIsValid(pulse, channelSpecs) {
return (
(pulse.name &&
diff --git a/frontend/src/metabase/lib/settings.js b/frontend/src/metabase/lib/settings.js
index 94d011e8e06..6195f55d349 100644
--- a/frontend/src/metabase/lib/settings.js
+++ b/frontend/src/metabase/lib/settings.js
@@ -248,6 +248,11 @@ class Settings {
return null;
}
}
+
+ subscriptionAllowedDomains() {
+ const setting = this.get("subscription-allowed-domains") ?? "";
+ return setting ? setting.split(",") : [];
+ }
}
const n2w = n => MetabaseUtils.numberToWord(n);
diff --git a/frontend/src/metabase/lib/utils.js b/frontend/src/metabase/lib/utils.js
index 4f92bb0d4db..715e220773c 100644
--- a/frontend/src/metabase/lib/utils.js
+++ b/frontend/src/metabase/lib/utils.js
@@ -24,6 +24,8 @@ const LAYOUT_PROPS = [
"bordered",
];
+const EMAIL_REGEX = /^(([^<>()[\]\\.,;:\s@\"]+(\.[^<>()[\]\\.,;:\s@\"]+)*)|(\".+\"))@((\[[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\])|(([a-zA-Z\-0-9]+\.)+[a-zA-Z]{2,}))$/;
+
export function stripLayoutProps(props) {
return _.omit(props, LAYOUT_PROPS);
}
@@ -144,8 +146,12 @@ const MetabaseUtils = {
},
isEmail(email) {
- const re = /^(([^<>()[\]\\.,;:\s@\"]+(\.[^<>()[\]\\.,;:\s@\"]+)*)|(\".+\"))@((\[[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\])|(([a-zA-Z\-0-9]+\.)+[a-zA-Z]{2,}))$/;
- return re.test(email);
+ return EMAIL_REGEX.test(email);
+ },
+
+ getEmailDomain(email) {
+ const match = EMAIL_REGEX.exec(email);
+ return match && match[5];
},
equals(a, b) {
diff --git a/frontend/src/metabase/pulse/components/PulseEdit.jsx b/frontend/src/metabase/pulse/components/PulseEdit.jsx
index 64bb0919fe7..056994e1485 100644
--- a/frontend/src/metabase/pulse/components/PulseEdit.jsx
+++ b/frontend/src/metabase/pulse/components/PulseEdit.jsx
@@ -202,6 +202,9 @@ export default class PulseEdit extends Component {
{...this.props}
setPulse={this.setPulse}
pulseIsValid={isValid}
+ invalidRecipientText={domains =>
+ t`You're only allowed to email pulses to addresses ending in ${domains}`
+ }
/>
</div>
<PulseEditSkip {...this.props} setPulse={this.setPulse} />
diff --git a/frontend/src/metabase/pulse/components/PulseEditChannels.jsx b/frontend/src/metabase/pulse/components/PulseEditChannels.jsx
index d9c513cd9c4..2c5d4d1e4c4 100644
--- a/frontend/src/metabase/pulse/components/PulseEditChannels.jsx
+++ b/frontend/src/metabase/pulse/components/PulseEditChannels.jsx
@@ -43,6 +43,7 @@ export default class PulseEditChannels extends Component {
cardPreviews: PropTypes.object,
hideSchedulePicker: PropTypes.bool,
emailRecipientText: PropTypes.string,
+ invalidRecipientText: PropTypes.func.isRequired,
};
static defaultProps = {};
@@ -203,6 +204,7 @@ export default class PulseEditChannels extends Component {
onRecipientsChange={recipients =>
this.onChannelPropertyChange(index, "recipients", recipients)
}
+ invalidRecipientText={this.props.invalidRecipientText}
/>
</div>
)}
diff --git a/frontend/src/metabase/pulse/components/RecipientPicker.jsx b/frontend/src/metabase/pulse/components/RecipientPicker.jsx
index f0b2da53aa3..f5f3751718f 100644
--- a/frontend/src/metabase/pulse/components/RecipientPicker.jsx
+++ b/frontend/src/metabase/pulse/components/RecipientPicker.jsx
@@ -2,13 +2,13 @@
import React, { Component } from "react";
import PropTypes from "prop-types";
import { t } from "ttag";
-
+import { recipientIsValid } from "metabase/lib/pulse";
+import MetabaseAnalytics from "metabase/lib/analytics";
+import MetabaseSettings from "metabase/lib/settings";
+import MetabaseUtils from "metabase/lib/utils";
import TokenField from "metabase/components/TokenField";
import UserAvatar from "metabase/components/UserAvatar";
-
-import MetabaseAnalytics from "metabase/lib/analytics";
-
-const VALID_EMAIL_REGEX = /^[^\s@]+@[^\s@]+\.[^\s@]+$/;
+import { ErrorMessage } from "./RecipientPicker.styled";
export default class RecipientPicker extends Component {
static propTypes = {
@@ -18,6 +18,7 @@ export default class RecipientPicker extends Component {
isNewPulse: PropTypes.bool.isRequired,
onRecipientsChange: PropTypes.func.isRequired,
autoFocus: PropTypes.bool,
+ invalidRecipientText: PropTypes.func.isRequired,
};
static defaultProps = {
@@ -50,49 +51,58 @@ export default class RecipientPicker extends Component {
}
render() {
- const { recipients, users, autoFocus } = this.props;
+ const { recipients, users, autoFocus, invalidRecipientText } = this.props;
+ const isValid = recipients.every(r => recipientIsValid(r));
+ const domains = MetabaseSettings.subscriptionAllowedDomains().join(", ");
+
return (
- <div className="bordered rounded" style={{ padding: "2px" }}>
- <TokenField
- value={recipients}
- options={
- users
- ? users.map(user => ({ label: user.common_name, value: user }))
- : []
- }
- onChange={this.handleOnChange}
- placeholder={
- recipients.length === 0
- ? t`Enter user names or email addresses`
- : null
- }
- autoFocus={autoFocus && recipients.length === 0}
- multi
- valueRenderer={value => value.common_name || value.email}
- optionRenderer={option => (
- <div className="flex align-center">
- <span className="text-white">
- <UserAvatar user={option.value} />
- </span>
- <span className="ml1">{option.value.common_name}</span>
- </div>
- )}
- filterOption={(option, filterString) =>
- // case insensitive search of name or email
- ~option.value.common_name
- .toLowerCase()
- .indexOf(filterString.toLowerCase()) ||
- ~option.value.email
- .toLowerCase()
- .indexOf(filterString.toLowerCase())
- }
- parseFreeformValue={inputValue => {
- if (VALID_EMAIL_REGEX.test(inputValue)) {
- return { email: inputValue };
+ <div>
+ <div className="bordered rounded" style={{ padding: "2px" }}>
+ <TokenField
+ value={recipients}
+ options={
+ users
+ ? users.map(user => ({ label: user.common_name, value: user }))
+ : []
+ }
+ onChange={this.handleOnChange}
+ placeholder={
+ recipients.length === 0
+ ? t`Enter user names or email addresses`
+ : null
+ }
+ autoFocus={autoFocus && recipients.length === 0}
+ multi
+ valueRenderer={value => value.common_name || value.email}
+ optionRenderer={option => (
+ <div className="flex align-center">
+ <span className="text-white">
+ <UserAvatar user={option.value} />
+ </span>
+ <span className="ml1">{option.value.common_name}</span>
+ </div>
+ )}
+ filterOption={(option, filterString) =>
+ // case insensitive search of name or email
+ ~option.value.common_name
+ .toLowerCase()
+ .indexOf(filterString.toLowerCase()) ||
+ ~option.value.email
+ .toLowerCase()
+ .indexOf(filterString.toLowerCase())
}
- }}
- updateOnInputBlur
- />
+ validateValue={value => recipientIsValid(value)}
+ parseFreeformValue={inputValue => {
+ if (MetabaseUtils.isEmail(inputValue)) {
+ return { email: inputValue };
+ }
+ }}
+ updateOnInputBlur
+ />
+ </div>
+ {!isValid && (
+ <ErrorMessage>{invalidRecipientText(domains)}</ErrorMessage>
+ )}
</div>
);
}
diff --git a/frontend/src/metabase/pulse/components/RecipientPicker.styled.jsx b/frontend/src/metabase/pulse/components/RecipientPicker.styled.jsx
new file mode 100644
index 00000000000..c7be949655d
--- /dev/null
+++ b/frontend/src/metabase/pulse/components/RecipientPicker.styled.jsx
@@ -0,0 +1,7 @@
+import styled from "styled-components";
+import { color } from "metabase/lib/colors";
+
+export const ErrorMessage = styled.div`
+ color: ${color("error")};
+ margin-top: 0.5rem;
+`;
diff --git a/frontend/src/metabase/query_builder/components/AlertModals.jsx b/frontend/src/metabase/query_builder/components/AlertModals.jsx
index 03d0db7b68a..2b0a4df3fc2 100644
--- a/frontend/src/metabase/query_builder/components/AlertModals.jsx
+++ b/frontend/src/metabase/query_builder/components/AlertModals.jsx
@@ -15,8 +15,7 @@ import Icon from "metabase/components/Icon";
import ChannelSetupModal from "metabase/components/ChannelSetupModal";
import ButtonWithStatus from "metabase/components/ButtonWithStatus";
import PulseEditChannels from "metabase/pulse/components/PulseEditChannels";
-import { getErrorMessage } from "metabase/components/form/FormMessage";
-import { AlertModalFooter, AlertModalError } from "./AlertModals.styled";
+import { AlertModalFooter } from "./AlertModals.styled";
import User from "metabase/entities/users";
@@ -50,6 +49,7 @@ import MetabaseAnalytics from "metabase/lib/analytics";
// types
import type { AlertType } from "metabase-lib/lib/Alert";
+import { alertIsValid } from "metabase/lib/alert";
const getScheduleFromChannel = channel =>
_.pick(
@@ -89,7 +89,6 @@ export class CreateAlertModalContent extends Component {
this.state = {
hasSeenEducationalScreen: MetabaseCookies.getHasSeenAlertSplash(),
alert: getDefaultAlert(question, user, visualizationSettings),
- formError: null,
};
}
@@ -125,19 +124,12 @@ export class CreateAlertModalContent extends Component {
} = this.props;
const { alert } = this.state;
- try {
- this.setState({ formError: null });
+ await apiUpdateQuestion(question);
+ await createAlert(alert);
+ await updateUrl(question.card(), { dirty: false });
- await apiUpdateQuestion(question);
- await createAlert(alert);
- await updateUrl(question.card(), { dirty: false });
-
- onAlertCreated();
- MetabaseAnalytics.trackEvent("Alert", "Create", alert.alert_condition);
- } catch (e) {
- this.setState({ formError: e });
- throw e;
- }
+ onAlertCreated();
+ MetabaseAnalytics.trackEvent("Alert", "Create", alert.alert_condition);
};
proceedFromEducationalScreen = () => {
@@ -156,11 +148,12 @@ export class CreateAlertModalContent extends Component {
user,
hasLoadedChannelInfo,
} = this.props;
- const { alert, hasSeenEducationalScreen, formError } = this.state;
+ const { alert, hasSeenEducationalScreen } = this.state;
const channelRequirementsMet = isAdmin
? hasConfiguredAnyChannel
: hasConfiguredEmailChannel;
+ const isValid = alertIsValid(alert);
if (hasLoadedChannelInfo && !channelRequirementsMet) {
return (
@@ -197,12 +190,10 @@ export class CreateAlertModalContent extends Component {
onAlertChange={this.onAlertChange}
/>
<AlertModalFooter>
- {formError && (
- <AlertModalError>{getErrorMessage(formError)}</AlertModalError>
- )}
<Button onClick={onCancel} className="mr2">{t`Cancel`}</Button>
<ButtonWithStatus
titleForState={{ default: t`Done` }}
+ disabled={!isValid}
onClickOperation={this.onCreateAlert}
/>
</AlertModalFooter>
@@ -318,7 +309,6 @@ export class UpdateAlertModalContent extends Component {
super();
this.state = {
modifiedAlert: props.alert,
- formError: null,
};
}
@@ -334,24 +324,16 @@ export class UpdateAlertModalContent extends Component {
} = this.props;
const { modifiedAlert } = this.state;
- try {
- this.setState({ formError: null });
-
- await apiUpdateQuestion();
- await updateAlert(modifiedAlert);
- await updateUrl(question.card(), { dirty: false });
-
- onAlertUpdated();
+ await apiUpdateQuestion();
+ await updateAlert(modifiedAlert);
+ await updateUrl(question.card(), { dirty: false });
+ onAlertUpdated();
- MetabaseAnalytics.trackEvent(
- "Alert",
- "Update",
- modifiedAlert.alert_condition,
- );
- } catch (e) {
- this.setState({ formError: e });
- throw e;
- }
+ MetabaseAnalytics.trackEvent(
+ "Alert",
+ "Update",
+ modifiedAlert.alert_condition,
+ );
};
onDeleteAlert = async () => {
@@ -369,10 +351,12 @@ export class UpdateAlertModalContent extends Component {
user,
isAdmin,
} = this.props;
- const { modifiedAlert, formError } = this.state;
+ const { modifiedAlert } = this.state;
const isCurrentUser = alert.creator.id === user.id;
const title = isCurrentUser ? t`Edit your alert` : t`Edit alert`;
+ const isValid = alertIsValid(alert);
+
// TODO: Remove PulseEdit css hack
return (
<ModalContent onClose={onCancel}>
@@ -394,12 +378,10 @@ export class UpdateAlertModalContent extends Component {
)}
<AlertModalFooter>
- {formError && (
- <AlertModalError>{getErrorMessage(formError)}</AlertModalError>
- )}
<Button onClick={onCancel} className="mr2">{t`Cancel`}</Button>
<ButtonWithStatus
titleForState={{ default: t`Save changes` }}
+ disabled={!isValid}
onClickOperation={this.onUpdateAlert}
/>
</AlertModalFooter>
@@ -680,6 +662,9 @@ export class AlertEditChannels extends Component {
setPulse={this.onSetPulse}
hideSchedulePicker={true}
emailRecipientText={t`Email alerts to:`}
+ invalidRecipientText={domains =>
+ t`You're only allowed to email alerts to addresses ending in ${domains}`
+ }
/>
</div>
</div>
diff --git a/frontend/src/metabase/query_builder/components/AlertModals.styled.jsx b/frontend/src/metabase/query_builder/components/AlertModals.styled.jsx
index ed338861bfe..c8aa54390d1 100644
--- a/frontend/src/metabase/query_builder/components/AlertModals.styled.jsx
+++ b/frontend/src/metabase/query_builder/components/AlertModals.styled.jsx
@@ -1,6 +1,5 @@
import styled from "styled-components";
import { space } from "metabase/styled-components/theme";
-import { color } from "metabase/lib/colors";
export const AlertModalFooter = styled.div`
display: flex;
@@ -8,7 +7,3 @@ export const AlertModalFooter = styled.div`
align-items: center;
margin-top: ${space(3)};
`;
-
-export const AlertModalError = styled.div`
- color: ${color("error")};
-`;
diff --git a/frontend/src/metabase/sharing/components/AddEditSidebar.jsx b/frontend/src/metabase/sharing/components/AddEditSidebar.jsx
index 89674b075e8..a383c626d51 100644
--- a/frontend/src/metabase/sharing/components/AddEditSidebar.jsx
+++ b/frontend/src/metabase/sharing/components/AddEditSidebar.jsx
@@ -56,7 +56,6 @@ export const AddEditSlackSidebar = connect(mapStateToProps)(
function _AddEditEmailSidebar({
pulse,
formInput,
- formError,
channel,
channelSpec,
users,
@@ -75,10 +74,11 @@ function _AddEditEmailSidebar({
handleArchive,
setPulseParameters,
}) {
+ const isValid = dashboardPulseIsValid(pulse, formInput.channels);
+
return (
<Sidebar
- closeIsDisabled={!dashboardPulseIsValid(pulse, formInput.channels)}
- formError={formError}
+ closeIsDisabled={!isValid}
onClose={handleSave}
onCancel={onCancel}
>
@@ -99,6 +99,9 @@ function _AddEditEmailSidebar({
onRecipientsChange={recipients =>
onChannelPropertyChange("recipients", recipients)
}
+ invalidRecipientText={domains =>
+ t`You're only allowed to email subscriptions to addresses ending in ${domains}`
+ }
/>
</div>
{channelSpec.fields && (
@@ -132,7 +135,7 @@ function _AddEditEmailSidebar({
testPulse={testPulse}
normalText={t`Send email now`}
successText={t`Email sent`}
- disabled={channel.recipients.length === 0}
+ disabled={!isValid}
/>
</div>
{PLUGIN_DASHBOARD_SUBSCRIPTION_PARAMETERS_SECTION_OVERRIDE.Component ? (
@@ -188,7 +191,6 @@ function _AddEditEmailSidebar({
_AddEditEmailSidebar.propTypes = {
pulse: PropTypes.object.isRequired,
formInput: PropTypes.object.isRequired,
- formError: PropTypes.object,
channel: PropTypes.object.isRequired,
channelSpec: PropTypes.object.isRequired,
users: PropTypes.array,
@@ -271,7 +273,6 @@ function getConfirmItems(pulse) {
function _AddEditSlackSidebar({
pulse,
formInput,
- formError,
channel,
channelSpec,
parameters,
@@ -287,10 +288,11 @@ function _AddEditSlackSidebar({
handleArchive,
setPulseParameters,
}) {
+ const isValid = dashboardPulseIsValid(pulse, formInput.channels);
+
return (
<Sidebar
- closeIsDisabled={!dashboardPulseIsValid(pulse, formInput.channels)}
- formError={formError}
+ closeIsDisabled={!isValid}
onClose={handleSave}
onCancel={onCancel}
>
@@ -331,7 +333,7 @@ function _AddEditSlackSidebar({
testPulse={testPulse}
normalText={t`Send to Slack now`}
successText={t`Slack sent`}
- disabled={channel.details.channel === undefined}
+ disabled={!isValid}
/>
</div>
{PLUGIN_DASHBOARD_SUBSCRIPTION_PARAMETERS_SECTION_OVERRIDE.Component ? (
@@ -371,7 +373,6 @@ function _AddEditSlackSidebar({
_AddEditSlackSidebar.propTypes = {
pulse: PropTypes.object.isRequired,
formInput: PropTypes.object.isRequired,
- formError: PropTypes.object,
channel: PropTypes.object.isRequired,
channelSpec: PropTypes.object.isRequired,
users: PropTypes.array,
diff --git a/frontend/src/metabase/sharing/components/SharingSidebar.jsx b/frontend/src/metabase/sharing/components/SharingSidebar.jsx
index 196d051e2b7..8390a664022 100644
--- a/frontend/src/metabase/sharing/components/SharingSidebar.jsx
+++ b/frontend/src/metabase/sharing/components/SharingSidebar.jsx
@@ -120,7 +120,6 @@ class SharingSidebar extends React.Component {
// use this to know where to go "back" to
returnMode: [],
isSaving: false,
- formError: null,
};
static propTypes = {
@@ -231,12 +230,10 @@ class SharingSidebar extends React.Component {
);
try {
- this.setState({ isSaving: true, formError: null });
+ this.setState({ isSaving: true });
await this.props.updateEditingPulse(cleanedPulse);
await this.props.saveEditingPulse();
this.setState({ editingMode: "list-pulses", returnMode: [] });
- } catch (e) {
- this.setState({ formError: e });
} finally {
this.setState({ isSaving: false });
}
@@ -283,7 +280,7 @@ class SharingSidebar extends React.Component {
};
render() {
- const { editingMode, formError } = this.state;
+ const { editingMode } = this.state;
const {
pulse,
pulses,
@@ -329,7 +326,6 @@ class SharingSidebar extends React.Component {
<AddEditEmailSidebar
pulse={pulse}
formInput={formInput}
- formError={formError}
channel={channel}
channelSpec={channelSpec}
handleSave={this.handleSave}
@@ -372,7 +368,6 @@ class SharingSidebar extends React.Component {
<AddEditSlackSidebar
pulse={pulse}
formInput={formInput}
- formError={formError}
channel={channel}
channelSpec={channelSpec}
handleSave={this.handleSave}
diff --git a/frontend/test/metabase/lib/pulse.unit.spec.js b/frontend/test/metabase/lib/pulse.unit.spec.js
index bd61e13f8ce..748972bb6eb 100644
--- a/frontend/test/metabase/lib/pulse.unit.spec.js
+++ b/frontend/test/metabase/lib/pulse.unit.spec.js
@@ -1,82 +1,117 @@
+import MetabaseSettings from "metabase/lib/settings";
import {
- getPulseParameters,
getActivePulseParameters,
+ getPulseParameters,
+ recipientIsValid,
} from "metabase/lib/pulse";
-describe("metabase/lib/pulse", () => {
- describe("getPulseParameters", () => {
- it("returns a pulse's parameters", () => {
- expect(
- getPulseParameters({
- parameters: [{ id: "foo", value: ["foo"] }],
- }),
- ).toEqual([{ id: "foo", value: ["foo"] }]);
- });
+describe("recipientIsValid", () => {
+ let originalDomains;
- it("defaults to an empty array", () => {
- expect(getPulseParameters()).toEqual([]);
- expect(getPulseParameters({})).toEqual([]);
- });
+ beforeEach(() => {
+ originalDomains = MetabaseSettings.get("subscription-allowed-domains");
});
- describe("getActivePulseParameters", () => {
- let pulse;
- let parametersList;
- beforeEach(() => {
- pulse = {
- parameters: [
- {
- id: "no default value",
- value: ["foo"],
- },
- {
- id: "overridden default value",
- default: ["bar"],
- value: ["baz"],
- },
- { id: "does not exist", value: ["does not exist"] },
- { id: "null value that should be overridden", value: null },
- ],
- };
+ afterEach(() => {
+ MetabaseSettings.set("subscription-allowed-domains", originalDomains);
+ });
- parametersList = [
- {
- id: "no default value",
- },
- { id: "unused", value: ["unused"] },
+ it("should be valid for every metabase user", () => {
+ const recipient = { id: 1, email: "user@metabase.example" };
+ MetabaseSettings.set("subscription-allowed-domains", "metabase.test");
+ expect(recipientIsValid(recipient)).toBeTruthy();
+ });
- { id: "foo" },
- { id: "overridden default value", default: ["bar"] },
- { id: "unadded default value", default: [123] },
- {
- id: "null value that should be overridden",
- default: ["not null value"],
- },
- ];
- });
+ it("should be valid when approved domains are not set", () => {
+ const recipient = { email: "user@metabase.example" };
+ expect(recipientIsValid(recipient)).toBeTruthy();
+ });
+
+ it("should not be valid for a recipient with another domain", () => {
+ const recipient = { email: "user@metabase.example" };
+ MetabaseSettings.set("subscription-allowed-domains", "metabase.test");
+ expect(recipientIsValid(recipient)).toBeFalsy();
+ });
+
+ it("should be valid for a recipient with the specified domain", () => {
+ const recipient = { email: "user@metabase.test" };
+ MetabaseSettings.set("subscription-allowed-domains", "metabase.test");
+ expect(recipientIsValid(recipient)).toBeTruthy();
+ });
+});
- it("should return a list of parameters that are applied to the pulse data", () => {
- expect(getActivePulseParameters(pulse, parametersList)).toEqual([
+describe("getPulseParameters", () => {
+ it("returns a pulse's parameters", () => {
+ expect(
+ getPulseParameters({
+ parameters: [{ id: "foo", value: ["foo"] }],
+ }),
+ ).toEqual([{ id: "foo", value: ["foo"] }]);
+ });
+
+ it("defaults to an empty array", () => {
+ expect(getPulseParameters()).toEqual([]);
+ expect(getPulseParameters({})).toEqual([]);
+ });
+});
+
+describe("getActivePulseParameters", () => {
+ let pulse;
+ let parametersList;
+ beforeEach(() => {
+ pulse = {
+ parameters: [
{
id: "no default value",
value: ["foo"],
},
{
- default: ["bar"],
id: "overridden default value",
+ default: ["bar"],
value: ["baz"],
},
- {
- default: [123],
- id: "unadded default value",
- value: [123],
- },
- {
- default: ["not null value"],
- id: "null value that should be overridden",
- value: ["not null value"],
- },
- ]);
- });
+ { id: "does not exist", value: ["does not exist"] },
+ { id: "null value that should be overridden", value: null },
+ ],
+ };
+
+ parametersList = [
+ {
+ id: "no default value",
+ },
+ { id: "unused", value: ["unused"] },
+
+ { id: "foo" },
+ { id: "overridden default value", default: ["bar"] },
+ { id: "unadded default value", default: [123] },
+ {
+ id: "null value that should be overridden",
+ default: ["not null value"],
+ },
+ ];
+ });
+
+ it("should return a list of parameters that are applied to the pulse data", () => {
+ expect(getActivePulseParameters(pulse, parametersList)).toEqual([
+ {
+ id: "no default value",
+ value: ["foo"],
+ },
+ {
+ default: ["bar"],
+ id: "overridden default value",
+ value: ["baz"],
+ },
+ {
+ default: [123],
+ id: "unadded default value",
+ value: [123],
+ },
+ {
+ default: ["not null value"],
+ id: "null value that should be overridden",
+ value: ["not null value"],
+ },
+ ]);
});
});
diff --git a/frontend/test/metabase/scenarios/sharing/approved-domains.cy.spec.js b/frontend/test/metabase/scenarios/sharing/approved-domains.cy.spec.js
index 765450aafce..3b12078d0ef 100644
--- a/frontend/test/metabase/scenarios/sharing/approved-domains.cy.spec.js
+++ b/frontend/test/metabase/scenarios/sharing/approved-domains.cy.spec.js
@@ -1,23 +1,24 @@
import {
+ describeWithToken,
+ modal,
restore,
setupSMTP,
- describeWithToken,
sidebar,
} from "__support__/e2e/cypress";
const allowedDomain = "metabase.test";
const deniedDomain = "metabase.example";
-const email = "mailer@" + deniedDomain;
+const allowedEmail = `mailer@${allowedDomain}`;
+const deniedEmail = `mailer@${deniedDomain}`;
+const subscriptionError = `You're only allowed to email subscriptions to addresses ending in ${allowedDomain}`;
+const alertError = `You're only allowed to email alerts to addresses ending in ${allowedDomain}`;
-const errorMessage = `You cannot create new subscriptions for the domain "${deniedDomain}". Allowed domains are: ${allowedDomain}`;
-
-describeWithToken("scenarios > alert (EE)", () => {
+describeWithToken("scenarios > sharing > approved domains (EE)", () => {
beforeEach(() => {
restore();
cy.signInAsAdmin();
-
setupSMTP();
- setAllowedEmailDomains(allowedDomain);
+ setAllowedDomains();
});
it("should validate approved email domains for a question alert", () => {
@@ -26,46 +27,80 @@ describeWithToken("scenarios > alert (EE)", () => {
cy.icon("bell").click();
cy.findByText("Set up an alert").click();
- addEmailRecipient(email);
+ cy.findByText("Email alerts to:")
+ .parent()
+ .within(() => addEmailRecipient(deniedEmail));
+ cy.button("Done").should("be.disabled");
+ cy.findByText(alertError);
+ });
+
+ it("should validate approved email domains for a question alert in the audit app", () => {
+ cy.visit("/question/1");
+ cy.icon("bell").click();
+ cy.findByText("Set up an alert").click();
cy.button("Done").click();
- cy.findByText(errorMessage);
+ cy.findByText("Your alert is all set up.");
+
+ cy.visit("/admin/audit/subscriptions/alerts");
+ cy.findByText("1").click();
+
+ modal().within(() => {
+ addEmailRecipient(deniedEmail);
+
+ cy.button("Update").should("be.disabled");
+ cy.findByText(alertError);
+ });
});
it("should validate approved email domains for a dashboard subscription (metabase#17977)", () => {
cy.visit("/dashboard/1");
-
cy.icon("share").click();
cy.findByText("Dashboard subscriptions").click();
cy.findByText("Email it").click();
- cy.findByPlaceholderText("Enter user names or email addresses")
- .click()
- .type(`${email}`)
- .blur();
-
sidebar().within(() => {
+ addEmailRecipient(deniedEmail);
+
// Reproduces metabase#17977
- cy.findByText("Send email now").click();
- cy.findByText("Sending failed");
+ cy.button("Send email now").should("be.disabled");
+ cy.button("Done").should("be.disabled");
+ cy.findByText(subscriptionError);
+ });
+ });
+
+ it("should validate approved email domains for a dashboard subscription in the audit app", () => {
+ cy.visit("/dashboard/1");
+ cy.icon("share").click();
+ cy.findByText("Dashboard subscriptions").click();
+ cy.findByText("Email it").click();
+ sidebar().within(() => {
+ addEmailRecipient(allowedEmail);
cy.button("Done").click();
- cy.findByText(errorMessage);
+ });
+
+ cy.visit("/admin/audit/subscriptions/subscriptions");
+ cy.findByText("1").click();
+
+ modal().within(() => {
+ addEmailRecipient(deniedEmail);
+
+ cy.button("Update").should("be.disabled");
+ cy.findByText(subscriptionError);
});
});
});
function addEmailRecipient(email) {
- cy.findByText("Email alerts to:")
- .parent()
- .find("input")
+ cy.get("input")
.click()
.type(`${email}`)
.blur();
}
-function setAllowedEmailDomains(domains) {
+function setAllowedDomains() {
cy.request("PUT", "/api/setting/subscription-allowed-domains", {
- value: domains,
+ value: allowedDomain,
});
}
--
GitLab