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

Fix error when deactivating an account #7899

Merged
merged 3 commits into from
Dec 18, 2024
Merged

Fix error when deactivating an account #7899

merged 3 commits into from
Dec 18, 2024

Conversation

aarongable
Copy link
Contributor

The RA's DeactivateAccount method expects the account provided to it by the WFE to still have status Valid. The new WFE deactivation code was hardcoding the status to Deactivated. Fix the WFE to pass the account's current status instead.

Add an integration test to confirm both the breakage and the fix. Also leave behind some TODOs to simplify this codepath further, and not require the status to be provided at all.

Part of #5554

@aarongable aarongable requested a review from a team as a code owner December 18, 2024 16:53
@aarongable aarongable requested a review from jsha December 18, 2024 16:53
jprenken
jprenken previously approved these changes Dec 18, 2024
@aarongable
Copy link
Contributor Author

I think I see the issue with the new integration test. The wfe is clearing the contacts field, but eggsampler isn't reflecting that in the in-memory account object because it unmarshals the response into the same object as it used for the request, and a missing contacts field doesn't overwrite an extant one:
https://github.com/eggsampler/acme/blob/79ababeef594ad243c4444ab292908aa9c9ba62e/account.go#L128
https://github.com/eggsampler/acme/blob/79ababeef594ad243c4444ab292908aa9c9ba62e/acme.go#L238

I'll replace that "contacts cleared" check in the integration test with a TODO.

@aarongable aarongable merged commit 0c658f2 into main Dec 18, 2024
12 checks passed
@aarongable aarongable deleted the reg-hotfix branch December 18, 2024 18:06
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.

3 participants