Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

change: [M3-8415] - Hide SMTP warning for Linodes and accounts that have SMTP enabled #10991

Merged
merged 7 commits into from
Oct 8, 2024
Merged

change: [M3-8415] - Hide SMTP warning for Linodes and accounts that have SMTP enabled #10991

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
454eae2
Hide SMTP warning for Linodes and accounts that have SMTP enabled
hkhalil-akamai Sep 23, 2024
aaf7e3e
Update unit tests
hkhalil-akamai Sep 23, 2024
a88b4aa
Added changeset: Hide SMTP warning for Linodes and accounts that have…
hkhalil-akamai Sep 23, 2024
b09b99e
Added changeset: 'SMTP Enabled' account & Linode capabilities
hkhalil-akamai Sep 23, 2024
822a751
Merge remote-tracking branch 'upstream/develop' into M3-8415-hide-SMT…
hkhalil-akamai Oct 4, 2024
9d3b403
Merge remote-tracking branch 'upstream/develop' into M3-8415-hide-SMT…
hkhalil-akamai Oct 7, 2024
09c78cb
Remove magic date
hkhalil-akamai Oct 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions packages/api-v4/.changeset/pr-10991-added-1727125536609.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@linode/api-v4": Added
---

'SMTP Enabled' account & Linode capabilities ([#10991](https://github.com/linode/manager/pull/10991))
1 change: 1 addition & 0 deletions packages/api-v4/src/account/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ export type AccountCapability =
| 'Object Storage Endpoint Types'
| 'Object Storage'
| 'Placement Group'
| 'SMTP Enabled'
| 'Support Ticket Severity'
| 'Vlans'
| 'VPCs';
Expand Down
2 changes: 1 addition & 1 deletion packages/api-v4/src/linodes/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ export interface LinodeBackups {
last_successful: string | null;
}

export type LinodeCapabilities = 'Block Storage Encryption';
export type LinodeCapabilities = 'Block Storage Encryption' | 'SMTP Enabled';

export type Window =
| 'Scheduling'
Expand Down
5 changes: 5 additions & 0 deletions packages/manager/.changeset/pr-10991-changed-1727125505081.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@linode/manager": Changed
---

Hide SMTP warning for Linodes and accounts that have SMTP enabled ([#10991](https://github.com/linode/manager/pull/10991))
2 changes: 0 additions & 2 deletions packages/manager/cypress/e2e/core/helpAndSupport/open-support-ticket.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ import {
import { createTestLinode } from 'support/util/linodes';
import { cleanUp } from 'support/util/cleanup';
import { authenticate } from 'support/api/authentication';
import { MAGIC_DATE_THAT_EMAIL_RESTRICTIONS_WERE_IMPLEMENTED } from 'src/constants';
import {
mockCreateLinodeAccountLimitError,
mockGetLinodes,
Expand Down Expand Up @@ -227,7 +226,6 @@ describe('open support tickets', () => {
first_name: 'Jane',
last_name: 'Doe',
company: 'Acme Co.',
active_since: MAGIC_DATE_THAT_EMAIL_RESTRICTIONS_WERE_IMPLEMENTED,
});

const mockFormFields = {
Expand Down
5 changes: 0 additions & 5 deletions packages/manager/src/constants.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,11 +233,6 @@ export const PAYMENT_HARD_MAX = 50_000;

export const DB_ROOT_USERNAME = 'linroot';

// "In an effort to fight spam, Linode restricts outbound connections on ports 25, 465, and 587 on all Linodes for new accounts created after November 5th, 2019."
// https://www.linode.com/docs/email/best-practices/running-a-mail-server/
export const MAGIC_DATE_THAT_EMAIL_RESTRICTIONS_WERE_IMPLEMENTED =
'2022-11-30T00:00:00.000Z'; // Date of release for Manager v1.81.0.

Comment on lines -236 to -240
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we are directly using the SMTP status of accounts/Linodes, we no longer need this magic date.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great! 🚀

// The date Linode switching to Akamai (for purposes of billing)
export const AKAMAI_DATE = '2022-12-15 00:00:00';

Expand Down
5 changes: 4 additions & 1 deletion packages/manager/src/features/Linodes/LinodesDetail/LinodesDetailNavigation.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,10 @@ const LinodesDetailNavigation = () => {
tabs[getIndex()]?.title ?? 'Detail View'
}`}
/>
<SMTPRestrictionText supportLink={{ id, label: linode?.label }}>
<SMTPRestrictionText
linode={linode}
supportLink={{ id, label: linode?.label }}
>
{({ text }) =>
text !== null ? (
<DismissibleBanner
Expand Down
59 changes: 19 additions & 40 deletions packages/manager/src/features/Linodes/SMTPRestrictionText.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,14 @@
import { waitFor } from '@testing-library/react';
import * as React from 'react';

import { MAGIC_DATE_THAT_EMAIL_RESTRICTIONS_WERE_IMPLEMENTED } from 'src/constants';
import { linodeFactory } from 'src/factories';
import { accountFactory } from 'src/factories/account';
import { HttpResponse, http, server } from 'src/mocks/testServer';
import { renderWithTheme } from 'src/utilities/testHelpers';

import {
SMTPRestrictionText,
SMTPRestrictionTextProps,
accountCreatedAfterRestrictions,
} from './SMTPRestrictionText';
import { SMTPRestrictionText } from './SMTPRestrictionText';

import type { SMTPRestrictionTextProps } from './SMTPRestrictionText';

const defaultChildren = (props: { text: React.ReactNode }) => (
<span>{props.text}</span>
Expand All @@ -20,28 +18,10 @@ const props: SMTPRestrictionTextProps = {
children: defaultChildren,
};

describe('accountCreatedAfterRestrictions', () => {
it('defaults to true with bad/no input', () => {
expect(accountCreatedAfterRestrictions()).toBe(true);
expect(accountCreatedAfterRestrictions('not a date!')).toBe(true);
});

it('only returns true when the account was created after the magic date', () => {
expect(accountCreatedAfterRestrictions('2022-11-27 00:00:00Z')).toBe(false);
expect(accountCreatedAfterRestrictions('2022-11-29 23:59:59Z')).toBe(false);
expect(
accountCreatedAfterRestrictions(
MAGIC_DATE_THAT_EMAIL_RESTRICTIONS_WERE_IMPLEMENTED
)
).toBe(true);
expect(accountCreatedAfterRestrictions('2022-11-30 00:00:01Z')).toBe(true);
});
});

describe('SMTPRestrictionText component', () => {
it('should render when user account is created on or after the magic date', async () => {
it('should not render for an account with the "SMTP Enabled" capability', async () => {
const account = accountFactory.build({
active_since: MAGIC_DATE_THAT_EMAIL_RESTRICTIONS_WERE_IMPLEMENTED,
capabilities: ['SMTP Enabled'],
});

server.use(
Expand All @@ -50,22 +30,24 @@ describe('SMTPRestrictionText component', () => {
})
);

const { findByText } = renderWithTheme(
const { queryByText } = renderWithTheme(
<SMTPRestrictionText
supportLink={{ id: 0, label: 'Test Linode' }}
{...props}
/>
);

await findByText('SMTP ports may be restricted on this Linode.', {
exact: false,
});
await waitFor(() =>
expect(
queryByText('SMTP ports may be restricted on this Linode.', {
exact: false,
})
).toBeNull()
);
});

it('should not render when user account is created before the magic date', async () => {
const account = accountFactory.build({
active_since: '2022-11-27 00:00:00Z',
});
it('should not render for a Linode with the "SMTP Enabled" capability', async () => {
const account = accountFactory.build();

server.use(
http.get('*/account', () => {
Expand All @@ -75,6 +57,7 @@ describe('SMTPRestrictionText component', () => {

const { queryByText } = renderWithTheme(
<SMTPRestrictionText
linode={linodeFactory.build({ capabilities: ['SMTP Enabled'] })}
supportLink={{ id: 0, label: 'Test Linode' }}
{...props}
/>
Expand All @@ -90,9 +73,7 @@ describe('SMTPRestrictionText component', () => {
});

it('should default to not including a link to open a support ticket', () => {
const account = accountFactory.build({
active_since: MAGIC_DATE_THAT_EMAIL_RESTRICTIONS_WERE_IMPLEMENTED,
});
const account = accountFactory.build();

server.use(
http.get('*/account', () => {
Expand All @@ -108,9 +89,7 @@ describe('SMTPRestrictionText component', () => {
});

it('should include a link to open a support ticket when the prop is provided', () => {
const account = accountFactory.build({
active_since: MAGIC_DATE_THAT_EMAIL_RESTRICTIONS_WERE_IMPLEMENTED,
});
const account = accountFactory.build();

server.use(
http.get('*/account', () => {
Expand Down
38 changes: 12 additions & 26 deletions packages/manager/src/features/Linodes/SMTPRestrictionText.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,27 +3,32 @@ import * as React from 'react';
import { Link } from 'src/components/Link';
import { SupportLink } from 'src/components/SupportLink';
import { Typography } from 'src/components/Typography';
import { MAGIC_DATE_THAT_EMAIL_RESTRICTIONS_WERE_IMPLEMENTED } from 'src/constants';
import { useAccount } from 'src/queries/account/account';
import { sendLinodeCreateDocsEvent } from 'src/utilities/analytics/customEventAnalytics';

import type { Linode } from '@linode/api-v4';

export interface SMTPRestrictionTextProps {
children: (props: { text: React.ReactNode }) => React.ReactNode;
linode?: Linode;
supportLink?: {
id: number;
label: string;
};
}

export const SMTPRestrictionText = (props: SMTPRestrictionTextProps) => {
const { supportLink } = props;
const { linode, supportLink } = props;
const { data: account } = useAccount();

const displayRestrictionText =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More of a backend question: Do we know if it's possible for an account to have SMTP_ENABLED, but an individual Linode is still disabled (with net exceptions on its ports)? If that state were possible, we're currently showing the notice for the Linode, which seems like what we'd want for transparency... unless, somehow, the individual net exceptions don't always get removed when SMTP_ENABLED is added account-wide, which could be confusing for customers.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we know if it's possible for an account to have SMTP_ENABLED, but an individual Linode is still disabled

Yes, it's possible.

I was surprised to learn that there is no logic in the API that links the customer tag with the Net Filter Exceptions of a particular Linode. Adding the tag is a manual process used by support representatives when a customer requests SMTP abilities on their Linode and it is only used to keep track of which customers have had this request granted.

The availability of SMTP for a Linode is determined solely by the presence (or lack thereof) of the SMTP Net Filter Exceptions. There is also a field on the customer called 'Default Net Filter Exceptions' that is applied by default to new Linodes.

So the choice to only use the capability from the Linode object was intentional, since that is the source of truth of whether a particular Linode is SMTP-capable.

For a detailed summary of this topic, I suggest the KB article SMTP Port Restrictions.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding the tag is a manual process used by support representatives when a customer requests SMTP abilities on their Linode and it is only used to keep track of which customers have had this request granted.

Super helpful context and surprises me also.

So the choice to only use the capability from the Linode object was intentional, since that is the source of truth of whether a particular Linode is SMTP-capable.

Got it; agree with this.

linode === undefined
? !account?.capabilities.includes('SMTP Enabled')
: !linode.capabilities?.includes('SMTP Enabled');

// If there account was created before restrictions were put into place,
// there's no need to display anything.
const text = !accountCreatedAfterRestrictions(
account?.active_since
) ? null : (
const text = displayRestrictionText ? (
<Typography variant="body1">
SMTP ports may be restricted on this Linode. Need to send email? Review
our{' '}
Expand All @@ -46,26 +51,7 @@ export const SMTPRestrictionText = (props: SMTPRestrictionTextProps) => {
)}
.
</Typography>
);

// eslint-disable-next-line
return <>{props.children({ text })}</>;
};

export const accountCreatedAfterRestrictions = (_accountCreated?: string) => {
// Default to `true` for bad input.
if (!_accountCreated) {
return true;
}

const restrictionsImplemented = new Date(
MAGIC_DATE_THAT_EMAIL_RESTRICTIONS_WERE_IMPLEMENTED
).getTime();
const accountCreated = new Date(_accountCreated).getTime();

if (isNaN(accountCreated)) {
return true;
}
) : null;

return accountCreated >= restrictionsImplemented;
return props.children({ text });
};