diff --git a/backend/internal/ads/service.go b/backend/internal/ads/service.go index ff6a7e0..939771d 100644 --- a/backend/internal/ads/service.go +++ b/backend/internal/ads/service.go @@ -139,14 +139,23 @@ func (s *Service) AddMessage(ctx context.Context, campaignID uuid.UUID, bodyEn, return s.store.AddMessage(ctx, Message{CampaignID: campaignID, Position: len(c.Messages), BodyEn: en, BodyRu: ru}) } -// EditMessage validates and updates a message's bilingual bodies (its position is -// unchanged). -func (s *Service) EditMessage(ctx context.Context, id uuid.UUID, bodyEn, bodyRu string) error { +// EditMessage validates and updates the bilingual bodies of a message that +// belongs to campaignID (its position is unchanged). It returns ErrNotFound when +// the message is not part of that campaign, so a mismatched campaign/message pair +// never edits an unrelated campaign's message. +func (s *Service) EditMessage(ctx context.Context, campaignID, messageID uuid.UUID, bodyEn, bodyRu string) error { en, ru, err := validBodies(bodyEn, bodyRu) if err != nil { return err } - return s.store.UpdateMessageBodies(ctx, id, en, ru) + c, err := s.store.Campaign(ctx, campaignID) + if err != nil { + return err + } + if !campaignOwnsMessage(c, messageID) { + return ErrNotFound + } + return s.store.UpdateMessageBodies(ctx, messageID, en, ru) } // MoveMessage reorders a message within its campaign by swapping its position @@ -178,19 +187,35 @@ func (s *Service) MoveMessage(ctx context.Context, campaignID, messageID uuid.UU return s.store.SetMessagePosition(ctx, b.ID, idx) } -// DeleteMessage removes a message, refusing to remove the default campaign's last -// remaining message (the default must always have a creative to show). +// DeleteMessage removes a message that belongs to campaignID, refusing to remove +// the default campaign's last remaining message (the default must always have a +// creative to show). It returns ErrNotFound when the message is not part of that +// campaign — so a mismatched campaign/message pair can neither delete an +// unrelated campaign's message nor bypass the default's last-message guard. func (s *Service) DeleteMessage(ctx context.Context, campaignID, messageID uuid.UUID) error { c, err := s.store.Campaign(ctx, campaignID) if err != nil { return err } + if !campaignOwnsMessage(c, messageID) { + return ErrNotFound + } if c.IsDefault && len(c.Messages) <= 1 { return fmt.Errorf("%w: the default campaign must keep at least one message", ErrValidation) } return s.store.DeleteMessage(ctx, messageID) } +// campaignOwnsMessage reports whether messageID is one of the campaign's messages. +func campaignOwnsMessage(c Campaign, messageID uuid.UUID) bool { + for _, m := range c.Messages { + if m.ID == messageID { + return true + } + } + return false +} + // Settings returns the global display timings. func (s *Service) Settings(ctx context.Context) (Timings, error) { return s.store.Settings(ctx) diff --git a/backend/internal/inttest/banner_console_test.go b/backend/internal/inttest/banner_console_test.go index 1cf0095..67ee9fa 100644 --- a/backend/internal/inttest/banner_console_test.go +++ b/backend/internal/inttest/banner_console_test.go @@ -144,6 +144,49 @@ func TestBannerConsoleCRUD(t *testing.T) { } } +// TestBannerMessageOwnership guards the campaign/message pairing: a message may +// only be edited or deleted through the URL of the campaign it belongs to. This +// closes a default-protection bypass — deleting the default's last message via an +// unrelated campaign's URL must be refused. +func TestBannerMessageOwnership(t *testing.T) { + ctx := context.Background() + srv, adsSvc := bannerServer(t) + h := srv.Handler() + base := "http://admin.test/_gm" + const origin = "http://admin.test" + + def := defaultCampaign(t, adsSvc) + if len(def.Messages) == 0 { + t.Fatal("default campaign has no seeded message") + } + defMsg := def.Messages[0].ID + + // A separate timed campaign whose URL we will (mis)use. + otherID, err := adsSvc.CreateCampaign(ctx, ads.Campaign{Name: "Own-" + uuid.NewString()[:8], Weight: 10, Enabled: true}) + if err != nil { + t.Fatalf("create campaign: %v", err) + } + t.Cleanup(func() { _ = adsSvc.DeleteCampaign(ctx, otherID) }) + + // Deleting the default's message through the other campaign's URL is refused, + // and the default keeps its message (the bypass is closed). + if code, body := consoleDo(h, http.MethodPost, base+"/banners/"+otherID.String()+"/messages/"+defMsg.String()+"/delete", "", origin); code != http.StatusOK || !strings.Contains(body, "Not found") { + t.Fatalf("cross-campaign delete = %d, has 'Not found'=%v", code, strings.Contains(body, "Not found")) + } + // Editing it through the wrong campaign is refused too. + if code, body := consoleDo(h, http.MethodPost, base+"/banners/"+otherID.String()+"/messages/"+defMsg.String(), "body_en=Hijacked&body_ru=Взлом", origin); code != http.StatusOK || !strings.Contains(body, "Not found") { + t.Fatalf("cross-campaign edit = %d, has 'Not found'=%v", code, strings.Contains(body, "Not found")) + } + // The default's message is intact. + again := defaultCampaign(t, adsSvc) + if len(again.Messages) != len(def.Messages) { + t.Fatalf("default messages = %d, want %d (unchanged)", len(again.Messages), len(def.Messages)) + } + if again.Messages[0].BodyEn != def.Messages[0].BodyEn { + t.Fatalf("default message body changed: %q -> %q", def.Messages[0].BodyEn, again.Messages[0].BodyEn) + } +} + // profileBanner is the banner block of the profile.get JSON response. type profileBanner struct { Banner *struct { diff --git a/backend/internal/server/handlers_admin_banners.go b/backend/internal/server/handlers_admin_banners.go index 86b89a5..0a8ecc6 100644 --- a/backend/internal/server/handlers_admin_banners.go +++ b/backend/internal/server/handlers_admin_banners.go @@ -159,7 +159,7 @@ func (s *Server) consoleEditBannerMessage(c *gin.Context) { return } back := "/_gm/banners/" + cid.String() - if err := s.ads.EditMessage(c.Request.Context(), mid, trimForm(c, "body_en"), trimForm(c, "body_ru")); err != nil { + if err := s.ads.EditMessage(c.Request.Context(), cid, mid, trimForm(c, "body_en"), trimForm(c, "body_ru")); err != nil { s.bannerError(c, err, back) return }