Skip to content

Confine contact addresses to the WFE #8245

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

Merged
merged 9 commits into from
Jun 25, 2025
Merged

Conversation

aarongable
Copy link
Contributor

@aarongable aarongable commented Jun 14, 2025

Change the WFE to stop populating the Contact field of the NewRegistration requests it sends to the RA. Similarly change the WFE to ignore the Contact field of any update-account requests it receives, thereby removing all calls to the RA's UpdateRegistrationContact method.

Hoist the RA's contact validation logic into the WFE, so that we can still return errors to clients which are presenting grossly malformed contact fields, and have a first layer of protection against trying to send malformed addresses to email-exporter.

A follow-up change (after a deploy cycle) will remove the deprecated RA and SA methods.

Part of #8199

@aarongable aarongable marked this pull request as ready for review June 14, 2025 00:31
@aarongable aarongable requested a review from a team as a code owner June 14, 2025 00:31
@aarongable aarongable requested a review from jsha June 14, 2025 00:31
jprenken
jprenken previously approved these changes Jun 19, 2025
@jsha
Copy link
Contributor

jsha commented Jun 24, 2025

Note that this does mean that we're no longer performing any validation of contact addresses, since that happened in the RA. But that's fine, because we're not storing them anymore, either.

Aren't we sending them to the email exporter? It would be nice to retain the validation we have, and continue failing registration and not sending them to the exporter.

@aarongable aarongable requested a review from jprenken June 24, 2025 20:58
@aarongable
Copy link
Contributor Author

It would be nice to retain the validation we have, and continue failing registration and not sending them to the exporter.

That's a good point; I was forgetting that we only send contacts to email-exporter after account creation has succeeded, and that therefore we were implicitly still validating the addresses before exporting them. I've hoisted this validation into the WFE directly now.

Copy link
Member

@beautifulentropy beautifulentropy left a comment

Choose a reason for hiding this comment

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

Overall the change looks great. I especially like that you moved the validation up near the top of NewAccount.

jprenken
jprenken previously approved these changes Jun 24, 2025
Copy link
Contributor

@aarongable, this PR appears to contain configuration and/or SQL schema changes. Please ensure that a corresponding deployment ticket has been filed with the new values.

@aarongable aarongable merged commit e110ec9 into main Jun 25, 2025
14 checks passed
@aarongable aarongable deleted the deprecate-update-contact branch June 25, 2025 22:51
aarongable added a commit that referenced this pull request Jun 25, 2025
This integration test was removed in the early versions of
#8245, because that PR had
removed all validation of contact addresses. However, later iterations
of that PR restored (most) contact validation, so this PR restores (most
of) the TestAccountEmailError integration test.
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.

4 participants