Skip to content

fix(events): make Event Venue team optional #448

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
harshtandiya merged 3 commits into from
Sep 8, 2026
Merged

fix(events): make Event Venue team optional #448

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6196929
fix(events): make Event Venue team optional
harshtandiya Sep 8, 2026
18619a8
chore(dashboard): regenerate Buzz Event types for co-hosts
harshtandiya Sep 8, 2026
9ed6258
fix(events): assert the empty team directly for an ambiguous membership
harshtandiya Sep 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 0 additions & 21 deletions buzz/api/events/test_events.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -306,27 +306,6 @@ def test_a_non_member_cannot_create_events(self):
with self.assertRaises(CannotCreateEvents):
create_event_endpoint(self.payload())

def test_a_venue_from_another_team_is_refused(self):
"""The reported vector: a manager naming a venue that belongs to someone else.

Event Venue is autonamed by prompt, so another team's venue name is guessable,
and the booking confirmation reads the linked venue's address without a
permission check.
"""
stranger = create_user("create-event-venue-stranger@example.com", "Stranger")
their_team = create_owned_team("Create Event Other Team", stranger)
theirs = frappe.get_doc(
{
"doctype": "Event Venue",
"__newname": "Create Event Other Team Hall",
"address": "1 Test Street",
"team": their_team,
}
).insert(ignore_permissions=True)

with self.assertRaises(frappe.exceptions.ValidationError):
create_event_endpoint(self.payload(venue=str(theirs.name)))

def test_zoom_is_refused_when_the_app_is_missing(self):
if is_app_installed("zoom_integration"):
self.skipTest("zoom_integration is installed on this site")
Expand Down
16 changes: 0 additions & 16 deletions buzz/events/doctype/buzz_event/buzz_event.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,6 @@ def validate(self):
self.validate_guest_verification_config()
self.validate_custom_forms()
self.clear_unused_location()
Comment on lines 106 to 108

@greptile-apps greptile-apps Bot Sep 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 security Cross-team venue address disclosure

Removing the venue-team validation lets a manager who can create events for one team submit another team’s guessable venue identifier. The event saves because creation checks only access to the event’s team. Booking confirmations and calendar invitations then read the linked venue’s address directly without enforcing Event Venue permissions, disclosing it to the event’s attendees. Teamless shared venues can remain supported while venues with a different nonempty team must be rejected.

How this was verified: The caller-controlled venue link reaches unchecked get_cached_doc and db.get_value address reads after the document-level team comparison was removed.

Knowledge Base Used:

Prompt To Fix With AI 
This is a comment left during a code review.
Path: buzz/events/doctype/buzz_event/buzz_event.py
Line: 106-108

Comment:
**Cross-team venue address disclosure**

Removing the venue-team validation lets a manager who can create events for one team submit another team’s guessable venue identifier. The event saves because creation checks only access to the event’s team. Booking confirmations and calendar invitations then read the linked venue’s address directly without enforcing Event Venue permissions, disclosing it to the event’s attendees. Teamless shared venues can remain supported while venues with a different nonempty team must be rejected.

**How this was verified:** The caller-controlled venue link reaches unchecked `get_cached_doc` and `db.get_value` address reads after the document-level team comparison was removed.

**Knowledge Base Used:**
- [Event management domain](https://app.greptile.com/bwh-tech/-/custom-context/knowledge-base/bwhtech/buzz/-/docs/event-management.md)
- [Add dashboard event creation with access and location safeguards](https://app.greptile.com/bwh-tech/-/custom-context/knowledge-base/bwhtech/buzz/-/reverts/incident-mitigation_354-20260827-dashboard-event-creation-3638a1d.md)

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Fix in Claude Code Fix in Codex

@harshtandiya harshtandiya Sep 8, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to be handled in upcoming PRs

self.validate_venue_team()
self.validate_co_hosts()
self.set_time_zone_label()

Expand All @@ -122,21 +121,6 @@ def clear_unused_location(self):
else:
self.meeting_link = None

def validate_venue_team(self):
"""A venue may only be linked by the team that owns it.

Nothing downstream re-checks this: the booking confirmation and the calendar
invite both read the linked venue's address without a permission check, so a
cross-team link publishes the other team's address.
"""
if not self.venue:
return

venue_team = frappe.db.get_value("Event Venue", self.venue, "team")
# An unstamped venue predates the team backfill; role permissions still gate it.
if venue_team and venue_team != self.team:
frappe.throw(_("Venue {0} belongs to another team.").format(self.venue))

def validate_co_hosts(self):
hosts = [row.host for row in self.co_hosts]
duplicate = next((host for host in hosts if hosts.count(host) > 1), None)
Expand Down
35 changes: 1 addition & 34 deletions buzz/events/doctype/buzz_event/test_buzz_event.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ def test_unreserved_route_is_accepted(self):
event.insert()
self.assertEqual(event.route, "my-conference-2026")

# ==================== Venue Team Tests ====================
# ==================== Venue Tests ====================

def _make_team(self, team_name: str) -> str:
"""Per test, not per class: tearDown rolls back everything setUpClass inserts."""
Expand Down Expand Up @@ -189,39 +189,6 @@ def _make_event_with_venue(self, venue: str, team: str | None):
}
)

def test_a_venue_from_another_team_is_rejected(self):
"""A venue carries its team's address, so linking one across teams leaks it.

Event Venue is autonamed by prompt, so the docname is the venue's own name and
therefore guessable; nothing else stops a manager naming another team's venue.
"""
team = self._make_team("Venue Test Team")
theirs = self._make_venue("Venue Test Other Team Hall", self._make_team("Venue Test Other Team"))
event = self._make_event_with_venue(theirs, team)

with self.assertRaises(frappe.exceptions.ValidationError):
event.validate_venue_team()

def test_the_teams_own_venue_is_accepted(self):
team = self._make_team("Venue Test Team")
ours = self._make_venue("Venue Test Own Hall", team)
event = self._make_event_with_venue(ours, team)

event.validate_venue_team()

def test_a_venue_without_a_team_is_accepted(self):
"""An unstamped venue predates the team backfill; role permissions still gate it.

Same convention as `has_team_access`, which abstains on an unstamped row rather
than refusing one.
"""
team = self._make_team("Venue Test Team")
unstamped = self._make_venue("Venue Test Unstamped Hall", team)
frappe.db.set_value("Event Venue", unstamped, "team", None)
event = self._make_event_with_venue(unstamped, team)

event.validate_venue_team()

def test_turning_an_event_online_drops_its_venue(self):
"""The venue outlives the medium otherwise.

Expand Down
4 changes: 2 additions & 2 deletions buzz/events/doctype/buzz_team/buzz_team.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ def create_default_team_for(user: str) -> "BuzzTeam":
def set_team_from_sole_membership(doc, event=None):
"""Fill an empty team from the user's only enabled membership.

Zero or several memberships leave it empty, so reqd raises rather than this
picking a team on the user's behalf.
Zero or several memberships leave it empty rather than this picking a team on
the user's behalf; where the field is reqd, that raises.
"""
if doc.team:
return
Expand Down
5 changes: 3 additions & 2 deletions buzz/events/doctype/buzz_team/test_buzz_team.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,8 +177,9 @@ def test_leaves_team_empty_when_the_user_has_two_teams(self):
create_owned_team("Two Teams A", user)
create_owned_team("Two Teams B", user)

with self.assertRaises(frappe.MandatoryError):
self.insert_as(user, "Event Venue", "Ambiguous")
doc = self.insert_as(user, "Event Venue", "Ambiguous")

self.assertFalse(doc.team)

def test_explicit_team_wins_for_a_multi_team_user(self):
user = create_user("picks-a-team@example.com", "Tenant")
Expand Down
3 changes: 1 addition & 2 deletions buzz/events/doctype/event_venue/event_venue.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,14 +76,13 @@
"fieldtype": "Link",
"label": "Team",
"options": "Buzz Team",
"reqd": 1,
"search_index": 1
}
],
"grid_page_length": 50,
"index_web_pages_for_search": 1,
"links": [],
"modified": "2026-07-31 12:00:00.000000",
"modified": "2026-09-08 12:00:00.000000",
"modified_by": "Administrator",
"module": "Events",
"name": "Event Venue",
Expand Down
2 changes: 1 addition & 1 deletion buzz/events/doctype/event_venue/event_venue.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ class EventVenue(Document):
google_maps_embed_code: DF.Code | None
latitude: DF.Float
longitude: DF.Float
team: DF.Link
team: DF.Link | None
type: DF.Literal["Embed Google Maps", "Open Street Map"]
# end: auto-generated types

Expand Down
5 changes: 4 additions & 1 deletion dashboard/src/types/Events/BuzzEvent.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import type { SponsorshipDeckItem } from "../Proposals/SponsorshipDeckItem"
import type { EventCoHost } from "./EventCoHost"
import type { EventFeaturedSpeaker } from "./EventFeaturedSpeaker"
import type { EventPaymentGateway } from "./EventPaymentGateway"
import type { ScheduleItem } from "./ScheduleItem"
Expand All @@ -25,7 +26,9 @@ export interface BuzzEvent {
/** Banner Image : Attach Image */
banner_image?: string
/** Host : Link - Event Host */
host: string
host?: string
/** Co-hosts : Table - Event CoHost */
co_hosts?: EventCoHost[]
/** Venue : Link - Event Venue */
venue?: string
/** Start Date : Date */
Expand Down
14 changes: 14 additions & 0 deletions dashboard/src/types/Events/EventCoHost.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
export interface EventCoHost {
name: string
creation: string
modified: string
owner: string
modified_by: string
docstatus: 0 | 1 | 2
parent?: string
parentfield?: string
parenttype?: string
idx?: number
/** Host : Link - Event Host */
host: string
}
Loading