From d13c5f72a4b9e6d40e573a0cf49d39b72cafa776 Mon Sep 17 00:00:00 2001 From: lucs7 Date: Fri, 9 Oct 2026 22:04:00 +0200 Subject: [PATCH] fix(calendar): send reservation emails as iTip REQUEST and CANCEL Every reservation email attached an ICS with METHOD:REQUEST, so Outlook offered to add the event again when a reservation was cancelled. Clients also differ on what they accept: Outlook only recognises REQUEST, and a REQUEST needs an ATTENDEE to follow RFC 5546. Add an IcsMethod enum and let each email class pick its method: CANCEL (STATUS:CANCELLED) for deletion emails and REQUEST for everything else. Feeds and exports keep PUBLISH. REQUEST and CANCEL name the site's default from address as the organizer and whoever receives the email as the only attendee: the owner, an invitee, a guest or a share recipient. The attendee needs action and no RSVP is requested, because invitations are answered through the links in the email. The ICS no longer carries a SEQUENCE. Thunderbird did not match a cancellation to its event when SEQUENCE was set, and a missing one counts as 0, so DTSTAMP orders the messages. A series without BookedBy() (from BuildSeries()) now falls back to the owner's session, with a timezone, instead of failing in the summary formatting. The log for a missing default from address is kept. The owner's update email used the subject meant for administrators ("Notification: Reservation Updated for ..."). Use the one for the owner, like the created and deleted emails do. Assisted-by: Claude:claude-sonnet-5-5 --- Pages/Export/CalendarExportDisplay.php | 32 +++- .../Schedule/iCalendarReservationView.php | 34 ++-- lib/Email/IEmailMessage.php | 2 +- .../Messages/ReservationDeletedEmail.php | 7 + .../Messages/ReservationEmailMessage.php | 27 ++- .../Messages/ReservationUpdatedEmail.php | 2 +- src/Calendar/IcsMethod.php | 15 ++ .../CalendarExportPresenterTest.php | 176 ++++++++++++++++-- tests/fakes/namespace.php | 1 + tests/lib/Email/EmailServiceTest.php | 4 +- .../Messages/ReservationEmailMessageTest.php | 170 +++++++++++++++-- tests/src/Calendar/IcsMethodTest.php | 24 +++ 12 files changed, 436 insertions(+), 58 deletions(-) create mode 100644 src/Calendar/IcsMethod.php create mode 100644 tests/src/Calendar/IcsMethodTest.php diff --git a/Pages/Export/CalendarExportDisplay.php b/Pages/Export/CalendarExportDisplay.php index b592b15e32..20c18961a0 100644 --- a/Pages/Export/CalendarExportDisplay.php +++ b/Pages/Export/CalendarExportDisplay.php @@ -1,5 +1,6 @@ '-//LibreBooking//NONSGML ' . Configuration::VERSION . '//EN', - 'METHOD' => 'REQUEST', + 'METHOD' => $method->value, ]); if ($calendarName !== null && $calendarName !== '') { @@ -54,10 +57,18 @@ public function Render(array $reservations, ?string $calendarName = null): strin $event->add('DTEND', $res->DateEnd->Format($isoFormat)); $event->add('LAST-MODIFIED', $res->LastModified->Format($isoFormat)); $event->add('LOCATION', $res->Location); - $event->add('ORGANIZER', 'mailto:' . $res->OrganizerEmail, ['CN' => $res->Organizer]); - $event->add('STATUS', $res->IsPending ? 'TENTATIVE' : 'CONFIRMED'); + if ($method !== IcsMethod::PUBLISH && !empty($res->OrganizerSentBy)) { + $event->add('ORGANIZER', 'mailto:' . $res->OrganizerSentBy, ['CN' => $res->SenderName]); + } elseif (!empty($res->OrganizerEmail)) { + $organizerParams = ['CN' => $res->Organizer]; + if (!empty($res->OrganizerSentBy)) { + // RFC 5545 ยง3.2.18: the site's address sends on the owner's behalf + $organizerParams['SENT-BY'] = 'mailto:' . $res->OrganizerSentBy; + } + $event->add('ORGANIZER', 'mailto:' . $res->OrganizerEmail, $organizerParams); + } + $event->add('STATUS', $method === IcsMethod::CANCEL ? 'CANCELLED' : ($res->IsPending ? 'TENTATIVE' : 'CONFIRMED')); $event->add('SUMMARY', $res->Summary); - $event->add('SEQUENCE', 0); $event->add('URL', $res->ReservationUrl); $event->add('X-MICROSOFT-CDO-BUSYSTATUS', 'BUSY'); @@ -88,6 +99,17 @@ public function Render(array $reservations, ?string $calendarName = null): strin } } + $attendeeEmail = $res->AttendeeEmail ?? $res->OrganizerEmail; + if ($method !== IcsMethod::PUBLISH && !empty($res->OrganizerSentBy) && !empty($attendeeEmail)) { + $attendeeName = $res->AttendeeEmail ? $res->AttendeeName : $res->Organizer; + $event->add('ATTENDEE', 'mailto:' . $attendeeEmail, array_filter([ + 'CN' => $attendeeName, + 'ROLE' => 'REQ-PARTICIPANT', + 'PARTSTAT' => 'NEEDS-ACTION', + 'RSVP' => 'FALSE', + ])); + } + if ($res->StartReminder !== null) { $alarm = new VAlarm($vcal, 'VALARM'); $event->add($alarm); diff --git a/lib/Application/Schedule/iCalendarReservationView.php b/lib/Application/Schedule/iCalendarReservationView.php index 8f6470d119..a28ab2e784 100644 --- a/lib/Application/Schedule/iCalendarReservationView.php +++ b/lib/Application/Schedule/iCalendarReservationView.php @@ -9,6 +9,10 @@ class iCalendarReservationView public $Description; public $Organizer; public $OrganizerEmail; + public $OrganizerSentBy; + public $SenderName; + public $AttendeeName; + public $AttendeeEmail; public $RecurRule; public $ReferenceNumber; public $Summary; @@ -60,12 +64,6 @@ public function __construct($res, UserSession $currentUser, IPrivacyFilter $priv $this->ExportFactory = PluginManager::Instance()->LoadExport(); $privateNotice = 'Private'; - $privateEmail = Configuration::Instance()->GetKey(ConfigKeys::EMAIL_DEFAULT_FROM_ADDRESS); - static $missingPrivateOrganizerEmailLogged = false; - if (!$canViewUser && $privateEmail === '' && !$missingPrivateOrganizerEmailLogged) { - Log::Error('ICS private organizer email is not configured. Set the email.default.from.address setting.'); - $missingPrivateOrganizerEmailLogged = true; - } $this->Classification = method_exists($this->ExportFactory, 'GetIcalendarClassification') ? $this->ExportFactory->GetIcalendarClassification($res) : 'PUBLIC'; if ($res->DateCreated) { @@ -78,9 +76,23 @@ public function __construct($res, UserSession $currentUser, IPrivacyFilter $priv $this->DateStart = $res->StartDate; $this->Summary = $canViewDetails ? $factory->Format($res, $summaryFormat) : $privateNotice; $this->Description = $canViewDetails ? ($res->Description ?? '') : $privateNotice; - $fullName = new FullName($res->OwnerFirstName, $res->OwnerLastName); - $this->Organizer = $canViewUser ? $fullName->__toString() : $privateNotice; - $this->OrganizerEmail = $canViewUser ? $res->OwnerEmailAddress : $privateEmail; + + $defaultFromAddress = Configuration::Instance()->GetKey(ConfigKeys::EMAIL_DEFAULT_FROM_ADDRESS); + if ($canViewUser) { + $ownerName = new FullName($res->OwnerFirstName, $res->OwnerLastName); + $this->Organizer = $ownerName->__toString(); + $this->OrganizerEmail = $res->OwnerEmailAddress; + $this->OrganizerSentBy = $defaultFromAddress; + $this->SenderName = Configuration::Instance()->GetKey(ConfigKeys::EMAIL_DEFAULT_FROM_NAME); + } else { + $this->Organizer = $privateNotice; + $this->OrganizerEmail = $defaultFromAddress; + static $missingPrivateOrganizerEmailLogged = false; + if ($defaultFromAddress === '' && !$missingPrivateOrganizerEmailLogged) { + Log::Error('ICS private organizer email is not configured. Set the email.default.from.address setting.'); + $missingPrivateOrganizerEmailLogged = true; + } + } $this->RecurRule = $this->CreateRecurRule($res); $this->ReferenceNumber = $res->ReferenceNumber; $this->ReservationUrl = sprintf( @@ -97,10 +109,6 @@ public function __construct($res, UserSession $currentUser, IPrivacyFilter $priv $this->LastModified = empty($res->ModifiedDate) || $res->ModifiedDate->ToString() == '' ? $this->DateCreated : $res->ModifiedDate; $this->IsPending = $res->RequiresApproval; - if ($canViewUser && $res->OwnerId == $currentUser->UserId) { - $this->OrganizerEmail = str_replace('@', '-noreply@', $res->OwnerEmailAddress); - } - $this->ExtraIcalLines = method_exists($this->ExportFactory, 'GetIcalendarExtraLines') ? $this->ExportFactory->GetIcalendarExtraLines($res) : null; } diff --git a/lib/Email/IEmailMessage.php b/lib/Email/IEmailMessage.php index b3e6f3f018..11c985b5ae 100644 --- a/lib/Email/IEmailMessage.php +++ b/lib/Email/IEmailMessage.php @@ -46,7 +46,7 @@ public function ReplyTo(); * @abstract * @param string $contents * @param string $fileName - * @param string|null $mimeType Full Content-Type value (e.g. 'text/calendar; method=REQUEST'). Null auto-detects from $fileName. + * @param string|null $mimeType Full Content-Type value (e.g. 'text/calendar; method=PUBLISH'). Null auto-detects from $fileName. */ public function AddStringAttachment($contents, $fileName, ?string $mimeType = null); diff --git a/lib/Email/Messages/ReservationDeletedEmail.php b/lib/Email/Messages/ReservationDeletedEmail.php index 83c9f513f8..93f13205ed 100644 --- a/lib/Email/Messages/ReservationDeletedEmail.php +++ b/lib/Email/Messages/ReservationDeletedEmail.php @@ -1,5 +1,7 @@ UserPreferences = $this->reservationOwner->GetPreferences(); $rv->OwnerEmailAddress = $this->reservationOwner->EmailAddress(); - $icsView = new iCalendarReservationView($rv, $this->reservationSeries->BookedBy(), new NullPrivacyFilter()); + // BookedBy() is null for series from ReservationRepository::BuildSeries(); use the + // owner, with a timezone for the {startdate}/{enddate} summary placeholders. + $currentUser = $this->reservationSeries->BookedBy(); + if ($currentUser === null) { + $currentUser = new UserSession($this->reservationOwner->Id()); + $currentUser->Timezone = $this->timezone; + } + $icsView = new iCalendarReservationView($rv, $currentUser, new NullPrivacyFilter()); + // the attendee is whoever the email goes to: the owner, or an invitee, guest or share recipient + $recipient = $this->To(); + $recipient = is_array($recipient) ? $recipient[0] : $recipient; + $icsView->AttendeeName = $recipient->Name(); + $icsView->AttendeeEmail = $recipient->Address(); + + $method = $this->GetIcsMethod(); $display = new CalendarExportDisplay(); - $icsContents = $display->Render([$icsView]); + $icsContents = $display->Render([$icsView], null, $method); $this->AddStringAttachment( contents: $icsContents, fileName: 'reservation.ics', - mimeType: 'text/calendar; charset=UTF-8; method=REQUEST' + mimeType: "text/calendar; charset=UTF-8; method={$method->value}" ); } + + protected function GetIcsMethod(): IcsMethod + { + return IcsMethod::REQUEST; + } } diff --git a/lib/Email/Messages/ReservationUpdatedEmail.php b/lib/Email/Messages/ReservationUpdatedEmail.php index c9951053b7..640f9d7706 100644 --- a/lib/Email/Messages/ReservationUpdatedEmail.php +++ b/lib/Email/Messages/ReservationUpdatedEmail.php @@ -6,7 +6,7 @@ class ReservationUpdatedEmail extends ReservationEmailMessage { public function Subject() { - return $this->Translate('ReservationUpdatedAdminSubjectWithResource', [$this->primaryResource->GetName()]); + return $this->Translate('ReservationUpdatedSubjectWithResource', [$this->primaryResource->GetName()]); } protected function GetTemplateName() diff --git a/src/Calendar/IcsMethod.php b/src/Calendar/IcsMethod.php new file mode 100644 index 0000000000..f893cb0755 --- /dev/null +++ b/src/Calendar/IcsMethod.php @@ -0,0 +1,15 @@ +presenter->PageLoad($this->fakeUser); } - public function testOrganizerIsOwnerIfCurrentUserIsNotOrganizer() + public function testOrganizerIsTheReservationOwnerWithSentByAsTheConfiguredDefaultAddress() { - // this fixes a bug in outlook which prevents you from adding a meeting that you are the organizer of + // ORGANIZER is the owner; SENT-BY is the site's default address. + $this->fakeConfig->SetKey(ConfigKeys::EMAIL_DEFAULT_FROM_ADDRESS, 'bookings@example.com'); + $this->fakeConfig->SetKey(ConfigKeys::EMAIL_DEFAULT_FROM_NAME, 'Example Bookings'); + $user = new FakeUserSession(); $res = new ReservationItemView(); - $res->OwnerId = $user->UserId + 1; + $res->OwnerId = $user->UserId; $res->OwnerFirstName = 'f'; $res->OwnerLastName = 'l'; $res->OwnerEmailAddress = 'e@m.com'; $reservationView = new iCalendarReservationView($res, $user, $this->privacyFilter); - $this->assertEquals($res->OwnerEmailAddress, $reservationView->OrganizerEmail); - $fullName = new FullName($res->OwnerFirstName, $res->OwnerLastName); - $this->assertEquals($fullName->__toString(), $reservationView->Organizer); + $this->assertEquals('e@m.com', $reservationView->OrganizerEmail); + $this->assertEquals('f l', $reservationView->Organizer); + $this->assertEquals('bookings@example.com', $reservationView->OrganizerSentBy); } - public function testOrganizerIsDefaultedIfCurrentUserIsOrganizer() + public function testOrganizerFallsBackToTheDefaultAddressWithNoSentByWhenPrivacyHidesTheOwner() { - // this fixes a bug in outlook which prevents you from adding a meeting that you are the organizer of + $this->fakeConfig->SetKey(ConfigKeys::EMAIL_DEFAULT_FROM_ADDRESS, 'bookings@example.com'); + $this->fakeConfig->SetKey(ConfigKeys::EMAIL_DEFAULT_FROM_NAME, 'Example Bookings'); + $user = new FakeUserSession(); $res = new ReservationItemView(); $res->OwnerId = $user->UserId; @@ -120,10 +127,55 @@ public function testOrganizerIsDefaultedIfCurrentUserIsOrganizer() $res->OwnerLastName = 'l'; $res->OwnerEmailAddress = 'e@m.com'; + $this->privacyFilter->_CanViewUser = false; + $reservationView = new iCalendarReservationView($res, $user, $this->privacyFilter); + + $this->assertEquals('bookings@example.com', $reservationView->OrganizerEmail); + $this->assertEquals('Private', $reservationView->Organizer); + // no real identity, so nothing to send on behalf of + $this->assertNull($reservationView->OrganizerSentBy); + } + + public function testOrganizerIsOmittedFromRenderedOutputWhenOwnerHasNoEmailOnFile() + { + // an owner without an email gets no ORGANIZER rather than an empty "mailto:" + $user = new FakeUserSession(); + $res = new ReservationItemView(); + $res->StartDate = Date::Now(); + $res->EndDate = Date::Now()->AddHours(1); + $reservationView = new iCalendarReservationView($res, $user, $this->privacyFilter); - $this->assertEquals('e-noreply@m.com', $reservationView->OrganizerEmail); - $fullName = new FullName($res->OwnerFirstName, $res->OwnerLastName); - $this->assertEquals($fullName->__toString(), $reservationView->Organizer); + $this->fakeConfig->_ScriptUrl = 'https://example.com/Web'; + $display = new CalendarExportDisplay(); + $ics = $display->Render([$reservationView]); + + $this->assertStringNotContainsString('ORGANIZER', $ics); + } + + public function testOrganizerRendersWithSentByAndNoAttendeeIsAdded() + { + // the owner is only ever ORGANIZER, never ATTENDEE + $this->fakeConfig->SetKey(ConfigKeys::EMAIL_DEFAULT_FROM_ADDRESS, 'bookings@example.com'); + + $user = new FakeUserSession(); + $res = new ReservationItemView(); + $res->OwnerId = $user->UserId; + $res->OwnerFirstName = 'f'; + $res->OwnerLastName = 'l'; + $res->OwnerEmailAddress = 'e@m.com'; + $res->StartDate = Date::Now(); + $res->EndDate = Date::Now()->AddHours(1); + + $this->privacyFilter->_CanViewDetails = true; + $reservationView = new iCalendarReservationView($res, $user, $this->privacyFilter); + + $this->fakeConfig->_ScriptUrl = 'https://example.com/Web'; + $display = new CalendarExportDisplay(); + $ics = $display->Render([$reservationView]); + $unfolded = str_replace("\r\n ", '', $ics); + + $this->assertMatchesRegularExpression('/ORGANIZER;CN=f l;SENT-BY="mailto:bookings@example\.com":mailto:e@m\.com/', $unfolded); + $this->assertStringNotContainsString('ATTENDEE', $ics); } public function testViewHidesDetailsWhenNoAccess() @@ -134,6 +186,7 @@ public function testViewHidesDetailsWhenNoAccess() $this->privacyFilter->_CanViewDetails = false; $this->privacyFilter->_CanViewUser = false; $this->fakeConfig->SetKey(ConfigKeys::EMAIL_DEFAULT_FROM_ADDRESS, 'noreply@example.com'); + $this->fakeConfig->SetKey(ConfigKeys::EMAIL_DEFAULT_FROM_NAME, 'LB'); $reservationView = new iCalendarReservationView($res, $user, $this->privacyFilter); @@ -143,8 +196,10 @@ public function testViewHidesDetailsWhenNoAccess() $this->assertEquals($res, $this->privacyFilter->_LastViewDetailsReservation); $this->assertEquals($res, $this->privacyFilter->_LastViewUserReservation); + // hidden owner: ORGANIZER is the default address, without SENT-BY $this->assertEquals('Private', $reservationView->Organizer); $this->assertEquals('noreply@example.com', $reservationView->OrganizerEmail); + $this->assertNull($reservationView->OrganizerSentBy); $this->assertEquals('Private', $reservationView->Summary); $this->assertEquals('Private', $reservationView->Description); } @@ -200,6 +255,7 @@ public function testAnonymousUserSeesPrivateWhenPublicReservationViewingIsDisabl // privacy.view.reservations=false (default) means anonymous users must not see any details $this->fakeConfig->SetKey(ConfigKeys::PRIVACY_VIEW_RESERVATIONS, false); $this->fakeConfig->SetKey(ConfigKeys::EMAIL_DEFAULT_FROM_ADDRESS, 'noreply@example.com'); + $this->fakeConfig->SetKey(ConfigKeys::EMAIL_DEFAULT_FROM_NAME, 'LB'); $this->privacyFilter->_CanViewDetails = true; $this->privacyFilter->_CanViewUser = true; @@ -207,8 +263,10 @@ public function testAnonymousUserSeesPrivateWhenPublicReservationViewingIsDisabl $this->assertEquals('Private', $reservationView->Summary); $this->assertEquals('Private', $reservationView->Description); + // anonymous viewers also lose the owner, as in the hidden-owner case $this->assertEquals('Private', $reservationView->Organizer); $this->assertEquals('noreply@example.com', $reservationView->OrganizerEmail); + $this->assertNull($reservationView->OrganizerSentBy); } public function testViewStoresRawTextInSummaryAndDescription() @@ -327,16 +385,16 @@ public function testExtraIcalLinesPreservesPropertyParametersAndNestedComponents $this->privacyFilter->_CanViewDetails = true; $reservationView = new iCalendarReservationView($res, $user, $this->privacyFilter, '{title}'); - // ATTENDEE carries parameters; VALARM is a nested component. Both must round-trip + // a parameterised property and a nested VALARM must round-trip // through Reader::read() rather than becoming malformed flat text. - $reservationView->ExtraIcalLines = "ATTENDEE;CN=JaneDoe;ROLE=REQ-PARTICIPANT:mailto:jane@example.com\r\n" + $reservationView->ExtraIcalLines = "CATEGORIES;LANGUAGE=en:Meeting\r\n" . "BEGIN:VALARM\r\nACTION:AUDIO\r\nTRIGGER:-PT15M\r\nEND:VALARM"; $this->fakeConfig->_ScriptUrl = 'https://example.com/Web'; $display = new CalendarExportDisplay(); $ics = $display->Render([$reservationView]); - $this->assertStringContainsString('ATTENDEE;CN=JaneDoe;ROLE=REQ-PARTICIPANT:mailto:jane@example.com', $ics); + $this->assertStringContainsString('CATEGORIES;LANGUAGE=en:Meeting', $ics); $this->assertStringContainsString('BEGIN:VALARM', $ics); $this->assertStringContainsString('ACTION:AUDIO', $ics); } @@ -401,4 +459,94 @@ public function testMalformedExtraIcalLinesIsSkippedWithoutBreakingTheExport() $this->assertStringContainsString('good-ref', $ics); $this->assertStringContainsString('bad-ref', $ics); } + + /** + * Both privacy settings through the real PrivacyFilter, for every method. + * + * @return array + */ + public static function privacyMatrix(): array + { + $rows = []; + foreach (IcsMethod::cases() as $method) { + foreach ([[false, false], [true, false], [false, true], [true, true]] as [$hideUser, $hideDetails]) { + $rows[sprintf('stranger user=%d details=%d %s', $hideUser, $hideDetails, $method->value)] = [$hideUser, $hideDetails, 'stranger', $method]; + } + foreach (['owner', 'admin'] as $viewer) { + $rows[sprintf('%s user=1 details=1 %s', $viewer, $method->value)] = [true, true, $viewer, $method]; + } + } + + return $rows; + } + + #[\PHPUnit\Framework\Attributes\DataProvider('privacyMatrix')] + public function testPrivacySettingsApplyToEveryMethod(bool $hideUser, bool $hideDetails, string $viewer, IcsMethod $method) + { + $this->fakeConfig->SetKey(ConfigKeys::EMAIL_DEFAULT_FROM_ADDRESS, 'bookings@example.com'); + $this->fakeConfig->SetKey(ConfigKeys::EMAIL_DEFAULT_FROM_NAME, 'Example Bookings'); + $this->fakeConfig->SetKey(ConfigKeys::PRIVACY_HIDE_USER_DETAILS, $hideUser ? 'true' : 'false'); + $this->fakeConfig->SetKey(ConfigKeys::PRIVACY_HIDE_RESERVATION_DETAILS, $hideDetails ? 'true' : 'false'); + $this->fakeConfig->_ScriptUrl = 'https://example.com/Web'; + // only the viewer's resource permission lookup returns a row + ServiceLocator::SetDatabase(new class () extends FakeDatabase { + public function Query(ISqlCommand $command) + { + parent::Query($command); + + return new FakeReader($command instanceof GetUserPermissionsCommand ? [[ColumnNames::RESOURCE_ID => 1]] : []); + } + }); + + $ownerId = 1; + $session = match ($viewer) { + 'owner' => new FakeUserSession(false, 'UTC', $ownerId), + 'admin' => new FakeUserSession(true, 'UTC', 3), + default => new FakeUserSession(false, 'UTC', 2), + }; + + $res = new ReservationItemView(); + $res->ResourceId = 1; + $res->UserId = $ownerId; + $res->UserLevelId = ReservationUserLevel::OWNER; + $res->OwnerId = $ownerId; + $res->OwnerFirstName = 'f'; + $res->OwnerLastName = 'l'; + $res->OwnerEmailAddress = 'e@m.com'; + $res->Title = 'Secret Title'; + $res->Description = 'Secret Description'; + $res->StartDate = Date::Now(); + $res->EndDate = Date::Now()->AddHours(1); + + $view = new iCalendarReservationView($res, $session, new PrivacyFilter($this->createMock(IReservationAuthorization::class)), '{title}'); + $ics = str_replace("\r\n ", '', (new CalendarExportDisplay())->Render([$view], null, $method)); + + $seesUser = !$hideUser || $viewer !== 'stranger'; + $seesDetails = !$hideDetails || $viewer !== 'stranger'; + + $this->assertStringContainsString('METHOD:' . $method->value, $ics); + $this->assertStringContainsString($method === IcsMethod::CANCEL ? 'STATUS:CANCELLED' : 'STATUS:CONFIRMED', $ics); + $invitation = $method !== IcsMethod::PUBLISH; + if ($seesUser && $invitation) { + // the site is the organizer and the owner the only attendee + $this->assertStringContainsString('ORGANIZER;CN=Example Bookings:mailto:bookings@example.com', $ics); + $this->assertSame(1, substr_count($ics, 'ATTENDEE;')); + $this->assertStringContainsString('ATTENDEE;CN=f l;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=FALSE:mailto:e@m.com', $ics); + } elseif ($seesUser) { + $this->assertStringContainsString('ORGANIZER;CN=f l;SENT-BY="mailto:bookings@example.com":mailto:e@m.com', $ics); + $this->assertStringNotContainsString('ATTENDEE', $ics); + } else { + $this->assertStringNotContainsString('ATTENDEE', $ics); + $this->assertStringContainsString('ORGANIZER;CN=Private:mailto:bookings@example.com', $ics); + $this->assertStringNotContainsString('e@m.com', $ics); + } + if ($seesDetails) { + $this->assertStringContainsString('SUMMARY:Secret Title', $ics); + $this->assertStringContainsString('DESCRIPTION:Secret Description', $ics); + } else { + $this->assertStringContainsString('SUMMARY:Private', $ics); + $this->assertStringContainsString('DESCRIPTION:Private', $ics); + $this->assertStringNotContainsString('Secret', $ics); + } + } } diff --git a/tests/fakes/namespace.php b/tests/fakes/namespace.php index e19107a945..03d354a73b 100644 --- a/tests/fakes/namespace.php +++ b/tests/fakes/namespace.php @@ -28,6 +28,7 @@ require_once(ROOT_DIR . 'tests/fakes/TestDateRange.php'); require_once(ROOT_DIR . 'tests/fakes/TestReservation.php'); require_once(ROOT_DIR . 'tests/fakes/TestReservationSeries.php'); +require_once(ROOT_DIR . 'tests/fakes/TestReservationEmailMessage.php'); require_once(ROOT_DIR . 'tests/fakes/FakeEmailMessage.php'); require_once(ROOT_DIR . 'tests/fakes/FakePluginManager.php'); require_once(ROOT_DIR . 'tests/fakes/FakeRegistrationPage.php'); diff --git a/tests/lib/Email/EmailServiceTest.php b/tests/lib/Email/EmailServiceTest.php index adb6511436..6430214273 100644 --- a/tests/lib/Email/EmailServiceTest.php +++ b/tests/lib/Email/EmailServiceTest.php @@ -67,7 +67,7 @@ public function testAttachmentIsAddedWithItsMimeType() $message->AddStringAttachment( contents: 'BEGIN:VCALENDAR...', fileName: 'reservation.ics', - mimeType: 'text/calendar; charset=UTF-8; method=REQUEST' + mimeType: 'text/calendar; charset=UTF-8; method=PUBLISH' ); $phpMailer->expects($this->once()) @@ -76,7 +76,7 @@ public function testAttachmentIsAddedWithItsMimeType() $this->equalTo('BEGIN:VCALENDAR...'), $this->equalTo('reservation.ics'), $this->anything(), - $this->equalTo('text/calendar; charset=UTF-8; method=REQUEST') + $this->equalTo('text/calendar; charset=UTF-8; method=PUBLISH') ); $service = new EmailService($phpMailer); diff --git a/tests/lib/Email/Messages/ReservationEmailMessageTest.php b/tests/lib/Email/Messages/ReservationEmailMessageTest.php index 86ce268df2..3b902eb222 100644 --- a/tests/lib/Email/Messages/ReservationEmailMessageTest.php +++ b/tests/lib/Email/Messages/ReservationEmailMessageTest.php @@ -2,14 +2,22 @@ declare(strict_types=1); -require_once(ROOT_DIR . 'lib/Email/EmailService.php'); -require_once(ROOT_DIR . 'tests/fakes/TestReservationEmailMessage.php'); +use LibreBooking\Calendar\IcsMethod; +use PHPUnit\Framework\Attributes\DataProvider; + +require_once(ROOT_DIR . 'lib/Email/Messages/ReservationEmailMessage.php'); +foreach (['ReservationCreatedEmail', 'ReservationUpdatedEmail', 'ReservationApprovedEmail', 'ReservationDeletedEmail', 'ReservationShareEmail', 'GuestAddedEmail', 'GuestDeletedEmail', 'InviteeAddedEmail', 'ParticipantAddedEmail'] as $emailClass) { + require_once(ROOT_DIR . "lib/Email/Messages/{$emailClass}.php"); +} class ReservationEmailMessageTest extends TestBase { public function setUp(): void { parent::setup(); + + $this->fakeConfig->SetKey(ConfigKeys::EMAIL_DEFAULT_FROM_ADDRESS, 'bookings@example.com'); + $this->fakeConfig->SetKey(ConfigKeys::EMAIL_DEFAULT_FROM_NAME, 'Example Bookings'); } public function teardown(): void @@ -17,35 +25,159 @@ public function teardown(): void parent::teardown(); } - public function testIcsAttachmentMimeTypeReachesPhpMailerThroughEmailService() + public function testIcsAttachmentListsOnlyTheOwnerAsAttendeeEvenWhenParticipantsExist() { + // other participants are invited by separate emails $ownerId = 1; $owner = new FakeUser($ownerId, 'owner@example.com'); $owner->ChangeName('Owner', 'Person'); + $participantId = 2; + $inviteeId = 3; + + $userRepo = new FakeUserRepository(); + $userRepo->_UserDtos[$participantId] = new UserDto($participantId, 'Part', 'One', 'part1@example.com'); + $userRepo->_UserDtos[$inviteeId] = new UserDto($inviteeId, 'Invite', 'Two', 'invite2@example.com'); + $instance = new TestReservation(); + $instance->WithParticipant($participantId); + $instance->WithInvitee($inviteeId); + $instance->WithParticipatingGuest('guest1@example.com'); + $instance->WithInvitedGuest('guest2@example.com'); $series = new TestReservationSeries(); $series->WithOwnerId($ownerId); $series->WithCurrentInstance($instance); - $message = new TestReservationEmailMessage($owner, $series, null, new FakeAttributeRepository(), new FakeUserRepository()); - $message->PopulateIcsAttachmentForTest($instance); + $attributeRepo = new FakeAttributeRepository(); + + $message = new TestReservationEmailMessage($owner, $series, null, $attributeRepo, $userRepo); + $message->PopulateIcsAttachmentForTest($instance, []); + $ics = $message->AttachmentContents(); + $unfolded = str_replace("\r\n ", '', $ics); + + $this->assertStringContainsString('METHOD:REQUEST', $ics); $this->assertEquals('text/calendar; charset=UTF-8; method=REQUEST', $message->AttachmentMimeType()); - $this->assertEquals('reservation.ics', $message->AttachmentFileName()); - - $phpMailer = $this->createMock('PHPMailer\PHPMailer\PHPMailer'); - $phpMailer->expects($this->once()) - ->method('addStringAttachment') - ->with( - $this->stringContains('BEGIN:VCALENDAR'), - $this->equalTo('reservation.ics'), - $this->anything(), - $this->equalTo('text/calendar; charset=UTF-8; method=REQUEST') - ); - - $service = new EmailService($phpMailer); - $service->Send($message); + // the site's default address is the organizer and the owner the only attendee + $this->assertStringContainsString('ORGANIZER;CN=Example Bookings:mailto:bookings@example.com', $unfolded); + $this->assertSame(1, substr_count($unfolded, 'ATTENDEE')); + $this->assertStringContainsString('ATTENDEE;CN=Owner Person;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=FALSE:mailto:owner@example.com', $unfolded); + } + + /** + * @return array + */ + public static function emailTypes(): array + { + $rows = []; + $types = [ + ['ReservationCreatedEmail', 'owner', IcsMethod::REQUEST], + ['ReservationUpdatedEmail', 'owner', IcsMethod::REQUEST], + ['ReservationApprovedEmail', 'owner', IcsMethod::REQUEST], + ['ReservationDeletedEmail', 'owner', IcsMethod::CANCEL], + ['ReservationShareEmail', 'address', IcsMethod::REQUEST], + ['GuestAddedEmail', 'address', IcsMethod::REQUEST], + ['GuestUpdatedEmail', 'address', IcsMethod::REQUEST], + ['GuestDeletedEmail', 'address', IcsMethod::CANCEL], + ['InviteeAddedEmail', 'user', IcsMethod::REQUEST], + ['InviteeUpdatedEmail', 'user', IcsMethod::REQUEST], + ['ParticipantAddedEmail', 'user', IcsMethod::REQUEST], + ['ParticipantUpdatedEmail', 'user', IcsMethod::REQUEST], + ['InviteeRemovedEmail', 'user', IcsMethod::CANCEL], + ]; + foreach ($types as [$class, $recipient, $method]) { + $rows[$class] = [$class, $recipient, $method]; + } + + return $rows; + } + + #[DataProvider('emailTypes')] + public function testEveryEmailTypeAttachesTheRightMethodWithTheRecipientAsAttendee(string $class, string $recipient, IcsMethod $method) + { + $owner = new FakeUser(1, 'owner@example.com'); + $owner->ChangeName('Owner', 'Person'); + + $participantId = 2; + $userRepo = new FakeUserRepository(); + $userRepo->_UserDtos[$participantId] = new UserDto($participantId, 'Part', 'One', 'part1@example.com'); + + $instance = new TestReservation(); + $instance->WithParticipant($participantId); + $instance->WithInvitedGuest('guest@example.com'); + + $series = new TestReservationSeries(); + $series->WithOwnerId(1); + $series->WithCurrentInstance($instance); + + $attributeRepo = new FakeAttributeRepository(); + $message = match ($recipient) { + 'owner' => new $class($owner, $series, null, $attributeRepo, $userRepo), + 'address' => new $class($owner, 'someone@example.com', $series, $attributeRepo, $userRepo), + 'user' => new $class($owner, new FakeUser(5, 'invitee@example.com'), $series, $attributeRepo, $userRepo), + }; + (fn () => $this->PopulateIcsAttachment($instance, []))->call($message); + + $ics = $message->AttachmentContents(); + + $this->assertStringContainsString('METHOD:' . $method->value, $ics); + $this->assertEquals('text/calendar; charset=UTF-8; method=' . $method->value, $message->AttachmentMimeType()); + $this->assertSame(1, substr_count($ics, 'ATTENDEE;')); + // the attendee is whoever receives the email + $attendee = ['owner' => 'owner@example.com', 'address' => 'someone@example.com', 'user' => 'invitee@example.com'][$recipient]; + $this->assertStringContainsString(':mailto:' . $attendee, str_replace("\r\n ", '', $ics)); + if ($method === IcsMethod::CANCEL) { + $this->assertStringContainsString('STATUS:CANCELLED', $ics); + } else { + $this->assertStringNotContainsString('STATUS:CANCELLED', $ics); + } + // a missing SEQUENCE counts as 0 and the DTSTAMP orders the messages + $this->assertStringNotContainsString('SEQUENCE', $ics); + } + + public function testUpdatedEmailHasTheSubjectForTheOwnerNotTheOneForAdmins() + { + $owner = new FakeUser(1, 'owner@example.com'); + $series = new TestReservationSeries(); + $series->WithOwnerId(1); + $series->WithCurrentInstance(new TestReservation()); + + $message = new ReservationUpdatedEmail($owner, $series, null, new FakeAttributeRepository(), new FakeUserRepository()); + + $resourceName = $series->Resource()->GetName(); + $this->assertSame(Resources::GetInstance()->GetString('ReservationUpdatedSubjectWithResource', [$resourceName]), $message->Subject()); + $this->assertNotSame(Resources::GetInstance()->GetString('ReservationUpdatedAdminSubjectWithResource', [$resourceName]), $message->Subject()); + } + + public function testIcsAttachmentRendersWithoutBookedBySet() + { + // date placeholders need a timezone on the fallback session, even if the owner has none + $this->fakeConfig->SetKey(ConfigKeys::RESERVATION_LABELS_ICS_SUMMARY, '{title} {startdate}'); + // the owner session needs permission to the resource to reach the formatting + $this->db->SetRows([[ColumnNames::RESOURCE_ID => 1]]); + + $ownerId = 1; + $owner = new FakeUser($ownerId, 'owner@example.com'); + $owner->ChangeName('Owner', 'Person'); + $owner->SetTimezone(''); + + $instance = new TestReservation(); + + $series = new TestReservationSeries(); + $series->WithOwnerId($ownerId); + $series->WithCurrentInstance($instance); + $series->WithBookedBy(null); + + $userRepo = new FakeUserRepository(); + $attributeRepo = new FakeAttributeRepository(); + + $message = new TestReservationEmailMessage($owner, $series, null, $attributeRepo, $userRepo); + $message->PopulateIcsAttachmentForTest($instance, []); + + $ics = $message->AttachmentContents(); + $unfolded = str_replace("\r\n ", '', $ics); + + $this->assertStringContainsString('ORGANIZER;CN=Example Bookings:mailto:bookings@example.com', $unfolded); } } diff --git a/tests/src/Calendar/IcsMethodTest.php b/tests/src/Calendar/IcsMethodTest.php new file mode 100644 index 0000000000..afba6cad05 --- /dev/null +++ b/tests/src/Calendar/IcsMethodTest.php @@ -0,0 +1,24 @@ +assertSame('PUBLISH', IcsMethod::PUBLISH->value); + $this->assertSame('REQUEST', IcsMethod::REQUEST->value); + $this->assertSame('CANCEL', IcsMethod::CANCEL->value); + } + + public function testOnlyTheMethodsOfReservationsAreSupported(): void + { + $this->assertCount(3, IcsMethod::cases()); + $this->assertNull(IcsMethod::tryFrom('REPLY')); + } +}