From 663916e8d33170db1fd724233e70f5dbe0f78f3f Mon Sep 17 00:00:00 2001 From: Mia Bennett Date: Wed, 5 Aug 2026 10:10:19 +0930 Subject: [PATCH] fix(public_events): fetch the metadata permission from the staff API (PPT-2247) The permission lives in the staff API `event_metadatas` table, it is not part of a calendar event, so it never appeared in the Bookings cache. The default of PRIVATE was applied to every event and the public list was always empty. Permissions are now looked up with `StaffAPI.query_metadata`, passing the id, ical_uid and recurring_event_id of every cached event as `event_ref` (batched, so the query string stays small). Metadata that belongs to an event instance takes precedence over the metadata of the recurring master, so a single public occurrence no longer makes the whole series public. Events that are private on the calendar remain excluded, their title and host have already been masked by the Bookings driver. A permission can also change without the event changing, and a driver only publishes a status when the value has changed, so the `bookings` subscription can't keep the cache fresh on its own. The filter is now re-applied on a schedule (`metadata_refresh_minutes`) and whenever `update_public_events` is called. --- drivers/place/public_events.cr | 131 ++++++++++++++- drivers/place/public_events_readme.md | 30 +++- drivers/place/public_events_spec.cr | 229 +++++++++++++++++++++++--- 3 files changed, 351 insertions(+), 39 deletions(-) diff --git a/drivers/place/public_events.cr b/drivers/place/public_events.cr index ef30882c6f4..c85d958712e 100644 --- a/drivers/place/public_events.cr +++ b/drivers/place/public_events.cr @@ -13,11 +13,37 @@ class Place::PublicEvents < PlaceOS::Driver accessor bookings : Bookings_1 accessor calendar : Calendar_1 + # the permission field lives in the staff API `EventMetadata` table, it is not + # part of a calendar event, so it can't be included in the Bookings cache + accessor staff_api : StaffAPI_1 + + alias Permission = PlaceOS::Model::EventMetadata::Permission + + # the number of event references we send to the staff API in a single request, + # this keeps the query string well below the HTTP request line size limit + REF_BATCH_SIZE = 50 + + default_settings({ + # how often we re-check the event metadata permissions + metadata_refresh_minutes: 5, + }) + @all_bookings : Array(PublicEvent) = [] of PublicEvent @public_event_ids : Set(String) = Set(String).new + @filter_mutex : Mutex = Mutex.new bind Bookings_1, :bookings, :on_bookings_change + def on_update + refresh_minutes = setting?(Int32, :metadata_refresh_minutes) || 5 + + # a permission can be changed without the event changing, and the Bookings + # driver only publishes `bookings` when the events have actually changed, + # so we can't rely on the subscription alone to keep the cache fresh + schedule.clear + schedule.every(refresh_minutes.minutes) { filter_and_cache } if refresh_minutes > 0 + end + private def on_bookings_change(_subscription, new_value : String) @all_bookings = Array(PublicEvent).from_json(new_value) filter_and_cache @@ -26,21 +52,90 @@ class Place::PublicEvents < PlaceOS::Driver end private def filter_and_cache : Array(PublicEvent) - logger.debug { "received #{@all_bookings.size} total events from bookings" } + @filter_mutex.synchronize do + events = @all_bookings + logger.debug { "received #{events.size} total events from bookings" } + + permissions = event_permissions(events) + + public_events = events.select do |event| + # a calendar event marked private has had its title and host masked by + # the Bookings driver, so there is nothing useful (or safe) to publish + permission_for(event, permissions).public? && !event.private? + end + + logger.debug { "#{public_events.size} events have PUBLIC permission" } + + @public_event_ids = public_events.compact_map(&.id).to_set + self["public_events"] = public_events + public_events + end + end + + # Looks the metadata permission up in the staff API. + # Returns the instance level permissions and the recurring master permissions + # separately, so instance metadata can take precedence over the master. + private def event_permissions(events : Array(PublicEvent)) : Permissions + by_event = {} of String => Permission + by_master = {} of String => Permission + permissions = {by_event, by_master} + return permissions if events.empty? + + system_id = system.id + refs = events.flat_map { |event| [event.id, event.ical_uid, event.recurring_event_id] }.compact + refs.uniq! + return permissions if refs.empty? + + refs.each_slice(REF_BATCH_SIZE) do |batch| + metadata(system_id, batch).each do |meta| + by_event[meta.event_id] = meta.permission + by_event[meta.ical_uid] = meta.permission + + # only the metadata of the series master applies to the whole series, + # instances have their own metadata which also references the master + if (master_id = meta.recurring_master_id) && master_id == meta.event_id + by_master[master_id] = meta.permission + if resource_master_id = meta.resource_master_id + by_master[resource_master_id] = meta.permission + end + end + end + end + + permissions + end + + private def metadata(system_id : String, event_ref : Array(String)) : Array(EventMetadata) + response = staff_api.query_metadata(system_id: system_id, event_ref: event_ref).get + Array(EventMetadata).from_json(response.to_json) + end + + private def permission_for(event : PublicEvent, permissions : Permissions) : Permission + by_event, by_master = permissions - public_events = @all_bookings.select(&.permission.public?) + if (event_id = event.id) && (permission = by_event[event_id]?) + return permission + end + + if (ical_uid = event.ical_uid) && (permission = by_event[ical_uid]?) + return permission + end - logger.debug { "#{public_events.size} events have PUBLIC permission" } + if (master_id = event.recurring_event_id) && (permission = by_master[master_id]?) + return permission + end - @public_event_ids = public_events.compact_map(&.id).to_set - self["public_events"] = public_events - public_events + Permission::PRIVATE end # Forces a Bookings re-poll then re-applies the public filter. @[Security(Level::Administrator)] def update_public_events : Nil bookings.poll_events.get + + # the re-poll only publishes `bookings` if the events have changed, so we + # always re-apply the filter to pick up metadata permission changes + filter_and_cache end # Appends an external attendee to the calendar event. @@ -68,7 +163,20 @@ class Place::PublicEvents < PlaceOS::Driver true end - alias Permission = PlaceOS::Model::EventMetadata::Permission + alias Permissions = Tuple(Hash(String, Permission), Hash(String, Permission)) + + # The subset of the staff API event metadata we require. + # NOTE:: we don't use `PlaceOS::Model::EventMetadata` as it is a database + # backed model that renders linked bookings on serialisation. + private struct EventMetadata + include JSON::Serializable + + getter event_id : String + getter ical_uid : String + getter recurring_master_id : String? + getter resource_master_id : String? + getter permission : Permission = Permission::PRIVATE + end # Fields that are safe to expose publicly. private struct PublicEvent @@ -83,7 +191,14 @@ class Place::PublicEvents < PlaceOS::Driver getter timezone : String? getter? all_day : Bool = false + # used for matching metadata and filtering, never exposed publicly @[JSON::Field(ignore_serialize: true)] - getter permission : Permission = Permission::PRIVATE + getter ical_uid : String? = nil + + @[JSON::Field(ignore_serialize: true)] + getter recurring_event_id : String? = nil + + @[JSON::Field(ignore_serialize: true)] + getter? private : Bool = false end end diff --git a/drivers/place/public_events_readme.md b/drivers/place/public_events_readme.md index f053d82b328..16dc690ccfd 100644 --- a/drivers/place/public_events_readme.md +++ b/drivers/place/public_events_readme.md @@ -3,7 +3,7 @@ Docs on the PlaceOS Public Events driver. This driver filters the Bookings event cache down to publicly visible events and handles guest registration, enabling unauthenticated access to selected calendar events. -* Subscribes to the Bookings driver's `:bookings` status and filters events where `private` is `false` +* Subscribes to the Bookings driver's `:bookings` status and filters events whose staff API event metadata `permission` is `PUBLIC` * Caches the filtered set of public events (with a reduced set of safe fields) as the `:public_events` status * Provides a `register_attendee` function for appending external (guest) attendees to a public event via the Calendar driver @@ -14,6 +14,7 @@ Requires the following drivers in the same system: * Bookings - for the room/calendar event cache and polling * Calendar - for reading and updating calendar events when registering attendees +* StaffAPI - for looking up the event metadata `permission` field The system must also have a calendar email configured (used as the `calendar_id` when calling the Calendar driver). @@ -21,9 +22,28 @@ The system must also have a calendar email configured (used as the `calendar_id` ## How It Works 1. The Bookings driver polls the calendar and publishes all events to its `:bookings` status -2. PublicEvents receives the update via the subscription binding and filters to non-private events (`private == false`) -3. The filtered events are stored in `:public_events` with only safe, non-sensitive fields exposed: `id`, `title`, `body`, `event_start`, `event_end`, `location`, `timezone`, `all_day` -4. When a guest registers, `register_attendee` checks the event is in the public set, fetches it from the Calendar driver, appends the attendee, and writes it back +2. PublicEvents receives the update via the subscription binding +3. The `permission` field lives in the staff API `event_metadatas` table, it is not part of a calendar event, so it is never present in the Bookings cache. PublicEvents looks it up with `StaffAPI.query_metadata`, passing the `id`, `ical_uid` and `recurring_event_id` of every cached event as `event_ref` (batched to keep the query string small) +4. Events are kept where the metadata permission is `PUBLIC`: + * `PRIVATE` (the default when no metadata exists) and `OPEN` are excluded. `OPEN` only allows users in the same tenant to join, so it is not suitable for unauthenticated access + * metadata that belongs to an event instance takes precedence over the metadata of the recurring master (i.e. `recurring_master_id == event_id`), so a single public occurrence does not make the whole series public + * events marked private on the calendar are always excluded, the Bookings driver has already masked their title and host +5. The filtered events are stored in `:public_events` with only safe, non-sensitive fields exposed: `id`, `title`, `body`, `event_start`, `event_end`, `location`, `timezone`, `all_day` +6. When a guest registers, `register_attendee` checks the event is in the public set, fetches it from the Calendar driver, appends the attendee, and writes it back + +A permission can be changed without the event itself changing, and a driver only publishes a status when its value has changed, so the `:bookings` subscription can't be relied on to keep the cache fresh. The metadata permissions are re-checked: + +* whenever the Bookings cache changes +* every `metadata_refresh_minutes` (defaults to 5, set to 0 to disable) +* when `update_public_events` is called + + +## Settings + +```yaml +# how often we re-check the event metadata permissions, 0 to disable +metadata_refresh_minutes: 5 +``` ## Public System Usage @@ -51,4 +71,4 @@ args: ### `update_public_events : Nil` -Administrator-only. Triggers a Bookings re-poll and repopulates the public events cache via the subscription binding. \ No newline at end of file +Administrator-only. Triggers a Bookings re-poll and repopulates the public events cache via the subscription binding. diff --git a/drivers/place/public_events_spec.cr b/drivers/place/public_events_spec.cr index e1512df3a68..d2971cdcb80 100644 --- a/drivers/place/public_events_spec.cr +++ b/drivers/place/public_events_spec.cr @@ -5,6 +5,7 @@ DriverSpecs.mock_driver "Place::PublicEvents" do system({ Bookings: {BookingsMock}, Calendar: {CalendarMock}, + StaffAPI: {StaffAPIMock}, }) # BookingsMock publishes its events in on_load, which triggers the @@ -15,43 +16,81 @@ DriverSpecs.mock_driver "Place::PublicEvents" do # Test 1: subscription populates the public events cache automatically # ----------------------------------------------------------------------- events = status[:public_events].as_a - events.size.should eq(1) - events[0]["id"].as_s.should eq("evt-public-1") + event_ids = events.map { |event| event["id"].as_s } + event_ids.should eq(["evt-public-1", "evt-series-instance", "evt-series-public-instance"]) events[0]["title"].as_s.should eq("Public Conference") # ----------------------------------------------------------------------- - # Test 2: private events are excluded + # Test 2: the permission field is not in the Bookings payload, so it has + # to be fetched from the staff API # ----------------------------------------------------------------------- - events.none? { |e| e["id"].as_s == "evt-private-no-ext" }.should be_true + queried = system(:StaffAPI)[:queried_refs].as_a.map(&.as_s) + queried.size.should eq(queried.uniq.size) + queried.should contain("evt-public-1") + queried.should contain("uid-public-1") + queried.should contain("evt-series-master") # ----------------------------------------------------------------------- - # Test 3: events explicitly marked private are also excluded + # Test 3: events without PUBLIC metadata permission are excluded # ----------------------------------------------------------------------- - events.none? { |e| e["id"].as_s == "evt-private-explicit" }.should be_true + # metadata says private + event_ids.should_not contain("evt-private-meta") + # metadata says open (tenant users only, not the public) + event_ids.should_not contain("evt-open-meta") + # no metadata at all, defaults to private + event_ids.should_not contain("evt-no-meta") # ----------------------------------------------------------------------- - # Test 4: only allowlisted fields are present in the public cache + # Test 4: instance metadata takes precedence over the recurring master + # ----------------------------------------------------------------------- + # the master is PUBLIC but this instance has its own PRIVATE metadata + event_ids.should_not contain("evt-series-instance-private") + # a sibling instance being PUBLIC must not make the whole series public + event_ids.should_not contain("evt-series-sibling") + + # ----------------------------------------------------------------------- + # Test 5: calendar private events are excluded, even when marked PUBLIC + # (the Bookings driver has already masked the title and host) + # ----------------------------------------------------------------------- + event_ids.should_not contain("evt-public-but-private-cal") + + # ----------------------------------------------------------------------- + # Test 6: only allowlisted fields are present in the public cache # ----------------------------------------------------------------------- events[0]["event_start"].as_i64.should be > 0_i64 events[0]["event_end"].as_i64.should be > 0_i64 + events[0]["body"]?.should_not be_nil events[0]["attendees"]?.should be_nil events[0]["host"]?.should be_nil - events[0]["body"]?.should_not be_nil events[0]["online_meeting_url"]?.should be_nil events[0]["creator"]?.should be_nil + events[0]["private"]?.should be_nil + events[0]["permission"]?.should be_nil + events[0]["ical_uid"]?.should be_nil + events[0]["recurring_event_id"]?.should be_nil # ----------------------------------------------------------------------- - # Test 5: update_public_events triggers a Bookings re-poll and returns nil; - # the cache is repopulated via the :bookings subscription binding. + # Test 7: update_public_events triggers a Bookings re-poll and re-checks the + # metadata permissions. + # + # A permission can change without the events changing, and the Bookings + # driver only publishes `bookings` when the value has changed, so the filter + # must be re-applied regardless of the subscription firing. + # StaffAPIMock marks `evt-no-meta` as PUBLIC from the second query onwards. # ----------------------------------------------------------------------- + system(:StaffAPI)[:query_count].as_i.should eq(1) + exec(:update_public_events).get sleep 200.milliseconds + + system(:StaffAPI)[:query_count].as_i.should eq(2) updated_events = status[:public_events].as_a - updated_events.size.should eq(1) - updated_events[0]["id"].as_s.should eq("evt-public-1") + updated_events.map { |event| event["id"].as_s }.should eq([ + "evt-public-1", "evt-no-meta", "evt-series-instance", "evt-series-public-instance", + ]) # ----------------------------------------------------------------------- - # Test 6: register_attendee appends the guest via the Calendar driver + # Test 8: register_attendee appends the guest via the Calendar driver # ----------------------------------------------------------------------- exec(:register_attendee, "evt-public-1", "Alice Smith", "alice@external.com").get.should be_true @@ -60,9 +99,9 @@ DriverSpecs.mock_driver "Place::PublicEvents" do attendees.any? { |a| a["name"].as_s == "Alice Smith" }.should be_true # ----------------------------------------------------------------------- - # Test 7: register_attendee returns false for unknown event IDs + # Test 9: register_attendee returns false for events that are not public # ----------------------------------------------------------------------- - exec(:register_attendee, "evt-private-no-ext", "Bob", "bob@example.com").get.should be_false + exec(:register_attendee, "evt-private-meta", "Bob", "bob@example.com").get.should be_false # Calendar must not have been called again — updated_attendees unchanged system(:Calendar)[:updated_attendees].as_a @@ -71,16 +110,56 @@ DriverSpecs.mock_driver "Place::PublicEvents" do end # :nodoc: -# Simulates the Bookings driver. Publishes a fixed set of three events on load -# so the PublicEvents driver's subscription fires immediately: -# - one non-private event (should appear in the cache) -# - two private events (should be excluded) +# A staff API event metadata record, as returned by `query_metadata` +struct MetadataFixture + include JSON::Serializable + + getter event_id : String + getter ical_uid : String + getter recurring_master_id : String? + getter resource_master_id : String? + getter permission : String + + def initialize( + @event_id, + @ical_uid, + @permission, + @recurring_master_id = nil, + @resource_master_id = nil, + ) + end + + # mirrors the staff API `by_events_or_master_ids` lookup + def matches?(refs : Array(String)) : Bool + return true if refs.includes?(event_id) || refs.includes?(ical_uid) + return true if (master = recurring_master_id) && refs.includes?(master) + return true if (master = resource_master_id) && refs.includes?(master) + false + end +end + +# :nodoc: +# Simulates the Bookings driver. Publishes a fixed set of events on load +# so the PublicEvents driver's subscription fires immediately. class BookingsMock < DriverSpecs::MockDriver def on_load + self[:bookings] = events + end + + def poll_events : Nil + # Re-publish the current bookings to exercise the subscription path. + # NOTE:: the payload is stable, so a re-poll will not publish a change + self[:bookings] = events + end + + # built once so that re-polling doesn't change the payload + private getter events : Array(PlaceCalendar::Event) do now = Time.utc - self[:bookings] = [ + [ + # metadata permission PUBLIC, should appear in the cache PlaceCalendar::Event.new( id: "evt-public-1", + ical_uid: "uid-public-1", host: "organizer@company.com", title: "Public Conference", event_start: now + 1.day, @@ -88,28 +167,126 @@ class BookingsMock < DriverSpecs::MockDriver body: "Join us for the annual public conference.", attendees: [PlaceCalendar::Event::Attendee.new(name: "Internal Person", email: "internal@company.com")], ), + # metadata permission PRIVATE PlaceCalendar::Event.new( - id: "evt-private-no-ext", + id: "evt-private-meta", + ical_uid: "uid-private-meta", host: "team@company.com", title: "Internal Meeting", event_start: now + 2.days, event_end: now + 2.days + 1.hour, - private: true, ), + # metadata permission OPEN (tenant users only) + PlaceCalendar::Event.new( + id: "evt-open-meta", + ical_uid: "uid-open-meta", + host: "team@company.com", + title: "Lunch and Learn", + event_start: now + 2.days, + event_end: now + 2.days + 1.hour, + ), + # no metadata record exists for this event PlaceCalendar::Event.new( - id: "evt-private-explicit", + id: "evt-no-meta", + ical_uid: "uid-no-meta", host: "exec@company.com", title: "Executive Briefing", event_start: now + 3.days, event_end: now + 3.days + 1.hour, + ), + # inherits the PUBLIC permission of the recurring master metadata + PlaceCalendar::Event.new( + id: "evt-series-instance", + ical_uid: "uid-series-instance", + recurring_event_id: "evt-series-master", + host: "organizer@company.com", + title: "Weekly Public Tour", + event_start: now + 4.days, + event_end: now + 4.days + 1.hour, + ), + # the master is PUBLIC, however this instance has its own PRIVATE metadata + PlaceCalendar::Event.new( + id: "evt-series-instance-private", + ical_uid: "uid-series-instance-private", + recurring_event_id: "evt-series-master", + host: "organizer@company.com", + title: "Weekly Public Tour (cancelled to the public)", + event_start: now + 11.days, + event_end: now + 11.days + 1.hour, + ), + # this instance is PUBLIC, its siblings are not + PlaceCalendar::Event.new( + id: "evt-series-public-instance", + ical_uid: "uid-series-public-instance", + recurring_event_id: "evt-series-master-2", + host: "organizer@company.com", + title: "Weekly Standup (open day)", + event_start: now + 5.days, + event_end: now + 5.days + 1.hour, + ), + PlaceCalendar::Event.new( + id: "evt-series-sibling", + ical_uid: "uid-series-sibling", + recurring_event_id: "evt-series-master-2", + host: "organizer@company.com", + title: "Weekly Standup", + event_start: now + 12.days, + event_end: now + 12.days + 1.hour, + ), + # marked PUBLIC, but private on the calendar so title / host are masked + PlaceCalendar::Event.new( + id: "evt-public-but-private-cal", + ical_uid: "uid-public-but-private-cal", + host: "Private", + title: "Private", + event_start: now + 6.days, + event_end: now + 6.days + 1.hour, private: true, ), ] end +end - def poll_events : Nil - # Re-publish current bookings to exercise the subscription path. - on_load +# :nodoc: +# Simulates the staff API driver, returning event metadata for the requested +# event references. Note: the recurring master metadata is the record where +# `recurring_master_id == event_id`. +class StaffAPIMock < DriverSpecs::MockDriver + METADATA = [ + MetadataFixture.new("evt-public-1", "uid-public-1", "public"), + MetadataFixture.new("evt-private-meta", "uid-private-meta", "private"), + MetadataFixture.new("evt-open-meta", "uid-open-meta", "open"), + MetadataFixture.new("evt-series-master", "uid-series-master", "public", + recurring_master_id: "evt-series-master", resource_master_id: "res-series-master"), + MetadataFixture.new("evt-series-instance-private", "uid-series-instance-private", "private", + recurring_master_id: "evt-series-master"), + MetadataFixture.new("evt-series-public-instance", "uid-series-public-instance", "public", + recurring_master_id: "evt-series-master-2"), + MetadataFixture.new("evt-public-but-private-cal", "uid-public-but-private-cal", "public"), + ] + + # simulates someone marking `evt-no-meta` as public after the initial lookup + LATE_METADATA = MetadataFixture.new("evt-no-meta", "uid-no-meta", "public") + + @queries : Int32 = 0 + + def query_metadata( + period_start : Int64? = nil, + period_end : Int64? = nil, + field_name : String? = nil, + value : String? = nil, + system_id : String? = nil, + event_ref : Array(String)? = nil, + ) : Array(MetadataFixture) + refs = event_ref || [] of String + @queries += 1 + self[:query_count] = @queries + self[:queried_system_id] = system_id + self[:queried_refs] = refs + return [] of MetadataFixture if refs.empty? + + metadata = @queries > 1 ? METADATA + [LATE_METADATA] : METADATA + metadata.select &.matches?(refs) end end