🐛 Updated support email verification flow #15029
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
refs https://github.com/TryGhost/Team/issues/584 The current support email verification flow uses an API endpoint as verification URL. This is a bad pattern, and also has the side effect that it shows a JSON error if something goes wrong. To fix this, this commit updates the whole flow to use the same pattern as newsletters: - You can update the `members_support_address` setting directly via the edit endpoint of settings. - Changes to that (and future 'guarded' email properties) are blocked and generate verification emails automatically. - When an email verification has been sent, the meta property `sent_email_verification` is set. Other changes: - Underlying, the implementation of email verificaton has moved from the (old) members service to the settings BREAD service. This makes it easier to add extra email addresses in settings later on that are not related to 'members'. - Because now you can update the `members_support_address` by updating the settings, the `updateMembersEmail` is no longer needed and has been removed. - The SingleUseTokenProvider threw a `UnauthorizedError` error if a token was expired or invalid. Those errors are caught by the admin app, and causes it to do a page reload (making the error message and modals invisible). To fix that, I've swapped it with a validation error. Future changes: - Existing emails that have been sent 24h before this change is applied, still use the `validateMembersEmailUpdate` API endpoint. This endpoint has not been removed for now, to not break those emails. In a future release, we should remove this. Changes to admin: TryGhost/Admin#2426
Codecov Report
@@ Coverage Diff @@
## main #15029 +/- ##
==========================================
+ Coverage 61.07% 61.47% +0.40%
==========================================
Files 593 597 +4
Lines 47557 47925 +368
Branches 4271 4322 +51
==========================================
+ Hits 29045 29462 +417
+ Misses 18463 18413 -50
- Partials 49 50 +1
Continue to review full report at Codecov.
|
This is technically a breaking change. Can we reinstate it, but map it internally using the deprecation header to a settings update to trigger the new flow? |
|
You are right. I readded the old endpoints ✅ |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
refs https://github.com/TryGhost/Team/issues/584
The current support email verification flow uses an API endpoint as verification URL inside the emails. This is a bad pattern, and also has the side effect that it shows a JSON error if something goes wrong.
To fix this, this commit updates the whole flow to use the same pattern as newsletters:
members_support_addresssetting directly via the edit endpoint of settings.sent_email_verificationis set.Other changes:
members_support_addressby updating the settings directly, so theupdateMembersEmailendpoint has been deprecated and is mapped to the new behaviour.UnauthorizedErrorerror if a token was expired or invalid. Those errors are caught by the admin app, and causes it to do a page reload (making the error message and modals invisible). To fix that, I've swapped it with a validation error.Future changes:
validateMembersEmailUpdateAPI endpoint. This endpoint has not been removed for now, to not break those emails. In a future release, we should remove this.Changes to admin: TryGhost/Admin#2426