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.

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

EOP-164: add support pages for MTA-STS in Momentum #760

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open

Conversation

juliebin
Copy link
Contributor

@juliebin juliebin commented May 7, 2024

  • Give your PR a recognizable title. For example: "FE-123: Add new prop to component" or "Resolve Issue Create new-pricing-increases-costs #123: Fix bug in component"
  • Your PR title will be visible in changelogs

What Changed

  • What changes does this PR propose?
  • Provide screenshots or screen recordings for any visual changes.

How To Test or Verify

  • Describe any steps that may help reviewers verify changes.
  • Anything beyond basic unit testing, such as assistive tech usage, or special interactions.

PR Checklist

Below are some checklists to follow for the correct procedure in different circumstance. The first list ("All PRs Checklist") should be followed for ALL PRs. The next 2 are additive to this list depending on what type of PR you are using.

For example: If you are submitting a content change to one of the support documents, your checklist would include the:

  • "All PRs Checklist"
  • AND the "Content Changes Checklist

If you are submitting a feature addition, enhancement, or bug fix, your checklist would include the:

  • "All PRs Checklist"
  • AND the "Development Changes Checklist"

All PRs Checklist

  • Give your pull request a meaningful name.
  • Use lowercase filenames.
  • Apply at least one team label according to which team is the content expert (ie. team-FE or team-SAZ)
  • Pull request approval from the FE team or content experts (see label applied above) that isn't the content creator.

Content Changes Checklist

  • Check that your article looks correct in the preview here or in a Netlify deploy preview.
  • Check the links in your article.
  • Check the images in your article (if there are any)
  • Check to make sure you are using markdown appropriately as outlined in examples/article.md in the root of the project directory and on the momentum doc's preface article
  • Check to make sure the Copy and Tone Guidelines are followed.

Development Changes Checklist (some checks are automatic github actions and will not be listed here. ie. "all tests pass")

  • The appropriate tests are created in cypress/ directory in the root of the project
  • The lighthouse score is passing according to the FE Support Docs' Service Outline SLI/SLOs

Copy link

netlify bot commented May 7, 2024

Deploy Preview for support-docs ready!

Name Link
🔨 Latest commit c1c29b3
🔍 Latest deploy log https://app.netlify.com/sites/support-docs/deploys/66719074e70fbc00084bf471
😎 Deploy Preview https://deploy-preview-760--support-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

content/momentum/4/config/mta-sts/enable-mta-sts.md Outdated Show resolved Hide resolved
content/momentum/4/mta-sts.md Outdated Show resolved Hide resolved
content/momentum/4/mta-sts.md Outdated Show resolved Hide resolved
content/momentum/4/mta-sts.md Show resolved Hide resolved
@@ -0,0 +1,27 @@
---
Copy link
Contributor

Choose a reason for hiding this comment

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

On this page, clicking the mta-sts link in the breadcrumb gives me a 404. Can we fix it? Please review breadcrumbs in all new pages.

content/momentum/4/mta-sts.md Show resolved Hide resolved

### Example 1: Recipient domain enforces MTA-STS and MX is allowed as per the MTA-STS policy

> **MTA-STS policy retrieved from https://mta-sts.domain.com/.well-known/mta-sts.txt**
Copy link
Contributor

Choose a reason for hiding this comment

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

(nit) These (bogus) links show up as links in the doc. Is there a way to defeat that (e.g. by marking as code?), or do we care?

- [mta_sts_dns_cname_max_depth](/momentum/4/config/mta-sts/mta-sts-dns-cname-max-depth)
- [mta_sts_policy_store](/momentum/4/config/mta-sts/mta-sts-policy-store)

## Related [Debug_Flags](/momentum/4/config/ref-debug-flags)
Copy link
Contributor

Choose a reason for hiding this comment

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

In this page, the default list for ERROR and CRITICAL list most but not all of the types, plus ALL. IMO, it should just say ALL.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I consider it serves not only "default" settings, but an example of listing the logging of interest. A partial list + ALL won't change the final value of the "default".
The page lists some there and followed by

Use ALL instead of listing each service debug flag.

I don't have a strong opinion to change it.

checks and encryption for delivering email. RFC 8461 describes the use of MTA-STS for improving
SMTP security between MTAs.

If the per-domain config option [use_mta_sts](/momentum/4/config/mta-sts/use-mta-sts) is set to `true`,
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems to ignore the global-level option. Maybe "global or per-domain"?


When this option is `true`, the MTA-STS policy mode will be considered along with the configuration
values for [TLS](/momentum/4/config/ref-tls) and [TLS_Verify](/momentum/4/config/tls-verify).
The more strict policy will apply. E.g. When the MTA-STS policy mode is `enforce`, regardless the
Copy link
Contributor

Choose a reason for hiding this comment

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

"regardless of the"

When the MTA-STS policy mode is `none`, the policy will be ignored and the configuration values for
`TLS` and `TLS_Verify` will be respected.

The default value is `true` when the domain level `use_mta_sts` is `true`.
Copy link
Contributor

Choose a reason for hiding this comment

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

how about "when use_mta_sts is true for a domain"?

<a name="config.use-mta-sts"></a>
## Name

use_mta_sts — specify whether Momentum should do MTA-STS policy fetching for a domain or not
Copy link
Contributor

Choose a reason for hiding this comment

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

IMO, the "or not" is implied and not needed

@@ -44,6 +44,8 @@ description: "This section documents all the non module specific console command
| [message fail quiet](/momentum/4/console-commands/message-fail-quiet) | fail a message and do not create a non-delivery receipt (NDR) |
| [message retry](/momentum/4/console-commands/message-retry) | perform an immediate delivery attempt on a message |
| [module](/momentum/4/console-commands/4-module) | manage loaded modules online |
| [mta_sts](/momentum/4/console-commands/mta_sts) | commands to display and manage MTA-STS policy for a domain |
| [\pager](/momentum/4/console-commands/pager) | Page output of long console commands in the console. |
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't this duplicated?

@@ -0,0 +1,100 @@
---
Copy link
Contributor

Choose a reason for hiding this comment

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

Kris had raised an issue on slack about the new feature not being searchable. Let's see if the issue gets resolved just by the virtue of merging the PR into main. If not, we'll have to revisit and seek help if needed.


## Scope

`mta_sts_dns_cname_max_depth` is valid in the global scope.
Copy link
Contributor

Choose a reason for hiding this comment

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

wrong name

Comment on lines 16 to 18
`mta-sts show domain {domain name}`

`mta-sts refresh policy {domain name}`
Copy link
Contributor

Choose a reason for hiding this comment

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

typo, should be "mta_sts"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants