diff --git a/drivers/place/public_events.cr b/drivers/place/public_events.cr index ef30882c6f..c85d958712 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 f053d82b32..16dc690ccf 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 e1512df3a6..d2971cdcb8 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