21990f2533
- Copy discipline for notifications: the store, the queue, workers, and API callers each own clones; no notification object is shared across goroutines (races previously flagged by -race between workers mutating Status/RetryCount and handlers JSON-encoding the same pointer). - Retry progress derives from QueueMessage.Attempt so it survives requeues; exponential backoff (1s base, 30s cap) honors the documented queue.retry_backoff setting instead of hammering failing providers in a tight loop; shutdown abandons pending backoff waits cleanly. - Stop() cancels a service-lifetime context so idle workers blocked in Dequeue exit immediately instead of waiting out their poll timeout. - LocalQueue no longer holds its mutex while sending on the queue channel (Enqueue/Nack) — with a full buffer this deadlocked the entire worker pool, since draining requires the same mutex. - Tenant scoping: notifications are stamped with the caller's ClientID; non-admin clients can only read/cancel/retry their own (cross-tenant access reports not-found to avoid leaking existence). - Sentinel errors ErrNotificationNotFound/ErrNotificationAlreadySent. - New race, backoff, and tenant-scoping test suites. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
195 lines
6.5 KiB
Go
195 lines
6.5 KiB
Go
package service
|
|
|
|
import (
|
|
"context"
|
|
"fmt"
|
|
"testing"
|
|
|
|
"github.com/google/uuid"
|
|
"github.com/igodwin/notifier/internal/auth"
|
|
"github.com/igodwin/notifier/internal/domain"
|
|
)
|
|
|
|
// ctxForClient builds a context carrying an auth.AuthContext for the given
|
|
// client and roles, as REST/gRPC middleware would attach after authenticating
|
|
// a request.
|
|
func ctxForClient(clientID string, roles ...string) context.Context {
|
|
return auth.ContextWithAuth(context.Background(), &auth.AuthContext{
|
|
ClientID: clientID,
|
|
Roles: roles,
|
|
})
|
|
}
|
|
|
|
// TestTenantScopingNonAdminSeesOnlyOwnNotifications verifies that a
|
|
// non-admin caller only sees notifications stamped with their own ClientID.
|
|
func TestTenantScopingNonAdminSeesOnlyOwnNotifications(t *testing.T) {
|
|
svc := createTestService(t)
|
|
|
|
tenantA := &domain.Notification{
|
|
ID: uuid.New().String(),
|
|
Type: domain.TypeStdout,
|
|
Status: domain.StatusSent,
|
|
Recipients: []string{"a@example.com"},
|
|
ClientID: "tenant-a",
|
|
}
|
|
tenantB := &domain.Notification{
|
|
ID: uuid.New().String(),
|
|
Type: domain.TypeStdout,
|
|
Status: domain.StatusSent,
|
|
Recipients: []string{"b@example.com"},
|
|
ClientID: "tenant-b",
|
|
}
|
|
|
|
svc.storeNotification(tenantA)
|
|
svc.storeNotification(tenantB)
|
|
|
|
ctxA := ctxForClient("tenant-a", "user")
|
|
|
|
list, err := svc.ListNotifications(ctxA, &domain.NotificationFilter{})
|
|
if err != nil {
|
|
t.Fatalf("ListNotifications failed: %v", err)
|
|
}
|
|
if len(list) != 1 || list[0].ID != tenantA.ID {
|
|
t.Fatalf("expected tenant-a to see only its own notification, got %d results", len(list))
|
|
}
|
|
|
|
if _, err := svc.GetNotification(ctxA, tenantA.ID); err != nil {
|
|
t.Errorf("tenant-a should be able to get its own notification: %v", err)
|
|
}
|
|
|
|
if _, err := svc.GetNotification(ctxA, tenantB.ID); err == nil {
|
|
t.Error("tenant-a should not be able to get tenant-b's notification")
|
|
}
|
|
|
|
stats, err := svc.GetStats(ctxA)
|
|
if err != nil {
|
|
t.Fatalf("GetStats failed: %v", err)
|
|
}
|
|
if stats.TotalSent != 1 {
|
|
t.Errorf("expected tenant-a stats to count only its own notification, got %d", stats.TotalSent)
|
|
}
|
|
}
|
|
|
|
// TestTenantScopingAdminSeesAll verifies that a caller with the admin role
|
|
// can see notifications belonging to any tenant.
|
|
func TestTenantScopingAdminSeesAll(t *testing.T) {
|
|
svc := createTestService(t)
|
|
|
|
tenantA := &domain.Notification{ID: uuid.New().String(), Type: domain.TypeStdout, Status: domain.StatusSent, Recipients: []string{"a@example.com"}, ClientID: "tenant-a"}
|
|
tenantB := &domain.Notification{ID: uuid.New().String(), Type: domain.TypeStdout, Status: domain.StatusSent, Recipients: []string{"b@example.com"}, ClientID: "tenant-b"}
|
|
|
|
svc.storeNotification(tenantA)
|
|
svc.storeNotification(tenantB)
|
|
|
|
adminCtx := ctxForClient("admin-client", "admin")
|
|
|
|
list, err := svc.ListNotifications(adminCtx, &domain.NotificationFilter{})
|
|
if err != nil {
|
|
t.Fatalf("ListNotifications failed: %v", err)
|
|
}
|
|
if len(list) != 2 {
|
|
t.Fatalf("expected admin to see both notifications, got %d", len(list))
|
|
}
|
|
|
|
if _, err := svc.GetNotification(adminCtx, tenantA.ID); err != nil {
|
|
t.Errorf("admin should be able to get tenant-a's notification: %v", err)
|
|
}
|
|
if _, err := svc.GetNotification(adminCtx, tenantB.ID); err != nil {
|
|
t.Errorf("admin should be able to get tenant-b's notification: %v", err)
|
|
}
|
|
|
|
stats, err := svc.GetStats(adminCtx)
|
|
if err != nil {
|
|
t.Fatalf("GetStats failed: %v", err)
|
|
}
|
|
if stats.TotalSent != 2 {
|
|
t.Errorf("expected admin stats to count both notifications, got %d", stats.TotalSent)
|
|
}
|
|
}
|
|
|
|
// TestTenantScopingAuthDisabledUnchanged verifies that when no auth context
|
|
// is present (auth disabled), behavior is unchanged: every notification is
|
|
// visible regardless of ClientID.
|
|
func TestTenantScopingAuthDisabledUnchanged(t *testing.T) {
|
|
svc := createTestService(t)
|
|
|
|
tenantA := &domain.Notification{ID: uuid.New().String(), Type: domain.TypeStdout, Status: domain.StatusSent, Recipients: []string{"a@example.com"}, ClientID: "tenant-a"}
|
|
tenantB := &domain.Notification{ID: uuid.New().String(), Type: domain.TypeStdout, Status: domain.StatusSent, Recipients: []string{"b@example.com"}, ClientID: ""}
|
|
|
|
svc.storeNotification(tenantA)
|
|
svc.storeNotification(tenantB)
|
|
|
|
ctx := context.Background() // no auth context attached
|
|
|
|
list, err := svc.ListNotifications(ctx, &domain.NotificationFilter{})
|
|
if err != nil {
|
|
t.Fatalf("ListNotifications failed: %v", err)
|
|
}
|
|
if len(list) != 2 {
|
|
t.Fatalf("expected both notifications visible when auth is disabled, got %d", len(list))
|
|
}
|
|
|
|
if _, err := svc.GetNotification(ctx, tenantA.ID); err != nil {
|
|
t.Errorf("expected to get tenant-a's notification with auth disabled: %v", err)
|
|
}
|
|
if _, err := svc.GetNotification(ctx, tenantB.ID); err != nil {
|
|
t.Errorf("expected to get tenant-b's notification with auth disabled: %v", err)
|
|
}
|
|
}
|
|
|
|
// TestTenantScopingCrossTenantAccessReturnsNotFound verifies that
|
|
// Get/Cancel/Retry on another tenant's notification return the exact same
|
|
// not-found error as a genuinely missing ID, so existence isn't leaked.
|
|
func TestTenantScopingCrossTenantAccessReturnsNotFound(t *testing.T) {
|
|
svc := createTestService(t)
|
|
|
|
tenantA := &domain.Notification{
|
|
ID: uuid.New().String(),
|
|
Type: domain.TypeStdout,
|
|
Status: domain.StatusPending,
|
|
Recipients: []string{"a@example.com"},
|
|
MaxRetries: 3,
|
|
ClientID: "tenant-a",
|
|
}
|
|
svc.storeNotification(tenantA)
|
|
|
|
ctxB := ctxForClient("tenant-b", "user")
|
|
|
|
wantMsg := fmt.Sprintf("notification not found: %s", tenantA.ID)
|
|
|
|
_, getErr := svc.GetNotification(ctxB, tenantA.ID)
|
|
if getErr == nil {
|
|
t.Fatal("expected not-found error for cross-tenant Get")
|
|
}
|
|
if getErr.Error() != wantMsg {
|
|
t.Errorf("expected cross-tenant Get error %q, got %q", wantMsg, getErr.Error())
|
|
}
|
|
|
|
_, missingErr := svc.GetNotification(ctxB, "does-not-exist")
|
|
if missingErr == nil {
|
|
t.Fatal("expected not-found error for missing ID")
|
|
}
|
|
|
|
cancelErr := svc.CancelNotification(ctxB, tenantA.ID)
|
|
if cancelErr == nil {
|
|
t.Fatal("expected not-found error for cross-tenant Cancel")
|
|
}
|
|
if cancelErr.Error() != wantMsg {
|
|
t.Errorf("expected cross-tenant Cancel error %q, got %q", wantMsg, cancelErr.Error())
|
|
}
|
|
|
|
_, retryErr := svc.RetryNotification(ctxB, tenantA.ID)
|
|
if retryErr == nil {
|
|
t.Fatal("expected not-found error for cross-tenant Retry")
|
|
}
|
|
if retryErr.Error() != wantMsg {
|
|
t.Errorf("expected cross-tenant Retry error %q, got %q", wantMsg, retryErr.Error())
|
|
}
|
|
|
|
// The owning tenant should still be able to see and act on it.
|
|
ctxA := ctxForClient("tenant-a", "user")
|
|
if _, err := svc.GetNotification(ctxA, tenantA.ID); err != nil {
|
|
t.Errorf("tenant-a should still be able to get its own notification: %v", err)
|
|
}
|
|
}
|