From e35baa7f58231f761277b7d9f6e0babc5ae42f64 Mon Sep 17 00:00:00 2001 From: Matthew Meszaros Date: Wed, 23 Sep 2026 21:12:09 -0700 Subject: [PATCH 1/4] feat: connect every inbox vendor with only its API key by discovering the workspaces and organizations the key reaches (InboxKit, Zapmail, Infraforge, ScaledMail), carry the workspace in vendor mailbox and domain ids with a lookup for ids stored before, skip a workspace the key may not read, refuse a key that reaches none with mailbox_vendor_no_workspace, and add a workspace switcher to the mailbox picker that filters, counts picks and selects a whole workspace --- docs/content/docs/api/error-codes.mdx | 3 +- docs/content/docs/development/sandbox.mdx | 4 +- docs/content/docs/guides/mailbox-import.mdx | 26 +- docs/public/openapi.json | 4 + internal/app/vendorconn/service.go | 10 +- internal/models/mailbox_import.go | 1 + internal/pkg/mailvendor/domains_test.go | 72 ++-- internal/pkg/mailvendor/forge.go | 60 +++- internal/pkg/mailvendor/inboxkit.go | 252 ++++++++----- internal/pkg/mailvendor/mailvendor.go | 45 +-- internal/pkg/mailvendor/mailvendor_test.go | 19 +- internal/pkg/mailvendor/scaledmail.go | 218 +++++++++--- internal/pkg/mailvendor/vendors_test.go | 189 +++++++--- internal/pkg/mailvendor/workspaces.go | 108 ++++++ internal/pkg/mailvendor/zapmail.go | 336 +++++++++++------- internal/sandbox/vendors.go | 74 +++- .../app/emails/import/PickTable.tsx | 154 +++++++- .../import/vendors/VendorImportWizard.tsx | 9 +- .../api/models/app/emails/MailboxSources.ts | 2 + 19 files changed, 1144 insertions(+), 442 deletions(-) create mode 100644 internal/pkg/mailvendor/workspaces.go diff --git a/docs/content/docs/api/error-codes.mdx b/docs/content/docs/api/error-codes.mdx index 7eb64626b..22658f693 100644 --- a/docs/content/docs/api/error-codes.mdx +++ b/docs/content/docs/api/error-codes.mdx @@ -183,7 +183,8 @@ The [inbox vendor](/guides/mailbox-import/#import-from-an-inbox-vendor) routes ( | `code` | Status | Meaning | |--------|--------|---------| -| `mailbox_vendor_unauthorized` | 400 | The vendor did not accept the API key, or it has no access to the workspace or organization entered. A saved connection is marked invalid until the key is updated | +| `mailbox_vendor_unauthorized` | 400 | The vendor did not accept the API key. A saved connection is marked invalid until the key is updated | +| `mailbox_vendor_no_workspace` | 400 | The key is accepted but the vendor lists no workspace or organization for it, so there is nothing to read. Create one at the vendor, then connect again | | `mailbox_vendor_invalid_fields` | 400 | A field the vendor requires is empty, or a field holds control characters. `GET /emails/vendors/catalog` lists each vendor's fields | | `mailbox_vendor_unknown` | 400 | `vendor` is not one of the supported vendors | | `mailbox_vendor_rate_limited` | 429 | The vendor is rate limiting Warmbly's requests. Try again in a minute | diff --git a/docs/content/docs/development/sandbox.mdx b/docs/content/docs/development/sandbox.mdx index c726a385e..ad38d63ee 100644 --- a/docs/content/docs/development/sandbox.mdx +++ b/docs/content/docs/development/sandbox.mdx @@ -109,11 +109,11 @@ Personas are derived from a hash of the contact's address, so behavior is stable The simulator also plays two inbox vendors, so connecting a vendor account, importing its mailboxes and managing its domains work with no vendor account. `make sandbox` starts it and points the backend at it with `MAILVENDOR_SANDBOX_URL=http://127.0.0.1:18099`. To run only the mock next to your own backend, use `go run ./cmd/sandbox -vendors-only` and start the backend with the same variable. The backend ignores it unless `APP_ENV` is `dev`. -Connect either vendor from **Add account > Inbox vendor** with any API key and workspace ID: +Connect either vendor from **Add account > Inbox vendor** with any API key. The InboxKit mock has two workspaces, so its mailboxes show which one each sits in: | Vendor | Mailboxes | Domains | |--------|-----------|---------| -| InboxKit | eight Google Workspace mailboxes on `sunrise-outbound.test` and `trysunrise.test`. Only a password comes back, as with the real API, and `.test` has no mail servers, so their rows ask for server settings; **Fix** on a row can point it at the sandbox's Mailpit (`localhost:11025`) and Dovecot (`localhost:10993`) | forwarding and DNS writes, so **Sending domains** can set the root forward and the tracking record in one click | +| InboxKit | eight Google Workspace mailboxes, on `sunrise-outbound.test` in the "Sunrise Outbound" workspace and `trysunrise.test` in "Sunrise Trials". Only a password comes back, as with the real API, and `.test` has no mail servers, so their rows ask for server settings; **Fix** on a row can point it at the sandbox's Mailpit (`localhost:11025`) and Dovecot (`localhost:10993`) | forwarding and DNS writes, so **Sending domains** can set the root forward and the tracking record in one click | | Mailforge | four mailboxes on `sunrisehq.test`, hosted on the sandbox's Mailpit and Dovecot, so they import and connect for real | forwarding only | The mock keeps its state in memory, so a restart puts every domain back the way it started. diff --git a/docs/content/docs/guides/mailbox-import.mdx b/docs/content/docs/guides/mailbox-import.mdx index ea4833241..b10b3aaa7 100644 --- a/docs/content/docs/guides/mailbox-import.mdx +++ b/docs/content/docs/guides/mailbox-import.mdx @@ -107,24 +107,26 @@ Proton Mail offers no IMAP or SMTP sign-in to other apps; only its Bridge does. ## Import from an inbox vendor -Choose **Add account**, then **Inbox vendor**, pick the vendor and enter what it asks for. Warmbly checks the key with the vendor before saving it, lists the mailboxes in that vendor account (marking the ones already in the workspace), and imports the ones you pick, or every one not yet connected. +Choose **Add account**, then **Inbox vendor**, pick the vendor and paste an API key. That is all any vendor asks for. Warmbly checks the key with the vendor before saving it, lists the mailboxes the key reaches (marking the ones already in the workspace), and imports the ones you pick, or every one not yet connected. -| Vendor | Asks for | Where to find the key | -| --- | --- | --- | -| InboxKit | API key and workspace ID (a UUID) | Settings > API & Integrations in the InboxKit dashboard | -| Zapmail | API key; optionally a workspace ID (blank is your primary workspace) and a mailbox type (`GOOGLE` or `MICROSOFT`, blank imports both) | Settings > Integrations > API in the Zapmail dashboard | -| Mailforge | API key | Settings > API keys in the Mailforge dashboard | -| Infraforge | API key; optionally a workspace ID (blank imports every workspace) | Settings > API keys in the Infraforge dashboard | -| Maildoso | API key (a personal access token) | Settings > API Keys in the Maildoso dashboard | -| Cheap Inboxes | API key, starting with `ci_live_` | Integrations > API in the Cheap Inboxes dashboard | -| ScaledMail | API key and organization ID | Settings in the ScaledMail dashboard | +A key that reaches several workspaces (or, at ScaledMail, organizations) lists the mailboxes of all of them, so there is no workspace ID to look up. Above the list, a workspace switcher narrows it to one workspace and shows how many of each you have picked, and **Select … in** picks every mailbox in that workspace that is not connected yet in one click. Pick the ones you want; the rest stay at the vendor. + +| Vendor | Where to find the key | +| --- | --- | +| InboxKit | Settings > API & Integrations in the InboxKit dashboard | +| Zapmail | Settings > Integrations > API in the Zapmail dashboard. Google and Microsoft mailboxes are both listed | +| Mailforge | Settings > API keys in the Mailforge dashboard | +| Infraforge | Settings > API keys in the Infraforge dashboard | +| Maildoso | Settings > API Keys in the Maildoso dashboard (a personal access token) | +| Cheap Inboxes | Integrations > API in the Cheap Inboxes dashboard (starts with `ci_live_`) | +| ScaledMail | Settings in the ScaledMail dashboard | One vendor account lists up to 10,000 mailboxes. - **Credentials are read when each row is worked.** Picking mailboxes stores only which ones you picked. The password, app password and servers of each are fetched from the vendor at the moment its row connects, and the row is then judged exactly like a file row: Google mailboxes need the app password the vendor holds (or [an admin grant](#connect-a-whole-google-workspace-domain) over the domain), and Microsoft mailboxes wait for sign-in unless [a grant](#connect-a-whole-microsoft-365-organization) covers them. -- **The key is sealed and never shown again.** The API key and IDs are sealed with the workspace's own encryption key. The list of connections shows each one's vendor, label, status and mailbox count, never the key. **Update** replaces the key after checking the new one with the vendor. +- **The key is sealed and never shown again.** The API key is sealed with the workspace's own encryption key. The list of connections shows each one's vendor, label, status and mailbox count, never the key. **Update** replaces the key after checking the new one with the vendor. - **Passwords the vendor rotates are picked up.** An SMTP and IMAP mailbox that came from a vendor and has stopped with an unresolved connection error is looked at every 15 minutes. When the vendor now holds a different password for it, Warmbly verifies that password against the server and reconnects the mailbox with it. Each mailbox is tried at most once every 6 hours, so a password that still fails is not retried on every pass. -- **A refused key marks the connection.** When the vendor stops accepting the key, the connection shows as invalid with the vendor's reason, and rows that needed it fail as `vendor_unauthorized` until the key is updated. +- **A refused key marks the connection.** A key the vendor accepts but that reaches no workspace is refused with `mailbox_vendor_no_workspace`, since there is nothing to list. When the vendor stops accepting the key, the connection shows as invalid with the vendor's reason, and rows that needed it fail as `vendor_unauthorized` until the key is updated. - **Deleting the connection keeps the mailboxes.** They stay connected and keep sending; they are no longer reconnected automatically when the vendor rotates a password. ## Connect a whole Google Workspace domain diff --git a/docs/public/openapi.json b/docs/public/openapi.json index 4872aac5a..a51c5bda4 100644 --- a/docs/public/openapi.json +++ b/docs/public/openapi.json @@ -37014,6 +37014,10 @@ "status": { "type": "string" }, + "workspace": { + "type": "string", + "description": "The vendor workspace or organization holding the mailbox, when the vendor has them." + }, "connected": { "type": "boolean" }, diff --git a/internal/app/vendorconn/service.go b/internal/app/vendorconn/service.go index e1841e1ad..f26fa0651 100644 --- a/internal/app/vendorconn/service.go +++ b/internal/app/vendorconn/service.go @@ -34,6 +34,7 @@ const ( ErrIDInvalidFields = "mailbox_vendor_invalid_fields" ErrIDRateLimited = "mailbox_vendor_rate_limited" ErrIDUnavailable = "mailbox_vendor_unavailable" + ErrIDNoWorkspace = "mailbox_vendor_no_workspace" ) // Mailboxes is the mailbox store's side. @@ -125,6 +126,9 @@ func (s *Service) Catalog() []Vendor { for _, d := range ds { v := Vendor{ID: d.ID, Label: d.Label, Website: d.Website, KeyHelpURL: d.KeyHelpURL} for _, f := range d.Fields { + if f.Legacy { + continue + } v.Fields = append(v.Fields, VendorField{Key: f.Key, Label: f.Label, Secret: f.Secret, Required: f.Required, Help: f.Help}) } out = append(out, v) @@ -137,7 +141,9 @@ func vendorError(label string, err error) *errx.Error { switch { case errors.Is(err, mailvendor.ErrUnauthorized): return errx.NewWithIdentifier(errx.BadRequest, mailboximport.ErrIDVendorUnauthorized, - label+" did not accept this API key. Check that it is current and has access to the workspace or organization entered.") + label+" did not accept this API key. Check that it is current and was copied in full.") + case errors.Is(err, mailvendor.ErrNoWorkspace): + return errx.NewWithIdentifier(errx.BadRequest, ErrIDNoWorkspace, label+" shows no workspace for this API key. Create one at "+label+", then try again.") case errors.Is(err, mailvendor.ErrRateLimited): return errx.NewWithIdentifier(errx.TooManyRequests, ErrIDRateLimited, label+" is rate limiting requests. Try again in a minute.") case errors.Is(err, mailvendor.ErrInvalidConfig): @@ -374,7 +380,7 @@ func (s *Service) Mailboxes(ctx context.Context, orgID, id uuid.UUID) ([]models. } out = append(out, models.VendorMailbox{ ID: m.ID, Email: m.Email, Name: strings.TrimSpace(m.FirstName + " " + m.LastName), - Domain: m.Domain, Provider: m.Provider, Status: m.Status, + Domain: m.Domain, Provider: m.Provider, Status: m.Status, Workspace: m.Workspace, }) emails = append(emails, m.Email) } diff --git a/internal/models/mailbox_import.go b/internal/models/mailbox_import.go index dc2b34e06..3d17335d1 100644 --- a/internal/models/mailbox_import.go +++ b/internal/models/mailbox_import.go @@ -344,6 +344,7 @@ type VendorMailbox struct { Domain string `json:"domain"` Provider string `json:"provider"` Status string `json:"status"` + Workspace string `json:"workspace,omitempty"` Connected bool `json:"connected"` AccountID *uuid.UUID `json:"email_account_id,omitempty"` } diff --git a/internal/pkg/mailvendor/domains_test.go b/internal/pkg/mailvendor/domains_test.go index 1da9579d7..1e8fcdda8 100644 --- a/internal/pkg/mailvendor/domains_test.go +++ b/internal/pkg/mailvendor/domains_test.go @@ -32,6 +32,14 @@ func decodeBody(t *testing.T, r seenRequest) map[string]any { // testDomain is a domain every vendor's calls accept. var testDomain = Domain{ID: "7", Name: "acme.io"} +// testDomainFor is testDomain as the vendor lists it; workspace-scoped vendors carry the workspace in the id. +func testDomainFor(vendor string) Domain { + if vendor == VendorInboxKit || vendor == VendorScaledMail || vendor == VendorZapmail { + return Domain{ID: "ws-1:7", Name: "acme.io"} + } + return testDomain +} + func TestDomainCapabilitiesAllVendors(t *testing.T) { want := map[string]DomainCapabilities{ VendorInboxKit: {Forwarding: true, ForwardingRemove: true, DNS: true, DNSTypes: allDNSTypes()}, @@ -66,11 +74,11 @@ func TestDomainCallsUnauthorizedAllVendors(t *testing.T) { dm := domainManager(t, newTestClient(t, d.ID, fieldsFor(d.ID), srv.URL, nil)) runs := map[string]func() error{ "domains": func() error { _, err := dm.Domains(context.Background()); return err }, - "forwarding": func() error { return dm.SetForwarding(context.Background(), testDomain, "https://acme.com") }, + "forwarding": func() error { return dm.SetForwarding(context.Background(), testDomainFor(d.ID), "https://acme.com") }, } if dm.DomainCapabilities().DNS { runs["dns"] = func() error { - return dm.UpsertDNSRecord(context.Background(), testDomain, DNSRecord{Type: "TXT", Name: "_warmbly.acme.io", Value: "warmbly-verify=abc"}) + return dm.UpsertDNSRecord(context.Background(), testDomainFor(d.ID), DNSRecord{Type: "TXT", Name: "_warmbly.acme.io", Value: "warmbly-verify=abc"}) } } for name, run := range runs { @@ -211,6 +219,8 @@ func TestInboxKitDomains(t *testing.T) { {"_id":"60f7b8c9e4b0b8c9e4b0b8c4","host":"@","type":"A","value":"192.0.2.2","ttl":3600}]}}` srv := newRecorder(t, func(w http.ResponseWriter, r *http.Request) { switch r.URL.Path { + case "/v1/api/workspaces/list": + writeJSON(w, 200, `{"error":false,"workspaces":[{"uid":"`+ws+`","name":"Main"}]}`) case "/v1/api/domains/list": var body struct{ Page int } _ = json.NewDecoder(r.Body).Decode(&body) @@ -233,18 +243,18 @@ func TestInboxKitDomains(t *testing.T) { writeJSON(w, 404, `{}`) } }) - dm := domainManager(t, newTestClient(t, VendorInboxKit, map[string]string{FieldAPIKey: testKey, FieldWorkspaceID: ws}, srv.URL, nil)) + dm := domainManager(t, newTestClient(t, VendorInboxKit, map[string]string{FieldAPIKey: testKey}, srv.URL, nil)) ctx := context.Background() got, err := dm.Domains(ctx) if err != nil { t.Fatal(err) } - want := []Domain{{ID: "3340bae3-53c3-4ab8-9c61-1d8205b86c70", Name: "example.com", Forwarding: "https://example.org"}, {ID: "u2", Name: "acme.io"}} + want := []Domain{{ID: ws + ":3340bae3-53c3-4ab8-9c61-1d8205b86c70", Name: "example.com", Forwarding: "https://example.org"}, {ID: ws + ":u2", Name: "acme.io"}} if !reflect.DeepEqual(got, want) { t.Fatalf("Domains = %+v", got) } - d := Domain{ID: "u2", Name: "acme.io"} + d := got[1] if err := dm.SetForwarding(ctx, d, "https://acme.com"); err != nil { t.Fatal(err) } @@ -260,14 +270,20 @@ func TestInboxKitDomains(t *testing.T) { t.Fatal(err) } // A new CNAME on a domain with no records yet. - if err := dm.UpsertDNSRecord(ctx, Domain{ID: "fresh", Name: "acme.io"}, DNSRecord{Type: "CNAME", Name: "track.acme.io", Value: "track.warmbly.com"}); err != nil { + if err := dm.UpsertDNSRecord(ctx, Domain{ID: ws + ":fresh", Name: "acme.io"}, DNSRecord{Type: "CNAME", Name: "track.acme.io", Value: "track.warmbly.com"}); err != nil { t.Fatal(err) } + // A domain id without its workspace cannot be addressed, so nothing is sent. + if err := dm.SetForwarding(ctx, Domain{ID: "u2", Name: "acme.io"}, "https://acme.com"); !errors.Is(err, ErrInvalidConfig) { + t.Fatalf("unscoped id: %v", err) + } reqs := srv.requests() for _, r := range reqs { assertHeader(t, r, "Authorization", "Bearer "+testKey) - assertHeader(t, r, "X-Workspace-Id", ws) + if r.Path != "/v1/api/workspaces/list" { + assertHeader(t, r, "X-Workspace-Id", ws) + } } var writes []map[string]any var paths []string @@ -305,7 +321,7 @@ func TestInboxKitDNSWithoutRecordIDs(t *testing.T) { writeJSON(w, 200, `{"error":false,"dns_record":{"records":[{"host":"_warmbly","type":"TXT","value":"warmbly-verify=old"}]}}`) }) dm := domainManager(t, newTestClient(t, VendorInboxKit, fieldsFor(VendorInboxKit), srv.URL, nil)) - err := dm.UpsertDNSRecord(context.Background(), Domain{ID: "u2", Name: "acme.io"}, DNSRecord{Type: "TXT", Name: "_warmbly.acme.io", Value: "warmbly-verify=new"}) + err := dm.UpsertDNSRecord(context.Background(), Domain{ID: "ws-1:u2", Name: "acme.io"}, DNSRecord{Type: "TXT", Name: "_warmbly.acme.io", Value: "warmbly-verify=new"}) if !errors.Is(err, ErrUnsupported) { t.Fatalf("err = %v, want ErrUnsupported", err) } @@ -313,7 +329,7 @@ func TestInboxKitDNSWithoutRecordIDs(t *testing.T) { t.Fatalf("%d requests, want only the list", n) } // An identical record needs no id: nothing is written. - if err := dm.UpsertDNSRecord(context.Background(), Domain{ID: "u2", Name: "acme.io"}, DNSRecord{Type: "TXT", Name: "_warmbly.acme.io", Value: "warmbly-verify=old"}); err != nil { + if err := dm.UpsertDNSRecord(context.Background(), Domain{ID: "ws-1:u2", Name: "acme.io"}, DNSRecord{Type: "TXT", Name: "_warmbly.acme.io", Value: "warmbly-verify=old"}); err != nil { t.Fatalf("no-op: %v", err) } } @@ -323,7 +339,7 @@ func TestInboxKitDomainErrorEnvelope(t *testing.T) { writeJSON(w, 200, `{"error":true,"message":"Domain not found"}`) }) dm := domainManager(t, newTestClient(t, VendorInboxKit, fieldsFor(VendorInboxKit), srv.URL, nil)) - if err := dm.SetForwarding(context.Background(), testDomain, "https://acme.com"); err == nil { + if err := dm.SetForwarding(context.Background(), testDomainFor(VendorInboxKit), "https://acme.com"); err == nil { t.Fatal("SetForwarding accepted an error envelope") } } @@ -331,6 +347,8 @@ func TestInboxKitDomainErrorEnvelope(t *testing.T) { func TestZapmailDomains(t *testing.T) { srv := newRecorder(t, func(w http.ResponseWriter, r *http.Request) { switch r.URL.Path { + case "/v2/workspaces": + writeJSON(w, 200, `{"status":200,"data":[{"id":"ws-123","name":"Main"}]}`) case "/v2/domains": if r.Header.Get("x-service-provider") == "MICROSOFT" { writeJSON(w, 200, `{"status":200,"message":"ok","data":{"totalSearchedCount":1,"currentPage":1,"nextPage":null,"totalPages":1,"domains":[{"id":"ms-d","domain":"contoso.co","status":"ACTIVE","forwardTo":null}]}}`) @@ -350,20 +368,20 @@ func TestZapmailDomains(t *testing.T) { writeJSON(w, 404, `{}`) } }) - dm := domainManager(t, newTestClient(t, VendorZapmail, map[string]string{FieldAPIKey: testKey, FieldWorkspaceID: "ws-123"}, srv.URL, nil)) + dm := domainManager(t, newTestClient(t, VendorZapmail, map[string]string{FieldAPIKey: testKey}, srv.URL, nil)) ctx := context.Background() got, err := dm.Domains(ctx) if err != nil { t.Fatal(err) } - if !reflect.DeepEqual(got, []Domain{{ID: "g-d", Name: "acme.io", Forwarding: "acme.com"}, {ID: "ms-d", Name: "contoso.co"}}) { + if !reflect.DeepEqual(got, []Domain{{ID: "ws-123:g-d", Name: "acme.io", Forwarding: "acme.com"}, {ID: "ws-123:ms-d", Name: "contoso.co"}}) { t.Fatalf("Domains = %+v", got) } - d := Domain{ID: "g-d", Name: "acme.io"} + d := got[0] if err := dm.SetForwarding(ctx, d, "https://acme.com"); err != nil { t.Fatal(err) } - if err := dm.SetForwarding(ctx, Domain{ID: "ms-d", Name: "contoso.co"}, ""); err != nil { + if err := dm.SetForwarding(ctx, got[1], ""); err != nil { t.Fatal(err) } if err := dm.UpsertDNSRecord(ctx, d, DNSRecord{Type: "CNAME", Name: "track.acme.io", Value: "track.warmbly.com", TTL: 60}); err != nil { @@ -376,7 +394,12 @@ func TestZapmailDomains(t *testing.T) { t.Fatal(err) } + // The workspace list comes first; drop it so the indexes below read the domain calls. reqs := srv.requests() + if reqs[0].Path != "/v2/workspaces" { + t.Fatalf("first call = %s", reqs[0].Path) + } + reqs = reqs[1:] for _, r := range reqs { assertHeader(t, r, "x-auth-zapmail", testKey) assertHeader(t, r, "x-workspace-key", "ws-123") @@ -432,20 +455,13 @@ func TestForgeDomains(t *testing.T) { writeJSON(w, 404, `{"code":404,"message":"Domain not found"}`) } }) - fields := map[string]string{FieldAPIKey: testKey} - if vendor == VendorInfraforge { - fields[FieldWorkspaceID] = "wks_1" - } - dm := domainManager(t, newTestClient(t, vendor, fields, srv.URL, nil)) + dm := domainManager(t, newTestClient(t, vendor, map[string]string{FieldAPIKey: testKey}, srv.URL, nil)) ctx := context.Background() got, err := dm.Domains(ctx) if err != nil { t.Fatal(err) } - want := []Domain{{ID: "dom_1", Name: "acme.io", Forwarding: "https://acme.com"}} - if vendor == VendorMailforge { - want = append(want, Domain{ID: "dom_2", Name: "other.com"}) - } + want := []Domain{{ID: "dom_1", Name: "acme.io", Forwarding: "https://acme.com"}, {ID: "dom_2", Name: "other.com"}} if !reflect.DeepEqual(got, want) { t.Fatalf("Domains = %+v", got) } @@ -615,6 +631,8 @@ func TestScaledMailDomains(t *testing.T) { const org = "recORG000000001" srv := newRecorder(t, func(w http.ResponseWriter, r *http.Request) { switch { + case r.URL.Path == "/organizations": + writeJSON(w, 200, `[{"id":"`+org+`","name":"Acme"}]`) case r.URL.Path == "/domains": writeJSON(w, 200, `{"total":2,"domains":[{"id":"recDOM1","domain":"outreach-one.com","tag":"","redirect":"https://example.com","order_type":"google","status":"Active"},{"id":"recDOM2","domain":"outreach-two.com","redirect":"","order_type":"outlook","status":"Active"}]}`) case strings.HasPrefix(r.URL.Path, "/swap-redirect/"): @@ -623,10 +641,10 @@ func TestScaledMailDomains(t *testing.T) { writeJSON(w, 404, `{"error":"Domain not found"}`) } }) - dm := domainManager(t, newTestClient(t, VendorScaledMail, map[string]string{FieldAPIKey: testKey, FieldOrganizationID: org}, srv.URL, nil)) + dm := domainManager(t, newTestClient(t, VendorScaledMail, map[string]string{FieldAPIKey: testKey}, srv.URL, nil)) ctx := context.Background() got, err := dm.Domains(ctx) - if err != nil || !reflect.DeepEqual(got, []Domain{{ID: "recDOM1", Name: "outreach-one.com", Forwarding: "https://example.com"}, {ID: "recDOM2", Name: "outreach-two.com"}}) { + if err != nil || !reflect.DeepEqual(got, []Domain{{ID: org + ":recDOM1", Name: "outreach-one.com", Forwarding: "https://example.com"}, {ID: org + ":recDOM2", Name: "outreach-two.com"}}) { t.Fatalf("Domains = %+v, %v", got, err) } if err := dm.SetForwarding(ctx, got[0], "https://mybrand.com/landing"); err != nil { @@ -642,9 +660,9 @@ func TestScaledMailDomains(t *testing.T) { if err := dm.SetForwarding(ctx, got[1], ""); err != nil { t.Fatal(err) } - reqs := srv.requests() + reqs := srv.requests()[1:] if len(reqs) != 3 { - t.Fatalf("%d requests, want 3", len(reqs)) + t.Fatalf("%d requests after the organization list, want 3", len(reqs)) } for _, r := range reqs[1:] { if r.Method != http.MethodPost || r.Path != "/swap-redirect/outreach-one.com" || r.q("organization_id") != org { diff --git a/internal/pkg/mailvendor/forge.go b/internal/pkg/mailvendor/forge.go index 0a54f42b4..be663f739 100644 --- a/internal/pkg/mailvendor/forge.go +++ b/internal/pkg/mailvendor/forge.go @@ -12,27 +12,59 @@ import ( // Mailforge (https://api.mailforge.ai/swagger/doc.json) and Infraforge // (https://api.infraforge.ai/public/swagger/doc.json) share one API shape. +// The key reaches every workspace of the account, and the mailbox list spans them all. type forge struct { - vendor string - t *transport - workspace string - cache credCache - dnsMu sync.Mutex + vendor string + t *transport + cache credCache + dnsMu sync.Mutex + workspaces workspaceCache } -func newForge(vendor, base, workspace string, vals map[string]string, o options) *forge { +func newForge(vendor, base string, vals map[string]string, o options) *forge { key := vals[FieldAPIKey] // Both take the raw key, with no Bearer prefix. auth := func(h http.Header) { h.Set("Authorization", key) } - return &forge{vendor: vendor, t: newTransport(vendor, base, o, auth, 0), workspace: workspace} + return &forge{vendor: vendor, t: newTransport(vendor, base, o, auth, 0)} } func (c *forge) Vendor() string { return c.vendor } +// listWorkspaces reads GET /workspaces; a body in another shape yields no names rather than an error. +func (c *forge) listWorkspaces(ctx context.Context) ([]workspace, error) { + var raw json.RawMessage + if err := c.t.do(ctx, call{method: http.MethodGet, path: "/workspaces"}, &raw); err != nil { + return nil, err + } + var res []struct { + ID string `json:"id"` + Name string `json:"name"` + } + _ = json.Unmarshal(raw, &res) + out := make([]workspace, 0, len(res)) + for _, w := range res { + out = append(out, workspace{ID: w.ID, Name: w.Name}) + } + return out, nil +} + func (c *forge) Verify(ctx context.Context) error { return c.t.do(ctx, call{method: http.MethodGet, path: "/workspaces"}, nil) } +// workspaceNames labels mailboxes by workspace; it is cosmetic, so a failed read leaves them unlabelled. +func (c *forge) workspaceNames(ctx context.Context) map[string]string { + wss, err := c.workspaces.get(ctx, c.listWorkspaces) + if err != nil { + return nil + } + names := make(map[string]string, len(wss)) + for _, w := range wss { + names[w.ID] = w.Name + } + return names +} + type forgeMailbox struct { ID string `json:"id"` Email string `json:"email"` @@ -40,6 +72,7 @@ type forgeMailbox struct { LastName string `json:"lastName"` Domain string `json:"domain"` Status string `json:"status"` + WorkspaceID string `json:"workspaceId"` Credentials *struct { IMAPHost string `json:"imapHost"` IMAPPort int `json:"imapPort"` @@ -84,16 +117,14 @@ func (m forgeMailbox) mailbox() Mailbox { } } -// List reads every mailbox with credentials in one unpaginated call. +// List reads every workspace's mailboxes with credentials in one unpaginated call. func (c *forge) List(ctx context.Context) ([]Mailbox, error) { q := url.Values{"with_credentials": {"true"}} - if c.workspace != "" { - q.Set("workspace_id", c.workspace) - } var res []forgeMailbox if err := c.t.do(ctx, call{method: http.MethodGet, path: "/mailboxes", query: q, limit: listBodyLimit}, &res); err != nil { return nil, err } + names := c.workspaceNames(ctx) out := make([]Mailbox, 0, min(len(res), MaxMailboxes)) for _, m := range res { if m.ID == "" { @@ -102,7 +133,9 @@ func (c *forge) List(ctx context.Context) ([]Mailbox, error) { if cr, ok := m.credentials(); ok { c.cache.put(m.ID, cr) } - out = append(out, m.mailbox()) + mb := m.mailbox() + mb.Workspace = names[m.WorkspaceID] + out = append(out, mb) if len(out) >= MaxMailboxes { break } @@ -137,7 +170,6 @@ type forgeDomain struct { SLD string `json:"sld"` TLD string `json:"tld"` ForwardToDomain string `json:"forwardToDomain"` - WorkspaceID string `json:"workspaceId"` } // Domains reads GET /domains in one unpaginated call. @@ -159,7 +191,7 @@ func (c *forge) Domains(ctx context.Context) ([]Domain, error) { } out := make([]Domain, 0, min(len(res), MaxDomains)) for _, d := range res { - if d.ID == "" || (c.workspace != "" && d.WorkspaceID != "" && d.WorkspaceID != c.workspace) { + if d.ID == "" { continue } name := d.SLD diff --git a/internal/pkg/mailvendor/inboxkit.go b/internal/pkg/mailvendor/inboxkit.go index a9c7b1ef8..c1bb26652 100644 --- a/internal/pkg/mailvendor/inboxkit.go +++ b/internal/pkg/mailvendor/inboxkit.go @@ -9,23 +9,54 @@ import ( ) // InboxKit: https://docs.inboxkit.com/llms.txt +// Every call but the workspace list is scoped by X-Workspace-Id, so the client reads all of the key's workspaces. type inboxKit struct { - t *transport + t *transport + workspaces workspaceCache } const inboxKitPageSize = 100 func newInboxKit(vals map[string]string, o options) *inboxKit { - key, ws := vals[FieldAPIKey], vals[FieldWorkspaceID] - auth := func(h http.Header) { - h.Set("Authorization", "Bearer "+key) - h.Set("X-Workspace-Id", ws) - } - return &inboxKit{t: newTransport(VendorInboxKit, "https://api.inboxkit.com", o, auth, 0)} + return &inboxKit{t: newTransport(VendorInboxKit, "https://api.inboxkit.com", o, bearer(vals[FieldAPIKey]), 0)} } func (c *inboxKit) Vendor() string { return VendorInboxKit } +func inboxKitHeader(ws string) http.Header { + h := http.Header{} + h.Set("X-Workspace-Id", ws) + return h +} + +// listWorkspaces reads GET /v1/api/workspaces/list, the one call the key alone answers. +func (c *inboxKit) listWorkspaces(ctx context.Context) ([]workspace, error) { + var res struct { + inboxKitEnvelope + Workspaces []struct { + UID string `json:"uid"` + Name string `json:"name"` + } `json:"workspaces"` + } + if err := c.t.do(ctx, call{method: http.MethodGet, path: "/v1/api/workspaces/list"}, &res); err != nil { + return nil, err + } + if err := res.err(); err != nil { + return nil, err + } + out := make([]workspace, 0, len(res.Workspaces)) + for _, w := range res.Workspaces { + if w.UID != "" { + out = append(out, workspace{ID: w.UID, Name: w.Name}) + } + } + return out, nil +} + +func (c *inboxKit) all(ctx context.Context) ([]workspace, error) { + return c.workspaces.get(ctx, c.listWorkspaces) +} + type inboxKitList struct { Error bool `json:"error"` Mailboxes []struct { @@ -41,10 +72,10 @@ type inboxKitList struct { CurrentPage int `json:"current_page"` } -func (c *inboxKit) page(ctx context.Context, page, limit int) (inboxKitList, error) { +func (c *inboxKit) page(ctx context.Context, ws string, page, limit int) (inboxKitList, error) { var out inboxKitList body := map[string]any{"page": page, "limit": limit} - if err := c.t.do(ctx, call{method: http.MethodPost, path: "/v1/api/mailboxes/list", body: body}, &out); err != nil { + if err := c.t.do(ctx, call{method: http.MethodPost, path: "/v1/api/mailboxes/list", body: body, header: inboxKitHeader(ws)}, &out); err != nil { return out, err } if out.Error { @@ -53,58 +84,84 @@ func (c *inboxKit) page(ctx context.Context, page, limit int) (inboxKitList, err return out, nil } -// Verify lists one mailbox, which checks the key and the workspace together. +// Verify reads the workspace list, which checks the key. func (c *inboxKit) Verify(ctx context.Context) error { - _, err := c.page(ctx, 1, 1) + _, err := c.listWorkspaces(ctx) return err } +// List pages through every workspace's mailboxes; ids carry their workspace. func (c *inboxKit) List(ctx context.Context) ([]Mailbox, error) { + wss, err := c.all(ctx) + if err != nil { + return nil, err + } var out []Mailbox - for page := 1; page <= maxPages; page++ { - res, err := c.page(ctx, page, inboxKitPageSize) - if err != nil { - return nil, err - } - for _, m := range res.Mailboxes { - email := m.Username - if m.DomainName != "" { - email = m.Username + "@" + m.DomainName + err = eachWorkspace(wss, func(ws workspace) error { + for page := 1; page <= maxPages; page++ { + res, err := c.page(ctx, ws.ID, page, inboxKitPageSize) + if err != nil { + return err } - out = append(out, Mailbox{ - ID: m.UID, - Email: email, - FirstName: m.FirstName, - LastName: m.LastName, - Domain: m.DomainName, - Provider: normalizeProvider(m.Platform), - Status: m.Status, - }) - if len(out) >= MaxMailboxes { - return out, nil + for _, m := range res.Mailboxes { + email := m.Username + if m.DomainName != "" { + email = m.Username + "@" + m.DomainName + } + out = append(out, Mailbox{ + ID: scoped(ws.ID, m.UID), + Email: email, + FirstName: m.FirstName, + LastName: m.LastName, + Domain: m.DomainName, + Provider: normalizeProvider(m.Platform), + Status: m.Status, + Workspace: ws.Name, + }) + if len(out) >= MaxMailboxes { + return errStop + } + } + if len(res.Mailboxes) == 0 || page >= res.Pages { + break } } - if len(res.Mailboxes) == 0 || page >= res.Pages { - break - } + return nil + }) + if err != nil { + return nil, err } return out, nil } -// Credentials reads show-credentials; InboxKit's list carries none. +// Credentials reads show-credentials in the mailbox's workspace; an id stored without one is looked up in each. func (c *inboxKit) Credentials(ctx context.Context, m Mailbox) (Credentials, error) { + ws, uid := unscoped(m.ID) + if ws != "" { + return c.show(ctx, ws, uid, m.Email) + } + wss, err := c.all(ctx) + if err != nil { + return Credentials{}, err + } + return findCredentials(VendorInboxKit, wss, func(w workspace) (Credentials, error) { + return c.show(ctx, w.ID, uid, m.Email) + }) +} + +func (c *inboxKit) show(ctx context.Context, ws, uid, email string) (Credentials, error) { q := url.Values{} - if m.ID != "" { - q.Set("uid", m.ID) + if uid != "" { + q.Set("uid", uid) } else { - q.Set("email", m.Email) + q.Set("email", email) } var res struct { Error bool `json:"error"` Password string `json:"password"` AppPassword string `json:"app_password"` } - if err := c.t.do(ctx, call{method: http.MethodGet, path: "/v1/api/mailboxes/show-credentials", query: q}, &res); err != nil { + if err := c.t.do(ctx, call{method: http.MethodGet, path: "/v1/api/mailboxes/show-credentials", query: q, header: inboxKitHeader(ws)}, &res); err != nil { return Credentials{}, err } if res.Error { @@ -131,57 +188,77 @@ func (c *inboxKit) DomainCapabilities() DomainCapabilities { return DomainCapabilities{Forwarding: true, ForwardingRemove: true, DNS: true, DNSTypes: allDNSTypes()} } -// Domains pages through POST /v1/api/domains/list. +// Domains pages through POST /v1/api/domains/list in every workspace; ids carry their workspace. func (c *inboxKit) Domains(ctx context.Context) ([]Domain, error) { + wss, err := c.all(ctx) + if err != nil { + return nil, err + } var out []Domain - for page := 1; page <= maxPages; page++ { - var res struct { - inboxKitEnvelope - Domains []struct { - UID string `json:"uid"` - Name string `json:"name"` - TLD string `json:"tld"` - ForwardingURL string `json:"forwarding_url"` - } `json:"domains"` - Pages int `json:"pages"` - } - body := map[string]any{"page": page, "limit": inboxKitDomainPageSize} - if err := c.t.do(ctx, call{method: http.MethodPost, path: "/v1/api/domains/list", body: body}, &res); err != nil { - return nil, err - } - if err := res.err(); err != nil { - return nil, err - } - for _, d := range res.Domains { - name := d.Name - // The Domain schema describes name as the label without its TLD; the list example carries both. - if name != "" && !strings.Contains(name, ".") && d.TLD != "" { - name += "." + strings.TrimPrefix(d.TLD, ".") + err = eachWorkspace(wss, func(ws workspace) error { + for page := 1; page <= maxPages; page++ { + var res struct { + inboxKitEnvelope + Domains []struct { + UID string `json:"uid"` + Name string `json:"name"` + TLD string `json:"tld"` + ForwardingURL string `json:"forwarding_url"` + } `json:"domains"` + Pages int `json:"pages"` } - out = append(out, Domain{ID: d.UID, Name: name, Forwarding: d.ForwardingURL}) - if len(out) >= MaxDomains { - return out, nil + body := map[string]any{"page": page, "limit": inboxKitDomainPageSize} + if err := c.t.do(ctx, call{method: http.MethodPost, path: "/v1/api/domains/list", body: body, header: inboxKitHeader(ws.ID)}, &res); err != nil { + return err + } + if err := res.err(); err != nil { + return err + } + for _, d := range res.Domains { + name := d.Name + // The Domain schema describes name as the label without its TLD; the list example carries both. + if name != "" && !strings.Contains(name, ".") && d.TLD != "" { + name += "." + strings.TrimPrefix(d.TLD, ".") + } + out = append(out, Domain{ID: scoped(ws.ID, d.UID), Name: name, Forwarding: d.ForwardingURL}) + if len(out) >= MaxDomains { + return errStop + } + } + if len(res.Domains) == 0 || page >= res.Pages { + break } } - if len(res.Domains) == 0 || page >= res.Pages { - break - } + return nil + }) + if err != nil { + return nil, err } return out, nil } +// inboxKitDomain splits a listed domain's id into its workspace and uid. +func inboxKitDomain(d Domain) (ws, uid string, err error) { + ws, uid = unscoped(d.ID) + if ws == "" || uid == "" { + return "", "", invalid(VendorInboxKit, "domain id is required") + } + return ws, uid, nil +} + // SetForwarding calls POST /v1/api/domains/forwarding; an empty forwarding_url removes it. func (c *inboxKit) SetForwarding(ctx context.Context, d Domain, target string) error { target, err := forwardingTarget(VendorInboxKit, target, c.DomainCapabilities()) if err != nil { return err } - if d.ID == "" { - return invalid(VendorInboxKit, "domain id is required") + ws, uid, err := inboxKitDomain(d) + if err != nil { + return err } var res inboxKitEnvelope - body := map[string]any{"uids": []string{d.ID}, "forwarding_url": target} - if err := c.t.do(ctx, call{method: http.MethodPost, path: "/v1/api/domains/forwarding", body: body}, &res); err != nil { + body := map[string]any{"uids": []string{uid}, "forwarding_url": target} + if err := c.t.do(ctx, call{method: http.MethodPost, path: "/v1/api/domains/forwarding", body: body, header: inboxKitHeader(ws)}, &res); err != nil { return err } return res.err() @@ -197,12 +274,9 @@ type inboxKitRecord struct { TTL *int `json:"ttl,omitempty"` } -// inboxKitTarget names the domain by uid, else by name; the DNS endpoints take either. -func inboxKitTarget(d Domain, domain string) map[string]any { - if d.ID != "" { - return map[string]any{"uid": d.ID} - } - return map[string]any{"domain": domain} +// inboxKitTarget names the domain by uid. +func inboxKitTarget(uid string) map[string]any { + return map[string]any{"uid": uid} } // UpsertDNSRecord reads /v1/api/dns/list, then adds, updates or deletes through /v1/api/dns/{add,update,delete}. @@ -211,19 +285,19 @@ func (c *inboxKit) UpsertDNSRecord(ctx context.Context, d Domain, r DNSRecord) e if err != nil { return err } - q := url.Values{} - if d.ID != "" { - q.Set("uid", d.ID) - } else { - q.Set("domain", domain) + ws, uid, err := inboxKitDomain(d) + if err != nil { + return err } + h := inboxKitHeader(ws) + q := url.Values{"uid": {uid}} var list struct { inboxKitEnvelope DNSRecord struct { Records []inboxKitRecord `json:"records"` } `json:"dns_record"` } - err = c.t.do(ctx, call{method: http.MethodGet, path: "/v1/api/dns/list", query: q}, &list) + err = c.t.do(ctx, call{method: http.MethodGet, path: "/v1/api/dns/list", query: q, header: h}, &list) // A 404 also means "no records yet"; a missing domain fails again on the write. if err != nil && !errors.Is(err, ErrNotFound) { return err @@ -250,10 +324,10 @@ func (c *inboxKit) UpsertDNSRecord(ctx context.Context, d Domain, r DNSRecord) e path = "/v1/api/dns/update" rec.RecordID = existing[p.update].ID } - body := inboxKitTarget(d, domain) + body := inboxKitTarget(uid) body["records"] = []inboxKitRecord{rec} var res inboxKitEnvelope - if err := c.t.do(ctx, call{method: http.MethodPost, path: path, body: body}, &res); err != nil { + if err := c.t.do(ctx, call{method: http.MethodPost, path: path, body: body, header: h}, &res); err != nil { return err } if err := res.err(); err != nil { @@ -266,10 +340,10 @@ func (c *inboxKit) UpsertDNSRecord(ctx context.Context, d Domain, r DNSRecord) e for i, idx := range p.remove { ids[i] = existing[idx].ID } - del := inboxKitTarget(d, domain) + del := inboxKitTarget(uid) del["record_ids"] = ids res = inboxKitEnvelope{} - if err := c.t.do(ctx, call{method: http.MethodPost, path: "/v1/api/dns/delete", body: del}, &res); err != nil { + if err := c.t.do(ctx, call{method: http.MethodPost, path: "/v1/api/dns/delete", body: del, header: h}, &res); err != nil { return err } return res.err() diff --git a/internal/pkg/mailvendor/mailvendor.go b/internal/pkg/mailvendor/mailvendor.go index 95bbbc672..e8f9360b3 100644 --- a/internal/pkg/mailvendor/mailvendor.go +++ b/internal/pkg/mailvendor/mailvendor.go @@ -24,10 +24,8 @@ const ( // Field keys a descriptor may ask for. const ( - FieldAPIKey = "api_key" - FieldWorkspaceID = "workspace_id" - FieldOrganizationID = "organization_id" - FieldServiceProvider = "service_provider" + FieldAPIKey = "api_key" + FieldOrganizationID = "organization_id" ) // Provider values on Mailbox. @@ -53,6 +51,7 @@ var ( ErrNotFound = errors.New("mailvendor: mailbox not found") ErrUnknownVendor = errors.New("mailvendor: unknown vendor") ErrInvalidConfig = errors.New("mailvendor: invalid configuration") + ErrNoWorkspace = errors.New("mailvendor: key reaches no workspace") ) // Field is a credential the customer must enter, keyed by Key. @@ -62,6 +61,8 @@ type Field struct { Secret bool Required bool Help string + // Legacy fields are read from connections saved before the value was discovered, and never asked for. + Legacy bool } // Descriptor describes a vendor and what connecting it asks for. @@ -84,6 +85,8 @@ type Mailbox struct { Domain string Provider string Status string + // Workspace is the name of the vendor workspace holding the mailbox, empty where the vendor has none. + Workspace string } // Endpoint is one server a mailbox connects to. @@ -145,23 +148,16 @@ var descriptors = []Descriptor{ Label: "InboxKit", Website: "https://inboxkit.com", KeyHelpURL: "https://app.inboxkit.com/settings/api", - Fields: []Field{ - apiKeyField("Settings > API & Integrations in the InboxKit dashboard."), - {Key: FieldWorkspaceID, Label: "Workspace ID", Required: true, Help: "The workspace whose mailboxes to import (a UUID)."}, - }, - Verified: true, + Fields: []Field{apiKeyField("Settings > API & Integrations in the InboxKit dashboard. Mailboxes from every workspace the key reaches are listed.")}, + Verified: true, }, { ID: VendorZapmail, Label: "Zapmail", Website: "https://zapmail.ai", KeyHelpURL: "https://docs.zapmail.ai/zapmail-docs-825990m0", - Fields: []Field{ - apiKeyField("Settings > Integrations > API in the Zapmail dashboard."), - {Key: FieldWorkspaceID, Label: "Workspace ID", Help: "Leave blank for your primary workspace."}, - {Key: FieldServiceProvider, Label: "Mailbox type", Help: "GOOGLE or MICROSOFT. Leave blank to import both."}, - }, - Verified: true, + Fields: []Field{apiKeyField("Settings > Integrations > API in the Zapmail dashboard. Mailboxes from every workspace the key reaches are listed.")}, + Verified: true, }, { ID: VendorMailforge, @@ -176,11 +172,8 @@ var descriptors = []Descriptor{ Label: "Infraforge", Website: "https://infraforge.ai", KeyHelpURL: "https://api.infraforge.ai/public/swagger/index.html", - Fields: []Field{ - apiKeyField("Settings > API keys in the Infraforge dashboard."), - {Key: FieldWorkspaceID, Label: "Workspace ID", Help: "Leave blank to import every workspace."}, - }, - Verified: true, + Fields: []Field{apiKeyField("Settings > API keys in the Infraforge dashboard. Mailboxes from every workspace are listed.")}, + Verified: true, }, { ID: VendorMaildoso, @@ -204,8 +197,8 @@ var descriptors = []Descriptor{ Website: "https://scaledmail.com", KeyHelpURL: "https://app.scaledmail.com/settings", Fields: []Field{ - apiKeyField("Settings in the ScaledMail dashboard."), - {Key: FieldOrganizationID, Label: "Organization ID", Required: true, Help: "The organization whose mailboxes to import."}, + apiKeyField("Settings in the ScaledMail dashboard. Mailboxes from every organization the key reaches are listed."), + {Key: FieldOrganizationID, Label: "Organization ID", Legacy: true}, }, Verified: true, }, @@ -244,7 +237,7 @@ func New(vendor string, fields map[string]string, opts ...Option) (Client, error vals := make(map[string]string, len(d.Fields)) for _, f := range d.Fields { v := strings.TrimSpace(fields[f.Key]) - if v == "" && f.Required { + if v == "" && f.Required && !f.Legacy { return nil, fmt.Errorf("%w: %s: %s is required", ErrInvalidConfig, vendor, f.Label) } if strings.ContainsFunc(v, func(r rune) bool { return r < 0x20 || r == 0x7f }) { @@ -263,11 +256,11 @@ func New(vendor string, fields map[string]string, opts ...Option) (Client, error case VendorInboxKit: return newInboxKit(vals, o), nil case VendorZapmail: - return newZapmail(vals, o) + return newZapmail(vals, o), nil case VendorMailforge: - return newForge(VendorMailforge, "https://api.mailforge.ai/public", "", vals, o), nil + return newForge(VendorMailforge, "https://api.mailforge.ai/public", vals, o), nil case VendorInfraforge: - return newForge(VendorInfraforge, "https://api.infraforge.ai/public", vals[FieldWorkspaceID], vals, o), nil + return newForge(VendorInfraforge, "https://api.infraforge.ai/public", vals, o), nil case VendorMaildoso: return newMaildoso(vals, o), nil case VendorCheapInboxes: diff --git a/internal/pkg/mailvendor/mailvendor_test.go b/internal/pkg/mailvendor/mailvendor_test.go index e9847c4fd..14d483859 100644 --- a/internal/pkg/mailvendor/mailvendor_test.go +++ b/internal/pkg/mailvendor/mailvendor_test.go @@ -99,10 +99,8 @@ func writeJSON(w http.ResponseWriter, status int, body string) { // fieldsFor returns a complete, valid field set for a vendor. func fieldsFor(vendor string) map[string]string { f := map[string]string{FieldAPIKey: testKey} - switch vendor { - case VendorInboxKit: - f[FieldWorkspaceID] = "6f1c2d3e-0000-4000-8000-000000000001" - case VendorScaledMail: + if vendor == VendorScaledMail { + // A legacy organization id keeps ScaledMail usable against a server that lists none. f[FieldOrganizationID] = "recORG000000001" } return f @@ -119,6 +117,12 @@ func TestDescriptorsStableAndComplete(t *testing.T) { if len(d.Fields) == 0 || d.Fields[0].Key != FieldAPIKey || !d.Fields[0].Secret || !d.Fields[0].Required { t.Errorf("%s: first field must be the required, secret api_key", d.ID) } + // Anything a vendor's API can discover is never asked for. + for _, f := range d.Fields[1:] { + if !f.Legacy { + t.Errorf("%s: asks for %s beyond the API key", d.ID, f.Key) + } + } } if !reflect.DeepEqual(got, want) { t.Fatalf("Descriptors order = %v, want %v", got, want) @@ -140,8 +144,8 @@ func TestNewValidatesFields(t *testing.T) { if _, err := New("nope", nil); !errors.Is(err, ErrUnknownVendor) { t.Fatalf("unknown vendor: %v", err) } - if _, err := New(VendorInboxKit, map[string]string{FieldAPIKey: testKey}); !errors.Is(err, ErrInvalidConfig) { - t.Fatalf("missing workspace: %v", err) + if _, err := New(VendorInboxKit, map[string]string{}); !errors.Is(err, ErrInvalidConfig) { + t.Fatalf("missing key: %v", err) } if _, err := New(VendorScaledMail, map[string]string{FieldAPIKey: " "}); !errors.Is(err, ErrInvalidConfig) { t.Fatalf("blank key: %v", err) @@ -150,9 +154,6 @@ func TestNewValidatesFields(t *testing.T) { if !errors.Is(err, ErrInvalidConfig) || strings.Contains(err.Error(), testKey) { t.Fatalf("control characters: %v", err) } - if _, err := New(VendorZapmail, map[string]string{FieldAPIKey: testKey, FieldServiceProvider: "yahoo"}); !errors.Is(err, ErrInvalidConfig) { - t.Fatalf("zapmail provider: %v", err) - } for _, d := range Descriptors() { c, err := New(d.ID, fieldsFor(d.ID)) if err != nil { diff --git a/internal/pkg/mailvendor/scaledmail.go b/internal/pkg/mailvendor/scaledmail.go index 1240567f5..28fe55e84 100644 --- a/internal/pkg/mailvendor/scaledmail.go +++ b/internal/pkg/mailvendor/scaledmail.go @@ -2,16 +2,20 @@ package mailvendor import ( "context" + "encoding/json" "net/http" "net/url" "strings" ) // ScaledMail: https://api.scaledmail.com/llms.txt +// Every call names an organization_id, so the client reads all of the key's organizations. type scaledMail struct { - t *transport - org string - cache credCache + t *transport + orgs workspaceCache + // legacyOrg is an organization id a connection saved before organizations were discovered. + legacyOrg string + cache credCache } // scaledMailPerSecond is the documented limit; going over it earns a temporary block. @@ -19,13 +23,90 @@ const scaledMailPerSecond = 5 func newScaledMail(vals map[string]string, o options) *scaledMail { t := newTransport(VendorScaledMail, "https://server.scaledmail.com/api/v1", o, bearer(vals[FieldAPIKey]), scaledMailPerSecond) - return &scaledMail{t: t, org: vals[FieldOrganizationID]} + return &scaledMail{t: t, legacyOrg: vals[FieldOrganizationID]} } func (c *scaledMail) Vendor() string { return VendorScaledMail } +// listOrganizations reads GET /organizations. Its response is undocumented, so any list of objects with an id is read. +func (c *scaledMail) listOrganizations(ctx context.Context) ([]workspace, error) { + var raw json.RawMessage + if err := c.t.do(ctx, call{method: http.MethodGet, path: "/organizations"}, &raw); err != nil { + return nil, err + } + out := scaledMailOrgs(raw) + if len(out) == 0 && c.legacyOrg != "" { + out = []workspace{{ID: c.legacyOrg}} + } + return out, nil +} + +type scaledMailOrg struct { + ID string `json:"id"` + UnderscoreID string `json:"_id"` + OrganizationID string `json:"organization_id"` + Name string `json:"name"` +} + +func (o scaledMailOrg) workspace() workspace { + return workspace{ID: firstNonEmpty(o.ID, o.OrganizationID, o.UnderscoreID), Name: o.Name} +} + +// scaledMailOrgs reads a top-level array, or the arrays and single objects one level inside an object. +func scaledMailOrgs(raw json.RawMessage) []workspace { + var out []workspace + seen := map[string]bool{} + add := func(o scaledMailOrg) { + if w := o.workspace(); w.ID != "" && !seen[w.ID] { + seen[w.ID] = true + out = append(out, w) + } + } + var list []scaledMailOrg + if json.Unmarshal(raw, &list) == nil { + for _, o := range list { + add(o) + } + return out + } + var obj map[string]json.RawMessage + if json.Unmarshal(raw, &obj) != nil { + return nil + } + for _, v := range obj { + var inner []scaledMailOrg + if json.Unmarshal(v, &inner) == nil { + for _, o := range inner { + add(o) + } + continue + } + var one scaledMailOrg + if json.Unmarshal(v, &one) == nil { + add(one) + } + } + return out +} + +func (c *scaledMail) all(ctx context.Context) ([]workspace, error) { + orgs, err := c.orgs.get(ctx, c.listOrganizations) + if err != nil { + return nil, err + } + if len(orgs) == 0 { + return nil, vendorErr(VendorScaledMail, http.StatusOK, "no organization", ErrNoWorkspace) + } + return orgs, nil +} + +// Verify reads the organizations, which checks the key and that it reaches one. func (c *scaledMail) Verify(ctx context.Context) error { - return c.t.do(ctx, call{method: http.MethodGet, path: "/organizations"}, nil) + orgs, err := c.listOrganizations(ctx) + if err == nil && len(orgs) == 0 { + err = vendorErr(VendorScaledMail, http.StatusOK, "no organization", ErrNoWorkspace) + } + return err } type scaledMailDomain struct { @@ -44,23 +125,23 @@ type scaledMailMailbox struct { Password string `json:"mailbox_password"` } -func (c *scaledMail) domains(ctx context.Context) ([]scaledMailDomain, error) { +func (c *scaledMail) domains(ctx context.Context, org string) ([]scaledMailDomain, error) { var res struct { Domains []scaledMailDomain `json:"domains"` } - q := url.Values{"organization_id": {c.org}} + q := url.Values{"organization_id": {org}} if err := c.t.do(ctx, call{method: http.MethodGet, path: "/domains", query: q}, &res); err != nil { return nil, err } return res.Domains, nil } -func (c *scaledMail) mailboxes(ctx context.Context, domainID string) ([]scaledMailMailbox, error) { +func (c *scaledMail) mailboxes(ctx context.Context, org, domainID string) ([]scaledMailMailbox, error) { var res struct { // Null when the domain belongs to another organization. Mailboxes []scaledMailMailbox `json:"mailboxes"` } - q := url.Values{"organization_id": {c.org}, "password": {"true"}} + q := url.Values{"organization_id": {org}, "password": {"true"}} if err := c.t.do(ctx, call{method: http.MethodGet, path: "/mailboxes/" + url.PathEscape(domainID), query: q}, &res); err != nil { return nil, err } @@ -77,54 +158,79 @@ func (m scaledMailMailbox) email(domain string) string { return "" } -// List reads the domains, then each domain's mailboxes with their passwords. +// List reads each organization's domains, then each domain's mailboxes with their passwords. func (c *scaledMail) List(ctx context.Context) ([]Mailbox, error) { - domains, err := c.domains(ctx) + orgs, err := c.all(ctx) if err != nil { return nil, err } var out []Mailbox - for _, d := range domains { - if d.ID == "" { - continue - } - mbs, err := c.mailboxes(ctx, d.ID) + err = eachWorkspace(orgs, func(org workspace) error { + domains, err := c.domains(ctx, org.ID) if err != nil { - return nil, err + return err } - for _, m := range mbs { - email := m.email(d.Domain) - if email == "" { + for _, d := range domains { + if d.ID == "" { continue } - // ScaledMail has no mailbox id; the address is the stable key. - id := strings.ToLower(email) - c.cache.put(id, Credentials{Password: m.Password}) - out = append(out, Mailbox{ - ID: id, - Email: email, - FirstName: m.FirstName, - LastName: m.LastName, - Domain: firstNonEmpty(d.Domain, domainOf(email)), - Provider: normalizeProvider(m.OrderType), - Status: m.Status, - }) - if len(out) >= MaxMailboxes { - return out, nil + mbs, err := c.mailboxes(ctx, org.ID, d.ID) + if err != nil { + return err + } + for _, m := range mbs { + email := m.email(d.Domain) + if email == "" { + continue + } + // ScaledMail has no mailbox id; the address is the stable key. + id := scoped(org.ID, strings.ToLower(email)) + c.cache.put(id, Credentials{Password: m.Password}) + out = append(out, Mailbox{ + ID: id, + Email: email, + FirstName: m.FirstName, + LastName: m.LastName, + Domain: firstNonEmpty(d.Domain, domainOf(email)), + Provider: normalizeProvider(m.OrderType), + Status: m.Status, + Workspace: org.Name, + }) + if len(out) >= MaxMailboxes { + return errStop + } } } + return nil + }) + if err != nil { + return nil, err } return out, nil } -// Credentials comes from List's cache, else re-reads the mailbox's domain. +// Credentials comes from List's cache, else re-reads the mailbox's domain in its organization. func (c *scaledMail) Credentials(ctx context.Context, m Mailbox) (Credentials, error) { - id := strings.ToLower(firstNonEmpty(m.ID, m.Email)) - if cr, ok := c.cache.get(id); ok { + if cr, ok := c.cache.get(m.ID); ok { return cr, nil } - domain := strings.ToLower(firstNonEmpty(m.Domain, domainOf(id))) - domains, err := c.domains(ctx) + org, id := unscoped(m.ID) + addr := strings.ToLower(firstNonEmpty(id, m.Email)) + domain := strings.ToLower(firstNonEmpty(m.Domain, domainOf(addr))) + if org != "" { + return c.find(ctx, org, domain, addr) + } + orgs, err := c.all(ctx) + if err != nil { + return Credentials{}, err + } + return findCredentials(VendorScaledMail, orgs, func(o workspace) (Credentials, error) { + return c.find(ctx, o.ID, domain, addr) + }) +} + +func (c *scaledMail) find(ctx context.Context, org, domain, addr string) (Credentials, error) { + domains, err := c.domains(ctx, org) if err != nil { return Credentials{}, err } @@ -132,12 +238,12 @@ func (c *scaledMail) Credentials(ctx context.Context, m Mailbox) (Credentials, e if !strings.EqualFold(d.Domain, domain) || d.ID == "" { continue } - mbs, err := c.mailboxes(ctx, d.ID) + mbs, err := c.mailboxes(ctx, org, d.ID) if err != nil { return Credentials{}, err } for _, mb := range mbs { - if strings.EqualFold(mb.email(d.Domain), id) { + if strings.EqualFold(mb.email(d.Domain), addr) { return Credentials{Password: mb.Password}, nil } } @@ -150,18 +256,28 @@ func (c *scaledMail) DomainCapabilities() DomainCapabilities { return DomainCapabilities{Forwarding: true, ForwardingRemove: true, ForwardingReviewed: true} } -// Domains reads GET /domains, which lists the organization's active domains in one call. +// Domains reads GET /domains, which lists one organization's active domains in one call, for each organization. func (c *scaledMail) Domains(ctx context.Context) ([]Domain, error) { - domains, err := c.domains(ctx) + orgs, err := c.all(ctx) if err != nil { return nil, err } - out := make([]Domain, 0, min(len(domains), MaxDomains)) - for _, d := range domains { - out = append(out, Domain{ID: d.ID, Name: d.Domain, Forwarding: d.Redirect}) - if len(out) >= MaxDomains { - break + var out []Domain + err = eachWorkspace(orgs, func(org workspace) error { + domains, err := c.domains(ctx, org.ID) + if err != nil { + return err } + for _, d := range domains { + out = append(out, Domain{ID: scoped(org.ID, d.ID), Name: d.Domain, Forwarding: d.Redirect}) + if len(out) >= MaxDomains { + return errStop + } + } + return nil + }) + if err != nil { + return nil, err } return out, nil } @@ -189,7 +305,11 @@ func (c *scaledMail) SetForwarding(ctx context.Context, d Domain, target string) if strings.EqualFold(strings.TrimRight(withScheme(d.Forwarding), "/"), strings.TrimRight(withScheme(target), "/")) { return nil } - q := url.Values{"organization_id": {c.org}} + org, _ := unscoped(d.ID) + if org == "" { + return invalid(VendorScaledMail, "domain id is required") + } + q := url.Values{"organization_id": {org}} body := map[string]string{"new_redirect": target} return c.t.do(ctx, call{method: http.MethodPost, path: "/swap-redirect/" + url.PathEscape(name), query: q, body: body}, nil) } diff --git a/internal/pkg/mailvendor/vendors_test.go b/internal/pkg/mailvendor/vendors_test.go index 713e1cc4b..74c538d64 100644 --- a/internal/pkg/mailvendor/vendors_test.go +++ b/internal/pkg/mailvendor/vendors_test.go @@ -43,23 +43,34 @@ func inboxKitMailbox(uid, user, domain, platform string) string { } func TestInboxKit(t *testing.T) { - const ws = "6f1c2d3e-0000-4000-8000-000000000001" + const ws1, ws2 = "6f1c2d3e-0000-4000-8000-000000000001", "6f1c2d3e-0000-4000-8000-000000000002" srv := newRecorder(t, func(w http.ResponseWriter, r *http.Request) { + ws := r.Header.Get("X-Workspace-Id") switch r.URL.Path { + case "/v1/api/workspaces/list": + writeJSON(w, 200, `{"error":false,"message":"Workspaces retrieved successfully","workspaces":[ + {"uid":"`+ws1+`","name":"Outbound","team":"t1","webhook_url":null,"domains":2}, + {"uid":"`+ws2+`","name":"Trials","team":"t1","webhook_url":null,"domains":1}]}`) case "/v1/api/mailboxes/list": var body struct{ Page, Limit int } _ = json.NewDecoder(r.Body).Decode(&body) - if body.Page == 1 { + switch { + case ws == ws2: + writeJSON(w, 200, `{"error":false,"mailboxes":[`+ + inboxKitMailbox("9c9c9c9c-2222-4333-8444-555566667777", "sam", "trials.io", "GOOGLE")+ + `],"total":1,"pages":1,"current_page":1,"limit":100}`) + case body.Page == 1: writeJSON(w, 200, `{"error":false,"message":"Mailboxes retrieved successfully","mailboxes":[`+ inboxKitMailbox("e88ae415-fe99-4831-b3fe-cdf2e7e25925", "marvinkassulke", "myzyng.net", "GOOGLE")+ `],"total":2,"pages":2,"current_page":1,"limit":1}`) - return + default: + writeJSON(w, 200, `{"error":false,"message":"Mailboxes retrieved successfully","mailboxes":[`+ + inboxKitMailbox("0b7c7a8a-1111-4222-8333-444455556666", "jane", "acme.io", "MICROSOFT")+ + `],"total":2,"pages":2,"current_page":2,"limit":1}`) } - writeJSON(w, 200, `{"error":false,"message":"Mailboxes retrieved successfully","mailboxes":[`+ - inboxKitMailbox("0b7c7a8a-1111-4222-8333-444455556666", "jane", "acme.io", "MICROSOFT")+ - `],"total":2,"pages":2,"current_page":2,"limit":1}`) case "/v1/api/mailboxes/show-credentials": - if r.URL.Query().Get("uid") == "gone" { + // Only ws2 holds the mailbox a stored, unscoped id names. + if uid := r.URL.Query().Get("uid"); uid == "gone" || (uid == "legacy-uid" && ws != ws2) { writeJSON(w, 404, `{"error":true,"message":"Mailbox not found"}`) return } @@ -68,20 +79,27 @@ func TestInboxKit(t *testing.T) { writeJSON(w, 404, `{}`) } }) - c := newTestClient(t, VendorInboxKit, map[string]string{FieldAPIKey: testKey, FieldWorkspaceID: ws}, srv.URL, nil) + c := newTestClient(t, VendorInboxKit, map[string]string{FieldAPIKey: testKey}, srv.URL, nil) + if err := c.Verify(context.Background()); err != nil { + t.Fatalf("Verify: %v", err) + } got := mustList(t, c) want := []Mailbox{ - {ID: "e88ae415-fe99-4831-b3fe-cdf2e7e25925", Email: "marvinkassulke@myzyng.net", FirstName: "marvin", LastName: "kassulke", Domain: "myzyng.net", Provider: ProviderGoogle, Status: "active"}, - {ID: "0b7c7a8a-1111-4222-8333-444455556666", Email: "jane@acme.io", FirstName: "marvin", LastName: "kassulke", Domain: "acme.io", Provider: ProviderMicrosoft, Status: "active"}, + {ID: ws1 + ":e88ae415-fe99-4831-b3fe-cdf2e7e25925", Email: "marvinkassulke@myzyng.net", FirstName: "marvin", LastName: "kassulke", Domain: "myzyng.net", Provider: ProviderGoogle, Status: "active", Workspace: "Outbound"}, + {ID: ws1 + ":0b7c7a8a-1111-4222-8333-444455556666", Email: "jane@acme.io", FirstName: "marvin", LastName: "kassulke", Domain: "acme.io", Provider: ProviderMicrosoft, Status: "active", Workspace: "Outbound"}, + {ID: ws2 + ":9c9c9c9c-2222-4333-8444-555566667777", Email: "sam@trials.io", FirstName: "marvin", LastName: "kassulke", Domain: "trials.io", Provider: ProviderGoogle, Status: "active", Workspace: "Trials"}, } if !reflect.DeepEqual(got, want) { t.Fatalf("List = %+v\nwant %+v", got, want) } - cr := mustCreds(t, c, got[0]) + cr := mustCreds(t, c, got[2]) if cr != (Credentials{Password: "pw-12345", AppPassword: "abcd efgh ijkl mnop"}) { t.Fatalf("Credentials = %+v", cr) } + if cr := mustCreds(t, c, Mailbox{ID: "legacy-uid", Email: "old@trials.io"}); cr.Password != "pw-12345" { + t.Fatalf("legacy id Credentials = %+v", cr) + } if _, err := c.Credentials(context.Background(), Mailbox{ID: "gone"}); !errors.Is(err, ErrNotFound) { t.Fatalf("gone: %v", err) } @@ -89,13 +107,44 @@ func TestInboxKit(t *testing.T) { reqs := srv.requests() for _, r := range reqs { assertHeader(t, r, "Authorization", "Bearer "+testKey) - assertHeader(t, r, "X-Workspace-Id", ws) + if r.Path == "/v1/api/workspaces/list" { + if r.Header.Get("X-Workspace-Id") != "" { + t.Errorf("workspace list sent X-Workspace-Id") + } + continue + } + if ws := r.Header.Get("X-Workspace-Id"); ws != ws1 && ws != ws2 { + t.Errorf("%s: X-Workspace-Id = %q", r.Path, ws) + } } - if reqs[0].Method != http.MethodPost || !strings.Contains(reqs[0].Body, `"page":1`) || !strings.Contains(reqs[1].Body, `"page":2`) { - t.Fatalf("list requests = %+v", reqs[:2]) + var shows []string + for _, r := range reqs { + if r.Path == "/v1/api/mailboxes/show-credentials" { + shows = append(shows, r.Header.Get("X-Workspace-Id")+"/"+r.q("uid")) + } } - if reqs[2].Query["uid"][0] != want[0].ID { - t.Fatalf("show-credentials query = %v", reqs[2].Query) + wantShows := []string{ws2 + "/9c9c9c9c-2222-4333-8444-555566667777", ws1 + "/legacy-uid", ws2 + "/legacy-uid", ws1 + "/gone", ws2 + "/gone"} + if !reflect.DeepEqual(shows, wantShows) { + t.Fatalf("show-credentials calls = %v\nwant %v", shows, wantShows) + } +} + +// A workspace the key may not read is skipped while another answers. +func TestInboxKitSkipsRefusedWorkspace(t *testing.T) { + srv := newRecorder(t, func(w http.ResponseWriter, r *http.Request) { + switch { + case r.URL.Path == "/v1/api/workspaces/list": + writeJSON(w, 200, `{"error":false,"workspaces":[{"uid":"ws-a","name":"A"},{"uid":"ws-b","name":"B"}]}`) + case r.Header.Get("X-Workspace-Id") == "ws-a": + writeJSON(w, 403, `{"error":true,"message":"Forbidden"}`) + default: + writeJSON(w, 200, `{"error":false,"mailboxes":[`+inboxKitMailbox("u1", "amy", "b.io", "GOOGLE")+`],"pages":1}`) + } + }) + c := newTestClient(t, VendorInboxKit, fieldsFor(VendorInboxKit), srv.URL, nil) + got := mustList(t, c) + if len(got) != 1 || got[0].ID != "ws-b:u1" || got[0].Workspace != "B" { + t.Fatalf("List = %+v", got) } } @@ -120,8 +169,8 @@ func TestZapmail(t *testing.T) { {"id":"d3","domain":"contoso.co","status":"ACTIVE","mailboxes":[{"id":"ms-1","username":"amy","email":"amy@contoso.co","firstName":"Amy","lastName":"Lee","password":"pw-ms","appPassword":null,"secret":null,"status":"ACTIVE","domain":"contoso.co"}]}]}}` srv := newRecorder(t, func(w http.ResponseWriter, r *http.Request) { switch r.URL.Path { - case "/v2/users": - writeJSON(w, 200, `{"status":200,"message":"ok","data":{}}`) + case "/v2/workspaces": + writeJSON(w, 200, `{"status":200,"message":"Workspaces fetched successfully","data":[{"id":"ws-123","name":"Main","domainCount":"15","mailboxCount":"8"}]}`) case "/v2/mailboxes/list": switch { case r.Header.Get("x-service-provider") == "MICROSOFT": @@ -141,16 +190,15 @@ func TestZapmail(t *testing.T) { writeJSON(w, 404, `{}`) } }) - fields := map[string]string{FieldAPIKey: testKey, FieldWorkspaceID: "ws-123"} - c := newTestClient(t, VendorZapmail, fields, srv.URL, nil) + c := newTestClient(t, VendorZapmail, map[string]string{FieldAPIKey: testKey}, srv.URL, nil) if err := c.Verify(context.Background()); err != nil { t.Fatalf("Verify: %v", err) } got := mustList(t, c) want := []Mailbox{ - {ID: "abcd1234-5678-90ef-ghij-klmn12345678", Email: "john.doe@example1.com", FirstName: "John", LastName: "Doe", Domain: "example1.com", Provider: ProviderGoogle, Status: "ACTIVE"}, - {ID: "xyz1234-5678-90ab-cdef-ghijk9876543", Email: "jane.smith@example2.net", FirstName: "Jane", LastName: "Smith", Domain: "example2.net", Provider: ProviderGoogle, Status: "IN_PROGRESS"}, - {ID: "ms-1", Email: "amy@contoso.co", FirstName: "Amy", LastName: "Lee", Domain: "contoso.co", Provider: ProviderMicrosoft, Status: "ACTIVE"}, + {ID: "ws-123:abcd1234-5678-90ef-ghij-klmn12345678", Email: "john.doe@example1.com", FirstName: "John", LastName: "Doe", Domain: "example1.com", Provider: ProviderGoogle, Status: "ACTIVE", Workspace: "Main"}, + {ID: "ws-123:xyz1234-5678-90ab-cdef-ghijk9876543", Email: "jane.smith@example2.net", FirstName: "Jane", LastName: "Smith", Domain: "example2.net", Provider: ProviderGoogle, Status: "IN_PROGRESS", Workspace: "Main"}, + {ID: "ws-123:ms-1", Email: "amy@contoso.co", FirstName: "Amy", LastName: "Lee", Domain: "contoso.co", Provider: ProviderMicrosoft, Status: "ACTIVE", Workspace: "Main"}, } if !reflect.DeepEqual(got, want) { t.Fatalf("List = %+v\nwant %+v", got, want) @@ -172,14 +220,18 @@ func TestZapmail(t *testing.T) { reqs := srv.requests() for _, r := range reqs { assertHeader(t, r, "x-auth-zapmail", testKey) - assertHeader(t, r, "x-workspace-key", "ws-123") + if r.Path != "/v2/workspaces" { + assertHeader(t, r, "x-workspace-key", "ws-123") + } if r.Header.Get("Authorization") != "" { t.Errorf("%s sent an Authorization header", r.Path) } } var providers []string - for _, r := range reqs[1:listCalls] { - providers = append(providers, r.Header.Get("x-service-provider")+"/"+r.q("page")) + for _, r := range reqs[:listCalls] { + if r.Path == "/v2/mailboxes/list" { + providers = append(providers, r.Header.Get("x-service-provider")+"/"+r.q("page")) + } } if !reflect.DeepEqual(providers, []string{"GOOGLE/1", "GOOGLE/2", "MICROSOFT/1"}) { t.Fatalf("list passes = %v", providers) @@ -199,17 +251,30 @@ func TestZapmailForbiddenEnvelope(t *testing.T) { } } -func TestZapmailSingleProviderNoWorkspace(t *testing.T) { - srv := newRecorder(t, func(w http.ResponseWriter, _ *http.Request) { +// With no workspace listed, calls go to the key's primary workspace, for both providers. +func TestZapmailNoWorkspacesUsesPrimary(t *testing.T) { + srv := newRecorder(t, func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/v2/workspaces" { + writeJSON(w, 200, `{"status":200,"data":[]}`) + return + } writeJSON(w, 200, `{"status":200,"data":{"currentPage":1,"totalPages":1,"domains":[]}}`) }) - c := newTestClient(t, VendorZapmail, map[string]string{FieldAPIKey: testKey, FieldServiceProvider: "microsoft"}, srv.URL, nil) + c := newTestClient(t, VendorZapmail, fieldsFor(VendorZapmail), srv.URL, nil) if got := mustList(t, c); len(got) != 0 { t.Fatalf("List = %+v", got) } - reqs := srv.requests() - if len(reqs) != 1 || reqs[0].Header.Get("x-service-provider") != "MICROSOFT" || reqs[0].Header.Get("x-workspace-key") != "" { - t.Fatalf("requests = %+v", reqs) + var seen []string + for _, r := range srv.requests() { + if r.Path == "/v2/mailboxes/list" { + if r.Header.Get("x-workspace-key") != "" { + t.Errorf("sent x-workspace-key %q", r.Header.Get("x-workspace-key")) + } + seen = append(seen, r.Header.Get("x-service-provider")) + } + } + if !reflect.DeepEqual(seen, []string{"GOOGLE", "MICROSOFT"}) { + t.Fatalf("providers = %v", seen) } } @@ -236,17 +301,13 @@ func TestForgeVendors(t *testing.T) { writeJSON(w, 404, `{"code":404,"message":"Mailbox not found"}`) } }) - fields := map[string]string{FieldAPIKey: testKey} - if vendor == VendorInfraforge { - fields[FieldWorkspaceID] = "wks_70my6ggvn5csfw3o27ojq" - } - c := newTestClient(t, vendor, fields, srv.URL, nil) + c := newTestClient(t, vendor, map[string]string{FieldAPIKey: testKey}, srv.URL, nil) if err := c.Verify(context.Background()); err != nil { t.Fatalf("Verify: %v", err) } got := mustList(t, c) want := []Mailbox{ - {ID: "mbx_1duj5a6534j37kzook2l9", Email: "jondoe@example.com", FirstName: "Jon", LastName: "Doe", Domain: "example.com", Provider: ProviderSMTP, Status: "active"}, + {ID: "mbx_1duj5a6534j37kzook2l9", Email: "jondoe@example.com", FirstName: "Jon", LastName: "Doe", Domain: "example.com", Provider: ProviderSMTP, Status: "active", Workspace: "Main"}, {ID: "mbx_2", Email: "amy@example.org", FirstName: "Amy", LastName: "Lee", Domain: "example.org", Provider: ProviderSMTP, Status: "pending"}, } if !reflect.DeepEqual(got, want) { @@ -281,8 +342,8 @@ func TestForgeVendors(t *testing.T) { if list.Query["with_credentials"][0] != "true" { t.Fatalf("list query = %v", list.Query) } - if ws := list.q("workspace_id"); (vendor == VendorInfraforge) != (ws == "wks_70my6ggvn5csfw3o27ojq") { - t.Fatalf("%s workspace_id = %q", vendor, ws) + if ws := list.q("workspace_id"); ws != "" { + t.Fatalf("%s workspace_id = %q, want every workspace", vendor, ws) } }) } @@ -414,7 +475,7 @@ func TestScaledMail(t *testing.T) { srv := newRecorder(t, func(w http.ResponseWriter, r *http.Request) { switch r.URL.Path { case "/organizations": - writeJSON(w, 200, `{"organizations":[{"id":"recORG000000001"}]}`) + writeJSON(w, 200, `{"organizations":[{"id":"recORG000000001","name":"Acme"}]}`) case "/domains": writeJSON(w, 200, `{"total":2,"domains":[ {"id":"recDOM1","domain":"outreach-one.com","tag":"campaign-q3","redirect":"https://example.com","order_type":"google","order_id":"recORD1","payment_id":"recPAY1","domain_provider":"Scaledmail","user_id":"recUSR1","total_mailboxes":2,"mailbox":[{"first_name":"Jane","last_name":"Doe","alias":"jane"}],"status":"Active"}, @@ -430,15 +491,15 @@ func TestScaledMail(t *testing.T) { } }) s := &sleeps{} - c := newTestClient(t, VendorScaledMail, map[string]string{FieldAPIKey: testKey, FieldOrganizationID: org}, srv.URL, s) + c := newTestClient(t, VendorScaledMail, map[string]string{FieldAPIKey: testKey}, srv.URL, s) if err := c.Verify(context.Background()); err != nil { t.Fatalf("Verify: %v", err) } got := mustList(t, c) want := []Mailbox{ - {ID: "jane@outreach-one.com", Email: "jane@outreach-one.com", FirstName: "Jane", LastName: "Doe", Domain: "outreach-one.com", Provider: ProviderGoogle, Status: "Active"}, - {ID: "joe@outreach-one.com", Email: "joe@outreach-one.com", FirstName: "Joe", LastName: "Roe", Domain: "outreach-one.com", Provider: ProviderGoogle, Status: "Active"}, - {ID: "kim@outreach-two.com", Email: "kim@outreach-two.com", FirstName: "Kim", LastName: "Park", Domain: "outreach-two.com", Provider: ProviderMicrosoft, Status: "Active"}, + {ID: org + ":jane@outreach-one.com", Email: "jane@outreach-one.com", FirstName: "Jane", LastName: "Doe", Domain: "outreach-one.com", Provider: ProviderGoogle, Status: "Active", Workspace: "Acme"}, + {ID: org + ":joe@outreach-one.com", Email: "joe@outreach-one.com", FirstName: "Joe", LastName: "Roe", Domain: "outreach-one.com", Provider: ProviderGoogle, Status: "Active", Workspace: "Acme"}, + {ID: org + ":kim@outreach-two.com", Email: "kim@outreach-two.com", FirstName: "Kim", LastName: "Park", Domain: "outreach-two.com", Provider: ProviderMicrosoft, Status: "Active", Workspace: "Acme"}, } if !reflect.DeepEqual(got, want) { t.Fatalf("List = %+v\nwant %+v", got, want) @@ -447,11 +508,14 @@ func TestScaledMail(t *testing.T) { t.Fatalf("Credentials = %+v", cr) } - // A fresh client has no cache, so it re-reads the mailbox's domain. - fresh := newTestClient(t, VendorScaledMail, map[string]string{FieldAPIKey: testKey, FieldOrganizationID: org}, srv.URL, nil) - if cr := mustCreds(t, fresh, Mailbox{ID: "kim@outreach-two.com", Email: "kim@outreach-two.com", Domain: "outreach-two.com"}); cr.Password != "Outl00k!" { + // A fresh client has no cache, so it re-reads the mailbox's domain, with or without the organization in the id. + fresh := newTestClient(t, VendorScaledMail, map[string]string{FieldAPIKey: testKey}, srv.URL, nil) + if cr := mustCreds(t, fresh, Mailbox{ID: org + ":kim@outreach-two.com", Email: "kim@outreach-two.com", Domain: "outreach-two.com"}); cr.Password != "Outl00k!" { t.Fatalf("fetched Credentials = %+v", cr) } + if cr := mustCreds(t, fresh, Mailbox{ID: "kim@outreach-two.com", Email: "kim@outreach-two.com"}); cr.Password != "Outl00k!" { + t.Fatalf("legacy id Credentials = %+v", cr) + } if _, err := fresh.Credentials(context.Background(), Mailbox{Email: "nobody@outreach-two.com"}); !errors.Is(err, ErrNotFound) { t.Fatalf("unknown mailbox: %v", err) } @@ -480,6 +544,35 @@ func TestScaledMail(t *testing.T) { } } +func TestScaledMailOrganizationShapes(t *testing.T) { + for body, want := range map[string][]workspace{ + `[{"id":"recA","name":"A"},{"id":"recB"}]`: {{ID: "recA", Name: "A"}, {ID: "recB"}}, + `{"data":[{"organization_id":"recC","name":"C"}]}`: {{ID: "recC", Name: "C"}}, + `{"organization":{"_id":"recD"},"message":"ok"}`: {{ID: "recD"}}, + `{"organizations":[{"id":"recE"},{"id":"recE"},{"name":"x"}]}`: {{ID: "recE"}}, + `{"message":"ok"}`: nil, + } { + if got := scaledMailOrgs([]byte(body)); !reflect.DeepEqual(got, want) { + t.Errorf("scaledMailOrgs(%s) = %+v, want %+v", body, got, want) + } + } +} + +func TestScaledMailNoOrganization(t *testing.T) { + srv := newRecorder(t, func(w http.ResponseWriter, _ *http.Request) { + writeJSON(w, 200, `{"organizations":[]}`) + }) + c := newTestClient(t, VendorScaledMail, map[string]string{FieldAPIKey: testKey}, srv.URL, nil) + if err := c.Verify(context.Background()); !errors.Is(err, ErrNoWorkspace) { + t.Fatalf("Verify = %v, want ErrNoWorkspace", err) + } + // A connection saved with an organization id keeps using it. + legacy := newTestClient(t, VendorScaledMail, map[string]string{FieldAPIKey: testKey, FieldOrganizationID: "recOLD"}, srv.URL, nil) + if err := legacy.Verify(context.Background()); err != nil { + t.Fatalf("legacy Verify = %v", err) + } +} + func TestListCapsAtMax(t *testing.T) { var sb strings.Builder sb.WriteString("[") diff --git a/internal/pkg/mailvendor/workspaces.go b/internal/pkg/mailvendor/workspaces.go new file mode 100644 index 000000000..626bdf3ca --- /dev/null +++ b/internal/pkg/mailvendor/workspaces.go @@ -0,0 +1,108 @@ +package mailvendor + +import ( + "context" + "errors" + "net/http" + "strings" + "sync" + "time" +) + +// workspace is one vendor workspace (or organization) the key reaches. +type workspace struct { + ID string + Name string +} + +// workspaceTTL bounds how long a discovered workspace list is reused, so one added at the vendor shows up. +const workspaceTTL = 5 * time.Minute + +// workspaceCache holds the workspaces a key reaches; vendors that scope every call to one are read across all of them. +type workspaceCache struct { + mu sync.Mutex + list []workspace + at time.Time +} + +func (w *workspaceCache) get(ctx context.Context, fetch func(context.Context) ([]workspace, error)) ([]workspace, error) { + w.mu.Lock() + if w.at.IsZero() || time.Since(w.at) >= workspaceTTL { + w.mu.Unlock() + list, err := fetch(ctx) + if err != nil { + return nil, err + } + w.mu.Lock() + w.list, w.at = list, time.Now() + } + out := append([]workspace(nil), w.list...) + w.mu.Unlock() + return out, nil +} + +// scopeSep joins a workspace id to an id the vendor only resolves inside that workspace. +const scopeSep = ":" + +func scoped(ws, id string) string { + if ws == "" || id == "" { + return id + } + return ws + scopeSep + id +} + +// unscoped splits a scoped id; an id stored before scoping comes back with no workspace. +func unscoped(id string) (ws, rest string) { + if i := strings.Index(id, scopeSep); i > 0 { + return id[:i], id[i+len(scopeSep):] + } + return "", id +} + +// findCredentials asks each workspace in turn, for an id stored before ids carried their workspace. +func findCredentials(vendor string, wss []workspace, fn func(workspace) (Credentials, error)) (Credentials, error) { + refused, read := error(nil), 0 + for _, ws := range wss { + cr, err := fn(ws) + switch { + case err == nil: + return cr, nil + case errors.Is(err, ErrUnauthorized): + refused = err + case errors.Is(err, ErrRateLimited), errors.Is(err, context.Canceled), errors.Is(err, context.DeadlineExceeded): + return Credentials{}, err + default: + read++ + } + } + if read == 0 && refused != nil { + return Credentials{}, refused + } + return Credentials{}, vendorErr(vendor, http.StatusOK, "not found", ErrNotFound) +} + +// errStop ends eachWorkspace early without an error, once a cap is reached. +var errStop = errors.New("mailvendor: stop") + +// eachWorkspace runs fn in every workspace. One the key may not read is skipped, unless the key can read none. +func eachWorkspace(wss []workspace, fn func(workspace) error) error { + var refused error + read := 0 + for _, ws := range wss { + err := fn(ws) + switch { + case err == nil: + read++ + case errors.Is(err, errStop): + return nil + case errors.Is(err, ErrUnauthorized): + refused = err + default: + return err + } + } + if read == 0 && refused != nil { + return refused + } + return nil +} diff --git a/internal/pkg/mailvendor/zapmail.go b/internal/pkg/mailvendor/zapmail.go index 6c6fce622..5495175ec 100644 --- a/internal/pkg/mailvendor/zapmail.go +++ b/internal/pkg/mailvendor/zapmail.go @@ -2,46 +2,34 @@ package mailvendor import ( "context" - "fmt" "net/http" "net/url" "strconv" - "strings" "sync" ) // Zapmail: https://docs.zapmail.ai/llms.txt +// Calls are scoped by workspace and by service provider, so the client reads every workspace for both providers. type zapmail struct { - t *transport - workspace string - providers []string - cache credCache - domains domainProvider + t *transport + workspaces workspaceCache + cache credCache + domains domainProvider } +// zapmailProviders are the service providers Zapmail lists separately. +var zapmailProviders = []string{"GOOGLE", "MICROSOFT"} + const ( zapmailPageSize = 100 // zapmailPerSecond is the documented general limit. zapmailPerSecond = 5 ) -func newZapmail(vals map[string]string, o options) (*zapmail, error) { +func newZapmail(vals map[string]string, o options) *zapmail { key := vals[FieldAPIKey] - var providers []string - switch sp := strings.ToUpper(vals[FieldServiceProvider]); sp { - case "": - providers = []string{"GOOGLE", "MICROSOFT"} - case "GOOGLE", "MICROSOFT": - providers = []string{sp} - default: - return nil, fmt.Errorf("%w: %s: Mailbox type must be GOOGLE or MICROSOFT", ErrInvalidConfig, VendorZapmail) - } auth := func(h http.Header) { h.Set("x-auth-zapmail", key) } - return &zapmail{ - t: newTransport(VendorZapmail, "https://api.zapmail.ai/api", o, auth, zapmailPerSecond), - workspace: vals[FieldWorkspaceID], - providers: providers, - }, nil + return &zapmail{t: newTransport(VendorZapmail, "https://api.zapmail.ai/api", o, auth, zapmailPerSecond)} } func (c *zapmail) Vendor() string { return VendorZapmail } @@ -58,10 +46,10 @@ func (e zapmailEnvelope) err() error { return statusErr(VendorZapmail, e.Status) } -func (c *zapmail) header(provider string) http.Header { +func (c *zapmail) header(ws, provider string) http.Header { h := http.Header{} - if c.workspace != "" { - h.Set("x-workspace-key", c.workspace) + if ws != "" { + h.Set("x-workspace-key", ws) } if provider != "" { h.Set("x-service-provider", provider) @@ -69,12 +57,52 @@ func (c *zapmail) header(provider string) http.Header { return h } -func (c *zapmail) Verify(ctx context.Context) error { - var res zapmailEnvelope - if err := c.t.do(ctx, call{method: http.MethodGet, path: "/v2/users", header: c.header("")}, &res); err != nil { - return err +// listWorkspaces pages through GET /v2/workspaces. +func (c *zapmail) listWorkspaces(ctx context.Context) ([]workspace, error) { + var out []workspace + for page := 1; page <= maxPages; page++ { + var res struct { + zapmailEnvelope + Data []struct { + ID string `json:"id"` + Name string `json:"name"` + } `json:"data"` + } + q := url.Values{"page": {strconv.Itoa(page)}, "limit": {strconv.Itoa(zapmailPageSize)}} + if err := c.t.do(ctx, call{method: http.MethodGet, path: "/v2/workspaces", query: q}, &res); err != nil { + return nil, err + } + if err := res.err(); err != nil { + return nil, err + } + for _, w := range res.Data { + if w.ID != "" { + out = append(out, workspace{ID: w.ID, Name: w.Name}) + } + } + if len(res.Data) < zapmailPageSize { + break + } } - return res.err() + return out, nil +} + +// all is every workspace the key reaches; with none listed, calls go to the key's primary workspace. +func (c *zapmail) all(ctx context.Context) ([]workspace, error) { + wss, err := c.workspaces.get(ctx, c.listWorkspaces) + if err != nil { + return nil, err + } + if len(wss) == 0 { + return []workspace{{}}, nil + } + return wss, nil +} + +// Verify reads the workspace list, which checks the key. +func (c *zapmail) Verify(ctx context.Context) error { + _, err := c.listWorkspaces(ctx) + return err } type zapmailMailbox struct { @@ -107,66 +135,80 @@ func (m zapmailMailbox) email() string { return "" } +// List pages through every workspace's mailboxes, once per service provider; ids carry their workspace. func (c *zapmail) List(ctx context.Context) ([]Mailbox, error) { + wss, err := c.all(ctx) + if err != nil { + return nil, err + } var out []Mailbox seen := make(map[string]bool) - for _, provider := range c.providers { - for page := 1; page <= maxPages; page++ { - var res struct { - zapmailEnvelope - Data struct { - CurrentPage int `json:"currentPage"` - NextPage int `json:"nextPage"` - TotalPages int `json:"totalPages"` - Domains []struct { - Domain string `json:"domain"` - Mailboxes []zapmailMailbox `json:"mailboxes"` - } `json:"domains"` - } `json:"data"` - } - q := url.Values{"page": {strconv.Itoa(page)}, "limit": {strconv.Itoa(zapmailPageSize)}} - if err := c.t.do(ctx, call{method: http.MethodGet, path: "/v2/mailboxes/list", query: q, header: c.header(provider)}, &res); err != nil { - return nil, err - } - if err := res.err(); err != nil { - return nil, err - } - for _, d := range res.Data.Domains { - for _, m := range d.Mailboxes { - if m.ID == "" || seen[m.ID] { - continue - } - seen[m.ID] = true - c.cache.put(m.ID, m.credentials()) - out = append(out, Mailbox{ - ID: m.ID, - Email: m.email(), - FirstName: m.FirstName, - LastName: m.LastName, - Domain: firstNonEmpty(m.Domain, d.Domain), - Provider: normalizeProvider(provider), - Status: m.Status, - }) - if len(out) >= MaxMailboxes { - return out, nil + err = eachWorkspace(wss, func(ws workspace) error { + for _, provider := range zapmailProviders { + for page := 1; page <= maxPages; page++ { + var res struct { + zapmailEnvelope + Data struct { + CurrentPage int `json:"currentPage"` + NextPage int `json:"nextPage"` + TotalPages int `json:"totalPages"` + Domains []struct { + Domain string `json:"domain"` + Mailboxes []zapmailMailbox `json:"mailboxes"` + } `json:"domains"` + } `json:"data"` + } + q := url.Values{"page": {strconv.Itoa(page)}, "limit": {strconv.Itoa(zapmailPageSize)}} + if err := c.t.do(ctx, call{method: http.MethodGet, path: "/v2/mailboxes/list", query: q, header: c.header(ws.ID, provider)}, &res); err != nil { + return err + } + if err := res.err(); err != nil { + return err + } + for _, d := range res.Data.Domains { + for _, m := range d.Mailboxes { + id := scoped(ws.ID, m.ID) + if m.ID == "" || seen[id] { + continue + } + seen[id] = true + c.cache.put(id, m.credentials()) + out = append(out, Mailbox{ + ID: id, + Email: m.email(), + FirstName: m.FirstName, + LastName: m.LastName, + Domain: firstNonEmpty(m.Domain, d.Domain), + Provider: normalizeProvider(provider), + Status: m.Status, + Workspace: ws.Name, + }) + if len(out) >= MaxMailboxes { + return errStop + } } } - } - more := res.Data.TotalPages > page || (res.Data.TotalPages == 0 && res.Data.NextPage > page) - if len(res.Data.Domains) == 0 || !more { - break + more := res.Data.TotalPages > page || (res.Data.TotalPages == 0 && res.Data.NextPage > page) + if len(res.Data.Domains) == 0 || !more { + break + } } } + return nil + }) + if err != nil { + return nil, err } return out, nil } -// Credentials comes from List's cache, else from the mailbox detail endpoint. +// Credentials comes from List's cache, else from the mailbox detail endpoint in the mailbox's workspace. func (c *zapmail) Credentials(ctx context.Context, m Mailbox) (Credentials, error) { if cr, ok := c.cache.get(m.ID); ok { return cr, nil } - if m.ID == "" { + ws, id := unscoped(m.ID) + if id == "" { return Credentials{}, vendorErr(VendorZapmail, 0, "mailbox id is required", ErrNotFound) } provider := "" @@ -176,14 +218,27 @@ func (c *zapmail) Credentials(ctx context.Context, m Mailbox) (Credentials, erro case ProviderMicrosoft: provider = "MICROSOFT" } + if ws != "" { + return c.detail(ctx, ws, provider, id) + } + wss, err := c.all(ctx) + if err != nil { + return Credentials{}, err + } + return findCredentials(VendorZapmail, wss, func(w workspace) (Credentials, error) { + return c.detail(ctx, w.ID, provider, id) + }) +} + +func (c *zapmail) detail(ctx context.Context, ws, provider, id string) (Credentials, error) { var res struct { zapmailEnvelope Data struct { Mailbox *zapmailMailbox `json:"mailbox"` } `json:"data"` } - q := url.Values{"id": {m.ID}} - if err := c.t.do(ctx, call{method: http.MethodGet, path: "/v2/mailboxes", query: q, header: c.header(provider)}, &res); err != nil { + q := url.Values{"id": {id}} + if err := c.t.do(ctx, call{method: http.MethodGet, path: "/v2/mailboxes", query: q, header: c.header(ws, provider)}, &res); err != nil { return Credentials{}, err } if err := res.err(); err != nil { @@ -220,75 +275,96 @@ func (c *zapmail) DomainCapabilities() DomainCapabilities { return DomainCapabilities{Forwarding: true, ForwardingRemove: true, DNS: true, DNSTypes: allDNSTypes()} } -// Domains pages through GET /v2/domains once per service provider. +// Domains pages through GET /v2/domains in every workspace, once per service provider; ids carry their workspace. func (c *zapmail) Domains(ctx context.Context) ([]Domain, error) { + wss, err := c.all(ctx) + if err != nil { + return nil, err + } var out []Domain seen := make(map[string]bool) - for _, provider := range c.providers { - for page := 1; page <= maxPages; page++ { - var res struct { - zapmailEnvelope - Data struct { - NextPage int `json:"nextPage"` - TotalPages int `json:"totalPages"` - Domains []struct { - ID string `json:"id"` - Domain string `json:"domain"` - ForwardTo *string `json:"forwardTo"` - } `json:"domains"` - } `json:"data"` - } - q := url.Values{"page": {strconv.Itoa(page)}, "limit": {strconv.Itoa(zapmailPageSize)}} - if err := c.t.do(ctx, call{method: http.MethodGet, path: "/v2/domains", query: q, header: c.header(provider)}, &res); err != nil { - return nil, err - } - if err := res.err(); err != nil { - return nil, err - } - for _, d := range res.Data.Domains { - if d.ID == "" || seen[d.ID] { - continue + err = eachWorkspace(wss, func(ws workspace) error { + for _, provider := range zapmailProviders { + for page := 1; page <= maxPages; page++ { + var res struct { + zapmailEnvelope + Data struct { + NextPage int `json:"nextPage"` + TotalPages int `json:"totalPages"` + Domains []struct { + ID string `json:"id"` + Domain string `json:"domain"` + ForwardTo *string `json:"forwardTo"` + } `json:"domains"` + } `json:"data"` } - seen[d.ID] = true - c.domains.put(d.ID, provider) - dom := Domain{ID: d.ID, Name: d.Domain} - if d.ForwardTo != nil { - dom.Forwarding = *d.ForwardTo + q := url.Values{"page": {strconv.Itoa(page)}, "limit": {strconv.Itoa(zapmailPageSize)}} + if err := c.t.do(ctx, call{method: http.MethodGet, path: "/v2/domains", query: q, header: c.header(ws.ID, provider)}, &res); err != nil { + return err } - out = append(out, dom) - if len(out) >= MaxDomains { - return out, nil + if err := res.err(); err != nil { + return err + } + for _, d := range res.Data.Domains { + id := scoped(ws.ID, d.ID) + if d.ID == "" || seen[id] { + continue + } + seen[id] = true + c.domains.put(id, provider) + dom := Domain{ID: id, Name: d.Domain} + if d.ForwardTo != nil { + dom.Forwarding = *d.ForwardTo + } + out = append(out, dom) + if len(out) >= MaxDomains { + return errStop + } + } + more := res.Data.TotalPages > page || (res.Data.TotalPages == 0 && res.Data.NextPage > page) + if len(res.Data.Domains) == 0 || !more { + break } - } - more := res.Data.TotalPages > page || (res.Data.TotalPages == 0 && res.Data.NextPage > page) - if len(res.Data.Domains) == 0 || !more { - break } } + return nil + }) + if err != nil { + return nil, err } return out, nil } +// zapmailDomain is a listed domain's workspace, vendor id and the headers its calls send. +func (c *zapmail) zapmailDomain(d Domain) (id string, h http.Header, err error) { + ws, id := unscoped(d.ID) + if id == "" { + return "", nil, invalid(VendorZapmail, "domain id is required") + } + h = c.header(ws, c.domains.get(d.ID)) + // The remove-forwarding page names the workspace header x-workspace-id, so both are sent. + if ws != "" { + h.Set("x-workspace-id", ws) + } + return id, h, nil +} + // SetForwarding calls POST /v2/domains/forwarding, or POST /v2/domains/remove-forwarding for an empty target. func (c *zapmail) SetForwarding(ctx context.Context, d Domain, target string) error { target, err := forwardingTarget(VendorZapmail, target, c.DomainCapabilities()) if err != nil { return err } - if d.ID == "" { - return invalid(VendorZapmail, "domain id is required") + id, h, err := c.zapmailDomain(d) + if err != nil { + return err } - h := c.header(c.domains.get(d.ID)) var cl call if target == "" { - // This endpoint's page names the workspace header x-workspace-id, so both are sent. - if c.workspace != "" { - h.Set("x-workspace-id", c.workspace) - } - cl = call{method: http.MethodPost, path: "/v2/domains/remove-forwarding", body: map[string]any{"domainId": d.ID}, header: h} + cl = call{method: http.MethodPost, path: "/v2/domains/remove-forwarding", body: map[string]any{"domainId": id}, header: h} } else { // contains and tagIds are filters that widen the target set, so they are left out: only this domain changes. - cl = call{method: http.MethodPost, path: "/v2/domains/forwarding", body: map[string]any{"domainIds": []string{d.ID}, "forwardTo": target}, header: h} + cl = call{method: http.MethodPost, path: "/v2/domains/forwarding", body: map[string]any{"domainIds": []string{id}, "forwardTo": target}, header: h} } var res zapmailEnvelope if err := c.t.do(ctx, cl, &res); err != nil { @@ -303,10 +379,10 @@ func (c *zapmail) UpsertDNSRecord(ctx context.Context, d Domain, r DNSRecord) er if err != nil { return err } - if d.ID == "" { - return invalid(VendorZapmail, "domain id is required") + id, h, err := c.zapmailDomain(d) + if err != nil { + return err } - h := c.header(c.domains.get(d.ID)) var list struct { zapmailEnvelope Data struct { @@ -318,7 +394,7 @@ func (c *zapmail) UpsertDNSRecord(ctx context.Context, d Domain, r DNSRecord) er } `json:"records"` } `json:"data"` } - if err := c.t.do(ctx, call{method: http.MethodGet, path: "/v2/dns/", query: url.Values{"id": {d.ID}}, header: h}, &list); err != nil { + if err := c.t.do(ctx, call{method: http.MethodGet, path: "/v2/dns/", query: url.Values{"id": {id}}, header: h}, &list); err != nil { return err } if err := list.err(); err != nil { @@ -340,12 +416,12 @@ func (c *zapmail) UpsertDNSRecord(ctx context.Context, d Domain, r DNSRecord) er if p.update >= 0 { // The documented body also names a zoneId, which no documented response carries; it is left out. cl = call{method: http.MethodPut, path: "/v2/dns", header: h, body: map[string]any{ - "assignedDomainId": d.ID, "dnsRecordId": existing[p.update].ID, + "assignedDomainId": id, "dnsRecordId": existing[p.update].ID, "host": host, "value": r.Value, "recordType": r.Type, }} } else { cl = call{method: http.MethodPost, path: "/v2/dns", header: h, body: map[string]any{ - "assignedDomainId": d.ID, + "assignedDomainId": id, "records": []map[string]string{{"host": host, "value": r.Value, "recordType": r.Type}}, }} } @@ -357,7 +433,7 @@ func (c *zapmail) UpsertDNSRecord(ctx context.Context, d Domain, r DNSRecord) er return err } for _, idx := range p.remove { - q := url.Values{"id": {existing[idx].ID}, "assignedDomainId": {d.ID}} + q := url.Values{"id": {existing[idx].ID}, "assignedDomainId": {id}} res = zapmailEnvelope{} if err := c.t.do(ctx, call{method: http.MethodDelete, path: "/v2/dns", query: q, header: h}, &res); err != nil { return err diff --git a/internal/sandbox/vendors.go b/internal/sandbox/vendors.go index 8b60f19ae..7a7706637 100644 --- a/internal/sandbox/vendors.go +++ b/internal/sandbox/vendors.go @@ -15,7 +15,9 @@ import ( // at /inboxkit and /mailforge. Any API key is accepted. // // InboxKit holds Google Workspace mailboxes (password only, as the real API -// returns) and lets its domains be forwarded and their DNS written. Mailforge +// returns) across two workspaces, scopes every call but the workspace list by +// X-Workspace-Id as the real API does, and lets its domains be forwarded and +// their DNS written. Mailforge // hosts its own SMTP/IMAP, pointed here at the sandbox's mailpit and dovecot so // its mailboxes import and connect for real, and only forwards domains. type VendorMock struct { @@ -34,8 +36,8 @@ type endpoint struct { } type mockDomain struct { - ID, Vendor, Name, Forwarding string - Records []mockRecord + ID, Vendor, Workspace, Name, Forwarding string + Records []mockRecord } type mockRecord struct { @@ -46,7 +48,13 @@ type mockRecord struct { } type mockBox struct { - ID, Vendor, User, Domain, First, Last, Platform string + ID, Vendor, Workspace, User, Domain, First, Last, Platform string +} + +// inboxKitWorkspaces are the mock InboxKit account's workspaces, by uid. +var inboxKitWorkspaces = []struct{ UID, Name string }{ + {"6f1c2d3e-0000-4000-8000-000000000001", "Sunrise Outbound"}, + {"6f1c2d3e-0000-4000-8000-000000000002", "Sunrise Trials"}, } // NewVendorMock seeds both vendors; smtp and imap are where Mailforge's mailboxes connect. @@ -58,9 +66,9 @@ func NewVendorMock(cfg Config) *VendorMock { latency: cfg.VendorLatency, } for _, d := range []mockDomain{ - {ID: "ik-d1", Vendor: "inboxkit", Name: "sunrise-outbound.test", Forwarding: "https://sunriselabs.test"}, - {ID: "ik-d2", Vendor: "inboxkit", Name: "trysunrise.test"}, - {ID: "mf-d1", Vendor: "mailforge", Name: "sunrisehq.test"}, + {ID: "ik-d1", Vendor: "inboxkit", Workspace: inboxKitWorkspaces[0].UID, Name: "sunrise-outbound.test", Forwarding: "https://sunriselabs.test"}, + {ID: "ik-d2", Vendor: "inboxkit", Workspace: inboxKitWorkspaces[1].UID, Name: "trysunrise.test"}, + {ID: "mf-d1", Vendor: "mailforge", Workspace: "mf-ws", Name: "sunrisehq.test"}, } { d := d m.domains[d.ID] = &d @@ -68,9 +76,9 @@ func NewVendorMock(cfg Config) *VendorMock { people := [][2]string{{"Ava", "Stone"}, {"Leo", "Park"}, {"Mia", "Chen"}, {"Noah", "Reed"}} for i, p := range people { m.boxes = append(m.boxes, - mockBox{ID: "ik-" + strconv.Itoa(i+1), Vendor: "inboxkit", User: strings.ToLower(p[0]), Domain: "sunrise-outbound.test", First: p[0], Last: p[1], Platform: "google"}, - mockBox{ID: "ik-" + strconv.Itoa(i+11), Vendor: "inboxkit", User: strings.ToLower(p[0]) + "." + strings.ToLower(p[1]), Domain: "trysunrise.test", First: p[0], Last: p[1], Platform: "google"}, - mockBox{ID: "mf-" + strconv.Itoa(i+1), Vendor: "mailforge", User: strings.ToLower(p[0]), Domain: "sunrisehq.test", First: p[0], Last: p[1], Platform: "smtp"}, + mockBox{ID: "ik-" + strconv.Itoa(i+1), Vendor: "inboxkit", Workspace: inboxKitWorkspaces[0].UID, User: strings.ToLower(p[0]), Domain: "sunrise-outbound.test", First: p[0], Last: p[1], Platform: "google"}, + mockBox{ID: "ik-" + strconv.Itoa(i+11), Vendor: "inboxkit", Workspace: inboxKitWorkspaces[1].UID, User: strings.ToLower(p[0]) + "." + strings.ToLower(p[1]), Domain: "trysunrise.test", First: p[0], Last: p[1], Platform: "google"}, + mockBox{ID: "mf-" + strconv.Itoa(i+1), Vendor: "mailforge", Workspace: "mf-ws", User: strings.ToLower(p[0]), Domain: "sunrisehq.test", First: p[0], Last: p[1], Platform: "smtp"}, ) } return m @@ -102,42 +110,74 @@ func (m *VendorMock) inboxKit(w http.ResponseWriter, r *http.Request, path strin defer m.mu.Unlock() var body map[string]any _ = json.NewDecoder(r.Body).Decode(&body) + if path == "/v1/api/workspaces/list" { + out := make([]map[string]any, 0, len(inboxKitWorkspaces)) + for _, ws := range inboxKitWorkspaces { + out = append(out, map[string]any{"uid": ws.UID, "name": ws.Name}) + } + writeMock(w, http.StatusOK, map[string]any{"error": false, "workspaces": out}) + return + } + ws := r.Header.Get("X-Workspace-Id") + known := false + for _, k := range inboxKitWorkspaces { + known = known || k.UID == ws + } + if !known { + writeMock(w, http.StatusBadRequest, map[string]any{"error": true, "message": "Workspace not found"}) + return + } + domain := func(uid string) *mockDomain { + if d := m.domains[uid]; d != nil && d.Vendor == "inboxkit" && d.Workspace == ws { + return d + } + return nil + } switch path { case "/v1/api/mailboxes/list": var out []map[string]any for _, b := range m.boxes { - if b.Vendor == "inboxkit" { + if b.Vendor == "inboxkit" && b.Workspace == ws { out = append(out, map[string]any{"uid": b.ID, "domain_name": b.Domain, "first_name": b.First, "last_name": b.Last, "username": b.User, "platform": b.Platform, "status": "active"}) } } writeMock(w, http.StatusOK, map[string]any{"error": false, "mailboxes": out, "pages": 1, "current_page": 1}) case "/v1/api/mailboxes/show-credentials": - writeMock(w, http.StatusOK, map[string]any{"error": false, "password": "sandbox", "app_password": ""}) + uid := r.URL.Query().Get("uid") + for _, b := range m.boxes { + if b.Vendor == "inboxkit" && b.Workspace == ws && b.ID == uid { + writeMock(w, http.StatusOK, map[string]any{"error": false, "password": "sandbox", "app_password": ""}) + return + } + } + writeMock(w, http.StatusNotFound, map[string]any{"error": true, "message": "Mailbox not found"}) case "/v1/api/domains/list": var out []map[string]any for _, d := range m.vendorDomains("inboxkit") { - out = append(out, map[string]any{"uid": d.ID, "name": d.Name, "forwarding_url": d.Forwarding}) + if d.Workspace == ws { + out = append(out, map[string]any{"uid": d.ID, "name": d.Name, "forwarding_url": d.Forwarding}) + } } writeMock(w, http.StatusOK, map[string]any{"error": false, "domains": out, "pages": 1}) case "/v1/api/domains/forwarding": target, _ := body["forwarding_url"].(string) uids, _ := body["uids"].([]any) for _, u := range uids { - if d, ok := m.domains[toString(u)]; ok && d.Vendor == "inboxkit" { + if d := domain(toString(u)); d != nil { d.Forwarding = target } } writeMock(w, http.StatusOK, map[string]any{"error": false}) case "/v1/api/dns/list": - d := m.domains[r.URL.Query().Get("uid")] + d := domain(r.URL.Query().Get("uid")) if d == nil { writeMock(w, http.StatusNotFound, map[string]any{"error": true}) return } writeMock(w, http.StatusOK, map[string]any{"error": false, "dns_record": map[string]any{"records": d.Records}}) case "/v1/api/dns/add", "/v1/api/dns/update", "/v1/api/dns/delete": - d := m.domains[toString(body["uid"])] + d := domain(toString(body["uid"])) if d == nil { writeMock(w, http.StatusNotFound, map[string]any{"error": true}) return @@ -231,7 +271,7 @@ func (m *VendorMock) mailforge(w http.ResponseWriter, r *http.Request, path stri func (m *VendorMock) forgeBox(b mockBox) map[string]any { email := b.User + "@" + b.Domain return map[string]any{ - "id": b.ID, "email": email, "firstName": b.First, "lastName": b.Last, "domain": b.Domain, "status": "active", + "id": b.ID, "email": email, "firstName": b.First, "lastName": b.Last, "domain": b.Domain, "status": "active", "workspaceId": b.Workspace, "credentials": map[string]any{ "imapHost": m.imap.Host, "imapPort": m.imap.Port, "imapUsername": email, "imapPassword": "sandbox", "smtpHost": m.smtp.Host, "smtpPort": m.smtp.Port, "smtpUsername": email, "smtpPassword": "sandbox", diff --git a/web/src/components/app/emails/import/PickTable.tsx b/web/src/components/app/emails/import/PickTable.tsx index ef85abe77..c033ff5a7 100644 --- a/web/src/components/app/emails/import/PickTable.tsx +++ b/web/src/components/app/emails/import/PickTable.tsx @@ -1,8 +1,9 @@ // The mailbox picker the vendor and admin-grant imports share: search, a -// filter for what is connected already, checkbox rows and paging. The caller -// owns the selection so the wizard's floating bar can act on it. +// filter for what is connected already, a workspace switcher when the rows +// span several, checkbox rows and paging. The caller owns the selection so the +// wizard's floating bar can act on it. import React from "react"; -import { ChevronLeftIcon, ChevronRightIcon, MailIcon, RefreshCwIcon } from "lucide-react"; +import { ChevronLeftIcon, ChevronRightIcon, LayersIcon, MailIcon, RefreshCwIcon } from "lucide-react"; import { SearchInput } from "@/components/ui/field"; import { CheckSquare } from "@/components/ui/check-square"; import ProviderLogo from "@/components/app/emails/ProviderLogo"; @@ -15,6 +16,8 @@ export interface PickItem { email: string; name: string; domain?: string; + /** A second line under the domain, such as the vendor workspace the mailbox sits in. */ + group?: string; /** Shows a Google / Microsoft / SMTP badge when set. */ provider?: "google" | "microsoft" | "smtp" | ""; status?: { label: string; cls: string }; @@ -52,6 +55,7 @@ export default function PickTable({ noun, loadingLogo, loadingSteps, + groupLabel = "Workspace", }: { items: PickItem[] | undefined; /** Whose mailboxes are being listed, and what is happening while they are. */ @@ -64,38 +68,55 @@ export default function PickTable({ selected: Set; setSelected: React.Dispatch>>; noun: { one: string; many: string }; + /** What PickItem.group names, for the switcher shown when rows span several. */ + groupLabel?: string; }) { const [query, setQuery] = React.useState(""); const [filter, setFilter] = React.useState("all"); + const [group, setGroup] = React.useState(null); const [page, setPage] = React.useState(0); const all = React.useMemo(() => items ?? [], [items]); + // Each group with its rows, in the order the source lists them; only worth a switcher with two or more. + const groups = React.useMemo(() => { + const by = new Map(); + for (const i of all) { + if (!i.group) continue; + const list = by.get(i.group); + if (list) list.push(i); + else by.set(i.group, [i]); + } + return by.size > 1 ? [...by.entries()].map(([name, rows]) => ({ name, rows })) : []; + }, [all]); + // A group that disappears on reload drops back to every row. + const activeGroup = group !== null && groups.some((g) => g.name === group) ? group : null; + const inScope = React.useMemo(() => (activeGroup === null ? all : all.filter((i) => i.group === activeGroup)), [all, activeGroup]); const showProvider = all.some((i) => i.provider !== undefined); const counts = React.useMemo( () => ({ - all: all.length, - new: all.filter((i) => !i.connected).length, - moving: all.filter((i) => i.connected && i.upgrade).length, - connected: all.filter((i) => i.connected).length, + all: inScope.length, + new: inScope.filter((i) => !i.connected).length, + moving: inScope.filter((i) => i.connected && i.upgrade).length, + connected: inScope.filter((i) => i.connected).length, }), - [all], + [inScope], ); const q = query.trim().toLowerCase(); const shown = React.useMemo( () => - all.filter((i) => { + inScope.filter((i) => { if (filter === "new" && i.connected) return false; if (filter === "connected" && !i.connected) return false; if (filter === "moving" && !(i.connected && i.upgrade)) return false; if (!q) return true; - return i.email.toLowerCase().includes(q) || i.name.toLowerCase().includes(q) || (i.domain ?? "").toLowerCase().includes(q); + return [i.email, i.name, i.domain ?? "", i.group ?? ""].some((v) => v.toLowerCase().includes(q)); }), - [all, filter, q], + [inScope, filter, q], ); - // A new search or filter starts from the first page. - React.useEffect(() => setPage(0), [q, filter]); + // A new search, filter or group starts from the first page. + React.useEffect(() => setPage(0), [q, filter, activeGroup]); const pages = Math.max(1, Math.ceil(shown.length / PAGE_SIZE)); const at = Math.min(page, pages - 1); @@ -114,6 +135,19 @@ export default function PickTable({ }); }; + // What "select the group" picks: rows that would be new here, or that move onto this source. + const wanted = (i: PickItem) => !i.disabledReason && (!i.connected || !!i.upgrade); + const setGroupPicked = (rows: PickItem[], on: boolean) => + setSelected((prev) => { + const next = new Set(prev); + for (const i of rows) { + if (!wanted(i)) continue; + if (on) next.add(i.id); + else next.delete(i.id); + } + return next; + }); + const toggleShown = () => setSelected((prev) => { const next = new Set(prev); @@ -194,6 +228,19 @@ export default function PickTable({ )} + {groups.length > 0 && ( + + )} +
{item.email} {item.name && {item.name}} - {item.domain || "-"} + + {item.domain || "-"} + {item.group && activeGroup === null && ( + {item.group} + )} + {showProvider && ( @@ -298,3 +350,77 @@ export default function PickTable({
); } + +// GroupSwitcher narrows the table to one workspace and picks or clears a whole +// workspace in one click, with how many of each are picked already. +function GroupSwitcher({ + label, + groups, + total, + active, + onPick, + selected, + wanted, + onPickAll, +}: { + label: string; + groups: { name: string; rows: PickItem[] }[]; + total: number; + active: string | null; + onPick: (group: string | null) => void; + selected: Set; + wanted: (i: PickItem) => boolean; + onPickAll: (rows: PickItem[], on: boolean) => void; +}) { + const current = groups.find((g) => g.name === active); + const pickable = current ? current.rows.filter(wanted) : []; + const picked = pickable.filter((i) => selected.has(i.id)).length; + const chip = (key: string | null, name: string, count: number, pickedHere: number) => { + const on = active === key; + return ( + + ); + }; + return ( +
+
+ + + {groups.length} {label.toLowerCase()}s + + {current && pickable.length > 0 && ( + + )} +
+
+ {chip(null, `All ${label.toLowerCase()}s`, total, 0)} + {groups.map((g) => chip(g.name, g.name, g.rows.length, g.rows.filter((i) => selected.has(i.id)).length))} +
+
+ ); +} diff --git a/web/src/components/app/emails/import/vendors/VendorImportWizard.tsx b/web/src/components/app/emails/import/vendors/VendorImportWizard.tsx index 243a051f7..cb0b0eb11 100644 --- a/web/src/components/app/emails/import/vendors/VendorImportWizard.tsx +++ b/web/src/components/app/emails/import/vendors/VendorImportWizard.tsx @@ -28,12 +28,13 @@ function statusPill(status: string): PickItem["status"] { return { label, cls: BAD_STATUS.test(s) ? "text-amber-700 bg-amber-50" : "text-slate-600 bg-slate-100" }; } -function toItem(m: VendorMailbox): PickItem { +function toItem(m: VendorMailbox, labelWorkspace: boolean): PickItem { return { id: m.id, email: m.email, name: m.name, domain: m.domain, + group: labelWorkspace ? m.workspace : undefined, provider: m.provider, status: statusPill(m.status), connected: m.connected, @@ -60,7 +61,11 @@ export default function VendorImportWizard({ const connections = React.useMemo(() => conns.data?.data ?? [], [conns.data]); const conn = connections.find((c) => c.id === connId) ?? null; const mailboxes = boxes.data?.data; - const items = React.useMemo(() => mailboxes?.map(toItem), [mailboxes]); + // The workspace is only worth a line when the key reaches more than one. + const items = React.useMemo(() => { + const several = new Set((mailboxes ?? []).map((m) => m.workspace ?? "")).size > 1; + return mailboxes?.map((m) => toItem(m, several)); + }, [mailboxes]); const sourceIssue = !conn ? "Pick a vendor account, or connect one." diff --git a/web/src/lib/api/models/app/emails/MailboxSources.ts b/web/src/lib/api/models/app/emails/MailboxSources.ts index fc00deea5..83a73ea2a 100644 --- a/web/src/lib/api/models/app/emails/MailboxSources.ts +++ b/web/src/lib/api/models/app/emails/MailboxSources.ts @@ -45,6 +45,8 @@ export interface VendorMailbox { provider: VendorMailboxProvider; /** The vendor's own status for the mailbox, as it reports it. */ status: string; + /** The vendor workspace or organization holding it, where the vendor has them. */ + workspace?: string; connected: boolean; email_account_id?: string; } From ce5eb4fd25d6fe656760a5d2c3636857d7e9a4d2 Mon Sep 17 00:00:00 2001 From: Matthew Meszaros Date: Wed, 23 Sep 2026 21:18:20 -0700 Subject: [PATCH 2/4] feat: let a Unibox reply or forward choose its sending mailbox in From (the shared MailboxPicker without Auto, kept in the draft and the undo-send), send the provider thread handle only from a mailbox that holds the thread so a switched reply threads on In-Reply-To alone, and scope scheduled sends, their cancel, count and cap to the organization whose mailboxes send them (#670) --- docs/content/docs/api/reference/unibox.mdx | 18 ++- docs/content/docs/guides/unibox.mdx | 16 +- docs/public/openapi.json | 11 +- internal/api/handler/unibox.go | 28 ++-- internal/app/aitools/tools_inbox.go | 4 +- internal/app/emailsend/service.go | 10 +- internal/app/instanceconfig/limits.go | 2 +- internal/app/unibox/overview.go | 4 +- internal/app/unibox/scheduled.go | 18 +-- internal/app/unibox/service.go | 13 +- internal/config/constants.go | 20 +-- internal/models/unibox.go | 2 +- internal/repository/pg_task.go | 89 ++++++----- .../scheduled_org_scope_live_test.go | 144 ++++++++++++++++++ .../tasks/unibox_reply_thread_live_test.go | 88 +++++++++++ internal/tasks/user_email_task.go | 11 ++ .../components/app/unibox/ReplyComposer.tsx | 108 ++++++++++--- web/src/components/app/unibox/ThreadView.tsx | 1 + .../app/unibox/compose/MailboxPicker.tsx | 45 +++++- .../app/unibox/replyComposerDraft.test.tsx | 95 +++++++++++- web/src/hooks/useOutboxStore.ts | 2 + .../api/models/app/unibox/UniboxOverview.ts | 4 +- web/src/lib/unibox/replyDraft.ts | 5 + 23 files changed, 608 insertions(+), 130 deletions(-) create mode 100644 internal/repository/scheduled_org_scope_live_test.go create mode 100644 internal/tasks/unibox_reply_thread_live_test.go diff --git a/docs/content/docs/api/reference/unibox.mdx b/docs/content/docs/api/reference/unibox.mdx index d4684664a..580b1c9bc 100644 --- a/docs/content/docs/api/reference/unibox.mdx +++ b/docs/content/docs/api/reference/unibox.mdx @@ -83,7 +83,7 @@ Returns the number of unread conversations in the Inbox folder, optionally scope `GET /unibox/overview` -Rolls up the scope rail and top metric strip in one call: unread, today, week, snoozed, awaiting-reply, and pending-scheduled counts, plus per-folder, per-mailbox, per-tag, and per-conversation-label breakdowns. The `folders` array always lists all six canonical folders, zero-filled, in sidebar order; the headline counts exclude `spam` and `trash`. All counts are threads, not messages. Auth: **Scope** `READ_UNIBOX` · **Org permission** `access_unibox`. +Rolls up the scope rail and top metric strip in one call: unread, today, week, snoozed, awaiting-reply, and pending-scheduled counts, plus per-folder, per-mailbox, per-tag, and per-conversation-label breakdowns. The `folders` array always lists all six canonical folders, zero-filled, in sidebar order; the headline counts exclude `spam` and `trash`. All counts are threads, not messages, except `scheduled_pending`, which counts the sends queued from the organization's mailboxes against the workspace cap in `scheduled_pending_max`. Auth: **Scope** `READ_UNIBOX` · **Org permission** `access_unibox`. ### Response @@ -96,7 +96,7 @@ Rolls up the scope rail and top metric strip in one call: unread, today, week, s "snoozed": 3, "awaiting_reply": 9, "scheduled_pending": 2, - "scheduled_pending_max": 50, + "scheduled_pending_max": 10000, "folders": [ { "folder": "inbox", "unread": 31, "total": 812 }, { "folder": "sent", "unread": 0, "total": 402 }, @@ -296,21 +296,21 @@ Echoes the request back. `POST /unibox/reply` -Sends or schedules a reply from one of your mailboxes. The send is routed through the per-mailbox scheduler according to `send_mode`. Requires an active organization. Auth: **Scope** `WRITE_UNIBOX` · **Org permission** `access_unibox`. +Sends or schedules a reply from any mailbox in the organization. The send is routed through the per-mailbox scheduler according to `send_mode`. Requires an active organization. Auth: **Scope** `WRITE_UNIBOX` · **Org permission** `access_unibox`. ### Request body | Field | Type | Required | Description | | --- | --- | --- | --- | -| `email_account_id` | string | Yes | UUID of the sending mailbox. | +| `email_account_id` | string | Yes | UUID of the sending mailbox. Any active mailbox in the organization, not only the one holding the thread. | | `to` | string[] | Yes | Recipient addresses (at least one). | | `cc` | string[] | No | CC addresses. | | `bcc` | string[] | No | BCC addresses. | | `subject` | string | Yes | Subject line. | | `body_html` | string | No | HTML body. | | `body_plain` | string | No | Plain-text body. | -| `in_reply_to` | string[] | No | Message-ID(s) this reply threads under. | -| `thread_id` | string | No | Thread to thread the reply into. | +| `in_reply_to` | string[] | No | Message-ID(s) this reply threads under. When omitted, the newest Message-ID in `thread_id` is used. | +| `thread_id` | string | No | The conversation being answered. | | `send_mode` | string | No | `instant` (default), `smart` (next mailbox gap), or `scheduled` (use `scheduled_at`). | | `scheduled_at` | string | No | RFC 3339 timestamp. Required when `send_mode` is `scheduled`; must be in the future. | @@ -338,6 +338,8 @@ Sends or schedules a reply from one of your mailboxes. The send is routed throug Instant sends are held for the sender's undo window (5 to 120 seconds, default 30) before they actually leave, so `scheduled_at` is that far in the future. Within the window the send can still be cancelled with `DELETE /unibox/scheduled/:task_id`. +A provider's thread handle belongs to the mailbox that holds the thread, so the reply only files into that provider thread when `email_account_id` holds a message in `thread_id`. From any other mailbox it goes out with the same In-Reply-To and References headers and the conversation's subject, which keeps it in the same conversation for the recipient; in the sending mailbox it starts a conversation of its own. The queued send still belongs to `thread_id`, so it is listed with that conversation's scheduled sends. + ## List active snoozes `GET /unibox/snoozes` @@ -414,7 +416,7 @@ Un-snoozes a thread immediately. Idempotent: deleting a snooze that does not exi `GET /unibox/scheduled` -Returns the outbound emails you have queued but not yet sent. Pass `thread_id` to scope to a single conversation (used to render queued replies inline); the response shape is identical either way. Auth: **Scope** `READ_UNIBOX` · **Org permission** `access_unibox`. +Returns the outbound emails queued from the organization's mailboxes but not yet sent, whichever member queued them. Pass `thread_id` to scope to a single conversation (used to render queued replies inline); the response shape is identical either way. Auth: **Scope** `READ_UNIBOX` · **Org permission** `access_unibox`. | Parameter | In | Type | Description | | --- | --- | --- | --- | @@ -447,7 +449,7 @@ The response wraps the items in a `data` array. Each item is a preview of the qu `DELETE /unibox/scheduled/:task_id` -Cancels a pending scheduled send before it fires. The queued task is marked cancelled and short-circuits to a no-op when its run time arrives. Auth: **Scope** `WRITE_UNIBOX` · **Org permission** `access_unibox`. +Cancels a pending scheduled send from any of the organization's mailboxes before it fires. The queued task is marked cancelled and short-circuits to a no-op when its run time arrives. Auth: **Scope** `WRITE_UNIBOX` · **Org permission** `access_unibox`. | Parameter | In | Type | Description | | --- | --- | --- | --- | diff --git a/docs/content/docs/guides/unibox.mdx b/docs/content/docs/guides/unibox.mdx index a753602ac..f062d7a72 100644 --- a/docs/content/docs/guides/unibox.mdx +++ b/docs/content/docs/guides/unibox.mdx @@ -51,7 +51,7 @@ The rail is two short groups and then your mailboxes. The first group is where y | Spam | Second | Junked at the provider | | Trash | Second | Deleted | -**Scheduled** sits between Sent and Archive. It is a view rather than a folder, listing replies queued to send later, and it reads naturally next to Sent. +**Scheduled** sits between Sent and Archive. It is a view rather than a folder, listing every send queued from the workspace's mailboxes, whichever teammate queued it, and it reads naturally next to Sent. Opening **Inbox** from the main navigation starts in the Inbox folder. Sent messages live in **Sent**; choose **All mail** when you want inbound and outbound messages together. An Inbox conversation still shows its complete history, including your replies, when you open it. @@ -72,7 +72,7 @@ The initial import that runs when a mailbox is first connected covers Inbox, Sen | Awaiting reply | You wrote last and they have not answered | | Agent drafts | Conversations where the [inbox agent](/guides/inbox-agent/) has a reply waiting for review | | Snoozed | Snoozed for later | -| Scheduled | Replies queued to send later | +| Scheduled | Sends queued from the workspace's mailboxes | Below those sit the premade **Views**, which answer the questions a pipeline turns on. They are built on the labels [automatic inbox tagging](/guides/inbox-tagging/) writes, exist from the moment the workspace is created, and need nothing set up: @@ -172,9 +172,15 @@ Unlike read state, Archive and Delete are Warmbly's own filing. The message keep ## Replying -Reply, forward, or hover any single message to reply to it specifically. The composer appears when you ask for it or return to a conversation with a saved reply or forward. Replies go from the mailbox that owns the thread, so conversations stay on one account. You can apply a saved **template** or **Insert booking link**. +Reply, forward, or hover any single message to reply to it specifically. The composer appears when you ask for it or return to a conversation with a saved reply or forward. You can apply a saved **template** or **Insert booking link**. -Replies and forwards save after a short typing pause, and save the latest edits when you close the composer, leave the conversation, or reload. Returning to the thread reopens a saved draft, including one addressed to an older message. **Discard** removes it; successfully queuing a send removes it too. Undoing a send restores the message for editing. +### Choosing the sending mailbox + +**From** starts on the mailbox that holds the message, so a reply you do not touch goes out exactly as before. Click it to send from any other active mailbox in the workspace: the same searchable picker as [Compose](#composing), with each mailbox's budget for today, auth health and history with the recipient. There is no Auto here; a reply always names its mailbox. The signature preview follows the mailbox you pick. When the mailbox holding the message is no longer active, the composer says so and **Send** stays off until you pick one that is. + +A reply from a different mailbox keeps the conversation's subject and its In-Reply-To and References headers, so the recipient sees it in the same conversation. A provider thread belongs to the mailbox that holds it, so unless the mailbox you pick already has a message in this conversation, the reply starts a conversation of its own there, and the recipient's answer arrives in that mailbox. A note under **From** says so, with **Switch back** to return to the original mailbox. A forward has no thread to keep, so it simply leaves from the mailbox you pick. + +Replies and forwards save after a short typing pause, and save the latest edits when you close the composer, leave the conversation, or reload. Returning to the thread reopens a saved draft, including one addressed to an older message, with the mailbox you picked. **Discard** removes it; successfully queuing a send removes it too. Undoing a send restores the message for editing, from the same mailbox. These reply drafts are personal to your signed-in user and workspace in this browser. They do not sync to other devices, appear in the mailbox's **Drafts** folder, or travel in a [workspace export](/guides/workspace-export-import/). Clearing browser data removes them. **Draft saved** confirms browser storage succeeded; **Draft not saved** means you should copy your text before leaving. @@ -219,7 +225,7 @@ With a recipient set, **History** slides out every conversation you have had wit | Control | Behavior | |---------|----------| | **Undo send** | Instant sends wait 30s by default (adjustable 5 to 120s under Settings, Profile). A header countdown offers **Cancel**, which returns the email to the composer intact. Applies to API sends with `send_mode: instant` too. | -| **Schedule** | Queue a send for later. It appears inline in the thread as a dashed card and in the **Scheduled** view, cancellable any time. A usage meter warns as you approach the queued-send cap. | +| **Schedule** | Queue a send for later. It appears inline in the thread as a dashed card and in the **Scheduled** view, where anyone in the workspace with Unibox access can cancel it. A usage meter warns as the workspace approaches its queued-send cap. | | **Snooze** | Clears a conversation until a time you pick, up to 90 days out, with presets or a custom time. **Un-snooze now** brings it back immediately. | Scheduled and smart-queued sends skip the undo window, since they already wait. Cancel them from the **Scheduled** view, or via `DELETE /unibox/scheduled/:task_id`. diff --git a/docs/public/openapi.json b/docs/public/openapi.json index 4872aac5a..40d52b93b 100644 --- a/docs/public/openapi.json +++ b/docs/public/openapi.json @@ -9118,7 +9118,7 @@ "post": { "operationId": "unibox_reply", "summary": "Reply from the inbox", - "description": "Sends or schedules a reply from one of your mailboxes, routed through the per-mailbox scheduler according to `send_mode`. Requires an active organization.", + "description": "Sends or schedules a reply from any mailbox in the organization, routed through the per-mailbox scheduler according to `send_mode`. Requires an active organization. `thread_id` names the conversation; the provider's thread handle is only used when the sending mailbox holds a message in that thread, and a reply from any other mailbox threads for the recipient on In-Reply-To and References alone.", "tags": [ "unibox" ], @@ -9421,7 +9421,7 @@ "get": { "operationId": "unibox_list_scheduled", "summary": "List scheduled sends", - "description": "Outbound emails you have queued but not yet sent. Pass `thread_id` to scope to a single conversation; the response shape is identical either way.", + "description": "Outbound emails queued from the organization's mailboxes but not yet sent, whichever member queued them. Pass `thread_id` to scope to a single conversation; the response shape is identical either way.", "tags": [ "unibox" ], @@ -9499,7 +9499,7 @@ "delete": { "operationId": "unibox_cancel_scheduled", "summary": "Cancel a scheduled send", - "description": "Cancels a pending scheduled send before it fires. The queued task is marked cancelled and short-circuits to a no-op when its run time arrives.", + "description": "Cancels a pending scheduled send from any of the organization's mailboxes before it fires. The queued task is marked cancelled and short-circuits to a no-op when its run time arrives.", "tags": [ "unibox" ], @@ -30138,7 +30138,7 @@ "email_account_id": { "type": "string", "format": "uuid", - "description": "UUID of the sending mailbox." + "description": "UUID of the sending mailbox. Any active mailbox in the organization; it does not have to be the one holding the thread." }, "to": { "type": "array", @@ -30177,7 +30177,8 @@ "description": "Message-ID(s) this reply threads under." }, "thread_id": { - "type": "string" + "type": "string", + "description": "The conversation being answered. Resolves In-Reply-To when that is omitted, and is used as the provider thread only by a mailbox holding a message in it." }, "send_mode": { "type": "string", diff --git a/internal/api/handler/unibox.go b/internal/api/handler/unibox.go index e720b73b9..0f0006fb4 100644 --- a/internal/api/handler/unibox.go +++ b/internal/api/handler/unibox.go @@ -664,10 +664,10 @@ func (h *Handler) DeleteUniboxSnooze(c *gin.Context) { c.Status(http.StatusNoContent) } -// ListUniboxScheduled returns every pending email task the user has -// queued: what the "Scheduled" scope in the dashboard reads from. -// When `thread_id` is set we scope the response to a single thread, -// which the ThreadView uses to render queued replies inline. The +// ListUniboxScheduled returns every pending email task queued from the +// organization's mailboxes: what the "Scheduled" scope in the dashboard +// reads from. When `thread_id` is set we scope the response to a single +// thread, which the ThreadView uses to render queued replies inline. The // response shape is identical either way so the same client + hook // handle both forms. // GET /unibox/scheduled @@ -676,15 +676,14 @@ func (h *Handler) ListUniboxScheduled(c *gin.Context) { if !h.gateUnibox(c) { return } - userID := middleware.GetUserID(c) - uid, err := uuid.Parse(userID) - if err != nil { - errx.Handle(c, errx.ErrUser) + orgID := middleware.GetOrganizationID(c) + if orgID == nil { + errx.Handle(c, errx.New(errx.BadRequest, "no organization selected")) return } if threadID := c.Query("thread_id"); threadID != "" { - items, xerr := h.UniboxService.ListScheduledByThread(c.Request.Context(), uid, threadID) + items, xerr := h.UniboxService.ListScheduledByThread(c.Request.Context(), *orgID, threadID) if xerr != nil { errx.Handle(c, xerr) return @@ -693,7 +692,7 @@ func (h *Handler) ListUniboxScheduled(c *gin.Context) { return } - items, xerr := h.UniboxService.ListScheduled(c.Request.Context(), uid) + items, xerr := h.UniboxService.ListScheduled(c.Request.Context(), *orgID) if xerr != nil { errx.Handle(c, xerr) return @@ -709,10 +708,9 @@ func (h *Handler) CancelUniboxScheduled(c *gin.Context) { if !h.gateUnibox(c) { return } - userID := middleware.GetUserID(c) - uid, err := uuid.Parse(userID) - if err != nil { - errx.Handle(c, errx.ErrUser) + orgID := middleware.GetOrganizationID(c) + if orgID == nil { + errx.Handle(c, errx.New(errx.BadRequest, "no organization selected")) return } taskID, err := uuid.Parse(c.Param("task_id")) @@ -721,7 +719,7 @@ func (h *Handler) CancelUniboxScheduled(c *gin.Context) { return } - if xerr := h.UniboxService.CancelScheduled(c.Request.Context(), uid, taskID); xerr != nil { + if xerr := h.UniboxService.CancelScheduled(c.Request.Context(), *orgID, taskID); xerr != nil { errx.Handle(c, xerr) return } diff --git a/internal/app/aitools/tools_inbox.go b/internal/app/aitools/tools_inbox.go index 7f4c4ebb5..c736a1188 100644 --- a/internal/app/aitools/tools_inbox.go +++ b/internal/app/aitools/tools_inbox.go @@ -206,7 +206,7 @@ func (d Deps) listScheduledSends(ctx context.Context, inv Invocation, _ json.Raw if err := d.requireUnibox(ctx, inv); err != nil { return "", err } - items, xerr := d.Unibox.ListScheduled(ctx, inv.UserID) + items, xerr := d.Unibox.ListScheduled(ctx, inv.OrgID) if xerr != nil { return "", fromErrx(xerr) } @@ -227,7 +227,7 @@ func (d Deps) cancelScheduledSend(ctx context.Context, inv Invocation, args json if err != nil { return "", err } - if xerr := d.Unibox.CancelScheduled(ctx, inv.UserID, tid); xerr != nil { + if xerr := d.Unibox.CancelScheduled(ctx, inv.OrgID, tid); xerr != nil { return "", fromErrx(xerr) } d.logAudit(ctx, inv, models.AuditActionUpdate, models.AuditEntityUnibox, &tid, nil) diff --git a/internal/app/emailsend/service.go b/internal/app/emailsend/service.go index ce5974377..51abb23cf 100644 --- a/internal/app/emailsend/service.go +++ b/internal/app/emailsend/service.go @@ -189,7 +189,7 @@ func (s *emailSendService) SendEmail(ctx context.Context, userID, orgID, account // Redis INCR; checked first because it's faster than a SELECT // COUNT and rejects bursts before they touch the DB. // - // Layer 2 (pending-count) — MaxPendingScheduledSendsPerUser + // Layer 2 (pending-count) — MaxPendingScheduledSendsPerOrg // bounds total queued state, so the DB doesn't accumulate // terabytes of pending message bodies even from a user who // schedules slowly over months. @@ -209,11 +209,11 @@ func (s *emailSendService) SendEmail(ctx context.Context, userID, orgID, account } } if s.taskRepo != nil { - pending, perr := s.taskRepo.CountScheduledForUser(ctx, userID) - if perr == nil && pending >= int64(config.MaxPendingScheduledSendsPerUser) { + pending, perr := s.taskRepo.CountScheduledInOrg(ctx, orgID) + if perr == nil && pending >= int64(config.MaxPendingScheduledSendsPerOrg) { return nil, errx.New(errx.TooManyRequests, fmt.Sprintf( - "you have %d scheduled sends queued (max %d). Cancel some from the Scheduled view before adding more.", - pending, config.MaxPendingScheduledSendsPerUser, + "this workspace has %d scheduled sends queued (max %d). Cancel some from the Scheduled view before adding more.", + pending, config.MaxPendingScheduledSendsPerOrg, )) } } diff --git a/internal/app/instanceconfig/limits.go b/internal/app/instanceconfig/limits.go index ca5dfcd10..3f61e4386 100644 --- a/internal/app/instanceconfig/limits.go +++ b/internal/app/instanceconfig/limits.go @@ -78,7 +78,7 @@ func Limits() []LimitGroup { Title: "Scheduled and undo sends", Entries: []LimitEntry{ {"New scheduled sends", n(config.DailyThrottleNewScheduledSends), "per user/day", "Rolling 24 hour window."}, - {"Pending scheduled sends", n(config.MaxPendingScheduledSendsPerUser), "per user", "Queued at once. Bounds queue storage rather than cost."}, + {"Pending scheduled sends", n(config.MaxPendingScheduledSendsPerOrg), "per workspace", "Queued at once. Bounds queue storage rather than cost."}, {"Undo send window", n(config.UndoSendSecondsDefault), "seconds", "Default hold before an instant send leaves. Range " + n(config.UndoSendSecondsMin) + " to " + n(config.UndoSendSecondsMax) + "."}, }, }, diff --git a/internal/app/unibox/overview.go b/internal/app/unibox/overview.go index ecef9513a..8856df006 100644 --- a/internal/app/unibox/overview.go +++ b/internal/app/unibox/overview.go @@ -31,7 +31,7 @@ func (s *uniboxService) Overview(ctx context.Context, orgID, userID uuid.UUID) ( // overview is more important than the badge — so we log via // the error reporter and continue with a zero count. if s.taskRepo != nil { - if n, err := s.taskRepo.CountScheduledForUser(ctx, userID); err == nil { + if n, err := s.taskRepo.CountScheduledInOrg(ctx, orgID); err == nil { o.ScheduledPending = n } else { errs.CaptureException(err) @@ -41,6 +41,6 @@ func (s *uniboxService) Overview(ctx context.Context, orgID, userID uuid.UUID) ( // dashboard render "N / max" so the user sees where they are // before they hit the wall. When this moves to per-plan, this is // the only line that needs a feature-gate lookup. - o.ScheduledPendingMax = int64(config.MaxPendingScheduledSendsPerUser) + o.ScheduledPendingMax = int64(config.MaxPendingScheduledSendsPerOrg) return o, nil } diff --git a/internal/app/unibox/scheduled.go b/internal/app/unibox/scheduled.go index bac076cf8..612033a1c 100644 --- a/internal/app/unibox/scheduled.go +++ b/internal/app/unibox/scheduled.go @@ -29,11 +29,11 @@ const ScheduledThreadListMax = 50 // to ship across the wire and short enough to render as one line. const snippetMaxLen = 240 -// ListScheduled returns the user's pending email tasks: every queued +// ListScheduled returns the organization's pending email tasks: every queued // outbound message that hasn't fired yet, ordered by next-to-fire. // The view is read-only — cancel is a separate explicit action. -func (s *uniboxService) ListScheduled(ctx context.Context, userID uuid.UUID) ([]models.UniboxScheduledItem, *errx.Error) { - rows, err := s.taskRepo.ListScheduledForUser(ctx, userID, ScheduledListMax) +func (s *uniboxService) ListScheduled(ctx context.Context, orgID uuid.UUID) ([]models.UniboxScheduledItem, *errx.Error) { + rows, err := s.taskRepo.ListScheduledInOrg(ctx, orgID, ScheduledListMax) if err != nil { errs.CaptureException(err) return nil, errx.InternalError() @@ -61,12 +61,12 @@ func (s *uniboxService) ListScheduled(ctx context.Context, userID uuid.UUID) ([] // ListScheduledByThread returns pending queued sends for the given // thread. Empty threadID is rejected up front so a malformed query -// can't silently fall back to the full per-user list. -func (s *uniboxService) ListScheduledByThread(ctx context.Context, userID uuid.UUID, threadID string) ([]models.UniboxScheduledItem, *errx.Error) { +// can't silently fall back to the full list. +func (s *uniboxService) ListScheduledByThread(ctx context.Context, orgID uuid.UUID, threadID string) ([]models.UniboxScheduledItem, *errx.Error) { if threadID == "" { return nil, errx.New(errx.BadRequest, "thread_id is required") } - rows, err := s.taskRepo.ListScheduledForUserByThread(ctx, userID, threadID, ScheduledThreadListMax) + rows, err := s.taskRepo.ListScheduledInOrgByThread(ctx, orgID, threadID, ScheduledThreadListMax) if err != nil { errs.CaptureException(err) return nil, errx.InternalError() @@ -110,14 +110,14 @@ func (s *uniboxService) ListScheduledByThread(ctx context.Context, userID uuid.U // DeleteTask fails for any reason — GCP outage, network blip, // already-fired — the handler's status check still short-circuits // the dispatch into a harmless no-op, so the user is always safe. -func (s *uniboxService) CancelScheduled(ctx context.Context, userID, taskID uuid.UUID) *errx.Error { - cloudTaskName, cancelled, err := s.taskRepo.CancelScheduledByUser(ctx, taskID, userID) +func (s *uniboxService) CancelScheduled(ctx context.Context, orgID, taskID uuid.UUID) *errx.Error { + cloudTaskName, cancelled, err := s.taskRepo.CancelScheduledInOrg(ctx, taskID, orgID) if err != nil { errs.CaptureException(err) return errx.InternalError() } if !cancelled { - // Either: task doesn't exist, isn't this user's, or already + // Either: task doesn't exist, isn't this workspace's, or already // left the pending state (fired / failed / cancelled). All // three look the same from the caller's perspective. return errx.New(errx.NotFound, "no pending scheduled send for this id") diff --git a/internal/app/unibox/service.go b/internal/app/unibox/service.go index 7f6f7ba39..e7cc046db 100644 --- a/internal/app/unibox/service.go +++ b/internal/app/unibox/service.go @@ -65,12 +65,13 @@ type UniboxService interface { // flip status to 'cancelled' and let the queued Cloud Task fire as // a no-op (handler short-circuits on non-pending status). Avoids // per-cancel API calls against Cloud Tasks. - ListScheduled(ctx context.Context, userID uuid.UUID) ([]models.UniboxScheduledItem, *errx.Error) - // ListScheduledByThread returns the user's pending queued sends - // for a single thread. ThreadView calls this so queued replies - // render inline alongside already-sent messages. - ListScheduledByThread(ctx context.Context, userID uuid.UUID, threadID string) ([]models.UniboxScheduledItem, *errx.Error) - CancelScheduled(ctx context.Context, userID, taskID uuid.UUID) *errx.Error + // All three are scoped to the organization, whose mailboxes send them. + ListScheduled(ctx context.Context, orgID uuid.UUID) ([]models.UniboxScheduledItem, *errx.Error) + // ListScheduledByThread returns the pending queued sends for a single + // thread. ThreadView calls this so queued replies render inline + // alongside already-sent messages. + ListScheduledByThread(ctx context.Context, orgID uuid.UUID, threadID string) ([]models.UniboxScheduledItem, *errx.Error) + CancelScheduled(ctx context.Context, orgID, taskID uuid.UUID) *errx.Error // ThreadGrounding and AddressGrounding return message text for AI prompts: // the stored body when it exists, the preview snippet as the fallback. diff --git a/internal/config/constants.go b/internal/config/constants.go index f3377a5cc..2d3d6219c 100644 --- a/internal/config/constants.go +++ b/internal/config/constants.go @@ -459,24 +459,24 @@ const ( // to 200 inbound messages a day couldn't hit it organically). DailyThrottleNewScheduledSends = 1000 - // MaxPendingScheduledSendsPerUser caps how many pending scheduled - // email sends one user can have queued at once. The DAILY rate - // (DailyThrottleNewScheduledSends) is the primary abuse defense; - // this is the DB-bloat defense — each pending row carries a body - // (~5KB), so capping pending count keeps total scheduled-queue - // storage bounded per user. + // MaxPendingScheduledSendsPerOrg caps how many pending scheduled + // email sends one workspace can have queued at once, across all of + // its mailboxes. The DAILY rate (DailyThrottleNewScheduledSends) is + // the primary abuse defense; this is the DB-bloat defense — each + // pending row carries a body (~5KB), so capping pending count keeps + // total scheduled-queue storage bounded per workspace. // - // 10,000 is generous: a user scheduling 100 sends/day for the next + // 10,000 is generous: a team scheduling 100 sends/day for the next // 100 days hits this exactly once. The combination of "1K new/day" - // + "10K total pending" means a legitimate user cannot organically + // + "10K total pending" means legitimate use cannot organically // hit either, while a scripted attacker is bounded on both axes. // // Cloud Tasks cost is negligible at this size — at $0.40/M - // operations, 10K pending = 20K ops = $0.008/user even at the + // operations, 10K pending = 20K ops = $0.008/workspace even at the // hardest abuse. The cap exists for DB sanity, not cost. // // Future: per-plan ceiling lookup. Today: single backstop. - MaxPendingScheduledSendsPerUser = 10000 + MaxPendingScheduledSendsPerOrg = 10000 // Undo send: instant sends are queued this many seconds in the // future so the sender can still cancel. Per-user setting stored in diff --git a/internal/models/unibox.go b/internal/models/unibox.go index 3b45461f3..6112a820f 100644 --- a/internal/models/unibox.go +++ b/internal/models/unibox.go @@ -437,7 +437,7 @@ type UniboxOverview struct { AwaitingAgentDraft int64 `json:"awaiting_agent_draft"` ScheduledPending int64 `json:"scheduled_pending"` // ScheduledPendingMax is the hard cap on pending scheduled email - // tasks per user. The dashboard shows current/max so the user + // tasks per workspace. The dashboard shows current/max so the user // sees how close they are to the limit before hitting it. ScheduledPendingMax int64 `json:"scheduled_pending_max"` Folders []UniboxFolderOverview `json:"folders"` diff --git a/internal/repository/pg_task.go b/internal/repository/pg_task.go index 45c610948..f2f2ca3f8 100644 --- a/internal/repository/pg_task.go +++ b/internal/repository/pg_task.go @@ -67,7 +67,7 @@ type TaskFailure struct { } // ScheduledEmailItem is the join shape returned by -// ListScheduledForUser — task + email_task + sender mailbox columns, +// ListScheduledInOrg — task + email_task + sender mailbox columns, // shaped for the dashboard's "Scheduled" view. type ScheduledEmailItem struct { TaskID uuid.UUID @@ -172,29 +172,32 @@ type TaskRepository interface { // Update campaign task with contact/sequence IDs (for tracking) UpdateCampaignTaskTracking(ctx context.Context, taskID, contactID, sequenceID uuid.UUID) error - // ListScheduledForUser returns every pending email task scheduled - // for the user's mailboxes, ordered by next-to-fire. Used by the - // unibox "Scheduled" view. - ListScheduledForUser(ctx context.Context, userID uuid.UUID, limit int) ([]ScheduledEmailItem, error) - // ListScheduledForUserByThread is the same query scoped to a + // ListScheduledInOrg returns every pending email task scheduled + // from the organization's mailboxes, ordered by next-to-fire. Used + // by the unibox "Scheduled" view. + ListScheduledInOrg(ctx context.Context, orgID uuid.UUID, limit int) ([]ScheduledEmailItem, error) + // ListScheduledInOrgByThread is the same query scoped to a // single email thread. ThreadView uses it to render queued sends // inline alongside already-sent messages so the user can see (and // cancel) what's about to fire on the conversation they're // reading. - ListScheduledForUserByThread(ctx context.Context, userID uuid.UUID, threadID string, limit int) ([]ScheduledEmailItem, error) - // CountScheduledForUser returns the number of pending email tasks + ListScheduledInOrgByThread(ctx context.Context, orgID uuid.UUID, threadID string, limit int) ([]ScheduledEmailItem, error) + // CountScheduledInOrg returns the number of pending email tasks // currently scheduled (regardless of fire time). Used for the - // scope-rail counter. - CountScheduledForUser(ctx context.Context, userID uuid.UUID) (int64, error) - // CancelScheduledByUser flips a pending email task to status - // 'cancelled' only when (a) it belongs to a mailbox the user owns, - // (b) it's still pending. Returns (cloudTaskName, ok, err) — the + // scope-rail counter and the pending-send cap. + CountScheduledInOrg(ctx context.Context, orgID uuid.UUID) (int64, error) + // CancelScheduledInOrg flips a pending email task to status + // 'cancelled' only when (a) it sends from one of the organization's + // mailboxes, (b) it's still pending. Returns (cloudTaskName, ok, err) — the // Cloud Task resource name is included so the caller can issue a // best-effort DeleteTask to clean the queue. The handler still // short-circuits on a non-pending status, so a failed DeleteTask // just degrades to a harmless no-op dispatch — never a real send. // `ok` distinguishes 404 (no row updated) from 200. - CancelScheduledByUser(ctx context.Context, taskID, userID uuid.UUID) (cloudTaskName *string, ok bool, err error) + CancelScheduledInOrg(ctx context.Context, taskID, orgID uuid.UUID) (cloudTaskName *string, ok bool, err error) + // ThreadHeldByMailbox reports whether the mailbox holds a message in the + // thread, which is what makes the thread id a handle its provider knows. + ThreadHeldByMailbox(ctx context.Context, emailAccountID uuid.UUID, threadID string) (bool, error) } type taskRepository struct { @@ -1082,7 +1085,7 @@ func (r *taskRepository) UpdateCampaignTaskTracking(ctx context.Context, taskID, // ListScheduledForUser returns user-initiated email tasks still in // 'pending' state, ordered by scheduled_at. Joins tasks → email_tasks // → email_accounts so callers don't need three lookups per row. -func (r *taskRepository) ListScheduledForUser(ctx context.Context, userID uuid.UUID, limit int) ([]ScheduledEmailItem, error) { +func (r *taskRepository) ListScheduledInOrg(ctx context.Context, orgID uuid.UUID, limit int) ([]ScheduledEmailItem, error) { if limit <= 0 || limit > 500 { limit = 200 } @@ -1104,13 +1107,13 @@ func (r *taskRepository) ListScheduledForUser(ctx context.Context, userID uuid.U FROM tasks t INNER JOIN email_tasks et ON et.task_id = t.id INNER JOIN email_accounts ea ON ea.id = t.email_account_id - WHERE ea.user_id = $1 + WHERE ea.organization_id = $1 AND t.task_type = 'email' AND t.status = 'pending' ORDER BY t.scheduled_at ASC NULLS LAST, t.created_at ASC LIMIT $2 ` - rows, err := r.db.Query(ctx, query, userID, limit) + rows, err := r.db.Query(ctx, query, orgID, limit) if err != nil { return nil, err } @@ -1145,11 +1148,11 @@ func (r *taskRepository) ListScheduledForUser(ctx context.Context, userID uuid.U return items, rows.Err() } -// ListScheduledForUserByThread is ListScheduledForUser scoped to a -// single thread. Same join + ownership enforcement, plus an extra +// ListScheduledInOrgByThread is ListScheduledInOrg scoped to a +// single thread. Same join + tenant enforcement, plus an extra // thread_id filter. Empty threadID is treated as "no rows" so the // caller can't accidentally fall back to the full list. -func (r *taskRepository) ListScheduledForUserByThread(ctx context.Context, userID uuid.UUID, threadID string, limit int) ([]ScheduledEmailItem, error) { +func (r *taskRepository) ListScheduledInOrgByThread(ctx context.Context, orgID uuid.UUID, threadID string, limit int) ([]ScheduledEmailItem, error) { if threadID == "" { return []ScheduledEmailItem{}, nil } @@ -1174,14 +1177,14 @@ func (r *taskRepository) ListScheduledForUserByThread(ctx context.Context, userI FROM tasks t INNER JOIN email_tasks et ON et.task_id = t.id INNER JOIN email_accounts ea ON ea.id = t.email_account_id - WHERE ea.user_id = $1 + WHERE ea.organization_id = $1 AND t.task_type = 'email' AND t.status = 'pending' AND et.thread_id = $2 ORDER BY t.scheduled_at ASC NULLS LAST, t.created_at ASC LIMIT $3 ` - rows, err := r.db.Query(ctx, query, userID, threadID, limit) + rows, err := r.db.Query(ctx, query, orgID, threadID, limit) if err != nil { return nil, err } @@ -1216,30 +1219,30 @@ func (r *taskRepository) ListScheduledForUserByThread(ctx context.Context, userI return items, rows.Err() } -// CountScheduledForUser returns how many email tasks are pending across -// every mailbox the user owns. Cheap enough to fold into the overview +// CountScheduledInOrg returns how many email tasks are pending across +// every mailbox in the organization. Cheap enough to fold into the overview // payload. -func (r *taskRepository) CountScheduledForUser(ctx context.Context, userID uuid.UUID) (int64, error) { +func (r *taskRepository) CountScheduledInOrg(ctx context.Context, orgID uuid.UUID) (int64, error) { query := ` SELECT COUNT(*) FROM tasks t INNER JOIN email_accounts ea ON ea.id = t.email_account_id - WHERE ea.user_id = $1 + WHERE ea.organization_id = $1 AND t.task_type = 'email' AND t.status = 'pending' ` var n int64 - err := r.db.QueryRow(ctx, query, userID).Scan(&n) + err := r.db.QueryRow(ctx, query, orgID).Scan(&n) return n, err } -// CancelScheduledByUser flips a single pending email task to -// 'cancelled', enforcing ownership through the email_accounts join. +// CancelScheduledInOrg flips a single pending email task to +// 'cancelled', enforcing the tenant through the email_accounts join. // Returns the Cloud Task resource name (if the row had one) so the // caller can issue a best-effort DeleteTask to clean up the GCP -// queue. ok=false when the task either doesn't exist, isn't owned by -// this user, isn't an email task, or already left the pending state. -func (r *taskRepository) CancelScheduledByUser(ctx context.Context, taskID, userID uuid.UUID) (*string, bool, error) { +// queue. ok=false when the task either doesn't exist, sends from +// another organization's mailbox, isn't an email task, or already left the pending state. +func (r *taskRepository) CancelScheduledInOrg(ctx context.Context, taskID, orgID uuid.UUID) (*string, bool, error) { query := ` UPDATE tasks t SET status = 'cancelled', @@ -1247,13 +1250,13 @@ func (r *taskRepository) CancelScheduledByUser(ctx context.Context, taskID, user FROM email_accounts ea WHERE t.id = $1 AND t.email_account_id = ea.id - AND ea.user_id = $2 + AND ea.organization_id = $2 AND t.task_type = 'email' AND t.status = 'pending' RETURNING t.cloud_task_name ` var cloudTaskName *string - err := r.db.QueryRow(ctx, query, taskID, userID).Scan(&cloudTaskName) + err := r.db.QueryRow(ctx, query, taskID, orgID).Scan(&cloudTaskName) if err != nil { if errors.Is(err, pgx.ErrNoRows) { return nil, false, nil @@ -1263,6 +1266,24 @@ func (r *taskRepository) CancelScheduledByUser(ctx context.Context, taskID, user return cloudTaskName, true, nil } +// ThreadHeldByMailbox reports whether the mailbox has a message in the thread, +// synced or sent and not yet synced back. A thread id is the provider's handle +// only for a mailbox that holds it. +func (r *taskRepository) ThreadHeldByMailbox(ctx context.Context, emailAccountID uuid.UUID, threadID string) (bool, error) { + threadID = strings.TrimSpace(threadID) + if threadID == "" { + return false, nil + } + var held bool + err := r.db.QueryRow(ctx, ` + SELECT EXISTS ( + SELECT 1 FROM unibox_emails WHERE email_id = $1 AND thread_id = $2 + ) OR EXISTS ( + SELECT 1 FROM tasks WHERE email_account_id = $1 AND status = 'completed' AND thread_id = $2 + )`, emailAccountID, threadID).Scan(&held) + return held, err +} + // MarkDirectOpened records the first open and lets a later human open replace a machine open. func (r *taskRepository) MarkDirectOpened(ctx context.Context, taskID uuid.UUID, at time.Time, machine bool) (bool, error) { const query = ` diff --git a/internal/repository/scheduled_org_scope_live_test.go b/internal/repository/scheduled_org_scope_live_test.go new file mode 100644 index 000000000..ba8037f07 --- /dev/null +++ b/internal/repository/scheduled_org_scope_live_test.go @@ -0,0 +1,144 @@ +package repository + +import ( + "context" + "testing" + "time" + + "github.com/google/uuid" +) + +// Queued sends leave from organization mailboxes, so the workspace that owns +// the mailbox lists, counts and cancels them, whichever member connected it, +// and no other workspace can. +func TestLiveScheduledSendsAreScopedToTheMailboxOrganization(t *testing.T) { + _, pool := liveContactDB(t) + ctx := context.Background() + owner, org, foreignOrg, mailbox := uuid.New(), uuid.New(), uuid.New(), uuid.New() + const thread = "thread-scheduled-scope" + + exec := func(query string, args ...any) { + t.Helper() + if _, err := pool.Exec(ctx, query, args...); err != nil { + t.Fatalf("fixture: %v", err) + } + } + exec(`INSERT INTO users (id, first_name, last_name, email) VALUES ($1, 'Sched', 'Scope', $2)`, + owner, "sched-"+uuid.NewString()+"@example.test") + exec(`INSERT INTO organizations (id, name, owner_user_id) VALUES ($1, 'Sched', $2), ($3, 'Foreign', $2)`, + org, owner, foreignOrg) + exec(`INSERT INTO email_accounts (id, user_id, organization_id, email, name, signature_plain, signature_html, provider) + VALUES ($1, $2, $3, $4, 'Sched', '', '', 'smtp_imap')`, + mailbox, owner, org, "sched-"+uuid.NewString()+"@example.test") + t.Cleanup(func() { + for _, step := range []struct { + query string + arg uuid.UUID + }{ + {`DELETE FROM tasks WHERE email_account_id = $1`, mailbox}, + {`DELETE FROM email_accounts WHERE id = $1`, mailbox}, + {`DELETE FROM organizations WHERE owner_user_id = $1`, owner}, + {`DELETE FROM users WHERE id = $1`, owner}, + } { + if _, err := pool.Exec(context.Background(), step.query, step.arg); err != nil { + t.Errorf("cleanup: %v", err) + } + } + }) + + repo := NewTaskRepository(pool) + at := time.Now().Add(time.Hour) + taskID := uuid.New() + threadID := thread + if err := repo.CreateEmailTaskFull(ctx, + &Task{ID: taskID, TaskType: "email", EmailAccountID: mailbox, Status: "pending", ScheduledAt: &at}, + &EmailTask{TaskID: taskID, To: []string{"them@example.test"}, Subject: "Re: Hi", Body: "Later", BodyPlain: "Later", ThreadID: &threadID, SendMode: "scheduled"}, + ); err != nil { + t.Fatalf("queue send: %v", err) + } + + for _, tc := range []struct { + org uuid.UUID + want int + }{{org, 1}, {foreignOrg, 0}} { + all, err := repo.ListScheduledInOrg(ctx, tc.org, 50) + if err != nil || len(all) != tc.want { + t.Errorf("list for %s = %d rows (err %v), want %d", tc.org, len(all), err, tc.want) + } + inThread, err := repo.ListScheduledInOrgByThread(ctx, tc.org, thread, 50) + if err != nil || len(inThread) != tc.want { + t.Errorf("thread list for %s = %d rows (err %v), want %d", tc.org, len(inThread), err, tc.want) + } + n, err := repo.CountScheduledInOrg(ctx, tc.org) + if err != nil || n != int64(tc.want) { + t.Errorf("count for %s = %d (err %v), want %d", tc.org, n, err, tc.want) + } + } + + if _, ok, err := repo.CancelScheduledInOrg(ctx, taskID, foreignOrg); err != nil || ok { + t.Fatalf("foreign cancel ok=%v err=%v, want refused", ok, err) + } + if _, ok, err := repo.CancelScheduledInOrg(ctx, taskID, org); err != nil || !ok { + t.Fatalf("workspace cancel ok=%v err=%v, want cancelled", ok, err) + } +} + +// A thread id is a mailbox's own provider handle once the mailbox holds a +// message in it, whether synced back or only recorded from its own send. +func TestLiveThreadHeldByMailbox(t *testing.T) { + _, pool := liveContactDB(t) + ctx := context.Background() + owner, org, holder, stranger := uuid.New(), uuid.New(), uuid.New(), uuid.New() + + exec := func(query string, args ...any) { + t.Helper() + if _, err := pool.Exec(ctx, query, args...); err != nil { + t.Fatalf("fixture: %v", err) + } + } + exec(`INSERT INTO users (id, first_name, last_name, email) VALUES ($1, 'Held', 'Thread', $2)`, + owner, "held-"+uuid.NewString()+"@example.test") + exec(`INSERT INTO organizations (id, name, owner_user_id) VALUES ($1, 'Held', $2)`, org, owner) + for _, id := range []uuid.UUID{holder, stranger} { + exec(`INSERT INTO email_accounts (id, user_id, organization_id, email, name, signature_plain, signature_html, provider) + VALUES ($1, $2, $3, $4, 'Held', '', '', 'gmail')`, id, owner, org, "held-"+uuid.NewString()+"@example.test") + } + t.Cleanup(func() { + c := context.Background() + for _, q := range []string{ + `DELETE FROM unibox_emails WHERE email_id IN (SELECT id FROM email_accounts WHERE organization_id = $1)`, + `DELETE FROM tasks WHERE email_account_id IN (SELECT id FROM email_accounts WHERE organization_id = $1)`, + `DELETE FROM email_accounts WHERE organization_id = $1`, + `DELETE FROM organizations WHERE id = $1`, + } { + if _, err := pool.Exec(c, q, org); err != nil { + t.Errorf("cleanup: %v", err) + } + } + if _, err := pool.Exec(c, `DELETE FROM users WHERE id = $1`, owner); err != nil { + t.Errorf("cleanup: %v", err) + } + }) + exec(`INSERT INTO unibox_emails (id, user_id, email_id, thread_id, subject, folder, seen) + VALUES ($1, $2, $3, 'synced-thread', 'Hi', 'inbox', true)`, uuid.New(), owner, holder) + exec(`INSERT INTO tasks (id, task_type, email_account_id, status, message_id, thread_id, completed_at) + VALUES ($1, 'email', $2, 'completed', '', 'sent-thread', NOW())`, uuid.New(), holder) + + repo := NewTaskRepository(pool) + for _, tc := range []struct { + mailbox uuid.UUID + thread string + want bool + }{ + {holder, "synced-thread", true}, + {holder, "sent-thread", true}, + {stranger, "synced-thread", false}, + {stranger, "sent-thread", false}, + {holder, "", false}, + } { + held, err := repo.ThreadHeldByMailbox(ctx, tc.mailbox, tc.thread) + if err != nil || held != tc.want { + t.Errorf("held(%s, %q) = %v (err %v), want %v", tc.mailbox, tc.thread, held, err, tc.want) + } + } +} diff --git a/internal/tasks/unibox_reply_thread_live_test.go b/internal/tasks/unibox_reply_thread_live_test.go new file mode 100644 index 000000000..d646fee3a --- /dev/null +++ b/internal/tasks/unibox_reply_thread_live_test.go @@ -0,0 +1,88 @@ +package tasks + +import ( + "context" + "testing" + "time" + + "github.com/google/uuid" + "github.com/jackc/pgx/v5/pgxpool" + + "github.com/warmbly/warmbly/internal/repository" + "github.com/warmbly/warmbly/internal/tasks/proto" +) + +// A unibox reply can leave from a mailbox other than the one holding the +// conversation. The provider thread handle belongs to the mailbox that holds +// it, so only that mailbox may send with it; any other one still threads for +// the recipient through In-Reply-To. +func TestLiveUniboxReplyUsesTheThreadHandleOnlyFromTheMailboxHoldingIt(t *testing.T) { + handle := liveCampaignDB(t) + f := newCampaignSendFixture(t, handle.Pool) + other := f.otherMailbox(t, handle.Pool) + const thread, parent = "gmail-thread-670", "" + holdThread(t, handle.Pool, f.user, f.mailbox, thread, parent) + + for _, tc := range []struct { + name string + mailbox uuid.UUID + wantThread string + }{ + {"same mailbox keeps the handle", f.mailbox, thread}, + {"another mailbox drops it", other, ""}, + } { + t.Run(tc.name, func(t *testing.T) { + sender := &recordingSender{} + svc := liveCampaignService(t, handle, sender) + taskID := queueUniboxReply(t, handle.Pool, tc.mailbox, thread, parent) + + if xerr := svc.HandleUserEmailTask(&proto.ProcessTask{TaskId: taskID.String()}); xerr != nil { + t.Fatalf("handle reply: %v", xerr) + } + msg := sender.message(t, 0) + if msg.ThreadID != tc.wantThread { + t.Errorf("thread handle = %q, want %q", msg.ThreadID, tc.wantThread) + } + if msg.InReplyTo != parent { + t.Errorf("in_reply_to = %q, want %q so the recipient still threads it", msg.InReplyTo, parent) + } + }) + } +} + +// holdThread stores one synced message in the thread for the mailbox. +func holdThread(t *testing.T, pool *pgxpool.Pool, user, mailbox uuid.UUID, thread, messageID string) { + t.Helper() + id := uuid.New() + if _, err := pool.Exec(context.Background(), ` + INSERT INTO unibox_emails (id, user_id, email_id, thread_id, message_id, subject, folder, seen) + VALUES ($1, $2, $3, $4, $5, 'Pricing', 'inbox', true)`, id, user, mailbox, thread, messageID); err != nil { + t.Fatalf("hold thread: %v", err) + } + t.Cleanup(func() { + if _, err := pool.Exec(context.Background(), `DELETE FROM unibox_emails WHERE id = $1`, id); err != nil { + t.Errorf("cleanup unibox email: %v", err) + } + }) +} + +// queueUniboxReply writes the task POST /unibox/reply creates, due now. +func queueUniboxReply(t *testing.T, pool *pgxpool.Pool, mailbox uuid.UUID, thread, inReplyTo string) uuid.UUID { + t.Helper() + now := time.Now() + id := uuid.New() + if err := repository.NewTaskRepository(pool).CreateEmailTaskFull(context.Background(), + &repository.Task{ID: id, TaskType: "email", EmailAccountID: mailbox, Status: "pending", ScheduledAt: &now}, + &repository.EmailTask{ + TaskID: id, To: []string{"them@test.local"}, InReplyTo: []string{inReplyTo}, + Subject: "Re: Pricing", Body: "Sure", BodyPlain: "Sure", ThreadID: &thread, SendMode: "instant", + }); err != nil { + t.Fatalf("queue reply: %v", err) + } + t.Cleanup(func() { + if _, err := pool.Exec(context.Background(), `DELETE FROM tasks WHERE id = $1`, id); err != nil { + t.Errorf("cleanup reply task: %v", err) + } + }) + return id +} diff --git a/internal/tasks/user_email_task.go b/internal/tasks/user_email_task.go index c12e7a7df..8c052a5d3 100644 --- a/internal/tasks/user_email_task.go +++ b/internal/tasks/user_email_task.go @@ -173,6 +173,17 @@ func (s *tasksService) HandleUserEmailTask(task *proto.ProcessTask) *errx.Error if emailTask.ThreadID != nil { threadID = *emailTask.ThreadID } + // The row keeps the conversation either way; a reply leaving from another + // mailbox threads on In-Reply-To alone, never on a handle it does not hold. + if threadID != "" { + held, herr := s.taskRepo.ThreadHeldByMailbox(ctx, account.ID, threadID) + if herr != nil { + log.Warn().Err(herr).Str("task_id", taskID.String()).Msg("user_email: could not confirm the mailbox holds the thread; sending without its handle") + } + if !held { + threadID = "" + } + } // STEP 9: Build EmailMessage and send via worker emailMsg := EmailMessage{ diff --git a/web/src/components/app/unibox/ReplyComposer.tsx b/web/src/components/app/unibox/ReplyComposer.tsx index e4ac68568..4702bf52f 100644 --- a/web/src/components/app/unibox/ReplyComposer.tsx +++ b/web/src/components/app/unibox/ReplyComposer.tsx @@ -10,6 +10,10 @@ // From, unlabelled Subject), the body textarea, the optional signature // preview, and the action bar (Send / Schedule / Template / Discard). // +// From defaults to the mailbox holding the message and can be switched to any +// active mailbox; the backend keeps the provider thread only for a mailbox +// that holds it, so a switched reply threads on its headers alone. +// // ⌘+Enter sends instantly. Each schedule preset calls /unibox/reply // with send_mode="scheduled" plus the concrete scheduled_at. @@ -34,6 +38,8 @@ import useTemplates from "@/lib/api/hooks/app/templates/useTemplates"; import TemplatePickerContent from "./TemplatePicker"; import InsertBookingLink from "./InsertBookingLink"; import ContactRecipientField from "./compose/ContactRecipientField"; +import MailboxPicker from "./compose/MailboxPicker"; +import useComposeCandidates from "@/lib/api/hooks/app/unibox/useComposeCandidates"; import useUniboxOverview from "@/lib/api/hooks/app/unibox/useUniboxOverview"; import { resolveSendAt, useOutboxStore } from "@/hooks/useOutboxStore"; import { useUserProfile } from "@/hooks/context/user"; @@ -177,9 +183,13 @@ export function ReplyComposer({ threadId, replyTo, mode, seed, onClose }: ReplyC const [bcc, setBcc] = React.useState(restored?.bcc ?? []); const [showCc, setShowCc] = React.useState((restored?.cc.length ?? 0) > 0); const [showBcc, setShowBcc] = React.useState((restored?.bcc.length ?? 0) > 0); + // The mailbox holding the message is the default sender; picking another + // one is a per-draft override. + const threadAccountId = replyTo.account_id ?? ""; + const [accountId, setAccountId] = React.useState(restored?.email_account_id || threadAccountId); const [isSending, setIsSending] = React.useState(false); - const draft = useReplyDraft(draftKey, { to, cc, bcc, subject, body }, { - to: initial.to, cc: [], bcc: [], subject: initial.subject, body: "", + const draft = useReplyDraft(draftKey, { to, cc, bcc, subject, body, email_account_id: accountId }, { + to: initial.to, cc: [], bcc: [], subject: initial.subject, body: "", email_account_id: threadAccountId, }); const closeKeepingDraft = () => { if (!draft.flush()) { @@ -236,13 +246,20 @@ export function ReplyComposer({ threadId, replyTo, mode, seed, onClose }: ReplyC setShowCc(seed.cc.length > 0); setShowBcc(seed.bcc.length > 0); setBody(seed.body); - }, [seed, resumeDraft]); + setAccountId(seed.email_account_id || threadAccountId); + }, [seed, resumeDraft, threadAccountId]); - // Resolve the sending mailbox from the target message's - // account_id. We look it up in the global emails store so we have - // the full Inbox record (signature_html, signature_plain, etc). - const accountId = replyTo.account_id ?? ""; + // The full Inbox record (signature_html, signature_plain, etc) of the + // chosen sender, from the global emails store. const mailbox = accounts.find((a) => a.id === accountId); + const switchedMailbox = !!threadAccountId && accountId !== threadAccountId; + // A queued send from an inactive mailbox is cancelled when it fires. + const mailboxInactive = !!mailbox && mailbox.status !== "active"; + const threadMailbox = accounts.find((a) => a.id === threadAccountId); + + // Scored like compose: history with the recipient, today's budget, auth. + const primary = to.length > 0 ? bareEmail(to[0]) : ""; + const candidatesQ = useComposeCandidates(primary); const templatesQuery = useTemplates(); @@ -255,7 +272,8 @@ export function ReplyComposer({ threadId, replyTo, mode, seed, onClose }: ReplyC const scheduleAtCap = scheduledCap > 0 && scheduledUsed >= scheduledCap; const trimmedBody = body.trim(); - const canSend = !!trimmedBody && to.length > 0 && to.every(looksLikeEmail) && !!accountId && !isSending; + const canSend = + !!trimmedBody && to.length > 0 && to.every(looksLikeEmail) && !!accountId && !mailboxInactive && !isSending; const send = async (scheduledAt?: Date) => { if (!canSend && !isSending) { @@ -275,9 +293,13 @@ export function ReplyComposer({ threadId, replyTo, mode, seed, onClose }: ReplyC toast.error("Couldn't resolve the sending mailbox"); return; } + if (mailboxInactive) { + toast.error(`${mailbox?.email} is not active. Pick another mailbox in From.`); + return; + } } - const submittedDraft = { to, cc, bcc, subject, body }; + const submittedDraft = { to, cc, bcc, subject, body, email_account_id: accountId }; draft.flush(); setIsSending(true); const sentSubject = subject.trim() || (mode === "forward" ? "Fwd:" : "Re:"); @@ -318,6 +340,7 @@ export function ReplyComposer({ threadId, replyTo, mode, seed, onClose }: ReplyC bcc, subject: sentSubject, body: trimmedBody, + emailAccountId: accountId, }, }); } else { @@ -526,21 +549,70 @@ export function ReplyComposer({ threadId, replyTo, mode, seed, onClose }: ReplyC )} - {mailbox ? ( -
- - {mailbox.name || mailbox.email} - - - {mailbox.email} - -
+ {accountId ? ( + setAccountId(next)} + candidates={candidatesQ.data} + loading={candidatesQ.isPending} + /> ) : ( No sending mailbox resolved )} + + {mailboxInactive && ( + +
+ + + {mailbox?.email} is not active, so it cannot send. Pick another mailbox in From, + or reconnect it under Emails. + +
+ + )} + {!mailboxInactive && switchedMailbox && mode === "reply" && ( + +
+ + + Replying from another mailbox. It stays in the same conversation for the + recipient, and their answer comes back to {mailbox?.email ?? "this mailbox"}. + + +
+ + )} +
(null); @@ -45,6 +55,7 @@ export default function MailboxPicker({ value, autoTag, onChange, candidates, lo // overflow, so the menu can't render inside it). const [anchor, setAnchor] = React.useState<{ top: number; left: number; up: boolean } | null>(null); const boxRef = React.useRef(null); + const triggerRef = React.useRef(null); useClickOutside(boxRef, () => setOpen(false)); const storeEmails = useAppStore((s) => s.emails); @@ -102,6 +113,9 @@ export default function MailboxPicker({ value, autoTag, onChange, candidates, lo ? (scopedBest(autoTag) ?? recommended) : recommended : accounts.find((a) => a.id === value); + // An explicit pick outside the candidates (still loading, or no longer + // active) is named from the store rather than shown as missing. + const stored = !selected && value !== "auto" ? storeEmails.find((e) => e.id === value) : undefined; const autoTagTitle = autoTag ? storeTags.find((t) => t.id === autoTag)?.title : undefined; // Every defined tag is offered (not just ones already in use) so a tag @@ -129,6 +143,7 @@ export default function MailboxPicker({ value, autoTag, onChange, candidates, lo return (
)} - {!mailboxInactive && switchedMailbox && mode === "reply" && ( + {!senderProblem && switchedMailbox && mode === "reply" && ( void; } const PANEL_WIDTH = 300; @@ -47,6 +49,7 @@ export default function MailboxPicker({ candidates, loading, allowAuto = true, + onOpen, }: MailboxPickerProps) { const [open, setOpen] = React.useState(false); const [search, setSearch] = React.useState(""); @@ -145,7 +148,10 @@ export default function MailboxPicker({