Merge pull request #593 from warmbly/fix/mailbox-disconnect-workspace-scope

Scope mailbox disconnect to the workspace and surface the API's reason

docs/content/docs/api/reference/mailboxes.mdx

@@ -642,7 +642,7 @@ Auth: **Scope** `WRITE_EMAILS` · **Org permission** `manage_emails`
 
 `DELETE /emails/:id`
 
-Disconnects and deletes a mailbox. It is removed from all warmup pools and an account-disconnected event fans out.
+Disconnects and deletes a mailbox. It is removed from all warmup pools and an account-disconnected event fans out. The mailbox is looked up in the caller's workspace, so any member with `manage_emails` (or a key with `WRITE_EMAILS`) can delete any mailbox the workspace holds, not only the member who connected it; a mailbox in another workspace answers `404`.
 
 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.
 

docs/content/docs/guides/mailboxes.mdx

@@ -288,7 +288,7 @@ Leads mid-sequence on that mailbox move to another one in their campaign as they
 
 It keeps its worker assignment while off, so switching it back on puts it back on the same machine, sending from the same IP, and it resumes syncing from where it stopped instead of re-importing.
 
-**Disconnecting** removes the mailbox for good. It is on the mailbox's own **More** menu in the list, as **Disconnect mailbox**, and at the bottom of its **Settings** tab under Danger zone. To remove several at once, tick their rows and use the selection bar. The machine syncing it is told to drop it before the record is removed, because afterwards there is nothing left to tell. If that instruction cannot be delivered, the disconnect fails with a `503` and nothing is removed, so retry it in a moment rather than assuming it worked.
+**Disconnecting** removes the mailbox for good. It is on the mailbox's own **More** menu in the list, as **Disconnect mailbox**, and at the bottom of its **Settings** tab under Danger zone. To remove several at once, tick their rows and use the selection bar. Any member with the manage mailboxes permission can disconnect any mailbox in the workspace, not only the member who connected it. The machine syncing it is told to drop it before the record is removed, because afterwards there is nothing left to tell. If that instruction cannot be delivered, the disconnect fails with a `503` and nothing is removed, so retry it in a moment rather than assuming it worked. When a disconnect is refused, the notice in the dashboard carries the reason the API gave, so a mailbox that could not be released from Warmbly Cloud or a machine that could not be reached reads as that rather than as a generic failure.
 
 Everything belonging to that mailbox goes with it: its imported mail in the unibox, its warmup history and pool membership, its credentials, its sender links, and any send still scheduled for it. A campaign that was using it keeps running on its remaining senders, and the leads it had been writing to move onto them at their next step. Export the workspace first if you want a copy. Disable the mailbox instead when you only want it to stop.
 

internal/api/handler/email.go

@@ -303,11 +303,15 @@ func (h *Handler) UpdateEmailTrackingDomain(c *gin.Context) {
 }
 
 func (h *Handler) DeleteEmail(c *gin.Context) {
-	userIDStr := middleware.GetUserID(c)
+	orgID := middleware.GetOrganizationID(c)
+	if orgID == nil {
+		errx.Handle(c, errx.New(errx.BadRequest, "no organization selected"))
+		return
+	}
 
 	emailAccountID := c.Param("id")
 
-	if err := h.EmailService.Delete(c.Request.Context(), userIDStr, emailAccountID); err != nil {
+	if err := h.EmailService.Delete(c.Request.Context(), orgID.String(), emailAccountID); err != nil {
 		errx.Handle(c, err)
 		return
 	}

internal/app/aitools/tools_mailboxes.go

@@ -290,7 +290,7 @@ func (d Deps) disconnectMailbox(ctx context.Context, inv Invocation, args json.R
 	if err != nil {
 		return "", err
 	}
-	if xerr := d.Emails.Delete(ctx, inv.UserID.String(), in.EmailAccountID); xerr != nil {
+	if xerr := d.Emails.Delete(ctx, inv.OrgID.String(), in.EmailAccountID); xerr != nil {
 		return "", fromErrx(xerr)
 	}
 	d.logAudit(ctx, inv, models.AuditActionDisconnect, models.AuditEntityEmailAccount, &aid, nil)

internal/app/cloudlink/disconnect_managed_test.go

@@ -45,12 +45,12 @@ type revokingEmails struct {
 	refused *[]*errx.Error
 }
 
-func (s revokingEmails) Delete(ctx context.Context, userID, accountID string) *errx.Error {
+func (s revokingEmails) Delete(ctx context.Context, orgID, accountID string) *errx.Error {
 	if xerr := s.svc.RevokeForDelete(ctx, s.org, uuid.MustParse(accountID)); xerr != nil {
 		*s.refused = append(*s.refused, xerr)
 		return xerr
 	}
-	return s.stubEmailDeletes.Delete(ctx, userID, accountID)
+	return s.stubEmailDeletes.Delete(ctx, orgID, accountID)
 }
 
 // Disconnect revokes the instance before it deletes the managed mirrors, so the

internal/app/cloudlink/managed.go

@@ -105,7 +105,7 @@ func (s *service) mirror(ctx context.Context, l *models.CloudLink, orgID, userID
 	}
 	if _, err := s.repo.Enroll(ctx, acc.ID, state.RemoteID, true); err != nil {
 		if s.emailSvc != nil {
-			_ = s.emailSvc.Delete(ctx, userID.String(), acc.ID.String())
+			_ = s.emailSvc.Delete(ctx, orgID.String(), acc.ID.String())
 		}
 		// Release the cloud side too: a mailbox left linked to this instance
 		// with no mirror here is hidden from the adoptable list and refused on
@@ -193,7 +193,7 @@ func (s *service) forgetToken(accountID uuid.UUID) {
 }
 
 // removeManaged deletes the local mirror; the cloud keeps the mailbox in the workspace.
-func (s *service) removeManaged(ctx context.Context, userID string, m *models.CloudLinkMailbox) *errx.Error {
+func (s *service) removeManaged(ctx context.Context, orgID uuid.UUID, m *models.CloudLinkMailbox) *errx.Error {
 	if l, err := s.repo.Get(ctx); err == nil && l != nil {
 		if xerr := s.clientFor(l).do(ctx, http.MethodDelete, "/instance/mailboxes/"+m.RemoteID.String(), nil, nil); xerr != nil && xerr.Identifier != "pool_link_mailbox_not_found" {
 			return xerr
@@ -201,7 +201,7 @@ func (s *service) removeManaged(ctx context.Context, userID string, m *models.Cl
 	}
 	s.forgetToken(m.EmailAccountID)
 	if s.emailSvc != nil {
-		if xerr := s.emailSvc.Delete(ctx, userID, m.EmailAccountID.String()); xerr != nil && xerr != errx.ErrNotFound {
+		if xerr := s.emailSvc.Delete(ctx, orgID.String(), m.EmailAccountID.String()); xerr != nil && xerr != errx.ErrNotFound {
 			return xerr
 		}
 	}

internal/app/cloudlink/service.go

@@ -310,9 +310,9 @@ func (s *service) Disconnect(ctx context.Context) *errx.Error {
 		if s.emailSvc == nil {
 			continue
 		}
-		if acc, xerr := s.emails.GetByID(ctx, m.EmailAccountID); xerr == nil {
+		if acc, xerr := s.emails.GetByID(ctx, m.EmailAccountID); xerr == nil && acc != nil && acc.OrganizationID != nil {
 			s.forgetToken(m.EmailAccountID)
-			_ = s.emailSvc.Delete(ctx, acc.UserID, acc.ID.String())
+			_ = s.emailSvc.Delete(ctx, acc.OrganizationID.String(), acc.ID.String())
 		}
 	}
 	if err := s.repo.UnenrollAll(ctx); err != nil {
@@ -483,8 +483,7 @@ func (s *service) Enroll(ctx context.Context, orgID, accountID uuid.UUID) (*mode
 }
 
 func (s *service) Unenroll(ctx context.Context, orgID, accountID uuid.UUID) *errx.Error {
-	acc, xerr := s.ownedAccount(ctx, orgID, accountID)
-	if xerr != nil {
+	if _, xerr := s.ownedAccount(ctx, orgID, accountID); xerr != nil {
 		return xerr
 	}
 	m, err := s.repo.GetByAccount(ctx, accountID)
@@ -495,7 +494,7 @@ func (s *service) Unenroll(ctx context.Context, orgID, accountID uuid.UUID) *err
 		return nil
 	}
 	if m.Managed {
-		return s.removeManaged(ctx, acc.UserID, m)
+		return s.removeManaged(ctx, orgID, m)
 	}
 	// Local row first, so a failed cloud call can be retried from a consistent
 	// state instead of leaving the mailbox with no warmup anywhere.

internal/app/email/cloud_unenroll_delete_test.go

@@ -57,7 +57,7 @@ func TestDeleteRevokesTheCloudEnrollmentBeforeTheRowGoes(t *testing.T) {
 	f := newRemovalFixture(t)
 	u := withCloudEnrollment(f, false)
 
-	if xerr := f.svc.Delete(context.Background(), f.user.String(), f.mailbox.String()); xerr != nil {
+	if xerr := f.svc.Delete(context.Background(), f.org.String(), f.mailbox.String()); xerr != nil {
 		t.Fatalf("delete: %v", xerr)
 	}
 	if len(u.calls) != 1 || u.calls[0] != f.mailbox {
@@ -84,7 +84,7 @@ func TestDeleteKeepsTheMailboxWhenTheCloudRefusesTheRevocation(t *testing.T) {
 	u := withCloudEnrollment(f, false)
 	u.err = errx.InternalError()
 
-	xerr := f.svc.Delete(context.Background(), f.user.String(), f.mailbox.String())
+	xerr := f.svc.Delete(context.Background(), f.org.String(), f.mailbox.String())
 	if xerr == nil {
 		t.Fatal("the mailbox was deleted while the pool still held its password")
 	}
@@ -106,7 +106,7 @@ func TestDeleteKeepsTheMailboxWhenTheEnrollmentCannotBeRead(t *testing.T) {
 	withCloudEnrollment(f, false)
 	f.svc.cloudLink = &stubCloudLinkRepo{err: errors.New("db down")}
 
-	if xerr := f.svc.Delete(context.Background(), f.user.String(), f.mailbox.String()); xerr == nil {
+	if xerr := f.svc.Delete(context.Background(), f.org.String(), f.mailbox.String()); xerr == nil {
 		t.Fatal("the mailbox was deleted on an unreadable cloud enrollment")
 	}
 	if f.repo.deleteCalls != 0 {
@@ -127,7 +127,7 @@ func TestDeleteKeepsTheMailboxWhenCloudRevocationIsNotWired(t *testing.T) {
 			f := newRemovalFixture(t)
 			tc.setup(f.svc)
 
-			xerr := f.svc.Delete(context.Background(), f.user.String(), f.mailbox.String())
+			xerr := f.svc.Delete(context.Background(), f.org.String(), f.mailbox.String())
 			if xerr == nil || xerr.Identifier != ErrCloudEnrollmentStuck.Identifier {
 				t.Fatalf("error = %v, want %q", xerr, ErrCloudEnrollmentStuck.Identifier)
 			}
@@ -145,7 +145,7 @@ 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 {
+	if xerr := f.svc.Delete(context.Background(), f.org.String(), f.mailbox.String()); xerr != nil {
 		t.Fatalf("delete: %v", xerr)
 	}
 	if len(u.calls) != 1 || u.calls[0] != f.mailbox {
@@ -176,7 +176,7 @@ func TestDeleteSkipsTheCloudWhenTheMailboxIsNotEnrolled(t *testing.T) {
 	u := withCloudEnrollment(f, false)
 	f.svc.cloudLink = &stubCloudLinkRepo{}
 
-	if xerr := f.svc.Delete(context.Background(), f.user.String(), f.mailbox.String()); xerr != nil {
+	if xerr := f.svc.Delete(context.Background(), f.org.String(), f.mailbox.String()); xerr != nil {
 		t.Fatalf("delete: %v", xerr)
 	}
 	if len(u.calls) != 0 {

internal/app/email/handler.go

@@ -321,25 +321,33 @@ func (s *emailService) resolveDomainAuth(ctx context.Context, orgID, emailAccoun
 // Delete disconnects a mailbox. The worker is told to drop it BEFORE the row
 // goes, because afterwards no assignment is left to read and nothing can repair
 // a missed removal, so a removal that cannot be sent fails the whole delete.
-func (s *emailService) Delete(ctx context.Context, userID, emailAccountID string) *errx.Error {
+//
+// Scoped to the workspace, like every other mailbox route: the list shows a
+// teammate every mailbox in it and the route is gated on manage_emails, so a
+// delete that only the connecting member could perform answered 404 to
+// everyone else on a row they could see.
+func (s *emailService) Delete(ctx context.Context, orgID, emailAccountID string) *errx.Error {
 	accountID, err := uuid.Parse(emailAccountID)
 	if err != nil {
 		return errx.ErrUuid
 	}
+	org, err := uuid.Parse(orgID)
+	if err != nil {
+		return errx.ErrUuid
+	}
 
-	// Read by id: Get is scoped by organization and was being handed a user id,
-	// so it never found the mailbox and every side effect below was skipped.
-	// Ownership moves here, or the removal below would be publishable for a
-	// mailbox the caller does not own.
+	// Ownership is proved here, before the removal below is publishable, and
+	// the repository deletes by id on the strength of it.
 	account, xerr := s.emailRepository.GetByID(ctx, accountID)
 	if xerr != nil {
 		return xerr
 	}
-	if account == nil || !sameUser(account.UserID, userID) {
+	if account == nil || account.OrganizationID == nil || *account.OrganizationID != org {
 		return errx.ErrNotFound
 	}
 
-	if xerr := s.dropFromWorker(ctx, userID, accountID); xerr != nil {
+	// The owner's id, not the caller's: the consumer's unibox cleanup is keyed on it.
+	if xerr := s.dropFromWorker(ctx, account.UserID, accountID); xerr != nil {
 		return xerr
 	}
 
@@ -353,7 +361,7 @@ func (s *emailService) Delete(ctx context.Context, userID, emailAccountID string
 	// nulls worker_id, so a worker not credited here stays charged for a
 	// mailbox that no longer exists, unrepairably.
 	refund := worker.MailboxWeight(account.Provider, account.Warmup != nil)
-	if xerr := s.emailRepository.Delete(ctx, userID, emailAccountID, refund); xerr != nil {
+	if xerr := s.emailRepository.Delete(ctx, emailAccountID, refund); xerr != nil {
 		// The removal already went out and the mailbox is still active: put it
 		// back now instead of leaving it dark until the reconciler's next pass.
 		s.loadAccountBestEffort(ctx, accountID)
@@ -408,14 +416,6 @@ func (s *emailService) unenrollFromCloud(ctx context.Context, account *models.Em
 	return nil
 }
 
-// sameUser compares user ids as uuids, the way the delete's own WHERE clause
-// does, so formatting alone never reads as a different owner.
-func sameUser(a, b string) bool {
-	left, aerr := uuid.Parse(a)
-	right, berr := uuid.Parse(b)
-	return aerr == nil && berr == nil && left == right
-}
-
 func (s *emailService) syncWarmupPoolMembership(ctx context.Context, account *models.Email) {
 	if s.warmupService == nil || account == nil {
 		return

internal/app/email/mailbox_erasure_live_test.go

@@ -60,7 +60,7 @@ func TestLiveDeleteRecordsTheErasureItCannotPerform(t *testing.T) {
 	exec(t, f, `INSERT INTO email_accounts_oauth (email_account_id, access_token, refresh_token, expires_at)
 	            VALUES ($1, 'sealed-access', 'sealed-refresh', now() + interval '1 hour')`, f.mailbox)
 
-	if xerr := f.svc.Delete(context.Background(), f.user.String(), f.mailbox.String()); xerr != nil {
+	if xerr := f.svc.Delete(context.Background(), f.org.String(), f.mailbox.String()); xerr != nil {
 		t.Fatalf("delete: %v", xerr)
 	}
 	if f.mailboxExists(t) {
@@ -91,7 +91,7 @@ func TestLiveDeleteQueuesErasureForAMailboxWithNoGrant(t *testing.T) {
 	f := newRemovalLiveFixture(t)
 	cleanErasure(t, f)
 
-	if xerr := f.svc.Delete(context.Background(), f.user.String(), f.mailbox.String()); xerr != nil {
+	if xerr := f.svc.Delete(context.Background(), f.org.String(), f.mailbox.String()); xerr != nil {
 		t.Fatalf("delete: %v", xerr)
 	}
 	row := readErasure(t, f)
@@ -110,7 +110,7 @@ func TestLiveAFailedDeleteQueuesNoErasure(t *testing.T) {
 	cleanErasure(t, f)
 	f.pub.removeErr = errBusDown
 
-	if xerr := f.svc.Delete(context.Background(), f.user.String(), f.mailbox.String()); xerr == nil {
+	if xerr := f.svc.Delete(context.Background(), f.org.String(), f.mailbox.String()); xerr == nil {
 		t.Fatal("the delete was reported as succeeding")
 	}
 	if !f.mailboxExists(t) {
@@ -157,7 +157,7 @@ func TestLiveDeleteTakesTheRowsThatHadNoForeignKey(t *testing.T) {
 	exec(t, f, `INSERT INTO warmup_pending_engagements (email_account_id, payload, fire_at)
 	            VALUES ($1, '{}'::jsonb, now())`, f.mailbox)
 
-	if xerr := f.svc.Delete(ctx, f.user.String(), f.mailbox.String()); xerr != nil {
+	if xerr := f.svc.Delete(ctx, f.org.String(), f.mailbox.String()); xerr != nil {
 		t.Fatalf("delete: %v", xerr)
 	}
 
@@ -222,7 +222,7 @@ func TestLiveDeleteClearsLabelsOnThreadsItEmptied(t *testing.T) {
 			f.user, m.thread)
 	}
 
-	if xerr := f.svc.Delete(ctx, f.user.String(), f.mailbox.String()); xerr != nil {
+	if xerr := f.svc.Delete(ctx, f.org.String(), f.mailbox.String()); xerr != nil {
 		t.Fatalf("delete: %v", xerr)
 	}
 

internal/app/email/service.go

@@ -60,7 +60,7 @@ type EmailService interface {
 	// lift the cold-send and warmup gate, so it sits behind the write
 	// permission while CheckDomainAuth stays readable.
 	RefreshDomainAuth(ctx context.Context, orgID, emailAccountID string) (*dnsauth.Result, *errx.Error)
-	Delete(ctx context.Context, userID, emailAccountID string) *errx.Error
+	Delete(ctx context.Context, orgID, emailAccountID string) *errx.Error
 
 	// GetSendIdentity reports which addresses the mailbox's provider will let
 	// it send as, which one is in use, and where the stored signature came

internal/app/email/worker_removal_live_test.go

@@ -159,7 +159,7 @@ func TestLiveDisablingAMailboxRemovesItFromItsWorker(t *testing.T) {
 func TestLiveDeletingAMailboxRemovesItFromItsWorkerFirst(t *testing.T) {
 	f := newRemovalLiveFixture(t)
 
-	if xerr := f.svc.Delete(context.Background(), f.user.String(), f.mailbox.String()); xerr != nil {
+	if xerr := f.svc.Delete(context.Background(), f.org.String(), f.mailbox.String()); xerr != nil {
 		t.Fatalf("delete: %v", xerr)
 	}
 
@@ -183,25 +183,26 @@ func TestLiveDeletingAMailboxRemovesItFromItsWorkerFirst(t *testing.T) {
 
 // The refund shares the delete's transaction, so a delete that matches no row
 // must leave the worker's capacity exactly as it was. Driven through the
-// repository, because the service refuses a foreign owner before it gets here.
+// repository, because the service refuses a mailbox outside the caller's
+// workspace before it gets here.
 func TestLiveDeleteThatMatchesNoRowRefundsNothing(t *testing.T) {
 	f := newRemovalLiveFixture(t)
 
-	if xerr := f.svc.emailRepository.Delete(context.Background(), uuid.New().String(), f.mailbox.String(), 1); xerr != errx.ErrNotFound {
+	if xerr := f.svc.emailRepository.Delete(context.Background(), uuid.New().String(), 1); xerr != errx.ErrNotFound {
 		t.Fatalf("error = %v, want not found", xerr)
 	}
 	if count, score := f.workerLoad(t); count != 1 || score != 1 {
 		t.Errorf("capacity was refunded for a mailbox that was not deleted: account_count=%d load_score=%v", count, score)
 	}
 	if !f.mailboxExists(t) {
-		t.Error("the mailbox was deleted by a caller that does not own it")
+		t.Error("a delete of an unknown id removed a different mailbox")
 	}
 }
 
-// The lookup that finds the mailbox is deliberately unscoped, so ownership is
-// checked in the service. A teammate's user id must not delete this mailbox or
-// publish a removal for it.
-func TestLiveDeleteRefusesAMailboxTheCallerDoesNotOwn(t *testing.T) {
+// The lookup that finds the mailbox is deliberately unscoped, so the workspace
+// is checked in the service. Another workspace's id must not delete this
+// mailbox or publish a removal for it.
+func TestLiveDeleteRefusesAMailboxOfAnotherWorkspace(t *testing.T) {
 	f := newRemovalLiveFixture(t)
 
 	if xerr := f.svc.Delete(context.Background(), uuid.New().String(), f.mailbox.String()); xerr != errx.ErrNotFound {
@@ -221,7 +222,7 @@ func TestLiveDeleteKeepsEverythingWhenTheWorkerCannotBeTold(t *testing.T) {
 	f := newRemovalLiveFixture(t)
 	f.pub.removeErr = errBusDown
 
-	xerr := f.svc.Delete(context.Background(), f.user.String(), f.mailbox.String())
+	xerr := f.svc.Delete(context.Background(), f.org.String(), f.mailbox.String())
 	if xerr == nil || xerr.Code != errx.ServiceUnavailable {
 		t.Fatalf("error = %v, want a 503 so the client retries", xerr)
 	}
@@ -258,7 +259,7 @@ func TestLiveDeletingAMailboxWithScheduledWork(t *testing.T) {
 		t.Fatalf("fixture admin action: %v", err)
 	}
 
-	if xerr := f.svc.Delete(ctx, f.user.String(), f.mailbox.String()); xerr != nil {
+	if xerr := f.svc.Delete(ctx, f.org.String(), f.mailbox.String()); xerr != nil {
 		t.Fatalf("disconnecting a mailbox that has scheduled work failed: %v", xerr)
 	}
 	if f.mailboxExists(t) {

internal/app/email/worker_removal_test.go

@@ -74,7 +74,7 @@ func (s *stubRemovalRepo) GetSMTPCredentials(ctx context.Context, emailAccountID
 	return &repository.SMTPCredentials{SMTPHost: "smtp.test.local", SMTPPort: 587, IMAPHost: "imap.test.local", IMAPPort: 993}, nil
 }
 
-func (s *stubRemovalRepo) Delete(ctx context.Context, userID, emailAccountID string, workerLoadRefund float64) *errx.Error {
+func (s *stubRemovalRepo) Delete(ctx context.Context, emailAccountID string, workerLoadRefund float64) *errx.Error {
 	s.deleteCalls++
 	s.refunded = append(s.refunded, workerLoadRefund)
 	s.record("delete")
@@ -269,7 +269,7 @@ func TestDisablingSucceedsEvenWhenTheBusIsDown(t *testing.T) {
 func TestDeleteTellsTheWorkerBeforeTheRowGoes(t *testing.T) {
 	f := newRemovalFixture(t)
 
-	if xerr := f.svc.Delete(context.Background(), f.user.String(), f.mailbox.String()); xerr != nil {
+	if xerr := f.svc.Delete(context.Background(), f.org.String(), f.mailbox.String()); xerr != nil {
 		t.Fatalf("delete: %v", xerr)
 	}
 
@@ -294,7 +294,7 @@ func TestDeleteKeepsTheMailboxWhenTheWorkerCannotBeTold(t *testing.T) {
 	f := newRemovalFixture(t)
 	f.pub.removeErr = errBusDown
 
-	xerr := f.svc.Delete(context.Background(), f.user.String(), f.mailbox.String())
+	xerr := f.svc.Delete(context.Background(), f.org.String(), f.mailbox.String())
 	if xerr == nil {
 		t.Fatal("the mailbox was deleted without the worker ever being told")
 	}
@@ -315,7 +315,7 @@ func TestDeleteKeepsTheMailboxWhenTheAssignmentCannotBeRead(t *testing.T) {
 	f := newRemovalFixture(t)
 	f.repo.workerErr = errx.InternalError()
 
-	if xerr := f.svc.Delete(context.Background(), f.user.String(), f.mailbox.String()); xerr == nil {
+	if xerr := f.svc.Delete(context.Background(), f.org.String(), f.mailbox.String()); xerr == nil {
 		t.Fatal("the mailbox was deleted on an unreadable assignment")
 	}
 	if f.repo.deleteCalls != 0 {
@@ -329,7 +329,7 @@ func TestDeleteWithoutAWorkerStillRemovesTheRow(t *testing.T) {
 	f.repo.workerID = nil
 	f.repo.account.WorkerID = nil
 
-	if xerr := f.svc.Delete(context.Background(), f.user.String(), f.mailbox.String()); xerr != nil {
+	if xerr := f.svc.Delete(context.Background(), f.org.String(), f.mailbox.String()); xerr != nil {
 		t.Fatalf("delete: %v", xerr)
 	}
 	if len(f.pub.removed) != 0 {
@@ -347,7 +347,7 @@ func TestDeleteWithoutAWorkerStillRemovesTheRow(t *testing.T) {
 func TestDeleteGivesTheWorkerItsCapacityBack(t *testing.T) {
 	f := newRemovalFixture(t)
 
-	if xerr := f.svc.Delete(context.Background(), f.user.String(), f.mailbox.String()); xerr != nil {
+	if xerr := f.svc.Delete(context.Background(), f.org.String(), f.mailbox.String()); xerr != nil {
 		t.Fatalf("delete: %v", xerr)
 	}
 	if len(f.repo.refunded) != 1 || f.repo.refunded[0] != worker.MailboxWeight("smtp_imap", false) {
@@ -363,7 +363,7 @@ func TestDeleteRefundsTheWeightTheMailboxWasChargedAt(t *testing.T) {
 	f.repo.account.Provider = "gmail"
 	f.repo.account.Warmup = &warming
 
-	if xerr := f.svc.Delete(context.Background(), f.user.String(), f.mailbox.String()); xerr != nil {
+	if xerr := f.svc.Delete(context.Background(), f.org.String(), f.mailbox.String()); xerr != nil {
 		t.Fatalf("delete: %v", xerr)
 	}
 	if len(f.repo.refunded) != 1 || f.repo.refunded[0] != worker.MailboxWeight("gmail", true) {
@@ -373,34 +373,40 @@ func TestDeleteRefundsTheWeightTheMailboxWasChargedAt(t *testing.T) {
 
 // The removal must never be reachable for a mailbox the caller does not own:
 // the lookup that finds it is unscoped, so ownership is checked here.
-func TestDeleteRefusesAMailboxTheCallerDoesNotOwn(t *testing.T) {
+func TestDeleteRefusesAMailboxOfAnotherWorkspace(t *testing.T) {
 	f := newRemovalFixture(t)
-	f.repo.account.UserID = uuid.New().String()
+	other := uuid.New()
+	f.repo.account.OrganizationID = &other
 
-	xerr := f.svc.Delete(context.Background(), f.user.String(), f.mailbox.String())
+	xerr := f.svc.Delete(context.Background(), f.org.String(), f.mailbox.String())
 	if xerr != errx.ErrNotFound {
 		t.Fatalf("error = %v, want not found", xerr)
 	}
 	if len(f.pub.removed) != 0 || f.repo.deleteCalls != 0 {
-		t.Errorf("acted on someone else's mailbox: %d removals, %d deletes", len(f.pub.removed), f.repo.deleteCalls)
+		t.Errorf("acted on another workspace's mailbox: %d removals, %d deletes", len(f.pub.removed), f.repo.deleteCalls)
 	}
 }
 
-// Owner ids arriving in different letter case are the same owner; Postgres
-// compares them as uuids and so does this.
-func TestDeleteAcceptsTheOwnerInAnyCase(t *testing.T) {
+// The mailbox belongs to the workspace, not to whoever connected it: a
+// teammate with the permission can disconnect it, and the removal still names
+// the owner, which is the id the consumer's unibox cleanup is keyed on.
+func TestDeleteByATeammateNamesTheOwner(t *testing.T) {
 	f := newRemovalFixture(t)
-	f.repo.account.UserID = uuidUpper(f.user)
+	owner := uuid.New().String()
+	f.repo.account.UserID = owner
 
-	if xerr := f.svc.Delete(context.Background(), f.user.String(), f.mailbox.String()); xerr != nil {
-		t.Fatalf("the owner was refused their own mailbox: %v", xerr)
+	if xerr := f.svc.Delete(context.Background(), f.org.String(), f.mailbox.String()); xerr != nil {
+		t.Fatalf("a teammate was refused a mailbox in their own workspace: %v", xerr)
+	}
+	if len(f.pub.removed) != 1 || f.pub.removed[0].userID != owner {
+		t.Errorf("removal = %+v, want one naming owner %s", f.pub.removed, owner)
 	}
 }
 
 func TestDeleteRejectsAMalformedID(t *testing.T) {
 	f := newRemovalFixture(t)
 
-	if xerr := f.svc.Delete(context.Background(), f.user.String(), "not-a-uuid"); xerr != errx.ErrUuid {
+	if xerr := f.svc.Delete(context.Background(), f.org.String(), "not-a-uuid"); xerr != errx.ErrUuid {
 		t.Fatalf("error = %v, want a uuid error", xerr)
 	}
 	if f.repo.deleteCalls != 0 {
@@ -414,7 +420,7 @@ func TestDeleteOfAMissingMailboxPublishesNothing(t *testing.T) {
 	f := newRemovalFixture(t)
 	f.repo.getErr = errx.ErrNotFound
 
-	if xerr := f.svc.Delete(context.Background(), f.user.String(), f.mailbox.String()); xerr != errx.ErrNotFound {
+	if xerr := f.svc.Delete(context.Background(), f.org.String(), f.mailbox.String()); xerr != errx.ErrNotFound {
 		t.Fatalf("error = %v, want not found", xerr)
 	}
 	if len(f.pub.removed) != 0 || f.repo.deleteCalls != 0 {
@@ -428,7 +434,7 @@ func TestDeleteWithNoPublisherWired(t *testing.T) {
 	f := newRemovalFixture(t)
 	f.svc.publisher = nil
 
-	if xerr := f.svc.Delete(context.Background(), f.user.String(), f.mailbox.String()); xerr != nil {
+	if xerr := f.svc.Delete(context.Background(), f.org.String(), f.mailbox.String()); xerr != nil {
 		t.Fatalf("delete: %v", xerr)
 	}
 	if f.repo.deleteCalls != 1 {
@@ -439,17 +445,6 @@ func TestDeleteWithNoPublisherWired(t *testing.T) {
 	}
 }
 
-// uuidUpper renders an id the way a caller that upper-cases its ids would.
-func uuidUpper(id uuid.UUID) string {
-	out := []rune(id.String())
-	for i, r := range out {
-		if r >= 'a' && r <= 'f' {
-			out[i] = r - 32
-		}
-	}
-	return string(out)
-}
-
 // A delete that fails after the removal was published leaves a mailbox that is
 // still active but no longer loaded anywhere. It goes straight back on rather
 // than waiting minutes for the reconciler.
@@ -457,7 +452,7 @@ func TestAFailedDeletePutsTheMailboxBackOnItsWorker(t *testing.T) {
 	f := newRemovalFixture(t)
 	f.repo.deleteErr = errx.InternalError()
 
-	if xerr := f.svc.Delete(context.Background(), f.user.String(), f.mailbox.String()); xerr == nil {
+	if xerr := f.svc.Delete(context.Background(), f.org.String(), f.mailbox.String()); xerr == nil {
 		t.Fatal("a failed delete was reported as success")
 	}
 	if len(f.pub.added) != 1 || f.pub.added[0] != f.mailbox {

internal/app/poollink/oauth.go

@@ -174,7 +174,7 @@ func (s *service) connectBrokered(ctx context.Context, st brokerState, code stri
 	}
 	remoteID := uuid.New()
 	if err := s.repo.EnrollMailbox(ctx, &models.PoolLinkMailbox{InstanceID: inst.ID, RemoteID: remoteID, EmailAccountID: acc.ID, Managed: true}); err != nil {
-		_ = s.emailSvc.Delete(ctx, userID, acc.ID.String())
+		_ = s.emailSvc.Delete(ctx, orgID.String(), acc.ID.String())
 		return uuid.Nil, errx.InternalError()
 	}
 	s.startWarmup(ctx, userID, acc.ID)

internal/app/poollink/service.go

@@ -463,7 +463,7 @@ func (s *service) Enroll(ctx context.Context, inst *models.PoolLinkInstance, req
 	}
 
 	if err := s.repo.EnrollMailbox(ctx, &models.PoolLinkMailbox{InstanceID: inst.ID, RemoteID: req.RemoteID, EmailAccountID: acc.ID}); err != nil {
-		_ = s.emailSvc.Delete(ctx, userID, acc.ID.String())
+		_ = s.emailSvc.Delete(ctx, orgID.String(), acc.ID.String())
 		return nil, errx.InternalError()
 	}
 
@@ -645,11 +645,7 @@ func (s *service) Unenroll(ctx context.Context, inst *models.PoolLinkInstance, r
 	}
 	// A managed mailbox belongs to the workspace; only the link goes.
 	if !m.Managed {
-		userID, xerr := s.ownerUserID(ctx, inst)
-		if xerr != nil {
-			return xerr
-		}
-		if xerr := s.emailSvc.Delete(ctx, userID, m.EmailAccountID.String()); xerr != nil && xerr != errx.ErrNotFound {
+		if xerr := s.emailSvc.Delete(ctx, inst.OrganizationID.String(), m.EmailAccountID.String()); xerr != nil && xerr != errx.ErrNotFound {
 			return xerr
 		}
 	}

internal/jobs/mailbox_erasure_live_test.go

@@ -103,7 +103,7 @@ func TestLiveErasureRemovesTheStoredMailAndTheQueueRow(t *testing.T) {
 
 	// The delete itself, through the repository the API uses.
 	emails := repository.NewEmailRepostory(handle, nil)
-	if xerr := emails.Delete(ctx, user.String(), mailbox.String(), 1); xerr != nil {
+	if xerr := emails.Delete(ctx, mailbox.String(), 1); xerr != nil {
 		t.Fatalf("delete mailbox: %v", xerr)
 	}
 

internal/repository/pg_email.go

@@ -142,7 +142,7 @@ type EmailRepository interface {
 	// Delete removes a mailbox and refunds workerLoadRefund of its worker's
 	// load in the same transaction, so a deleted mailbox can never leave a
 	// worker permanently charged for it.
-	Delete(ctx context.Context, userID, emailAccountID string, workerLoadRefund float64) *errx.Error
+	Delete(ctx context.Context, emailAccountID string, workerLoadRefund float64) *errx.Error
 
 	NewOauthAccount(ctx context.Context, userID string, data models.NewOauthAccount) (*models.Email, *errx.Error)
 	// NewManagedAccount creates an OAuth mailbox whose credential lives on Warmbly Cloud, so no token row is written.
@@ -1385,16 +1385,18 @@ const deleteDeadlockAttempts = 3
 // that side is this one. Nothing is wrong when it happens and the work is
 // entirely redoable, so surfacing it meant someone clicking Disconnect got an
 // error for an operation that would have succeeded a moment later.
-func (r *emailRepository) Delete(ctx context.Context, userID, emailAccountID string, workerLoadRefund float64) *errx.Error {
+func (r *emailRepository) Delete(ctx context.Context, emailAccountID string, workerLoadRefund float64) *errx.Error {
 	for attempt := 1; ; attempt++ {
-		xerr, deadlocked := r.deleteOnce(ctx, userID, emailAccountID, workerLoadRefund)
+		xerr, deadlocked := r.deleteOnce(ctx, emailAccountID, workerLoadRefund)
 		if !deadlocked || attempt >= deleteDeadlockAttempts {
 			return xerr
 		}
 	}
 }
 
-func (r *emailRepository) deleteOnce(ctx context.Context, userID, emailAccountID string, workerLoadRefund float64) (*errx.Error, bool) {
+// deleteOnce deletes by id alone: the service has already proved the mailbox
+// belongs to the caller's workspace, and no other scope is narrower than that.
+func (r *emailRepository) deleteOnce(ctx context.Context, emailAccountID string, workerLoadRefund float64) (*errx.Error, bool) {
 	tx, err := r.DB.Begin(ctx)
 	if err != nil {
 		db.CaptureError(err, "", nil, "begin")
@@ -1410,11 +1412,11 @@ func (r *emailRepository) deleteOnce(ctx context.Context, userID, emailAccountID
 		UPDATE warmup_reputation_ledger l
 		   SET recorded_at = now()
 		  FROM email_accounts a
-		 WHERE a.user_id = $1 AND a.id = $2
+		 WHERE a.id = $1
 		   AND l.organization_id = a.organization_id
 		   AND l.email = lower(btrim(a.email))
 	`
-	bumpParams := []any{userID, emailAccountID}
+	bumpParams := []any{emailAccountID}
 	if _, err := tx.Exec(ctx, bump, bumpParams...); err != nil {
 		if isDeadlock(err) {
 			return errx.InternalError(), true
@@ -1427,23 +1429,23 @@ func (r *emailRepository) deleteOnce(ctx context.Context, userID, emailAccountID
 	// sealed refresh token lives in email_accounts_oauth, which cascades away
 	// with the mailbox, so reading it afterwards is impossible and the grant
 	// would stay live at the provider forever.
-	const scope = `a.user_id = $1 AND a.id = $2`
-	if _, err := EnqueueMailboxErasures(ctx, tx, scope, userID, emailAccountID); err != nil {
+	const scope = `a.id = $1`
+	if _, err := EnqueueMailboxErasures(ctx, tx, scope, emailAccountID); err != nil {
 		return errx.InternalError(), isDeadlock(err)
 	}
 
 	// The threads this mailbox holds messages in, read while they still exist.
-	threads, err := CollectMailboxThreadState(ctx, tx, scope, userID, emailAccountID)
+	threads, err := CollectMailboxThreadState(ctx, tx, scope, emailAccountID)
 	if err != nil {
 		return errx.InternalError(), isDeadlock(err)
 	}
 
 	query := `
 		DELETE FROM email_accounts
-		WHERE user_id = $1 AND id = $2
+		WHERE id = $1
 		RETURNING worker_id
 	`
-	params := []any{userID, emailAccountID}
+	params := []any{emailAccountID}
 
 	var workerID *uuid.UUID
 	if err := tx.QueryRow(ctx, query, params...).Scan(&workerID); err != nil {

internal/repository/warmup_reputation_ledger_live_test.go

@@ -96,9 +96,9 @@ func (f *ledgerFixture) penalise(t *testing.T, id uuid.UUID, score float64, stat
 	           WHERE email_account_id = $1`, id, score, state, until)
 }
 
-func (f *ledgerFixture) remove(t *testing.T, user, id uuid.UUID) {
+func (f *ledgerFixture) remove(t *testing.T, id uuid.UUID) {
 	t.Helper()
-	if xerr := f.emails.Delete(context.Background(), user.String(), id.String(), 0); xerr != nil {
+	if xerr := f.emails.Delete(context.Background(), id.String(), 0); xerr != nil {
 		t.Fatalf("Delete: %v", xerr)
 	}
 }
@@ -176,7 +176,7 @@ func TestLiveReputationMirrorFollowsTheAddressAcrossRemoval(t *testing.T) {
 
 	written := m.recordedAt
 	time.Sleep(20 * time.Millisecond)
-	f.remove(t, f.user, first)
+	f.remove(t, first)
 	m = f.mirrorRow(t)
 	if m == nil {
 		t.Fatal("removing the mailbox lost its standing")
@@ -203,7 +203,7 @@ func TestLiveReputationMirrorLeavesNothingForGoodStanding(t *testing.T) {
 	f := newLedgerFixture(t)
 	first := f.addMailbox(t, f.user)
 	f.join(t, first)
-	f.remove(t, f.user, first)
+	f.remove(t, first)
 	if m := f.mirrorRow(t); m != nil {
 		t.Fatalf("a mailbox in good standing left a mirror row: %+v", m)
 	}
@@ -273,7 +273,7 @@ func TestLiveReputationMirrorNeverLapsesAReviewBlock(t *testing.T) {
 	if m == nil || m.standing != nil {
 		t.Fatalf("a review block must mirror with no standing_until: %+v", m)
 	}
-	f.remove(t, f.user, id)
+	f.remove(t, id)
 	f.exec(t, `UPDATE warmup_reputation_ledger SET recorded_at = now() - interval '400 days' WHERE organization_id = $1`, f.org)
 	if _, err := f.warmups.PurgeExpiredReputationLedger(context.Background()); err != nil {
 		t.Fatalf("purge: %v", err)
@@ -308,7 +308,7 @@ func TestLiveReputationMirrorLapsesOnlyWithoutALiveRow(t *testing.T) {
 	}
 
 	// Removed and lapsed: forgotten, and reported.
-	f.remove(t, f.user, id)
+	f.remove(t, id)
 	age()
 	purged, err := f.warmups.PurgeExpiredReputationLedger(context.Background())
 	if err != nil {
@@ -326,7 +326,7 @@ func TestLiveReputationMirrorDoesNotInheritALapsedStanding(t *testing.T) {
 	id := f.addMailbox(t, f.user)
 	f.join(t, id)
 	f.penalise(t, id, 40, "quarantined", ptr(time.Now().Add(7*24*time.Hour)))
-	f.remove(t, f.user, id)
+	f.remove(t, id)
 	f.exec(t, `UPDATE warmup_reputation_ledger SET standing_until = now() - interval '200 days', recorded_at = now() - interval '200 days' WHERE organization_id = $1`, f.org)
 
 	again := f.addMailbox(t, f.user)

web/src/app/app/emails/page.tsx

@@ -525,9 +525,12 @@ function bulkRevocationNote(providers: Set<string>): string {
 }
 
 // removeErrorMessage pulls the API's own explanation out of a failed request.
+// The client's interceptor has already flattened the axios error into an
+// AppError, so the message sits at the top; reading `response.data` here found
+// nothing and every refusal showed as "couldn't be disconnected".
 function removeErrorMessage(err: unknown): string | undefined {
-    const e = err as { response?: { data?: { message?: string } } };
-    return e?.response?.data?.message;
+    const message = (err as AppError | undefined)?.message;
+    return message || undefined;
 }
 
 function MailboxRow({