Details

Reviewers

None

Group Reviewers

Maniphest Tasks

T15941: Calendar Import: support Available/Busy
T15564: Allow yourself to be Invited in Imported Events

Summary

If one of *your* verified email addresses is invited in *your* ICS Calendar,
you are now imported as yourself, instead of being a "Private User".

For example, if you own a Google Calendar, and if you import that in
Phorge, and if your email is mentioned in the Invitees:

you are not shown anymore as "Private User 1" but as yourself
the "Busy" or "Available" badge is shown from your Profile (instead of nothing), respecting the "Time Transparency" RFC 5545 section 3.8.2.7 from your ICS event https://icalendar.org/iCalendar-RFC-5545/3-8-2-7-time-transparency.html
the widget "Profile Calendar" in your user page shows your imported Event, instead of nothing. No "Clear Sailing ahead" anymore. As usual - this happens only if the event happens today, tomorrow, or the day after tomorrow.

Example situation:

User "test" imports a Calendar. An Event has two invited emails:

1 email is verified and belongs to the very same user "test"
1 email belongs to another user

Before	After

See that the calendar importer named "test" is not an alien anymore.

Allowing to match yourself makes sense because you trust your imported
Calendar file, and we trust your verified email addresses.

WE DO NOT MATCH OTHER USERS BUT THE CALENDAR OWNER.
Matching other users must involve serious privacy measures,
coherent with the rest of Phorge.

Closes T15564
Closes T15941

Test Plan

Download this example ICS file:

Example_for_patch_D25363.ics1 KBDownload

Replace the email boz+asdlol@reyboz.it with one of your verified email of your Phorge account.

Import the event in Phorge using Calendar → Imports → Import Events → Import .ics File

/calendar/import/edit/?importType=icsuri

Two imported events are created successfully:

In the event "Very busy opaque" (25 December 2024):
- you are finally shown as "Busy"
- there is also another "Private user"
In the other event "Very available transparent" (25 December 2024)
- you are finall shown as "Available"
- there is also another "Private user"

Then nuke these example events by visiting the import and "Delete Imported Events".

Try again from scratch in these alternatives:

if you import the ICS file as-is:
- you get two "Private User" in all events (since none of the invitees matches one of your verified emails)
if you import the ICS file, setting one of your un-verified emails:
- you get two "Private User" in all events (since none of the invitees matches one of your verified emails)
if you import the ICS file, setting a verified email of *another* user:
- you get two "Private User" in all events (since none of the invitees matches one of your verified emails)

As additional test, from the file you can also manually set these events to today, tomorrow, or the day after tomorrow; so you can test the user profile's calendar widget, and see that finally it does not show "Clear sailing" anymore, but it shows your calendar invitations (if the ICS file contains one of your verified email - as already said):

Calendar widget finally showing something.png (507×300 px, 20 KB)

Diff Detail

Repository

rP Phorge

Branch

arcpatch-D25363_2

Lint

Lint Passed

Unit

Tests Passed

Build Status

Buildable 922
Build 922: arc lint + arc unit

Event Timeline

valerio.bozzolan created this revision.Jul 28 2023, 15:52

Owners added a reviewer: O1: Blessed Committers.Jul 28 2023, 15:52

Herald added subscribers: Cigaryno, Matthew, tobiaswiese, speck. · View Herald TranscriptJul 28 2023, 15:52

valerio.bozzolan requested review of this revision.Jul 28 2023, 15:52

Harbormaster completed remote builds in B700: Diff 1168.Jul 28 2023, 15:52

valerio.bozzolan added inline comments.Jul 28 2023, 16:04

src/applications/calendar/controller/PhabricatorCalendarEventViewController.php
169	✅ This is consistent with the error "Can Not Edit Imported Event".
633	✅ This is consistent with the error "Can Not Edit Imported Event".
641	✅ This is consistent with the error "Can Not Edit Imported Event".
src/applications/calendar/import/PhabricatorCalendarImportEngine.php
230	This was a map of names.
238–240	The `$name` variable was re-used too much. So, renamed when: $attendee_mail = new PhutilEmailAddress($name);
359	This contains external attendees by name. So, added `_name` in the name to avoid confusion.
364–365	This part was kept, but moved in a dedicated loop above. So, we first loop external attendees, then real users. Then, this loop handles all of them.
417–422	✅ If this is your event, and you are self-invited, of course you are attending (you are not just a random invited). Note that the manual Event creation form relies on the same assumption.

remove newline

Harbormaster completed remote builds in B701: Diff 1169.Jul 28 2023, 16:05

valerio.bozzolan mentioned this in T15564: Allow yourself to be Invited in Imported Events.Jul 28 2023, 16:06

Could this be abused, e.g. create an event with a thousand emails then import it and see if those emails are registered? If so how does that compare to existing means of discovering registered users?

In D25363#10531, @speck wrote:

Could this be abused, e.g. create an event with a thousand emails then import it and see if those emails are registered? If so how does that compare to existing means of discovering registered users?

Thanks speck, absolutely not! :)

If Valerio imports a Calendar, only Valerio is eventually recognized. The others are still "Private user"

I've added a couple of screenshots

(to notice the difference you must see them quickly with your arrow keys ← → ihih)

avivey subscribed.Sep 23 2023, 09:05

avivey added inline comments.

src/applications/calendar/import/PhabricatorCalendarImportEngine.php
211	Use PhabricatorPeopleUserEmailQuery, possibly augmenting it with `withIsVerified` and/or `withUserPhid`
261	Don't rely on `$foo[$x][$y] = ...` to create array in `[$x]` - create that expliclitly, see line 237 above. the idx in line 378 can be replaced with direct access then.
398	make the parameter clearer:

Apply review tips

Harbormaster completed remote builds in B922: Diff 1499.Nov 13 2023, 13:33

Thanks!

I've adopted PhabricatorPeopleUserEmailQuery as suggested.

But note that I've used the omnipotent user there. Edited:

Since we are already limiting to the author of that import, and it's not useful to check that permission again.

Also because setViewer($viewer) was causing this:

EXCEPTION: (PhabricatorDataNotAttachedException) Attempting to access attached data on PhabricatorUserEmail (via getUser()), but the data is not actually attached. Before accessing attachable data on an object, you must load and attach it.\n\nData is normally attached by calling the corresponding needX() method on the Query class when the object is loaded. You can also call the corresponding attachX() method explicitly. at [<phorge>/src/infrastructure/storage/lisk/PhabricatorLiskDAO.php:275]
arcanist(head=master, ref.master=98d16d27cf3e), phorge(head=arcpatch-D25363_2, ref.master=aa8af1d79e8b, ref.arcpatch-D25363_2=544e5ed24a07)
   #0 <#2> PhabricatorLiskDAO::assertAttached(string) called at [<phorge>/src/applications/people/storage/PhabricatorUserEmail.php:65]
   #1 <#2> PhabricatorUserEmail::getUser() called at [<phorge>/src/applications/people/storage/PhabricatorUserEmail.php:325]
   #2 <#2> PhabricatorUserEmail::getPolicy(string) called at [<phorge>/src/applications/policy/filter/PhabricatorPolicyFilter.php:884]
   #3 <#2> PhabricatorPolicyFilter::getObjectPolicy(PhabricatorUserEmail, string) called at [<phorge>/src/applications/policy/filter/PhabricatorPolicyFilter.php:201]
   #4 <#2> PhabricatorPolicyFilter::apply(array) called at [<phorge>/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php:273]
   #5 <#2> PhabricatorPolicyAwareQuery::execute() called at [<phorge>/src/applications/calendar/import/PhabricatorCalendarImportEngine.php:217]
   #6 <#2> PhabricatorCalendarImportEngine::importEventDocument(PhabricatorUser, PhabricatorCalendarImport, PhutilCalendarRootNode) called at [<phorge>/src/applications/calendar/import/PhabricatorCalendarICSImportEngine.php:42]
   #7 <#2> PhabricatorCalendarICSImportEngine::importICSData(PhabricatorUser, PhabricatorCalendarImport, string) called at [<phorge>/src/applications/calendar/import/PhabricatorCalendarICSFileImportEngine.php:91]
   #8 <#2> PhabricatorCalendarICSFileImportEngine::importEventsFromSource(PhabricatorUser, PhabricatorCalendarImport, boolean) called at [<phorge>/src/applications/calendar/editor/PhabricatorCalendarImportEditor.php:57]
   #9 <#2> PhabricatorCalendarImportEditor::applyFinalEffects(PhabricatorCalendarImport, array) called at [<phorge>/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php:1412]
   #10 <#2> PhabricatorApplicationTransactionEditor::applyTransactions(PhabricatorCalendarImport, array) called at [<phorge>/src/applications/calendar/controller/PhabricatorCalendarImportReloadController.php:37]
   #11 <#2> PhabricatorCalendarImportReloadController::handleRequest(AphrontRequest) called at [<phorge>/src/aphront/configuration/AphrontApplicationConfiguration.php:284]
   #12 phlog(PhabricatorDataNotAttachedException) called at [<phorge>/src/aphront/handler/PhabricatorDefaultRequestExceptionHandler.php:41]
   #13 PhabricatorDefaultRequestExceptionHandler::handleRequestThrowable(AphrontRequest, PhabricatorDataNotAttachedException) called at [<phorge>/src/aphront/configuration/AphrontApplicationConfiguration.php:751]
   #14 AphrontApplicationConfiguration::handleThrowable(PhabricatorDataNotAttachedException) called at [<phorge>/src/aphront/configuration/AphrontApplicationConfiguration.php:296]
   #15 AphrontApplicationConfiguration::processRequest(AphrontRequest, PhutilDeferredLog, AphrontPHPHTTPSink, MultimeterControl) called at [<phorge>/src/aphront/configuration/AphrontApplicationConfiguration.php:204]
   #16 AphrontApplicationConfiguration::runHTTPRequest(AphrontPHPHTTPSink) called at [<phorge>/webroot/index.php:35]

See PhabricatorCalendarImportEngine line 215

valerio.bozzolan edited the test plan for this revision. (Show Details)Jan 23 2024, 16:49

Tested again with git rebase master. Ready.

Harbormaster completed remote builds in B1100: Diff 1751.Mar 7 2024, 09:58

git rebase master again :) ready.

Harbormaster completed remote builds in B1102: Diff 1754.Mar 7 2024, 10:33

I admit I am quite interested in the changes in src/applications/people/query/PhabricatorPeopleUserEmailQuery.php adding $userPhids and $isVerified as I have a downstream use case for them.
Would be lovely to get them in, nevertheless where the actual calendar feature request is going.

Could you clarify if this is only supposed to work for import an .ics URI and not for importing a single event via an .ics file? Maybe I'm overcomplicating things, what I tried:

Go to http://phorge.localhost/people/new/standard/ and create four users:
- Meeting organizer user d25363_00 with email address d25363_00@example.com
- User d25363_01 with email address d25363_01@example.com
- User d25363_02 with email address d25363_02a@example.com to use a secondary address
- User d25363_03 with email address d25363_03uv@example.com to keep unverified
In MariaDB, run USE phabricator_user; UPDATE phabricator_user.user u SET isEmailVerified = "1" WHERE (u.realName = "d25363_00" OR u.userName = "d25363_01" OR u.userName = "d25363_02");
Log in as user d25363_02 (via ./bin/auth recover d25363_02) and add secondary email address d25363_02b@example.com on http://phorge.localhost/settings/panel/email/
Set up .ics testcase at P42 which has d25363_00@example.com as meeting organizer and invites the five addresses d25363_01@example.com, d25363_02a@example.com, d25363_02b@example.com, d25363_03uv@example.com, d25363_04@example.com (the latter non-existing in Phab)
As user d25363_00, import that testcase: Go to http://phorge.localhost/calendar/import/ and select "Import Events", "Import .ics File", select the test case, set "Visible To" to "Public", click "Create New Import", end up on http://phorge.localhost/calendar/import/9/ linking to resulting http://phorge.localhost/E12
- Resulting E12 shows "Private User 1" to "Private User 5" with no user matching
- As user d25363_02, go to http://phorge.localhost/E12 - says "You do not have permission to view this object. Users with the "Can View" capability: d25363_00 (d25363_00) can take this action."
As user d25363_02, go to http://phorge.localhost/calendar/import/ and select "Import Events", "Import .ics File", select the test case, set "Visible To" to "Public", click "Create New Import", go to resulting http://phorge.localhost/E13
- Resulting E13 still shows "Private User 1" to "Private User 5" with no user matching - I don't think that is expected but I'm not sure we want to care as that is no regression?

aklapper mentioned this in T15852: PHP 8.1 "preg_match(null)"/"strlen(null)" exceptions importing ics file with no attendee names into calendar.Jun 10 2024, 17:08

valerio.bozzolan mentioned this in D25686: Fix PHP 8.1 exceptions importing ICS file without attendee names.Jun 10 2024, 22:20

valerio.bozzolan added a parent revision: D25767: Calendar Import: add unit tests to cover participants.Aug 8 2024, 11:53

be based on unit tests

Harbormaster completed remote builds in B1510: Diff 2349.Aug 8 2024, 11:54

valerio.bozzolan retitled this revision from Match yourself from Imported Events Invitees to Calendar Import: calendar uploader is not anymore an alien.Aug 8 2024, 12:00

rebase

Harbormaster completed remote builds in B1519: Diff 2360.Aug 13 2024, 09:23

git rebase master

Harbormaster completed remote builds in B1573: Diff 2435.Aug 28 2024, 07:35

valerio.bozzolan added inline comments.Aug 28 2024, 07:37

src/applications/calendar/import/PhabricatorCalendarImportEngine.php
213	The `attendee_map` was a map of userPHID so it's renamed to `attendee_user_map`. In this way we can introduce `attendee_name_map` (map of string names) with less confusion.

valerio.bozzolan mentioned this in Organizations Using Phorge.Sep 9 2024, 07:24

valerio.bozzolan mentioned this in T15941: Calendar Import: support Available/Busy.Sep 11 2024, 09:21

valerio.bozzolan edited the summary of this revision. (Show Details)Nov 2 2024, 12:13

valerio.bozzolan edited the test plan for this revision. (Show Details)

valerio.bozzolan added a task: T15941: Calendar Import: support Available/Busy.

it works!

Harbormaster completed remote builds in B1620: Diff 2522.Nov 2 2024, 12:13

valerio.bozzolan edited the summary of this revision. (Show Details)Nov 2 2024, 13:30

valerio.bozzolan edited the test plan for this revision. (Show Details)

fix lint, happy review

Harbormaster completed remote builds in B1621: Diff 2523.Nov 2 2024, 13:31

valerio.bozzolan mentioned this in Z1: Phorge.Nov 2 2024, 13:32

valerio.bozzolan edited the test plan for this revision. (Show Details)Nov 2 2024, 13:40

helping reviewers, adding comment on future export possibility

Harbormaster completed remote builds in B1622: Diff 2524.Nov 2 2024, 13:53

Calendar Import: calendar uploader is not anymore an alien
Needs ReviewPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 1499

src/applications/calendar/controller/PhabricatorCalendarEventViewController.php

src/applications/calendar/import/PhabricatorCalendarImportEngine.php

src/applications/people/query/PhabricatorPeopleUserEmailQuery.php

Calendar Import: calendar uploader is not anymore an alienNeeds ReviewPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 1499

src/applications/calendar/controller/PhabricatorCalendarEventViewController.php

src/applications/calendar/import/PhabricatorCalendarImportEngine.php

src/applications/people/query/PhabricatorPeopleUserEmailQuery.php

Calendar Import: calendar uploader is not anymore an alien
Needs ReviewPublic
Actions

Revision Contents
Changeset List