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

Jump to bottom

feat(console): DKIM, DMARC & SPF records for custom domains #2347

Merged
merged 17 commits into from
Jun 7, 2023
Merged

feat(console): DKIM, DMARC & SPF records for custom domains #2347

merged 17 commits into from
Jun 7, 2023

Conversation

Cosmin-Parvulescu
Copy link
Contributor

@Cosmin-Parvulescu Cosmin-Parvulescu commented May 31, 2023 •
edited
Loading

Description

  • Added Email worker endpoint for getting email security DNS values so that we DRY
  • Added Email fetcher to Starbase worker so that we can query DNS values
  • Updated DNS record value fetcher to work with arrays in order to support more complex scenarios
  • Enabled custom domains on e-mails
  • Added additional env vars
  • disable pre validation fields after validated (don't show copy icons)
  • fix copy icons squeeze on long values
  • show transition animation during refresh
  • show CNAME key value
  • drop spf? set spf records to passport domains

Related Issues

Testing

  1. Added a custom domain to application
  2. Updated custom domain DNS settings to match domain-wip screen
  3. Waited a bit
  4. Revalidated
  5. Got all green lights
  6. Tried to login to profile app
  7. Used e-mail
  8. Got e-mail
  9. Checked that it matches OTP template
  10. Checked that SPF, DKIM and DMARC pass in Gmail
image
  1. Checked DNS for SPF, DKIM and DMARC in https://powerdmarc.com/analyzer/

Checklist

  • I have read the CONTRIBUTING guidelines
  • I have tested my code (manually and/or automated if applicable)
  • I have updated the documentation (if necessary)

@Cosmin-Parvulescu Cosmin-Parvulescu changed the base branch from main to feat/passport/support-designer-customized-authn-authz-screens May 31, 2023 14:44
@Cosmin-Parvulescu Cosmin-Parvulescu force-pushed the feat/passport/support-designer-customized-authn-authz-screens branch from 6416414 to 9a9abb1 Compare May 31, 2023 14:47
@Cosmin-Parvulescu Cosmin-Parvulescu force-pushed the feat/console/DKIM,-DMARC-&-SPF-records-for-custom-domains branch 2 times, most recently from 00b96f4 to 3c5b1d4 Compare June 1, 2023 07:30
@Cosmin-Parvulescu Cosmin-Parvulescu force-pushed the feat/passport/support-designer-customized-authn-authz-screens branch 2 times, most recently from 3b08b9f to a71dfef Compare June 1, 2023 08:20
@Cosmin-Parvulescu Cosmin-Parvulescu force-pushed the feat/console/DKIM,-DMARC-&-SPF-records-for-custom-domains branch from 3c5b1d4 to ff60c62 Compare June 1, 2023 11:17
@Cosmin-Parvulescu Cosmin-Parvulescu force-pushed the feat/passport/support-designer-customized-authn-authz-screens branch 2 times, most recently from 6ad0129 to a7dcb28 Compare June 2, 2023 05:32
Base automatically changed from feat/passport/support-designer-customized-authn-authz-screens to main June 2, 2023 05:39
@Cosmin-Parvulescu Cosmin-Parvulescu force-pushed the feat/console/DKIM,-DMARC-&-SPF-records-for-custom-domains branch 2 times, most recently from 0ca8389 to 8fa6523 Compare June 6, 2023 10:44
@Cosmin-Parvulescu Cosmin-Parvulescu changed the title Feat/console/dkim, dmarc & spf records for custom domains feat(console): DKIM, DMARC & SPF records for custom domains Jun 6, 2023
@Cosmin-Parvulescu Cosmin-Parvulescu self-assigned this Jun 6, 2023
@Cosmin-Parvulescu Cosmin-Parvulescu added the enhancement Indicates new feature requests label Jun 6, 2023
@Cosmin-Parvulescu Cosmin-Parvulescu force-pushed the feat/console/DKIM,-DMARC-&-SPF-records-for-custom-domains branch from 82065af to 282ddc2 Compare June 7, 2023 07:41
@Cosmin-Parvulescu Cosmin-Parvulescu marked this pull request as ready for review June 7, 2023 12:11
betimshahini
betimshahini requested changes Jun 7, 2023
View reviewed changes
.github/workflows/next-starbase.yaml
@@ -54,3 +54,4 @@ jobs:
env:
INTERNAL_CLOUDFLARE_ZONE_ID: ${{ secrets.INTERNAL_CLOUDFLARE_ZONE_ID }}
TOKEN_CLOUDFLARE_API: ${{ secrets.TOKEN_CLOUDFLARE_API }}
INTERNAL_DKIM_SELECTOR: ${{ secrets.INTERNAL_DKIM_SELECTOR }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Need to add it to secret stanza above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added

.github/workflows/release-starbase.yaml
@@ -54,3 +54,4 @@ jobs:
NODE_ENV: 'development'
INTERNAL_CLOUDFLARE_ZONE_ID: ${{ secrets.INTERNAL_CLOUDFLARE_ZONE_ID }}
TOKEN_CLOUDFLARE_API: ${{ secrets.TOKEN_CLOUDFLARE_API }}
INTERNAL_DKIM_SELECTOR: ${{ secrets.INTERNAL_DKIM_SELECTOR }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Add to secret stanza above as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added

apps/passport/app/routes/connect/email/otp.ts Outdated
@@ -70,9 +70,9 @@ export const loader: LoaderFunction = getRollupReqFunctionErrorWrapper(
// we need to figure out DKIM
// for custom domains
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this comment need to remain? Remove, if not.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed

packages/utils/getDNSRecordValue.ts Outdated
const values = packet.answers.map((a) => {
if (a.type === 'TXT') {
const strParts = new Array(a.data.length)
for (let i = 0; i < a.data.length; i++) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we do this as a for..of so we avoid any potential out-of-bounds or off-by-one errors?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

@betimshahini betimshahini merged commit 9b25567 into main Jun 7, 2023
@betimshahini betimshahini deleted the feat/console/DKIM,-DMARC-&-SPF-records-for-custom-domains branch June 7, 2023 14:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@betimshahini betimshahini betimshahini approved these changes

@maurerbot maurerbot Awaiting requested review from maurerbot

Assignees

@Cosmin-Parvulescu Cosmin-Parvulescu

Labels
enhancement Indicates new feature requests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

feat(console): DKIM, DMARC & SPF records for custom domains
2 participants
@Cosmin-Parvulescu @betimshahini