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

Restrict email reply-to domains to domains used by current team members #2060

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

Conversation

whabanks
Copy link
Contributor

Summary | Résumé

This PR updates the email reply-to validation, allowing only email domains matching the email domains of current team members.

Test instructions | Instructions pour tester la modification

Add a reply-to with an invalid domain

  1. Add a new reply to where the email's domain is not used by any current team members
  2. Note the error message

Add a reply-to with a valid domain

  1. Add a new reply to where the email domain is used but one of the current team members. Use a +someString alias to identify the notification
  2. When you reach the "verifying this email address" screen, go to the notification table in your local DB and find the verification notification by searching for the alias you used in step 1
  3. Change the status to delivered
  4. Note that the process completes as expected

Edit an existing reply-to to an invalid domain

  1. Edit the reply-to you created in the steps above to a domain not used by any team members
  2. Note the error message preventing you from moving forward

Edit an existing reply-to to a valid domain

  1. Edit the reply-to again and use a domain used by one of the service's team members, again using a +someString alias
  2. Repeat the steps above, setting the notification to delivered in your local DB to complete the verification process
  3. Note the reply-to was successfully updated.

@whabanks whabanks changed the title Restrict email reply-tos to domains of current team members Restrict email reply-to domains to domains used by current team members Jan 28, 2025
return
email_domain = field.data.split("@")[-1]

if email_domain not in g.team_member_email_domains:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Content to be reviewed / reworked.

Copy link

Copy link
Member

@andrewleith andrewleith left a comment

Choose a reason for hiding this comment

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

Didn't review the code yet but wrote a query to see the state of this right now. There are 55 services whose reply-to's don't match any of their members. If we were to safelist @canada.ca that would get us down to 35.

Safelisting *.gc.ca, gets us down to 21.

WITH service_domains AS (
  SELECT 
    services.id as service_id,
    services.name as service_name,
    STRING_AGG(DISTINCT SUBSTRING(users.email_address FROM '@(.*)$'), ', ') as user_email_domains,
    STRING_AGG(DISTINCT SUBSTRING(service_email_reply_to.email_address FROM '@(.*)$'), ', ') as reply_to_domains,
    BOOL_OR(
      SUBSTRING(service_email_reply_to.email_address FROM '@(.*)$') = 
      SUBSTRING(users.email_address FROM '@(.*)$')
    ) as has_matching_domain
  FROM users
  JOIN user_to_service ON users.id = user_to_service.user_id
  JOIN services ON services.id = user_to_service.service_id
  JOIN service_email_reply_to ON services.id = service_email_reply_to.service_id
  GROUP BY services.id, services.name
  HAVING COUNT(service_email_reply_to.id) > 0
)
SELECT 
  service_name,
  user_email_domains,
  reply_to_domains,
  CASE 
    WHEN has_matching_domain THEN 'Yes'
    ELSE 'No'
  END as domains_match
FROM service_domains
where has_matching_domain = False
ORDER BY domains_match, service_name

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

Successfully merging this pull request may close these issues.

2 participants