fix(server): validate cross-org resource ownership
Review of the org-scoping pass found that org_id on a query filter protects the row you look up, but does nothing when a handler accepts a foreign resource ID as data and a downstream unscoped query consumes it. - AssignKey: verify key and server both belong to the org - BuildAuthorizedKeys: resolve server first, scope assignments and keys to that server's org (was honouring foreign assignment rows) - Workflows: validate TargetServerIDs on create/update and re-check at trigger time - Monitor incidents/uptime handlers: gate on org-scoped GetMonitor - GetChannels: take orgID; validate channel_ids on monitor create/update - Secret and default-step unique indexes: scope to org_id so a second org no longer hits E11000 - DeleteServer/DeleteMonitor: scope cascading deletes
This commit is contained in:
@@ -41,14 +41,14 @@ func GetChannel(orgID, channelID string) (*models.NotificationChannel, error) {
|
||||
return &ch, nil
|
||||
}
|
||||
|
||||
// GetChannels loads multiple channels by ID, skipping any not found.
|
||||
func GetChannels(channelIDs []string) ([]models.NotificationChannel, error) {
|
||||
// GetChannels loads multiple channels by ID within an org, skipping any not found.
|
||||
func GetChannels(orgID string, channelIDs []string) ([]models.NotificationChannel, error) {
|
||||
if len(channelIDs) == 0 {
|
||||
return nil, nil
|
||||
}
|
||||
ctx, cancel := monCtx()
|
||||
defer cancel()
|
||||
cur, err := db.Col("notification_channels").Find(ctx, bson.M{"channel_id": bson.M{"$in": channelIDs}})
|
||||
cur, err := db.Col("notification_channels").Find(ctx, bson.M{"org_id": orgID, "channel_id": bson.M{"$in": channelIDs}})
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
@@ -59,6 +59,21 @@ func GetChannels(channelIDs []string) ([]models.NotificationChannel, error) {
|
||||
return out, nil
|
||||
}
|
||||
|
||||
// validateChannelIDs rejects any channel that does not belong to the org.
|
||||
// Channel IDs arrive from the client as data on monitor writes.
|
||||
func validateChannelIDs(orgID string, channelIDs []string) error {
|
||||
for _, id := range channelIDs {
|
||||
ch, err := GetChannel(orgID, id)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if ch == nil {
|
||||
return errors.New("channel " + id + " not found")
|
||||
}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func CreateChannel(orgID string, ch *models.NotificationChannel) (*models.NotificationChannel, error) {
|
||||
ctx, cancel := monCtx()
|
||||
defer cancel()
|
||||
|
||||
@@ -175,6 +175,15 @@ func AssignKey(orgID, keyID, serverID string) (*models.Assignment, error) {
|
||||
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
|
||||
defer cancel()
|
||||
|
||||
// Both sides must belong to the caller's org — the IDs arrive from the
|
||||
// client as data and are consumed by unscoped agent-path queries later.
|
||||
if _, err := GetKey(orgID, keyID); err != nil {
|
||||
return nil, fmt.Errorf("key not found")
|
||||
}
|
||||
if _, err := GetServer(orgID, serverID); err != nil {
|
||||
return nil, fmt.Errorf("server not found")
|
||||
}
|
||||
|
||||
// Check if already assigned and active
|
||||
var existing models.Assignment
|
||||
err := db.Col("assignments").FindOne(ctx, bson.M{
|
||||
|
||||
@@ -101,6 +101,9 @@ func getMonitorByID(monitorID string) (*models.Monitor, error) {
|
||||
func CreateMonitor(orgID string, m *models.Monitor) (*models.Monitor, error) {
|
||||
ctx, cancel := monCtx()
|
||||
defer cancel()
|
||||
if err := validateChannelIDs(orgID, m.ChannelIDs); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
m.OrgID = orgID
|
||||
m.MonitorID = uuid.NewString()
|
||||
m.CreatedAt = time.Now()
|
||||
@@ -123,6 +126,11 @@ func CreateMonitor(orgID string, m *models.Monitor) (*models.Monitor, error) {
|
||||
func UpdateMonitor(orgID, monitorID string, upd bson.M) error {
|
||||
ctx, cancel := monCtx()
|
||||
defer cancel()
|
||||
if ids, ok := upd["channel_ids"].([]string); ok {
|
||||
if err := validateChannelIDs(orgID, ids); err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
_, err := db.Col("monitors").UpdateOne(ctx, bson.M{"monitor_id": monitorID, "org_id": orgID}, bson.M{"$set": upd})
|
||||
return err
|
||||
}
|
||||
@@ -130,9 +138,15 @@ func UpdateMonitor(orgID, monitorID string, upd bson.M) error {
|
||||
func DeleteMonitor(orgID, monitorID string) error {
|
||||
ctx, cancel := monCtx()
|
||||
defer cancel()
|
||||
if _, err := db.Col("monitors").DeleteOne(ctx, bson.M{"monitor_id": monitorID, "org_id": orgID}); err != nil {
|
||||
res, err := db.Col("monitors").DeleteOne(ctx, bson.M{"monitor_id": monitorID, "org_id": orgID})
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
// Only cascade when the org-scoped delete actually removed a monitor —
|
||||
// incidents/rollups carry no org_id of their own.
|
||||
if res.DeletedCount == 0 {
|
||||
return nil
|
||||
}
|
||||
db.Col("incidents").DeleteMany(ctx, bson.M{"monitor_id": monitorID})
|
||||
db.Col("monitor_rollups").DeleteMany(ctx, bson.M{"monitor_id": monitorID})
|
||||
return nil
|
||||
@@ -263,7 +277,7 @@ func notifyTransition(m *models.Monitor, newStatus, message string) {
|
||||
if len(m.ChannelIDs) == 0 {
|
||||
return
|
||||
}
|
||||
channels, err := GetChannels(m.ChannelIDs)
|
||||
channels, err := GetChannels(m.OrgID, m.ChannelIDs)
|
||||
if err != nil {
|
||||
log.Printf("notify: load channels for %s: %v", m.MonitorID, err)
|
||||
return
|
||||
|
||||
@@ -2,6 +2,7 @@ package services
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"fmt"
|
||||
"sort"
|
||||
"time"
|
||||
@@ -13,18 +14,34 @@ import (
|
||||
"go.mongodb.org/mongo-driver/v2/mongo/options"
|
||||
)
|
||||
|
||||
// EnsureSecretIndexes creates the unique compound index on (group, key).
|
||||
// EnsureSecretIndexes creates the unique compound index on (org_id, group, key).
|
||||
// The pre-multi-tenant index was on (group, key) alone, which made a second org
|
||||
// collide on the same group/key — drop it if a live DB still carries it.
|
||||
func EnsureSecretIndexes() error {
|
||||
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
|
||||
defer cancel()
|
||||
|
||||
if err := db.Col("secrets").Indexes().DropOne(ctx, "group_1_key_1"); err != nil && !isIndexNotFound(err) {
|
||||
return err
|
||||
}
|
||||
|
||||
_, err := db.Col("secrets").Indexes().CreateOne(ctx, mongo.IndexModel{
|
||||
Keys: bson.D{{Key: "group", Value: 1}, {Key: "key", Value: 1}},
|
||||
Keys: bson.D{{Key: "org_id", Value: 1}, {Key: "group", Value: 1}, {Key: "key", Value: 1}},
|
||||
Options: options.Index().SetUnique(true),
|
||||
})
|
||||
return err
|
||||
}
|
||||
|
||||
// isIndexNotFound reports whether err is Mongo's IndexNotFound (27), returned
|
||||
// when dropping an index that was never created.
|
||||
func isIndexNotFound(err error) bool {
|
||||
var ce mongo.CommandError
|
||||
if errors.As(err, &ce) {
|
||||
return ce.Code == 27 || ce.Name == "IndexNotFound"
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
// ListSecretGroups returns a summary of every group with its key count and
|
||||
// most recent update time.
|
||||
func ListSecretGroups(orgID string) ([]models.GroupSummary, error) {
|
||||
|
||||
@@ -257,7 +257,7 @@ func DeleteServer(orgID, serverID string) error {
|
||||
return err
|
||||
}
|
||||
// Also remove assignments
|
||||
_, err = db.Col("assignments").DeleteMany(ctx, bson.M{"server_id": serverID})
|
||||
_, err = db.Col("assignments").DeleteMany(ctx, bson.M{"server_id": serverID, "org_id": orgID})
|
||||
return err
|
||||
}
|
||||
|
||||
|
||||
@@ -13,7 +13,15 @@ func BuildAuthorizedKeys(serverID string) ([]string, error) {
|
||||
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
|
||||
defer cancel()
|
||||
|
||||
// Agent path — no session, so the org comes from the server record itself
|
||||
// and both follow-up queries are scoped to it.
|
||||
srv, err := getServerByID(serverID)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
cursor, err := db.Col("assignments").Find(ctx, bson.M{
|
||||
"org_id": srv.OrgID,
|
||||
"server_id": serverID,
|
||||
"revoked_at": nil,
|
||||
})
|
||||
@@ -30,7 +38,7 @@ func BuildAuthorizedKeys(serverID string) ([]string, error) {
|
||||
var lines []string
|
||||
for _, a := range assignments {
|
||||
var key models.Key
|
||||
err := db.Col("keys").FindOne(ctx, bson.M{"key_id": a.KeyID}).Decode(&key)
|
||||
err := db.Col("keys").FindOne(ctx, bson.M{"key_id": a.KeyID, "org_id": srv.OrgID}).Decode(&key)
|
||||
if err != nil {
|
||||
continue
|
||||
}
|
||||
|
||||
@@ -30,6 +30,11 @@ func TriggerWorkflow(orgID, workflowID, actor string) (string, error) {
|
||||
if len(wf.Steps) == 0 {
|
||||
return "", fmt.Errorf("workflow has no steps")
|
||||
}
|
||||
// Re-check ownership at trigger time — targets may predate validation or a
|
||||
// server may have been removed since the workflow was saved.
|
||||
if err := validateTargetServers(orgID, wf.TargetServerIDs); err != nil {
|
||||
return "", err
|
||||
}
|
||||
|
||||
// Reject a concurrent run of the same workflow.
|
||||
ctx, cancel := wfCtx()
|
||||
|
||||
@@ -25,8 +25,13 @@ func EnsureWorkflowIndexes() error {
|
||||
}); err != nil {
|
||||
return err
|
||||
}
|
||||
// The pre-multi-tenant index was on slug alone, so seeding defaults for a
|
||||
// second org collided — drop it if a live DB still carries it.
|
||||
if err := db.Col("workflow_steps").Indexes().DropOne(ctx, "slug_1"); err != nil && !isIndexNotFound(err) {
|
||||
return err
|
||||
}
|
||||
if _, err := db.Col("workflow_steps").Indexes().CreateOne(ctx, mongo.IndexModel{
|
||||
Keys: bson.D{{Key: "slug", Value: 1}},
|
||||
Keys: bson.D{{Key: "org_id", Value: 1}, {Key: "slug", Value: 1}},
|
||||
Options: options.Index().SetUnique(true).
|
||||
SetPartialFilterExpression(bson.M{"source": "default"}),
|
||||
}); err != nil {
|
||||
@@ -219,6 +224,9 @@ func CreateWorkflow(orgID string, w models.Workflow) (*models.Workflow, error) {
|
||||
if err := ValidateWorkflow(w); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
if err := validateTargetServers(orgID, w.TargetServerIDs); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
normalizeInlineSteps(&w)
|
||||
if _, err := db.Col("workflows").InsertOne(ctx, w); err != nil {
|
||||
return nil, err
|
||||
@@ -232,6 +240,9 @@ func UpdateWorkflow(orgID, id string, w models.Workflow) error {
|
||||
if err := ValidateWorkflow(w); err != nil {
|
||||
return err
|
||||
}
|
||||
if err := validateTargetServers(orgID, w.TargetServerIDs); err != nil {
|
||||
return err
|
||||
}
|
||||
normalizeInlineSteps(&w)
|
||||
_, err := db.Col("workflows").UpdateOne(ctx, bson.M{"workflow_id": id, "org_id": orgID}, bson.M{"$set": bson.M{
|
||||
"name": w.Name,
|
||||
@@ -242,6 +253,18 @@ func UpdateWorkflow(orgID, id string, w models.Workflow) error {
|
||||
return err
|
||||
}
|
||||
|
||||
// validateTargetServers rejects any target server that does not belong to the
|
||||
// org. The IDs are client-supplied and are later consumed by the runner's
|
||||
// unscoped lookups, so ownership has to be proven at the write boundary.
|
||||
func validateTargetServers(orgID string, serverIDs []string) error {
|
||||
for _, sid := range serverIDs {
|
||||
if _, err := GetServer(orgID, sid); err != nil {
|
||||
return fmt.Errorf("target server %s not found", sid)
|
||||
}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// normalizeInlineSteps derives outputs for inline steps and strips fields that
|
||||
// only belong to library steps.
|
||||
func normalizeInlineSteps(w *models.Workflow) {
|
||||
|
||||
Reference in New Issue
Block a user