mirror of
https://github.com/warmbly/warmbly.git
synced 2026-10-08 16:02:18 +00:00
feat: release a cloud-managed mirror's cloud link when it is deleted through the mailbox delete, using the delete-only revocation that never calls back into the mailbox delete, so the mailbox returns to the cloud workspace instead of staying claimed by an instance that no longer holds it (issue #582)
This commit is contained in:
tunglambk
@@ -230,7 +230,7 @@ The `/unibox/drafts` endpoints hold autosaved compose drafts, scoped to the call
|
||||
|
||||
`PATCH /emails/:id` accepts `save_to_sent` (boolean) on SMTP/IMAP mailboxes: when true, which is the default, the worker files a copy of each outbound message in the mailbox's Sent folder. It has no effect on Gmail and Outlook mailboxes, whose APIs file their own copy. See [keeping a copy of sent mail](/guides/mailboxes/#keeping-a-copy-of-sent-mail).
|
||||
|
||||
`DELETE /emails/:id` revokes any Warmbly Cloud pool enrollment before removing the local mailbox. If the cloud cannot confirm that it released the stored credential, the delete returns `409 mailbox_cloud_unenroll_failed` and keeps the mailbox record so the request can be retried safely. Warmbly attempts to restore the mailbox onto its worker immediately, and the worker reconciler may restore it later if that attempt fails. See [mailboxes](/guides/mailboxes/).
|
||||
`DELETE /emails/:id` releases the mailbox on Warmbly Cloud before removing the local mailbox. An enrolled mailbox's stored credential comes out of the pool, and a cloud-managed mirror's claim is released so the mailbox returns to the cloud workspace and can be adopted again. If the cloud cannot confirm the release, the delete returns `409 mailbox_cloud_unenroll_failed` and keeps the mailbox record so the request can be retried safely. Warmbly attempts to restore the mailbox onto its worker immediately, and the worker reconciler may restore it later if that attempt fails. See [mailboxes](/guides/mailboxes/).
|
||||
|
||||
`GET /unibox` and `GET /unibox/thread` return message previews: each row carries `snippet`, a one-line summary, not the message body. Read a full message with `GET /unibox/:id`, which returns `body_plain` plus `body_html`. The HTML is sanitized before it leaves the API (scripts, event handlers, embedded frames, and unsafe URL schemes are removed), so it is safe to render, and links carry `target="_blank"` with `rel="noopener"`. Warmbly open-tracking pixels are also removed from this display copy, including quoted history, so rendering it does not record a campaign open. Stored and delivered copies keep their pixels. `body_truncated` is `true` on the rare message whose stored body could not be read, where `body_plain` falls back to the snippet.
|
||||
|
||||
|
||||
@@ -240,9 +240,9 @@ One conflict carries its own `code`. A contact's email address has to be free, s
|
||||
| `code` | Status | Meaning |
|
||||
|--------|--------|---------|
|
||||
| `contact_email_taken` | 409 | The address given to [update a contact](/api/reference/contacts/#update-a-contact) already belongs to another contact |
|
||||
| `mailbox_cloud_unenroll_failed` | 409 | The mailbox is enrolled in [Warmbly Cloud](/guides/warmbly-cloud/) and its enrollment could not be revoked, so `DELETE /emails/{id}` would leave its password in the pool. The mailbox record remains, and restoration onto its worker is attempted |
|
||||
| `mailbox_cloud_unenroll_failed` | 409 | The mailbox is linked to [Warmbly Cloud](/guides/warmbly-cloud/) and its link could not be released, so `DELETE /emails/{id}` would leave Warmbly Cloud holding its credential or its claim on it. The mailbox record remains, and restoration onto its worker is attempted |
|
||||
|
||||
`mailbox_cloud_unenroll_failed` is a self-hosted instance losing contact with Warmbly Cloud mid-delete. Deleting an enrolled mailbox has to revoke its enrollment before the record goes, because the pool holds the mailbox's own SMTP/IMAP credentials and that record is the only thing that knows the enrollment exists. The mailbox record remains. Warmbly attempts to restore it onto its worker immediately, and the worker reconciler may restore it later if that attempt fails. Retry the delete once the instance can reach the cloud again. Unenrolling under **Settings > Warmbly Cloud** first does not help: it makes the same call.
|
||||
`mailbox_cloud_unenroll_failed` is a self-hosted instance losing contact with Warmbly Cloud mid-delete. Deleting an enrolled mailbox has to revoke its enrollment before the record goes, because the pool holds the mailbox's own SMTP/IMAP credentials and that record is the only thing that knows the enrollment exists. The same call releases a cloud-managed mirror before the record goes, so the mailbox returns to the cloud workspace and can be adopted again instead of staying claimed by an instance that no longer keeps it. The mailbox record remains. Warmbly attempts to restore it onto its worker immediately, and the worker reconciler may restore it later if that attempt fails. Retry the delete once the instance can reach the cloud again. Unenrolling under **Settings > Warmbly Cloud** first does not help: it makes the same call.
|
||||
|
||||
### 422 Unprocessable
|
||||
|
||||
|
||||
@@ -644,7 +644,7 @@ Auth: **Scope** `WRITE_EMAILS` · **Org permission** `manage_emails`
|
||||
|
||||
Disconnects and deletes a mailbox. It is removed from all warmup pools and an account-disconnected event fans out.
|
||||
|
||||
Nothing is removed unless the two steps that cannot be repaired afterwards succeed first: the machine syncing the mailbox is told to drop it, and a mailbox enrolled in [Warmbly Cloud](/guides/warmbly-cloud/) has its enrollment revoked, which is what takes its stored credentials out of the pool. A failure at either point puts the mailbox back as it was and is safe to retry. If the record itself then fails to delete, the mailbox remains but its enrollment is already revoked, so its warmup moves back to this instance until the delete is retried.
|
||||
Nothing is removed unless the two steps that cannot be repaired afterwards succeed first: the machine syncing the mailbox is told to drop it, and the mailbox's [Warmbly Cloud](/guides/warmbly-cloud/) link is released, which takes an enrolled mailbox's stored credentials out of the pool and returns a cloud-managed mirror to the cloud workspace. A failure at either point puts the mailbox back as it was and is safe to retry. If the record itself then fails to delete, the mailbox remains but its link is already released, so its warmup moves back to this instance until the delete is retried.
|
||||
|
||||
Auth: **Scope** `WRITE_EMAILS` · **Org permission** `manage_emails`
|
||||
|
||||
@@ -658,7 +658,7 @@ Auth: **Scope** `WRITE_EMAILS` · **Org permission** `manage_emails`
|
||||
|
||||
| Status | `code` | Meaning |
|
||||
|--------|--------|---------|
|
||||
| `409` | `mailbox_cloud_unenroll_failed` | The mailbox is enrolled in Warmbly Cloud and the enrollment could not be revoked. See [error codes](/api/error-codes/#409-conflict) |
|
||||
| `409` | `mailbox_cloud_unenroll_failed` | The mailbox's Warmbly Cloud link could not be released. See [error codes](/api/error-codes/#409-conflict) |
|
||||
| `503` | `mailbox_worker_unreachable` | The machine syncing the mailbox could not be told to drop it. See [error codes](/api/error-codes/#mailbox_worker_unreachable) |
|
||||
|
||||
## Send from a mailbox
|
||||
|
||||
@@ -9,6 +9,7 @@ import (
|
||||
|
||||
"github.com/google/uuid"
|
||||
|
||||
"github.com/warmbly/warmbly/internal/app/email"
|
||||
"github.com/warmbly/warmbly/internal/errx"
|
||||
"github.com/warmbly/warmbly/internal/models"
|
||||
"github.com/warmbly/warmbly/internal/repository"
|
||||
@@ -195,3 +196,72 @@ func TestRevokeForDeleteRefusesAForeignMailbox(t *testing.T) {
|
||||
t.Errorf("called the cloud for a foreign mailbox: %v", *f.deletes)
|
||||
}
|
||||
}
|
||||
|
||||
// stubEmailDeletes records any call the revocation makes into the mailbox
|
||||
// service, which is what would make it recursive.
|
||||
type stubEmailDeletes struct {
|
||||
email.EmailService
|
||||
|
||||
deletes *[]string
|
||||
onDelete func()
|
||||
}
|
||||
|
||||
func (s stubEmailDeletes) Delete(context.Context, string, string) *errx.Error {
|
||||
*s.deletes = append(*s.deletes, "delete")
|
||||
if s.onDelete != nil {
|
||||
s.onDelete()
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// RevokeForDelete is the leaf that keeps the managed delete from recursing, so
|
||||
// it must not call back into the mailbox service at all.
|
||||
func TestRevokeForDeleteDoesNotCallIntoTheMailboxDelete(t *testing.T) {
|
||||
f := newRevokeFixture(t, http.StatusNoContent)
|
||||
calls := &[]string{}
|
||||
f.svc.emailSvc = stubEmailDeletes{deletes: calls}
|
||||
|
||||
if xerr := f.svc.RevokeForDelete(context.Background(), f.org, f.account); xerr != nil {
|
||||
t.Fatalf("RevokeForDelete: %v", xerr)
|
||||
}
|
||||
if len(*calls) != 0 {
|
||||
t.Fatalf("the revocation called into the mailbox service: %v", *calls)
|
||||
}
|
||||
}
|
||||
|
||||
// Unenrolling a managed mirror releases the cloud link and then deletes the
|
||||
// mirror. The delete revokes too, so the cloud answers the repeat call with
|
||||
// pool_link_mailbox_not_found, which is tolerated: neither call loops.
|
||||
func TestUnenrollReleasesTheCloudBeforeDeletingAManagedMirror(t *testing.T) {
|
||||
f := newRevokeFixture(t, http.StatusNoContent)
|
||||
deletes := &[]string{}
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
*deletes = append(*deletes, r.URL.Path)
|
||||
if len(*deletes) == 1 {
|
||||
w.WriteHeader(http.StatusNoContent)
|
||||
return
|
||||
}
|
||||
w.WriteHeader(http.StatusNotFound)
|
||||
_, _ = w.Write([]byte(`{"code":"pool_link_mailbox_not_found","message":"That mailbox is not enrolled."}`))
|
||||
}))
|
||||
t.Cleanup(srv.Close)
|
||||
f.repo.link.CloudURL = srv.URL
|
||||
f.deletes = deletes
|
||||
|
||||
calls := &[]string{}
|
||||
f.svc.emailSvc = stubEmailDeletes{
|
||||
deletes: calls,
|
||||
onDelete: func() { _ = f.svc.RevokeForDelete(context.Background(), f.org, f.account) },
|
||||
}
|
||||
f.repo.mailbox.Managed = true
|
||||
|
||||
if xerr := f.svc.Unenroll(context.Background(), f.org, f.account); xerr != nil {
|
||||
t.Fatalf("Unenroll: %v", xerr)
|
||||
}
|
||||
if len(*calls) != 1 {
|
||||
t.Fatalf("the mirror was deleted %d times, want 1", len(*calls))
|
||||
}
|
||||
if len(*deletes) != 2 || (*deletes)[0] != (*deletes)[1] {
|
||||
t.Fatalf("cloud deletes = %v, want the mailbox path twice", *deletes)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -98,7 +98,8 @@ type Service interface {
|
||||
ListMailboxes(ctx context.Context, orgID uuid.UUID) ([]models.CloudLinkMailboxRow, *errx.Error)
|
||||
Enroll(ctx context.Context, orgID, accountID uuid.UUID) (*models.CloudLinkMailboxRow, *errx.Error)
|
||||
Unenroll(ctx context.Context, orgID, accountID uuid.UUID) *errx.Error
|
||||
// RevokeForDelete confirms the cloud no longer holds the mailbox credential.
|
||||
// RevokeForDelete releases the mailbox on the cloud, credential and link
|
||||
// alike, without ever calling back into the email service.
|
||||
RevokeForDelete(ctx context.Context, orgID, accountID uuid.UUID) *errx.Error
|
||||
SetLifecycle(ctx context.Context, orgID, accountID uuid.UUID, action string) (*models.CloudLinkMailboxRow, *errx.Error)
|
||||
|
||||
@@ -510,7 +511,9 @@ func (s *service) Unenroll(ctx context.Context, orgID, accountID uuid.UUID) *err
|
||||
return nil
|
||||
}
|
||||
|
||||
// RevokeForDelete removes the remote credential before local deletion makes retries impossible.
|
||||
// RevokeForDelete releases the mailbox on the cloud before local deletion makes
|
||||
// retries impossible. It is a leaf, so the email service can call it for a
|
||||
// managed mirror without recursing through removeManaged.
|
||||
func (s *service) RevokeForDelete(ctx context.Context, orgID, accountID uuid.UUID) *errx.Error {
|
||||
if _, xerr := s.ownedAccount(ctx, orgID, accountID); xerr != nil {
|
||||
return xerr
|
||||
|
||||
@@ -138,16 +138,32 @@ func TestDeleteKeepsTheMailboxWhenCloudRevocationIsNotWired(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// Cloud-managed mirrors skip revocation to avoid recursive deletion.
|
||||
func TestDeleteDoesNotCallBackForAManagedMailbox(t *testing.T) {
|
||||
// A cloud-managed mirror's delete releases the cloud's claim, so the mailbox
|
||||
// goes back to the cloud workspace instead of staying linked to an instance
|
||||
// that no longer keeps a mirror of it.
|
||||
func TestDeleteReleasesTheCloudLinkForAManagedMailbox(t *testing.T) {
|
||||
f := newRemovalFixture(t)
|
||||
u := withCloudEnrollment(f, true)
|
||||
|
||||
if xerr := f.svc.Delete(context.Background(), f.user.String(), f.mailbox.String()); xerr != nil {
|
||||
t.Fatalf("delete: %v", xerr)
|
||||
}
|
||||
if len(u.calls) != 0 {
|
||||
t.Fatalf("a managed mailbox was unenrolled through its own delete path: %v", u.calls)
|
||||
if len(u.calls) != 1 || u.calls[0] != f.mailbox {
|
||||
t.Fatalf("revoked %v, want one call for %s", u.calls, f.mailbox)
|
||||
}
|
||||
if u.orgs[0] != f.org {
|
||||
t.Errorf("revoked under org %s, want the mailbox's own %s", u.orgs[0], f.org)
|
||||
}
|
||||
// removeManaged removes the mirror by calling back into this delete, so the
|
||||
// managed path must not reach it: one entry per step, no repeat.
|
||||
want := []string{"remove", "unenroll", "delete"}
|
||||
if len(f.trace) != len(want) {
|
||||
t.Fatalf("delete re-entered itself: trace = %v, want %v", f.trace, want)
|
||||
}
|
||||
for i := range want {
|
||||
if f.trace[i] != want[i] {
|
||||
t.Fatalf("delete re-entered itself: trace = %v, want %v", f.trace, want)
|
||||
}
|
||||
}
|
||||
if f.repo.deleteCalls != 1 {
|
||||
t.Errorf("delete called %d times, want 1", f.repo.deleteCalls)
|
||||
|
||||
@@ -373,14 +373,16 @@ func (s *emailService) Delete(ctx context.Context, userID, emailAccountID string
|
||||
return nil
|
||||
}
|
||||
|
||||
// ErrCloudEnrollmentStuck keeps the local mailbox when its cloud credential cannot be revoked.
|
||||
// ErrCloudEnrollmentStuck keeps the local mailbox when its cloud link cannot be released.
|
||||
var ErrCloudEnrollmentStuck = errx.NewWithIdentifier(
|
||||
errx.Conflict,
|
||||
"mailbox_cloud_unenroll_failed",
|
||||
"This mailbox is enrolled in Warmbly Cloud and its enrollment could not be removed, so deleting it would leave its password in the pool. The mailbox record remains. Worker restoration is retried automatically; try the delete again once this instance can reach Warmbly Cloud.",
|
||||
"This mailbox is linked to Warmbly Cloud and its link could not be released, so deleting it would leave Warmbly Cloud holding its credential or its claim on it. The mailbox record remains. Worker restoration is retried automatically; try the delete again once this instance can reach Warmbly Cloud.",
|
||||
)
|
||||
|
||||
// unenrollFromCloud removes the mailbox credential held by Warmbly Cloud.
|
||||
// unenrollFromCloud releases the mailbox on Warmbly Cloud before its local
|
||||
// record goes, so a managed mirror returns to the cloud workspace instead of
|
||||
// staying claimed by an instance that no longer holds it.
|
||||
func (s *emailService) unenrollFromCloud(ctx context.Context, account *models.Email) *errx.Error {
|
||||
if account.OrganizationID == nil {
|
||||
return nil
|
||||
@@ -394,8 +396,9 @@ func (s *emailService) unenrollFromCloud(ctx context.Context, account *models.Em
|
||||
log.Warn().Err(err).Str("account_id", account.ID.String()).Msg("cloud enrollment unreadable; delete refused rather than leaving the credential in the pool")
|
||||
return ErrCloudEnrollmentStuck
|
||||
}
|
||||
// Cloud-managed mirrors hold no local enrollment and would recurse here.
|
||||
if link == nil || link.Managed {
|
||||
// RevokeForDelete is a leaf: unlike removeManaged it never calls back into
|
||||
// this delete, so the managed mirror can revoke here without recursing.
|
||||
if link == nil {
|
||||
return nil
|
||||
}
|
||||
if xerr := s.cloudUnenroll.RevokeForDelete(ctx, *account.OrganizationID, account.ID); xerr != nil {
|
||||
|
||||
Reference in New Issue
Block a user