diff --git a/docs/content/docs/api/error-codes.mdx b/docs/content/docs/api/error-codes.mdx index 7bb1b7a2f..5ad58296f 100644 --- a/docs/content/docs/api/error-codes.mdx +++ b/docs/content/docs/api/error-codes.mdx @@ -145,8 +145,8 @@ A password must be 8 to 128 characters. There is no composition rule, but it is | `code` | Status | Meaning | |--------|--------|---------| -| `cloud_link_workspace_required` | 409 | The selected workspace only has a legacy instance-wide connection. Existing enrollments keep working; connect the workspace separately before adding mailboxes or placement tests | -| `pool_link_workspace_required` | 409 | Cloud refused a new connection or enrollment without local workspace scope. Upgrade the self-hosted instance and connect per workspace; existing legacy enrollments keep working | +| `cloud_link_workspace_required` | 409 | Disconnecting a shared server connection requires explicit confirmation of the server-wide scope; it affects all local workspaces using that connection | +| `pool_link_workspace_required` | 409 | Cloud refused a new connection request without local workspace scope. Update the self-hosted instance before creating a new connection; existing connections can still enroll mailboxes and start placement tests | | `pool_link_workspace_connected` | 409 | The Cloud workspace already has an active self-hosted connection, including a legacy link. Use another Cloud workspace and plan, or disconnect the existing link first | | `cloud_link_upgrade_required` | 409 | This Cloud deployment does not support workspace-scoped connections. Upgrade Cloud first | | `organization_cloud_connected` | 409 | Disconnect the workspace's Cloud connection before deleting it or scheduling deletion of it or its owner account | diff --git a/docs/content/docs/guides/warmbly-cloud.mdx b/docs/content/docs/guides/warmbly-cloud.mdx index bdbb0f460..f93cf4806 100644 --- a/docs/content/docs/guides/warmbly-cloud.mdx +++ b/docs/content/docs/guides/warmbly-cloud.mdx @@ -53,11 +53,13 @@ The link belongs to the selected local workspace. Connecting and disconnecting i Upgrading does not revoke an existing connection or move its mailboxes. The migration records which connection enrolled each mailbox, and existing warmup, brokered access tokens, warmup verification, analytics, standing synchronization, redirects and placement results keep using that original connection. Old placement results and Cloud-served redirects are backfilled to that connection too. -A legacy connection cannot enroll more mailboxes, adopt Cloud mailboxes, start new Google or Microsoft sign-ins, or start new placement tests. New Cloud-served redirects also require a workspace-scoped connection; existing redirects stay manageable. **Settings > Warmbly Cloud** labels the old connection and offers **Connect this workspace**. Existing mailbox rows say **legacy connection**, even after the workspace establishes a new link. +Existing connections work normally after upgrading: you can enroll more mailboxes, connect Cloud mailboxes, sign in with Google or Microsoft, create Cloud-served redirects, and start placement tests within the connected Cloud workspace's plan. **Settings > Warmbly Cloud** shows the normal connection, plan and mailbox controls, with no separate migration step. You do not need to disconnect or buy another plan to keep using your existing connection. -You can leave the old connection in place and connect each workspace to a separate, unused Cloud workspace, moving mailboxes at your own pace. No mailbox changes connection automatically. To reuse the old Cloud workspace, first disconnect its legacy link; that stops its remaining enrollments across **all** local workspaces. Workspace-scoped connections are not affected. Removing an SMTP/IMAP enrollment deletes its old Cloud mailbox and history; enrolling it again starts a new Cloud enrollment. Cloud-managed mailboxes remain on their original Cloud workspace, but disconnecting removes their local mirrors. Sign in again through the new connection to create new mirrors. +The one-active-connection limit applies when creating an additional connection, not when using an existing one. A local workspace with an existing connection does not offer another connection setup. A Cloud workspace that already has a connection cannot approve another, including when that connection is shared across a self-hosted server's local workspaces. Existing connections and their mailboxes remain in place. New connection requests are scoped to an unconnected local workspace and require an unused Cloud workspace. -Deploy the Cloud backend and migration **before** publishing the self-hosted upgrade. Old self-hosted clients keep using existing enrollments, but cannot establish new instance-wide connections or add enrollments to them. New self-hosted clients refuse to connect to older Cloud versions that do not advertise workspace-scoped support. No new environment variable is required. +Disconnect is still deliberate and potentially disruptive. Disconnecting a shared server connection affects **all** local workspaces using it; the confirmation states that scope. Disconnecting a workspace's own connection affects only its mailboxes and redirects. Other connections are unaffected. Removing an SMTP/IMAP enrollment deletes its Cloud mailbox and history; enrolling it again starts a new Cloud enrollment. Cloud-managed mailboxes remain on Cloud, but disconnecting removes their local mirrors. None of this is required by an upgrade. + +Deploy the Cloud backend and migration **before** publishing the self-hosted upgrade. Old self-hosted clients can keep using existing connections, including adding mailboxes, but cannot establish new instance-wide connections. New self-hosted clients refuse to connect to older Cloud versions that do not advertise workspace-scoped support. No new environment variable or data backfill is required for restoring use of existing connections. ## Google and Microsoft mailboxes: sign in through Warmbly Cloud diff --git a/internal/app/cloudlink/managed.go b/internal/app/cloudlink/managed.go index a70c5f8bc..b248bd637 100644 --- a/internal/app/cloudlink/managed.go +++ b/internal/app/cloudlink/managed.go @@ -57,7 +57,7 @@ func (s *service) FinishOAuth(ctx context.Context, orgID, userID uuid.UUID, sess } func (s *service) ListWorkspaceMailboxes(ctx context.Context, orgID uuid.UUID) ([]models.PoolLinkWorkspaceMailbox, *errx.Error) { - l, xerr := s.newLink(ctx, orgID) + l, xerr := s.link(ctx, orgID) if xerr != nil { return nil, xerr } diff --git a/internal/app/cloudlink/managed_consent.go b/internal/app/cloudlink/managed_consent.go index b76f56a3a..4380a9af4 100644 --- a/internal/app/cloudlink/managed_consent.go +++ b/internal/app/cloudlink/managed_consent.go @@ -19,7 +19,7 @@ import ( var ErrManagedProtocol = errx.NewWithIdentifier(errx.Conflict, "cloud_link_managed_protocol", "Warmbly Cloud must support durable managed consent before connecting or adopting another mailbox. Existing mailboxes are unchanged.") func (s *service) managedLink(ctx context.Context, orgID uuid.UUID) (*models.CloudLink, repository.CloudManagedConsentRepository, *errx.Error) { - l, xerr := s.newLink(ctx, orgID) + l, xerr := s.link(ctx, orgID) if xerr != nil { return nil, nil, xerr } diff --git a/internal/app/cloudlink/placement.go b/internal/app/cloudlink/placement.go index 3cf746301..763a42c84 100644 --- a/internal/app/cloudlink/placement.go +++ b/internal/app/cloudlink/placement.go @@ -11,7 +11,7 @@ import ( ) func (s *service) PlacementPanel(ctx context.Context, orgID uuid.UUID) (*models.PlacementCloudPanel, *errx.Error) { - l, xerr := s.newLink(ctx, orgID) + l, xerr := s.link(ctx, orgID) if xerr != nil { return nil, xerr } @@ -23,7 +23,7 @@ func (s *service) PlacementPanel(ctx context.Context, orgID uuid.UUID) (*models. } func (s *service) StartPlacement(ctx context.Context, orgID uuid.UUID, req models.PlacementCloudStartRequest) (*models.PlacementCloudStart, *errx.Error) { - l, xerr := s.newLink(ctx, orgID) + l, xerr := s.link(ctx, orgID) if xerr != nil { return nil, xerr } diff --git a/internal/app/cloudlink/reconciliation_test.go b/internal/app/cloudlink/reconciliation_test.go index 462e1ffea..61d466a02 100644 --- a/internal/app/cloudlink/reconciliation_test.go +++ b/internal/app/cloudlink/reconciliation_test.go @@ -77,6 +77,50 @@ func (e enrollmentEmails) GetAllActiveInScope(context.Context, repository.Accoun return []models.Email{*e.account}, nil } +type existingEnrollmentRepo struct { + *enrollmentFaultRepo + workspace *models.CloudLink +} + +func (r *existingEnrollmentRepo) Get(_ context.Context, org *uuid.UUID) (*models.CloudLink, error) { + if org != nil && r.workspace != nil { + return r.workspace, nil + } + return r.link, nil +} + +func TestExistingServerConnectionEnrollsAndRefreshesWithoutMovingMailboxes(t *testing.T) { + t.Setenv("APP_ENV", "dev") + org, account, instance := uuid.New(), uuid.New(), uuid.New() + posts := 0 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Header.Get("Authorization") != "Bearer original" { + t.Error("mailbox used a different connection") + } + if r.Method == http.MethodPost { + posts++ + } + _, _ = w.Write([]byte(`{}`)) + })) + defer srv.Close() + r := &existingEnrollmentRepo{enrollmentFaultRepo: &enrollmentFaultRepo{stubLinkRepo: &stubLinkRepo{ + link: &models.CloudLink{InstanceID: instance, CloudURL: srv.URL, Token: "original"}, + }}} + s := NewService(r, enrollmentEmails{stubEmails{account: &models.Email{ + ID: account, OrganizationID: &org, Status: "active", Provider: "smtp_imap", + }}}, nil).(*service) + if _, xerr := s.Enroll(context.Background(), org, account); xerr != nil { + t.Fatal(xerr) + } + r.workspace = &models.CloudLink{InstanceID: uuid.New(), OrganizationID: &org, CloudURL: "http://127.0.0.1:1"} + if xerr := s.RefreshCredentials(context.Background(), org, account); xerr != nil { + t.Fatal(xerr) + } + if posts != 2 || r.mailbox == nil || r.mailbox.InstanceID != instance { + t.Fatalf("enrollment moved or was not refreshed: %+v, posts=%d", r.mailbox, posts) + } +} + func TestEnrollmentRecordsIntentBeforeRemoteAndRetriesAmbiguousConfirmation(t *testing.T) { for _, failure := range []string{"lost_ack", "local_confirmation", "intent_write"} { t.Run(failure, func(t *testing.T) { diff --git a/internal/app/cloudlink/service.go b/internal/app/cloudlink/service.go index e38fa34dc..2736caef7 100644 --- a/internal/app/cloudlink/service.go +++ b/internal/app/cloudlink/service.go @@ -22,7 +22,7 @@ import ( const DefaultCloudURL = "https://api.warmbly.com" var ( - ErrLegacyLink = errx.NewWithIdentifier(errx.Conflict, "cloud_link_workspace_required", "Connect this workspace separately to add Cloud mailboxes. Existing legacy enrollments continue working.") + ErrLegacyLink = errx.NewWithIdentifier(errx.Conflict, "cloud_link_workspace_required", "This connection is shared across this server's workspaces. Confirm disconnecting the server-wide connection.") ErrNotConnected = errx.NewWithIdentifier(errx.Conflict, "cloud_link_not_connected", "This workspace is not connected to Warmbly Cloud.") ErrAlreadyLinked = errx.NewWithIdentifier(errx.Conflict, "cloud_link_connected", "This workspace is already connected. Disconnect first to link a different Cloud workspace.") ErrNoPendingCode = errx.NewWithIdentifier(errx.NotFound, "cloud_link_no_pending", "No connection in progress. Start again.") @@ -192,17 +192,6 @@ func (s *service) link(ctx context.Context, orgID uuid.UUID) (*models.CloudLink, return l, nil } -func (s *service) newLink(ctx context.Context, orgID uuid.UUID) (*models.CloudLink, *errx.Error) { - l, xerr := s.link(ctx, orgID) - if xerr != nil { - return nil, xerr - } - if l.OrganizationID == nil { - return nil, ErrLegacyLink - } - return l, nil -} - func (s *service) mailboxLink(ctx context.Context, m *models.CloudLinkMailbox) (*models.CloudLink, *errx.Error) { l, err := s.repo.GetByInstance(ctx, m.InstanceID) if err != nil { @@ -255,7 +244,7 @@ func (s *service) StartConnect(ctx context.Context, orgID, userID uuid.UUID, clo defer s.connectMu.Unlock() if l, err := s.repo.Get(ctx, &orgID); err != nil { return nil, errx.InternalError() - } else if l != nil && l.OrganizationID != nil { + } else if l != nil { return nil, ErrAlreadyLinked } // The handshake is one-time: refuse it now rather than lose the token @@ -310,7 +299,7 @@ func (s *service) PollConnect(ctx context.Context, orgID, userID uuid.UUID) (*Co s.mu.Unlock() if p == nil { // Another tab may have finished the handshake already. - if l, err := s.repo.Get(ctx, &orgID); err == nil && l != nil && l.OrganizationID != nil { + if l, err := s.repo.Get(ctx, &orgID); err == nil && l != nil { return &ConnectPollResult{Status: models.PoolLinkCodeApproved, Link: l}, nil } return nil, ErrNoPendingCode @@ -593,13 +582,6 @@ func (s *service) Enroll(ctx context.Context, orgID, accountID uuid.UUID) (*mode }) return row, xerr } - l, xerr := s.newLink(ctx, orgID) - if xerr != nil { - return nil, xerr - } - if l.DisconnectPending { - return nil, ErrAlreadyLinked - } acc, xerr := s.ownedAccount(ctx, orgID, accountID) if xerr != nil { return nil, xerr @@ -607,6 +589,22 @@ func (s *service) Enroll(ctx context.Context, orgID, accountID uuid.UUID) (*mode if acc.Status != "active" { return nil, ErrMailboxInactive } + m, err := s.repo.GetByAccount(ctx, accountID) + if err != nil { + return nil, errx.InternalError() + } + var l *models.CloudLink + if m != nil { + l, xerr = s.mailboxLink(ctx, m) + } else { + l, xerr = s.link(ctx, orgID) + } + if xerr != nil { + return nil, xerr + } + if l.DisconnectPending { + return nil, ErrAlreadyLinked + } req := models.PoolLinkEnrollRequest{ RemoteID: acc.ID, Email: acc.Email, diff --git a/internal/app/cloudlink/workspace_links_test.go b/internal/app/cloudlink/workspace_links_test.go index b02ed0a56..fabc3fe6c 100644 --- a/internal/app/cloudlink/workspace_links_test.go +++ b/internal/app/cloudlink/workspace_links_test.go @@ -171,24 +171,73 @@ func TestWarmupReportsUseEachMailboxesOriginalLink(t *testing.T) { } } -func TestWorkspaceResolutionDoesNotGrantNewAccessOnLegacyLinks(t *testing.T) { +func TestWorkspaceResolutionKeepsExistingConnectionsUsable(t *testing.T) { org, other := uuid.New(), uuid.New() legacy := &models.CloudLink{InstanceID: uuid.New()} scoped := &models.CloudLink{InstanceID: uuid.New(), OrganizationID: &org} repo := &workspaceLinkRepo{links: map[uuid.UUID]*models.CloudLink{legacy.InstanceID: legacy, scoped.InstanceID: scoped}} svc := &service{repo: repo} - if got, xerr := svc.newLink(context.Background(), org); xerr != nil || got != scoped { + if got, xerr := svc.link(context.Background(), org); xerr != nil || got != scoped { t.Fatalf("workspace link = %v, %v", got, xerr) } - if _, xerr := svc.newLink(context.Background(), other); xerr != ErrLegacyLink { - t.Fatalf("legacy granted new access: %v", xerr) - } - if _, xerr := svc.StartOAuth(context.Background(), other, uuid.New(), models.InboxProviderGoogle); xerr != ErrLegacyLink { - t.Fatalf("legacy OAuth = %v", xerr) + if got, xerr := svc.link(context.Background(), other); xerr != nil || got != legacy { + t.Fatalf("existing server connection = %v, %v", got, xerr) } svc.emails = stubEmails{account: &models.Email{ID: uuid.New(), OrganizationID: &other}} - if _, xerr := svc.Enroll(context.Background(), other, uuid.New()); xerr != ErrLegacyLink { - t.Fatalf("legacy enrollment = %v", xerr) + if _, xerr := svc.Enroll(context.Background(), org, uuid.New()); xerr == nil { + t.Fatal("another workspace's mailbox was accepted") + } +} + +func TestExistingServerConnectionCanStartManagedOAuth(t *testing.T) { + f := newConsentFixture(t) + f.r.link.OrganizationID = nil + start, xerr := f.s.StartOAuth(context.Background(), f.org, f.user, models.InboxProviderGoogle) + if xerr != nil || start == nil || f.starts != 1 { + t.Fatalf("OAuth on existing connection = %+v, %v", start, xerr) + } + account, xerr := f.s.FinishOAuth(context.Background(), f.org, f.user, start.Session) + if xerr != nil || account == nil || f.r.mailbox.InstanceID != f.r.link.InstanceID { + t.Fatalf("managed enrollment on existing connection = %+v, %v", account, xerr) + } +} + +func TestExistingConnectionsPreventAdditionalConnectionRequests(t *testing.T) { + org, user := uuid.New(), uuid.New() + for _, scope := range []*uuid.UUID{nil, &org} { + l := &models.CloudLink{InstanceID: uuid.New(), OrganizationID: scope} + s := &service{repo: &workspaceLinkRepo{links: map[uuid.UUID]*models.CloudLink{l.InstanceID: l}}} + if _, xerr := s.StartConnect(context.Background(), org, user, "https://cloud.test"); xerr != ErrAlreadyLinked { + t.Fatalf("existing connection allowed another handshake (scope=%v): %v", scope, xerr) + } + if got, xerr := s.PollConnect(context.Background(), org, user); xerr != nil || got.Link != l || got.Status != models.PoolLinkCodeApproved { + t.Fatalf("existing connection not recognized after polling: %+v, %v", got, xerr) + } + } +} + +func TestExistingServerConnectionCanStartPlacementTests(t *testing.T) { + t.Setenv("APP_ENV", "dev") + org := uuid.New() + posts := 0 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Header.Get("Authorization") != "Bearer original" { + t.Error("placement changed connection") + } + if r.Method == http.MethodPost { + posts++ + } + _, _ = w.Write([]byte(`{}`)) + })) + defer srv.Close() + l := &models.CloudLink{InstanceID: uuid.New(), CloudURL: srv.URL, Token: "original"} + s := &service{repo: &workspaceLinkRepo{links: map[uuid.UUID]*models.CloudLink{l.InstanceID: l}}} + if _, xerr := s.PlacementPanel(context.Background(), org); xerr != nil { + t.Fatal(xerr) + } + result, xerr := s.StartPlacement(context.Background(), org, models.PlacementCloudStartRequest{Tests: 1}) + if xerr != nil || result == nil || result.InstanceID != l.InstanceID || posts != 1 { + t.Fatalf("placement on existing connection = %+v, %v", result, xerr) } } diff --git a/internal/app/placement/remote.go b/internal/app/placement/remote.go index dca20a7fc..1f9c4efd1 100644 --- a/internal/app/placement/remote.go +++ b/internal/app/placement/remote.go @@ -39,9 +39,6 @@ func (s *service) RemotePanel(ctx context.Context, inst *models.PoolLinkInstance // RemoteStart opens one test per requested variant against the same seeds, // charged to the linked workspace's allowance. func (s *service) RemoteStart(ctx context.Context, inst *models.PoolLinkInstance, req models.PlacementCloudStartRequest) (*models.PlacementCloudStart, *errx.Error) { - if inst.RemoteOrganizationID == nil { - return nil, placementErr(errx.Conflict, "pool_link_workspace_required", "Connect per workspace to start placement tests. Existing legacy results keep working.") - } if req.Tests < 1 || req.Tests > 2 { return nil, errx.New(errx.BadRequest, "tests must be 1 or 2") } diff --git a/internal/app/placement/service_test.go b/internal/app/placement/service_test.go index 927ac0cc8..dde9c5254 100644 --- a/internal/app/placement/service_test.go +++ b/internal/app/placement/service_test.go @@ -20,13 +20,6 @@ import ( "github.com/warmbly/warmbly/internal/tasks/proto" ) -func TestLegacyCloudLinkCannotStartNewPlacementTests(t *testing.T) { - svc := &service{} - if _, xerr := svc.RemoteStart(context.Background(), &models.PoolLinkInstance{ID: uuid.New()}, models.PlacementCloudStartRequest{Tests: 1}); xerr == nil || xerr.Identifier != "pool_link_workspace_required" { - t.Fatalf("legacy placement = %v", xerr) - } -} - // Each fake embeds the interface it stands in for, so a call the test does // not expect panics instead of passing silently. @@ -168,6 +161,25 @@ func (h *harness) input() CreateInput { return CreateInput{OrgID: h.org, SenderAccountID: h.sender, Subject: "Quick question", BodyPlain: "Hi there"} } +func TestExistingServerConnectionCanStartPlacementWithinWorkspaceQuota(t *testing.T) { + h := newHarness(t) + t.Setenv("DEPLOYMENT_MODE", "cloud") + h.svc.Gate = fakeGate{paid: false} + inst := &models.PoolLinkInstance{ID: uuid.New(), OrganizationID: h.org} + result, xerr := h.svc.RemoteStart(context.Background(), inst, models.PlacementCloudStartRequest{Tests: 1}) + if xerr != nil || result == nil || len(result.TestIDs) != 1 || len(h.repo.created) != 1 { + t.Fatalf("placement on existing connection = %+v, %v", result, xerr) + } + created := h.repo.created[0] + if created.OrganizationID == nil || *created.OrganizationID != h.org || created.RemoteInstanceID == nil || *created.RemoteInstanceID != inst.ID { + t.Fatalf("placement lost original connection ownership: %+v", created) + } + h.repo.metered = config.PlacementTestsPerMonthTrialDefault + if _, xerr := h.svc.RemoteStart(context.Background(), inst, models.PlacementCloudStartRequest{Tests: 1}); xerr == nil || xerr.Identifier != "placement_quota_exceeded" { + t.Fatalf("existing connection bypassed quota: %v", xerr) + } +} + func TestCreateTestsPicksAcrossFamiliesAndSkipsTheSendersDomain(t *testing.T) { h := newHarness(t) views, xerr := h.svc.CreateTests(context.Background(), h.input()) diff --git a/internal/app/poollink/adopt_warmup_test.go b/internal/app/poollink/adopt_warmup_test.go index 178af1f24..03095c46c 100644 --- a/internal/app/poollink/adopt_warmup_test.go +++ b/internal/app/poollink/adopt_warmup_test.go @@ -6,6 +6,7 @@ import ( "github.com/google/uuid" "github.com/warmbly/warmbly/internal/app/email" + "github.com/warmbly/warmbly/internal/app/feature" "github.com/warmbly/warmbly/internal/errx" "github.com/warmbly/warmbly/internal/models" "github.com/warmbly/warmbly/internal/repository" @@ -34,6 +35,48 @@ type adoptAccounts struct { acc *models.Email } +type existingConnectionAccounts struct { + managedOpsEmails + createdOrg uuid.UUID +} + +func (r *existingConnectionAccounts) NewSMTPIMAPAccount(_ context.Context, _ string, req models.NewSMTPIMAPAccount) (*models.Email, *errx.Error) { + r.createdOrg = *req.OrganizationID + a := &models.Email{ID: uuid.New(), OrganizationID: req.OrganizationID, Email: req.Email, Provider: "smtp_imap", Status: "active"} + r.accounts[a.ID] = a + return a, nil +} + +type freeConnectionGate struct{ feature.FeatureGateService } + +func (freeConnectionGate) HasPremiumWarmup(context.Context, uuid.UUID) (bool, *errx.Error) { + return false, nil +} + +func TestExistingServerConnectionEnrollsWithinItsCloudWorkspaceAllowance(t *testing.T) { + t.Setenv("BILLING_PROVIDER", "stripe") + org, owner := uuid.New(), uuid.New() + r := &adoptLinkRepo{} + accounts := &existingConnectionAccounts{managedOpsEmails: managedOpsEmails{accounts: map[uuid.UUID]*models.Email{}}} + emails := &adoptEmails{org: org} + s := &service{repo: r, emails: accounts, emailSvc: emails, gate: freeConnectionGate{}} + inst := &models.PoolLinkInstance{ID: uuid.New(), OrganizationID: org, CreatedBy: &owner} + tls := &models.Service{Host: "mail.example.test", Port: 993, Security: models.MailSecurityTLS} + req := models.PoolLinkEnrollRequest{RemoteID: uuid.New(), Email: "sender@example.test", Provider: models.InboxProviderSMTPIMAP, SMTPIMAP: &models.SmtpImap{SMTP: tls, IMAP: tls}} + got, xerr := s.Enroll(context.Background(), inst, req) + if xerr != nil || got == nil || r.enrolled.InstanceID != inst.ID || accounts.createdOrg != org || !emails.started { + t.Fatalf("enrollment on existing connection = %+v, %v", got, xerr) + } + for len(accounts.accounts) < models.FreeWorkspaceMailboxLimit { + accounts.accounts[uuid.New()] = &models.Email{} + } + r.enrolled = nil + req.RemoteID = uuid.New() + if _, xerr := s.Enroll(context.Background(), inst, req); xerr != ErrMailboxLimit { + t.Fatalf("existing connection bypassed allowance: %v", xerr) + } +} + func (r adoptAccounts) GetByID(context.Context, uuid.UUID) (*models.Email, *errx.Error) { return r.acc, nil } @@ -76,30 +119,20 @@ func TestApprovalExplainsThatCloudWorkspaceIsAlreadyConnected(t *testing.T) { } } -func TestLegacyInstancesCannotAddCloudMailboxes(t *testing.T) { +func TestNewConnectionsStillRequireWorkspaceScope(t *testing.T) { s := &service{repo: conflictApprovalRepo{}} - inst := &models.PoolLinkInstance{ID: uuid.New()} if _, xerr := s.StartCode(context.Background(), models.PoolLinkStartRequest{InstanceName: "Legacy"}); xerr != ErrLegacyLink { t.Fatalf("legacy device handshake = %v", xerr) } - if _, xerr := s.Adopt(context.Background(), inst, models.PoolLinkAdoptRequest{}); xerr != ErrLegacyLink { - t.Fatalf("legacy adoption = %v", xerr) - } - if _, xerr := s.Enroll(context.Background(), inst, models.PoolLinkEnrollRequest{RemoteID: uuid.New(), Email: "user@example.com"}); xerr != ErrLegacyLink { - t.Fatalf("legacy enrollment = %v", xerr) - } - if _, xerr := s.StartOAuth(context.Background(), inst, models.PoolLinkOAuthStartRequest{}); xerr != ErrLegacyLink { - t.Fatalf("legacy OAuth = %v", xerr) - } } // Adopting a workspace mailbox into a linked instance must start its warmup on the cloud. -func TestAdoptStartsWarmupInTheInstanceWorkspace(t *testing.T) { +func TestExistingServerConnectionAdoptsAndStartsWarmupInItsCloudWorkspace(t *testing.T) { org, owner := uuid.New(), uuid.New() acc := &models.Email{ID: uuid.New(), OrganizationID: &org, Status: "active", Provider: string(models.InboxProviderGoogle)} emails := &adoptEmails{org: org} s := &service{repo: &adoptLinkRepo{}, emails: adoptAccounts{acc: acc}, emailSvc: emails} - inst := &models.PoolLinkInstance{ID: uuid.New(), OrganizationID: org, RemoteOrganizationID: &org, CreatedBy: &owner} + inst := &models.PoolLinkInstance{ID: uuid.New(), OrganizationID: org, CreatedBy: &owner} if _, xerr := s.Adopt(context.Background(), inst, models.PoolLinkAdoptRequest{RemoteID: uuid.New(), EmailAccountID: acc.ID}); xerr != nil { t.Fatalf("Adopt: %v", xerr) @@ -107,4 +140,9 @@ func TestAdoptStartsWarmupInTheInstanceWorkspace(t *testing.T) { if !emails.started { t.Fatal("warmup was not started in the workspace; the mailbox would sit enrolled and never warm") } + other := uuid.New() + acc.OrganizationID = &other + if _, xerr := s.Adopt(context.Background(), inst, models.PoolLinkAdoptRequest{RemoteID: uuid.New(), EmailAccountID: acc.ID}); xerr != ErrNotAdoptable { + t.Fatalf("another workspace's mailbox was accepted: %v", xerr) + } } diff --git a/internal/app/poollink/oauth.go b/internal/app/poollink/oauth.go index b07d9babe..948cb62da 100644 --- a/internal/app/poollink/oauth.go +++ b/internal/app/poollink/oauth.go @@ -104,9 +104,6 @@ func returnURLAllowed(raw, instanceURL string) bool { } func (s *service) StartOAuth(ctx context.Context, inst *models.PoolLinkInstance, req models.PoolLinkOAuthStartRequest) (*models.PoolLinkOAuthStartResponse, *errx.Error) { - if inst.RemoteOrganizationID == nil { - return nil, ErrLegacyLink - } if req.Protocol != 0 { if req.Protocol != models.ManagedConsentProtocol || req.RemoteID == uuid.Nil || len(req.Session) < 32 || len(req.Session) > 128 { return nil, ErrBadRequest @@ -404,9 +401,6 @@ func (s *service) connectBrokered(ctx context.Context, st brokerState, code stri if inst == nil || inst.RevokedAt != nil { return uuid.Nil, ErrInstanceRevoked } - if inst.RemoteOrganizationID == nil { - return uuid.Nil, ErrLegacyLink - } userID, xerr := s.ownerUserID(ctx, inst) if xerr != nil { return uuid.Nil, xerr @@ -540,9 +534,6 @@ func (s *service) AccessToken(ctx context.Context, inst *models.PoolLinkInstance } func (s *service) ListWorkspaceMailboxes(ctx context.Context, inst *models.PoolLinkInstance) ([]models.PoolLinkWorkspaceMailbox, *errx.Error) { - if inst.RemoteOrganizationID == nil { - return nil, ErrLegacyLink - } list, err := s.repo.ListAdoptableMailboxes(ctx, inst.OrganizationID) if err != nil { return nil, errx.InternalError() @@ -552,9 +543,6 @@ func (s *service) ListWorkspaceMailboxes(ctx context.Context, inst *models.PoolL // Adopt links a mailbox that was connected directly on the workspace. func (s *service) Adopt(ctx context.Context, inst *models.PoolLinkInstance, req models.PoolLinkAdoptRequest) (*models.PoolLinkMailboxState, *errx.Error) { - if inst.RemoteOrganizationID == nil { - return nil, ErrLegacyLink - } if req.Protocol != 0 { if req.Protocol != models.ManagedConsentProtocol || req.RemoteID == uuid.Nil || req.EmailAccountID == uuid.Nil { return nil, ErrBadRequest diff --git a/internal/app/poollink/service.go b/internal/app/poollink/service.go index 1ef1a6112..b628a0ca1 100644 --- a/internal/app/poollink/service.go +++ b/internal/app/poollink/service.go @@ -28,7 +28,7 @@ import ( ) var ( - ErrLegacyLink = errx.NewWithIdentifier(errx.Conflict, "pool_link_workspace_required", "Reconnect per workspace to add mailboxes. Existing legacy enrollments continue working.") + ErrLegacyLink = errx.NewWithIdentifier(errx.Conflict, "pool_link_workspace_required", "Update this self-hosted instance before creating a new connection. Existing connections continue working.") ErrWorkspaceLinked = errx.NewWithIdentifier(errx.Conflict, "pool_link_workspace_connected", "This Cloud workspace already has an active connection. Use a separate Cloud workspace and subscription, or disconnect the existing link first.") ErrCodeNotFound = errx.NewWithIdentifier(errx.NotFound, "pool_link_code_not_found", "That code is unknown or has expired. Start the connection again from your instance.") ErrCodeNotPending = errx.NewWithIdentifier(errx.Conflict, "pool_link_code_used", "That code has already been used.") @@ -450,10 +450,6 @@ func (s *service) Enroll(ctx context.Context, inst *models.PoolLinkInstance, req return s.PatchMailbox(ctx, inst, req.RemoteID, models.PoolLinkMailboxPatch{Warmup: &req.Warmup, OAuth: req.OAuth, SMTPIMAP: req.SMTPIMAP}) } - if inst.RemoteOrganizationID == nil { - return nil, ErrLegacyLink - } - plan, xerr := s.Plan(ctx, inst.OrganizationID) if xerr != nil { return nil, xerr diff --git a/internal/repository/pg_cloudlink.go b/internal/repository/pg_cloudlink.go index 89e5cfd43..c9b5bbac0 100644 --- a/internal/repository/pg_cloudlink.go +++ b/internal/repository/pg_cloudlink.go @@ -124,7 +124,8 @@ func (r *cloudLinkRepository) ListLinks(ctx context.Context) ([]models.CloudLink func (r *cloudLinkRepository) GetForRedirect(ctx context.Context, orgID uuid.UUID, domain string) (*models.CloudLink, error) { query := `SELECT ` + cloudLinkColumns + ` FROM cloud_link WHERE instance_id = COALESCE( (SELECT cloud_link_instance_id FROM domain_redirects WHERE organization_id = $1 AND domain = $2), - (SELECT instance_id FROM cloud_link WHERE organization_id = $1 LIMIT 1) + (SELECT instance_id FROM cloud_link WHERE organization_id = $1 OR organization_id IS NULL + ORDER BY organization_id IS NULL LIMIT 1) )` return r.scanLink(r.db.QueryRow(ctx, query, orgID, domain)) } diff --git a/internal/repository/workspace_cloud_links_live_test.go b/internal/repository/workspace_cloud_links_live_test.go index 3a0f23b99..99f658767 100644 --- a/internal/repository/workspace_cloud_links_live_test.go +++ b/internal/repository/workspace_cloud_links_live_test.go @@ -101,8 +101,11 @@ func TestLiveWorkspaceLinksPreserveLegacyEnrollmentOwnership(t *testing.T) { if err != nil || got == nil || got.InstanceID != legacy.InstanceID { t.Fatalf("legacy fallback = %v, %v", got, err) } - if got, err := repo.GetForRedirect(ctx, other, "unused.test"); err != nil || got != nil { - t.Fatalf("new redirect used legacy link = %v, %v", got, err) + if got, err := repo.GetForRedirect(ctx, other, "unused.test"); err != nil || got == nil || got.InstanceID != legacy.InstanceID { + t.Fatalf("existing server connection unavailable for redirect = %v, %v", got, err) + } + if got, err := repo.GetForRedirect(ctx, f.org, "unused.test"); err != nil || got == nil || got.InstanceID != scoped.InstanceID { + t.Fatalf("workspace redirect used another connection = %v, %v", got, err) } if err := NewOrganizationRepository(f.pool).Delete(ctx, f.org); !errors.Is(err, ErrOrganizationCloudLinked) { t.Fatalf("linked workspace deletion = %v", err) diff --git a/web/src/app/app/settings/warmbly-cloud/MailboxTable.tsx b/web/src/app/app/settings/warmbly-cloud/MailboxTable.tsx index a026f8002..b062c6620 100644 --- a/web/src/app/app/settings/warmbly-cloud/MailboxTable.tsx +++ b/web/src/app/app/settings/warmbly-cloud/MailboxTable.tsx @@ -19,7 +19,7 @@ import { TableSurface, Toggle } from "../_components/SectionShell"; import { providerLabel, providerSupported } from "./providers"; import { cloudWarmupPaused } from "@/lib/cloudWarmup"; -export default function MailboxTable({ allowEnrollment = true }: { allowEnrollment?: boolean }) { +export default function MailboxTable() { const rows = useCloudLinkMailboxes(); const enroll = useEnrollCloudLinkMailbox(); const unenroll = useUnenrollCloudLinkMailbox(); @@ -92,7 +92,6 @@ export default function MailboxTable({ allowEnrollment = true }: { allowEnrollme

{providerLabel(row.provider)} {row.managed && " · signed in through Warmbly Cloud"} - {row.legacy && " · legacy connection"} {!supported && " · signed in with this instance's own OAuth app; add it again through Warmbly Cloud to warm it"} {row.enrolled && !cloud && " · waiting for the cloud"} {cloud?.errors && cloud.errors.length > 0 && ( @@ -148,7 +147,7 @@ export default function MailboxTable({ allowEnrollment = true }: { allowEnrollme {busy === row.id ? ( ) : ( - flip(row)} /> + flip(row)} /> )} diff --git a/web/src/app/app/settings/warmbly-cloud/page.test.tsx b/web/src/app/app/settings/warmbly-cloud/page.test.tsx new file mode 100644 index 000000000..39901fad0 --- /dev/null +++ b/web/src/app/app/settings/warmbly-cloud/page.test.tsx @@ -0,0 +1,103 @@ +import { fireEvent, render, screen, waitFor } from "@testing-library/react"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import type { CloudLinkMailboxRow, CloudLinkStatus } from "@/lib/api/models/app/cloudlink/CloudLink"; +import WarmblyCloudSettingsPage from "./page"; + +const fixture = vi.hoisted(() => ({ + status: {} as CloudLinkStatus, + rows: [] as CloudLinkMailboxRow[], + enroll: vi.fn(), + disconnect: vi.fn(), + confirm: vi.fn(), +})); + +vi.mock("@/hooks/usePermission", () => ({ + usePermission: () => true, + useInstanceAdmin: () => ({ allowed: true }), +})); +vi.mock("@/lib/api/hooks/auth/useAuthConfig", () => ({ default: () => ({ data: { self_hosted: true } }) })); +vi.mock("@/lib/api/hooks/app/organizations/useCurrentOrganization", () => ({ default: () => ({ data: { id: "local" } }) })); +vi.mock("@/hooks/context/confirm", () => ({ useConfirm: () => ({ show: fixture.confirm }) })); +vi.mock("@/lib/api/hooks/app/cloudlink/useCloudLink", () => ({ + useCloudLinkStatus: () => ({ data: fixture.status, refetch: vi.fn() }), + useDisconnectCloudLink: () => ({ mutateAsync: fixture.disconnect }), + useCloudLinkMailboxes: () => ({ data: fixture.rows }), + useEnrollCloudLinkMailbox: () => ({ mutateAsync: fixture.enroll }), + useUnenrollCloudLinkMailbox: () => ({ mutateAsync: vi.fn() }), + useCloudLinkMailboxLifecycle: () => ({ mutateAsync: vi.fn() }), +})); +vi.mock("./ConnectFlow", () => ({ default: () =>

Connection setup
})); +vi.mock("./LinkedInstances", () => ({ default: () => null })); + +describe("existing Cloud connections", () => { + beforeEach(() => { + vi.clearAllMocks(); + fixture.enroll.mockResolvedValue(undefined); + fixture.disconnect.mockResolvedValue(undefined); + fixture.status = { + connected: true, + reachable: true, + legacy_connected: true, + default_cloud_url: "https://cloud.test", + link: { + instance_id: "original", + cloud_url: "https://cloud.test", + organization_name: "Existing Cloud workspace", + connected_at: new Date("2025-01-01"), + }, + }; + fixture.rows = [{ + id: "mailbox", email: "sender@test.local", name: "Sender", + provider: "smtp_imap", status: "active", enrolled: false, managed: false, legacy: true, + }]; + }); + + it("shows the normal connection and allows enrollment without a migration", async () => { + render(); + expect(screen.getByText("Existing Cloud workspace")).toBeInTheDocument(); + expect(screen.queryByText(/legacy/i)).not.toBeInTheDocument(); + expect(screen.queryByText("Connect this workspace")).not.toBeInTheDocument(); + expect(screen.queryByText("Connection setup")).not.toBeInTheDocument(); + expect(screen.getByRole("switch")).toBeEnabled(); + fireEvent.click(screen.getByRole("switch")); + await waitFor(() => expect(fixture.enroll).toHaveBeenCalledWith("mailbox")); + }); + + it("still prevents enrollment of an unsupported local OAuth mailbox", () => { + fixture.rows[0].provider = "gmail"; + render(); + expect(screen.getByRole("switch")).toBeDisabled(); + }); + + it("requires confirmation of server-wide disconnect scope", async () => { + render(); + fireEvent.click(screen.getByRole("button", { name: "Disconnect" })); + expect(fixture.disconnect).not.toHaveBeenCalled(); + expect(fixture.confirm.mock.calls[0][0]).toContain("across all local workspaces"); + await fixture.confirm.mock.calls[0][1](); + expect(fixture.disconnect).toHaveBeenCalledWith(true); + }); + + it("keeps both disconnect scopes explicit when connections coexist", async () => { + fixture.status.link!.organization_id = "local"; + render(); + expect(screen.queryByText(/legacy/i)).not.toBeInTheDocument(); + expect(screen.queryByText("Connection setup")).not.toBeInTheDocument(); + fireEvent.click(screen.getByRole("button", { name: "Disconnect" })); + expect(fixture.confirm.mock.calls[0][0]).toContain("this workspace"); + await fixture.confirm.mock.calls[0][1](); + expect(fixture.disconnect).toHaveBeenLastCalledWith(false); + fireEvent.click(screen.getByRole("button", { name: "Disconnect shared connection" })); + expect(fixture.confirm.mock.calls[1][0]).toContain("across all local workspaces"); + await fixture.confirm.mock.calls[1][1](); + expect(fixture.disconnect).toHaveBeenLastCalledWith(true); + }); + + it("only offers connection setup when disconnected", () => { + fixture.status.connected = false; + fixture.status.legacy_connected = false; + fixture.status.link = null; + render(); + expect(screen.getByText("Connection setup")).toBeInTheDocument(); + }); +}); diff --git a/web/src/app/app/settings/warmbly-cloud/page.tsx b/web/src/app/app/settings/warmbly-cloud/page.tsx index eb68dc1f5..5753f0d0a 100644 --- a/web/src/app/app/settings/warmbly-cloud/page.tsx +++ b/web/src/app/app/settings/warmbly-cloud/page.tsx @@ -75,7 +75,7 @@ function SelfHostedCloud() { } const st = status.data; const plan = st.info?.plan; - const legacy = st.connected && !st.link?.organization_id; + const serverWide = st.connected && !st.link?.organization_id; return ( - {legacy && ( -
- -
- )} - {st.legacy_connected && !legacy && ( -
- -
- )} {showFlow ? ( { setFlow(false); void status.refetch(); @@ -146,7 +127,7 @@ function SelfHostedCloud() { /> {plan && ( -
+
)} @@ -154,20 +135,20 @@ function SelfHostedCloud() { eyebrow="Mailboxes" description="Enrolled mailboxes are warmed by Warmbly Cloud; their local warmup stops. Campaigns keep sending from this server." > - +
+ {st.legacy_connected && !serverWide && ( + + + + )}
)}