Merge pull request #177 from warmbly/fix/167-sender-org-scoping

fix: resolve campaign senders by organization, not by the campaign owner (#167)
This commit is contained in:
Matthew Meszaros
2026-08-24 09:24:54 -07:00
committed by GitHub
9 changed files with 423 additions and 51 deletions
+2
View File
@@ -23,6 +23,8 @@ Pick mailboxes in **Sending accounts** three ways, and the first two combine:
- **Individually**: specific mailboxes by hand.
- **All active mailboxes**: the default when you pick neither.
Sending accounts always resolve inside the campaign's own workspace. If you belong to more than one workspace, a tag you reuse across them still only picks up mailboxes belonging to the workspace that owns the campaign, so one workspace's sending reputation, daily caps, and warmup are never spent on another's traffic.
<Callout type="info" title="Follow-ups stay on the same mailbox">
Each mailbox stays within its own daily limit, and follow-ups always send from the mailbox that sent the first email to that contact, so threads stay consistent.
</Callout>
+1 -1
View File
@@ -78,7 +78,7 @@ For drafts written automatically on every inbound reply and held for approval, t
### Auto mailbox selection
**From** defaults to **Auto**, scoring every active mailbox for the recipient: an existing conversation with them wins first (contacts should keep hearing from one address), then the most remaining daily budget, with domain auth health as the tiebreaker.
**From** defaults to **Auto**, scoring every active mailbox in the current workspace for the recipient: an existing conversation with them wins first (contacts should keep hearing from one address), then the most remaining daily budget, with domain auth health as the tiebreaker.
The picker shows the pick and its reason before sending. Opening it reveals every mailbox with a budget bar (sent today against its limit), a history badge, and an auth-health dot, so overriding is informed. The menu filters by mailbox tag; with a tag selected, Auto picks within that group only, which is the `from_tag_id` field on `POST /unibox/compose`.
+2 -1
View File
@@ -1509,7 +1509,8 @@ func (s *service) RunPreflight(ctx context.Context, organizationID, campaignID u
}
if settings.Preflight.CheckTrackingDomain && (campaign.OpenTracking || campaign.LinkTracking) {
accounts, err := s.emailRepo.GetByTags(ctx, campaign.UserID, campaign.EmailTags)
scope := repository.NewAccountScope(campaign.OrganizationID)
accounts, err := s.emailRepo.GetByTags(ctx, scope, campaign.EmailTags)
if err != nil || len(accounts) == 0 {
checks = append(checks, models.PreflightCheckResult{
Key: "tracking_domain",
+6 -3
View File
@@ -66,7 +66,9 @@ func NewService(emailRepo repository.EmailRepository, composeRepo repository.Com
}
func (s *service) Candidates(ctx context.Context, userID, orgID uuid.UUID, address string) ([]Candidate, *errx.Error) {
accounts, xerr := s.emailRepo.GetAllActiveByUser(ctx, userID.String())
// Scoped to the caller's current organization, not to the caller: a member of
// two workspaces must not compose from the other workspace's mailboxes.
accounts, xerr := s.emailRepo.GetAllActiveInScope(ctx, repository.NewAccountScope(&orgID))
if xerr != nil {
return nil, xerr
}
@@ -148,9 +150,10 @@ func (s *service) Resolve(ctx context.Context, userID, orgID uuid.UUID, accountI
// Tag-scoped auto: restrict the pool to mailboxes carrying the tag,
// then apply the same best-with-budget-first rule within it. GetByTags
// is already user-scoped, so a foreign tag id just yields no members.
// is scoped to the same organization, so a foreign tag id just yields no
// members.
if tagID != nil {
members, merr := s.emailRepo.GetByTags(ctx, userID.String(), []string{tagID.String()})
members, merr := s.emailRepo.GetByTags(ctx, repository.NewAccountScope(&orgID), []string{tagID.String()})
if merr != nil {
return nil, false, merr
}
+8 -12
View File
@@ -43,7 +43,6 @@ type CampaignRepository interface {
Update(ctx context.Context, userID, query string, data *models.UpdateCampaign) (*models.Campaign, *errx.Error)
UpdateStatus(ctx context.Context, campaignID uuid.UUID, status string) error
UpdateStatusWithLock(ctx context.Context, campaignID uuid.UUID, status string) error
PauseAllByUserID(ctx context.Context, userID uuid.UUID, reason string) error
Delete(ctx context.Context, userID, id string) error
// Campaign start/stop
@@ -1375,13 +1374,6 @@ func (r *campaignRepository) UpdateStatus(ctx context.Context, campaignID uuid.U
return err
}
// PauseAllByUserID pauses all active campaigns for a user
func (r *campaignRepository) PauseAllByUserID(ctx context.Context, userID uuid.UUID, reason string) error {
query := `UPDATE campaigns SET status = $1, updated_at = NOW() WHERE user_id = $2 AND status = 'active'`
_, err := r.DB.Exec(ctx, query, reason, userID)
return err
}
// StartCampaign sets campaign status to active and updates last_status_change_at.
// When ramp is enabled and not yet started (ramp_level = 0), it also seeds the
// ramp at ramp_start for today so the first day sends at the ramp floor rather
@@ -1562,10 +1554,13 @@ func (r *campaignRepository) AccountHasActiveCampaign(ctx context.Context, accou
AND ea.status = 'active'
AND c.status = 'active'
UNION ALL
-- "all" campaigns (no tags, no enabled senders) back every active mailbox of their owner
-- "all" campaigns (no tags, no enabled senders) back every active
-- mailbox in their tenant. Joined on the organization, like the
-- scheduler's AccountScope: on the owner it also counted a multi-org
-- user's OTHER workspace.
SELECT 1
FROM campaigns c
JOIN email_accounts ea ON ea.user_id = c.user_id
JOIN email_accounts ea ON ea.organization_id = c.organization_id
WHERE ea.id = $1
AND ea.status = 'active'
AND c.status = 'active'
@@ -1600,10 +1595,11 @@ func (r *campaignRepository) CountActiveCampaignsForAccount(ctx context.Context,
AND cs.enabled
AND c.status = 'active'
UNION
-- "all" campaigns (no tags, no enabled senders) count for every active mailbox of their owner
-- "all" campaigns (no tags, no enabled senders) count for every active
-- mailbox in their tenant — same scope rule as the scheduler.
SELECT c.id
FROM campaigns c
JOIN email_accounts ea ON ea.user_id = c.user_id
JOIN email_accounts ea ON ea.organization_id = c.organization_id
WHERE ea.id = $1
AND ea.status = 'active'
AND c.status = 'active'
+67 -22
View File
@@ -39,19 +39,50 @@ type OAuthCredentials struct {
ExpiresAt time.Time
}
// AccountScope confines a mailbox lookup to one tenant. The organization is the
// tenant boundary: a multi-org user must never send organization A's campaign
// from an organization B mailbox, so resolution keys on organization_id and
// never on the owner.
type AccountScope struct {
// OrgID is the tenant key. Nil is not a wildcard — it resolves to no
// mailboxes at all, matching how the campaign task halts an orgless
// campaign rather than sending it unchecked.
OrgID *uuid.UUID
}
// NewAccountScope builds the scope for an organization, treating the nil UUID
// the same as no organization.
func NewAccountScope(orgID *uuid.UUID) AccountScope {
if orgID != nil && *orgID == uuid.Nil {
orgID = nil
}
return AccountScope{OrgID: orgID}
}
// tenant returns the organization to query for, and false when the scope has
// none — the caller then answers "no mailboxes" instead of running a query with
// an unbound tenant.
func (s AccountScope) tenant() (uuid.UUID, bool) {
if s.OrgID == nil {
return uuid.Nil, false
}
return *s.OrgID, true
}
type EmailRepository interface {
Search(ctx context.Context, userID, search string, cursor, tag *string, limit int32, allowedAccountIDs []uuid.UUID) (*models.EmailsResult, *errx.Error)
Get(ctx context.Context, userID, emailAccountID string) (*models.Email, *errx.Error)
GetByID(ctx context.Context, emailAccountID uuid.UUID) (*models.Email, *errx.Error)
GetByTags(ctx context.Context, userID string, tags []string) ([]models.Email, *errx.Error)
// GetAllActiveByUser returns every active mailbox for a user (no tag/sender
// filter) — the "all" sender pool used when a campaign picks neither tags nor
// explicit accounts.
GetAllActiveByUser(ctx context.Context, userID string) ([]models.Email, *errx.Error)
// GetByTags returns the scope's active mailboxes carrying any of the tags.
GetByTags(ctx context.Context, scope AccountScope, tags []string) ([]models.Email, *errx.Error)
// GetAllActiveInScope returns every active mailbox in the scope (no
// tag/sender filter) — the "all" sender pool used when a campaign picks
// neither tags nor explicit accounts.
GetAllActiveInScope(ctx context.Context, scope AccountScope) ([]models.Email, *errx.Error)
// GetByCampaignSenders returns the active mailboxes in a campaign's explicit
// sender pool, carrying each sender's rotation metadata (weight,
// rotation_position, last_sent_at) for the scheduler's rotation modes.
GetByCampaignSenders(ctx context.Context, userID string, campaignID uuid.UUID) ([]CampaignSenderAccount, *errx.Error)
GetByCampaignSenders(ctx context.Context, scope AccountScope, campaignID uuid.UUID) ([]CampaignSenderAccount, *errx.Error)
GetSMTPCredentials(ctx context.Context, emailAccountID uuid.UUID) (*SMTPCredentials, *errx.Error)
GetOAuthCredentials(ctx context.Context, emailAccountID uuid.UUID) (*OAuthCredentials, *errx.Error)
GetWorkerID(ctx context.Context, emailAccountID uuid.UUID) (*uuid.UUID, *errx.Error)
@@ -1309,9 +1340,13 @@ func (r *emailRepository) GetByID(ctx context.Context, emailAccountID uuid.UUID)
return &i, nil
}
// GetByTags retrieves email accounts matching any of the specified tags
func (r *emailRepository) GetByTags(ctx context.Context, userID string, tags []string) ([]models.Email, *errx.Error) {
if len(tags) == 0 {
// GetByTags retrieves the scope's active mailboxes matching any of the tags.
// Tags themselves are owned by a user, not an organization, so a multi-org
// user's tag can span workspaces — the scope predicate is what keeps the
// resolved senders inside one tenant.
func (r *emailRepository) GetByTags(ctx context.Context, scope AccountScope, tags []string) ([]models.Email, *errx.Error) {
orgID, ok := scope.tenant()
if len(tags) == 0 || !ok {
return []models.Email{}, nil
}
@@ -1326,15 +1361,15 @@ func (r *emailRepository) GetByTags(ctx context.Context, userID string, tags []s
ea.created_at, ea.updated_at
FROM email_accounts ea
JOIN email_tags eat ON eat.email_id = ea.id
WHERE ea.user_id = $1
WHERE ea.organization_id = $1
AND eat.tag_id = ANY($2)
AND ea.status = 'active'
ORDER BY ea.id
`
rows, err := r.DB.Query(ctx, query, userID, tags)
rows, err := r.DB.Query(ctx, query, orgID, tags)
if err != nil {
db.CaptureError(err, query, []any{userID, tags}, "query")
db.CaptureError(err, query, []any{orgID, tags}, "query")
return nil, errx.InternalError()
}
defer rows.Close()
@@ -1362,9 +1397,14 @@ func (r *emailRepository) GetByTags(ctx context.Context, userID string, tags []s
return emails, nil
}
// GetAllActiveByUser returns every active mailbox for a user (the "all" sender
// pool). Same projection as GetByTags, without the tag join.
func (r *emailRepository) GetAllActiveByUser(ctx context.Context, userID string) ([]models.Email, *errx.Error) {
// GetAllActiveInScope returns every active mailbox in the scope (the "all"
// sender pool). Same projection as GetByTags, without the tag join.
func (r *emailRepository) GetAllActiveInScope(ctx context.Context, scope AccountScope) ([]models.Email, *errx.Error) {
orgID, ok := scope.tenant()
if !ok {
return []models.Email{}, nil
}
query := `
SELECT
ea.id, ea.user_id, ea.email, ea.name, ea.signature_plain, ea.signature_html, ea.signature_sync, ea.signature_code,
@@ -1375,14 +1415,14 @@ func (r *emailRepository) GetAllActiveByUser(ctx context.Context, userID string)
ea.auth_state, ea.auth_failing_since,
ea.created_at, ea.updated_at
FROM email_accounts ea
WHERE ea.user_id = $1
WHERE ea.organization_id = $1
AND ea.status = 'active'
ORDER BY ea.id
`
rows, err := r.DB.Query(ctx, query, userID)
rows, err := r.DB.Query(ctx, query, orgID)
if err != nil {
db.CaptureError(err, query, []any{userID}, "query")
db.CaptureError(err, query, []any{orgID}, "query")
return nil, errx.InternalError()
}
defer rows.Close()
@@ -1424,7 +1464,12 @@ type CampaignSenderAccount struct {
// explicit campaign_senders pool instead of email tags. Only enabled senders
// backing an active mailbox are returned; the per-sender weight/cursor/last-send
// ride along for rotation.
func (r *emailRepository) GetByCampaignSenders(ctx context.Context, userID string, campaignID uuid.UUID) ([]CampaignSenderAccount, *errx.Error) {
func (r *emailRepository) GetByCampaignSenders(ctx context.Context, scope AccountScope, campaignID uuid.UUID) ([]CampaignSenderAccount, *errx.Error) {
orgID, ok := scope.tenant()
if !ok {
return nil, nil
}
query := `
SELECT
ea.id, ea.user_id, ea.email, ea.name, ea.signature_plain, ea.signature_html, ea.signature_sync, ea.signature_code,
@@ -1439,14 +1484,14 @@ func (r *emailRepository) GetByCampaignSenders(ctx context.Context, userID strin
JOIN campaign_senders cs ON cs.email_account_id = ea.id
WHERE cs.campaign_id = $2
AND cs.enabled
AND ea.user_id = $1
AND ea.organization_id = $1
AND ea.status = 'active'
ORDER BY ea.id
`
rows, err := r.DB.Query(ctx, query, userID, campaignID)
rows, err := r.DB.Query(ctx, query, orgID, campaignID)
if err != nil {
db.CaptureError(err, query, []any{userID, campaignID}, "query")
db.CaptureError(err, query, []any{orgID, campaignID}, "query")
return nil, errx.InternalError()
}
defer rows.Close()
+11 -4
View File
@@ -52,8 +52,15 @@ func (s *schedulerService) CalculateNextCampaignTime(ctx context.Context, campai
// UNION of the explicit campaign_senders pool and the tag-resolved mailboxes
// (one dropdown picks both — they're no longer mutually exclusive). When the
// campaign selects NEITHER tags nor explicit accounts, it sends from ALL of
// the owner's active mailboxes ("all").
senders, serr := s.emailRepo.GetByCampaignSenders(ctx, campaign.UserID, campaignID)
// the active mailboxes in the campaign's tenant ("all").
//
// Tenancy is the campaign's organization, never its owner: a user who belongs
// to two organizations must not have organization A's campaign pick up an
// organization B mailbox and burn B's reputation, caps and warmup state. A
// campaign with no organization resolves to no mailboxes, the same way the
// campaign task halts it rather than sending unchecked.
scope := repository.NewAccountScope(campaign.OrganizationID)
senders, serr := s.emailRepo.GetByCampaignSenders(ctx, scope, campaignID)
if serr != nil {
return time.Time{}, nil, uuid.Nil, serr
}
@@ -68,7 +75,7 @@ func (s *schedulerService) CalculateNextCampaignTime(ctx context.Context, campai
}
}
if len(campaign.EmailTags) > 0 {
tagAccounts, terr := s.emailRepo.GetByTags(ctx, campaign.UserID, campaign.EmailTags)
tagAccounts, terr := s.emailRepo.GetByTags(ctx, scope, campaign.EmailTags)
if terr != nil {
return time.Time{}, nil, uuid.Nil, terr
}
@@ -80,7 +87,7 @@ func (s *schedulerService) CalculateNextCampaignTime(ctx context.Context, campai
}
}
if len(senders) == 0 && len(campaign.EmailTags) == 0 {
allAccts, aerr := s.emailRepo.GetAllActiveByUser(ctx, campaign.UserID)
allAccts, aerr := s.emailRepo.GetAllActiveInScope(ctx, scope)
if aerr != nil {
return time.Time{}, nil, uuid.Nil, aerr
}
+316
View File
@@ -0,0 +1,316 @@
package scheduler
import (
"context"
"errors"
"strings"
"testing"
"github.com/google/uuid"
"github.com/jackc/pgx/v5/pgxpool"
"github.com/warmbly/warmbly/internal/infrastructure/db"
"github.com/warmbly/warmbly/internal/models"
"github.com/warmbly/warmbly/internal/pkg/encrypt"
"github.com/warmbly/warmbly/internal/repository"
)
// Regression cover for issue #167: campaign sender resolution used to filter on
// the campaign OWNER's user_id, so a user who belongs to two organizations
// could have organization A's campaign pick up an organization B mailbox and
// burn B's reputation, daily caps and warmup state.
//
// Run against the dev stack:
//
// WARMBLY_TEST_DB=postgres://warmbly:warmbly@localhost:15432/warmbly_dev?sslmode=disable \
// go test ./internal/scheduler/ -run LiveSender -v
// foreignOrg adds a SECOND organization owned by the same user, with its own
// active mailbox — the neighbouring tenant that must never be reachable.
type foreignOrg struct {
org uuid.UUID
mailbox uuid.UUID
}
func newForeignOrg(t *testing.T, pool *pgxpool.Pool, f *liveFixture) *foreignOrg {
t.Helper()
ctx := context.Background()
o := &foreignOrg{org: uuid.New(), mailbox: uuid.New()}
exec := func(sql string, args ...any) {
t.Helper()
if _, err := pool.Exec(ctx, sql, args...); err != nil {
t.Fatalf("foreign org fixture %q: %v", sql[:min(60, len(sql))], err)
}
}
exec(`INSERT INTO organizations (id, name, slug, owner_user_id)
VALUES ($1, 'Foreign Tenant', $2, $3)`, o.org, "foreign-"+o.org.String()[:8], f.user)
// The condition the bug needs: one login, membership in both workspaces.
exec(`INSERT INTO organization_members (organization_id, user_id, role, accepted_at)
VALUES ($1, $2, 'owner', NOW()), ($3, $2, 'owner', NOW())`, f.org, f.user, o.org)
exec(`INSERT INTO email_accounts (id, user_id, organization_id, email, name,
signature_plain, signature_html, provider, status, campaign_limit, min_wait_time, timezone)
VALUES ($1, $2, $3, $4, 'Foreign', '', '', 'smtp_imap', 'active', 50, 600, 'UTC')`,
o.mailbox, f.user, o.org, "foreign-"+o.mailbox.String()[:8]+"@test.local")
t.Cleanup(func() {
c := context.Background()
steps := []struct {
sql string
arg any
}{
{`DELETE FROM campaign_senders WHERE email_account_id = $1`, o.mailbox},
{`DELETE FROM email_tags WHERE email_id = $1`, o.mailbox},
{`DELETE FROM email_accounts WHERE id = $1`, o.mailbox},
{`DELETE FROM organization_members WHERE user_id = $1`, f.user},
{`DELETE FROM organizations WHERE id = $1`, o.org},
}
for _, step := range steps {
if _, err := pool.Exec(c, step.sql, step.arg); err != nil {
t.Errorf("cleanup %q: %v", step.sql, err)
}
}
})
return o
}
// tagMailboxes creates ONE user-owned tag, attaches it to the given mailboxes
// and puts it on the campaign. Tags carry no organization of their own, which is
// exactly why the tag path needed the organization predicate: one user's tag
// legitimately spans both workspaces.
func tagMailboxes(t *testing.T, pool *pgxpool.Pool, f *liveFixture, mailboxes ...uuid.UUID) uuid.UUID {
t.Helper()
ctx := context.Background()
tag := uuid.New()
if _, err := pool.Exec(ctx,
`INSERT INTO tags (id, user_id, title, color, "position") VALUES ($1, $2, 'senders', '#aabbcc', 0)`,
tag, f.user); err != nil {
t.Fatalf("insert tag: %v", err)
}
for _, mailbox := range mailboxes {
if _, err := pool.Exec(ctx,
`INSERT INTO email_tags (email_id, tag_id) VALUES ($1, $2)`, mailbox, tag); err != nil {
t.Fatalf("tag mailbox %s: %v", mailbox, err)
}
}
if _, err := pool.Exec(ctx,
`INSERT INTO campaign_email_tags (campaign_id, tag_id) VALUES ($1, $2)`, f.campaign, tag); err != nil {
t.Fatalf("attach tag to campaign: %v", err)
}
t.Cleanup(func() {
c := context.Background()
for _, sql := range []string{
`DELETE FROM campaign_email_tags WHERE tag_id = $1`,
`DELETE FROM email_tags WHERE tag_id = $1`,
`DELETE FROM tags WHERE id = $1`,
} {
if _, err := pool.Exec(c, sql, tag); err != nil {
t.Errorf("cleanup %q: %v", sql, err)
}
}
})
return tag
}
func liveEmailRepo(t *testing.T, handle *db.DB) repository.EmailRepository {
t.Helper()
enc, err := encrypt.NewEncrypter([]byte("0123456789abcdef0123456789abcdef"))
if err != nil {
t.Fatalf("encrypter: %v", err)
}
return repository.NewEmailRepostory(handle, enc)
}
// mailboxIDs renders a result set as ids, so a failure names the mailboxes that
// leaked rather than just their count.
func mailboxIDs(accounts []models.Email) string {
ids := make([]uuid.UUID, len(accounts))
for i, a := range accounts {
ids[i] = a.ID
}
return idsOf(ids)
}
func idsOf(ids []uuid.UUID) string {
if len(ids) == 0 {
return "no mailboxes"
}
out := make([]string, len(ids))
for i, id := range ids {
out[i] = id.String()
}
return strings.Join(out, ", ")
}
// TestLiveSenderResolutionStaysInsideTheCampaignOrg walks all three resolution
// paths — explicit senders, tags, and the "all active mailboxes" fallback —
// and asserts each one returns only the campaign organization's mailbox.
func TestLiveSenderResolutionStaysInsideTheCampaignOrg(t *testing.T) {
handle, pool := liveDB(t)
f := newLiveFixture(t, pool, "UTC")
o := newForeignOrg(t, pool, f)
ctx := context.Background()
repo := liveEmailRepo(t, handle)
scope := repository.NewAccountScope(&f.org)
// 1. The "all" fallback: no tags, no explicit senders.
all, xerr := repo.GetAllActiveInScope(ctx, scope)
if xerr != nil {
t.Fatalf("all-active lookup: %v", xerr)
}
if len(all) != 1 || all[0].ID != f.mailbox {
t.Fatalf("all-active fallback returned %s, want only the campaign org's mailbox %s",
mailboxIDs(all), f.mailbox)
}
// 2. The tag path, with ONE tag on both mailboxes.
tag := tagMailboxes(t, pool, f, f.mailbox, o.mailbox)
tagged, xerr := repo.GetByTags(ctx, scope, []string{tag.String()})
if xerr != nil {
t.Fatalf("tag lookup: %v", xerr)
}
if len(tagged) != 1 || tagged[0].ID != f.mailbox {
t.Fatalf("tag resolution returned %s, want only the campaign org's mailbox %s",
mailboxIDs(tagged), f.mailbox)
}
// 3. The explicit sender pool, with the FOREIGN mailbox pinned to the
// campaign (a row that predates the ownership check, or one written while
// the campaign had no organization).
if _, err := pool.Exec(ctx,
`INSERT INTO campaign_senders (campaign_id, email_account_id, weight, enabled)
VALUES ($1, $2, 1, true), ($1, $3, 1, true)`, f.campaign, f.mailbox, o.mailbox); err != nil {
t.Fatalf("pin senders: %v", err)
}
senders, xerr := repo.GetByCampaignSenders(ctx, scope, f.campaign)
if xerr != nil {
t.Fatalf("sender lookup: %v", xerr)
}
if len(senders) != 1 || senders[0].Account.ID != f.mailbox {
ids := make([]uuid.UUID, len(senders))
for i, s := range senders {
ids[i] = s.Account.ID
}
t.Fatalf("explicit sender pool returned %s, want only the campaign org's mailbox %s",
idsOf(ids), f.mailbox)
}
}
// TestLiveSenderSchedulerNeverPicksAnotherOrgMailbox is the end-to-end shape of
// the same bug, arranged so only the WRONG answer is reachable: the campaign's
// tag sits on the other organization's mailbox alone. Before the fix the
// scheduler resolved it by owner and handed back that foreign mailbox; now the
// campaign correctly has nothing to send from.
func TestLiveSenderSchedulerNeverPicksAnotherOrgMailbox(t *testing.T) {
handle, pool := liveDB(t)
f := newLiveFixture(t, pool, "UTC")
o := newForeignOrg(t, pool, f)
tagMailboxes(t, pool, f, o.mailbox)
_, _, accountID, err := liveScheduler(t, handle, pool).
CalculateNextCampaignTime(context.Background(), f.campaign)
if accountID == o.mailbox {
t.Fatalf("scheduler picked the OTHER organization's mailbox %s", o.mailbox)
}
if !errors.Is(err, ErrNoEmailAccounts) {
t.Fatalf("want ErrNoEmailAccounts when the only tagged mailbox belongs to another organization, got %v (account %s)",
err, accountID)
}
}
// TestLiveSenderSchedulerPicksTheCampaignOrgMailbox is the other half: when both
// workspaces' mailboxes carry the campaign's tag, the campaign's own mailbox is
// the one that sends.
func TestLiveSenderSchedulerPicksTheCampaignOrgMailbox(t *testing.T) {
handle, pool := liveDB(t)
f := newLiveFixture(t, pool, "UTC")
o := newForeignOrg(t, pool, f)
tagMailboxes(t, pool, f, f.mailbox, o.mailbox)
_, accountID := scheduleSlot(t, liveScheduler(t, handle, pool), f.campaign)
if accountID != f.mailbox {
t.Fatalf("scheduler picked %s, want the campaign org's mailbox %s", accountID, f.mailbox)
}
}
// TestLiveSenderScopeWithoutAnOrganizationReachesNothing pins the fail-closed
// half of the rule. organization_id is NOT NULL since migration 000092, so this
// scope should be unreachable — but if one ever appears, it must resolve to no
// mailboxes rather than widening to the owner, which is how a campaign reached
// another workspace in the first place.
func TestLiveSenderScopeWithoutAnOrganizationReachesNothing(t *testing.T) {
handle, pool := liveDB(t)
f := newLiveFixture(t, pool, "UTC")
ctx := context.Background()
repo := liveEmailRepo(t, handle)
empty := repository.NewAccountScope(nil)
all, xerr := repo.GetAllActiveInScope(ctx, empty)
if xerr != nil {
t.Fatalf("all-active lookup: %v", xerr)
}
if len(all) != 0 {
t.Fatalf("a scope with no organization reached %s", mailboxIDs(all))
}
tag := tagMailboxes(t, pool, f, f.mailbox)
tagged, xerr := repo.GetByTags(ctx, empty, []string{tag.String()})
if xerr != nil {
t.Fatalf("tag lookup: %v", xerr)
}
if len(tagged) != 0 {
t.Fatalf("a scope with no organization reached %s through tags", mailboxIDs(tagged))
}
senders, xerr := repo.GetByCampaignSenders(ctx, empty, f.campaign)
if xerr != nil {
t.Fatalf("sender lookup: %v", xerr)
}
if len(senders) != 0 {
t.Fatalf("a scope with no organization reached %d explicit senders", len(senders))
}
}
// TestLiveActiveCampaignLookupIsOrgScoped covers the same tenancy rule on the
// other side of the join: the warmup floor asks "does this mailbox back an
// active campaign?", and an "all" campaign (no tags, no explicit senders) used
// to answer yes for every mailbox its OWNER had, in any workspace.
func TestLiveActiveCampaignLookupIsOrgScoped(t *testing.T) {
handle, pool := liveDB(t)
f := newLiveFixture(t, pool, "UTC")
o := newForeignOrg(t, pool, f)
ctx := context.Background()
repo := repository.NewCampaignRepostory(handle)
// The fixture campaign is active, in org A, with neither tags nor explicit
// senders — so it backs org A's mailbox and nothing else.
backs, err := repo.AccountHasActiveCampaign(ctx, f.mailbox)
if err != nil {
t.Fatalf("own-org lookup: %v", err)
}
if !backs {
t.Fatal("the campaign's own organization mailbox should back its active campaign")
}
backs, err = repo.AccountHasActiveCampaign(ctx, o.mailbox)
if err != nil {
t.Fatalf("foreign-org lookup: %v", err)
}
if backs {
t.Fatalf("mailbox %s in another organization counts as backing the campaign", o.mailbox)
}
count, err := repo.CountActiveCampaignsForAccount(ctx, o.mailbox)
if err != nil {
t.Fatalf("foreign-org count: %v", err)
}
if count != 0 {
t.Fatalf("mailbox %s in another organization counts %d active campaigns, want 0", o.mailbox, count)
}
}
+10 -8
View File
@@ -2,6 +2,7 @@ package tasks
import (
"context"
"errors"
"os"
"sync"
"testing"
@@ -318,12 +319,10 @@ func liveCampaignService(t *testing.T, handle *db.DB, sender EmailSender) *tasks
// The regression issue #168 asks for: a campaign with no organization, whose
// recipient is on the suppression list, must not send.
//
// Suppression is enforced twice, and the missing tenant defeats both. Routing
// Suppression is enforced twice, and the missing tenant defeats both: routing
// excludes a suppressed lead by joining suppressed_recipients on the campaign's
// organization_id, which matches nothing when that is NULL, so the scheduler
// still hands the suppressed contact back (asserted here, because it is what
// makes this a real regression rather than a test that passes by accident).
// The send gate then has to be the thing that stops it.
// organization_id, which matches nothing when that is NULL. The send gate is
// what has to stop it, and that is what this asserts.
func TestLiveOrglessCampaignDoesNotSendToSuppressedRecipient(t *testing.T) {
handle := liveCampaignDB(t)
svc := liveCampaignService(t, handle, nil)
@@ -332,10 +331,13 @@ func TestLiveOrglessCampaignDoesNotSendToSuppressedRecipient(t *testing.T) {
f.suppress(t, "unsubscribed")
f.dropOrganization(t)
// Routing's own suppression filter is bypassed: the lead is still routable.
// Sender resolution is org-scoped too (issue #167), so an orgless campaign
// now finds no mailboxes before routing is even consulted. That is a second
// lock on the same door, not the one under test: the tick below still runs
// the send gate, which is what must refuse.
_, pair, _, err := svc.scheduler.CalculateNextCampaignTime(context.Background(), f.campaign)
if err != nil || pair == nil {
t.Fatalf("expected the suppressed lead to still route on an orgless campaign, got pair=%v err=%v", pair, err)
if pair != nil || !errors.Is(err, scheduler.ErrNoEmailAccounts) {
t.Fatalf("expected an orgless campaign to resolve no senders, got pair=%v err=%v", pair, err)
}
taskID := f.queueTick(t, svc.taskRepo)