Details

Reviewers

None

Group Reviewers

Maniphest Tasks

T15564: Allow yourself to be Invited in Imported Events

Summary

If one of your verified email addresses is mentioned in the "Invitees"
section of your upstream Calendar Event, you are now imported as such.

For example, if you have 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" orange marker is shown from your Profile (instead of nothing)
the Profile Calendar widget shows your imported Event (instead of nothing - no "Clear Sailing ahead")

Example situation:

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

1 email belongs to user "test"
1 email belongs to another user

Before	After

So the Calendar owner is not an alien anymore.

Allowing to match yourself makes sense because you trust your imported
Calendar 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

Test Plan

create an Event in Google Calendar (or whatever)
put 1+ verified emails of yours as invited
put 1+ other people emails as invited
import the Calendar (or refresh the data) /calendar/import/edit/?importType=icsuri

Verify the Event page:

you are now Invited in your own event (you don't see anymore yourself as one - or more - of "Private User" in your own event)
the above thing happens only if your email is confirmed
other people is still never disclosed and are still "Private User"
you cannot change Event details (just like before)

Diff Detail

Repository

rP Phorge

Branch

arcpatch-D25363_2

Lint

Lint Passed

Unit

Tests Passed

Build Status

Buildable 1102
Build 1102: 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

Match yourself from Imported Events Invitees
Needs ReviewPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 1754

src/applications/calendar/controller/PhabricatorCalendarEventViewController.php

src/applications/calendar/import/PhabricatorCalendarImportEngine.php

src/applications/people/query/PhabricatorPeopleUserEmailQuery.php

Match yourself from Imported Events InviteesNeeds ReviewPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 1754

src/applications/calendar/controller/PhabricatorCalendarEventViewController.php

src/applications/calendar/import/PhabricatorCalendarImportEngine.php

src/applications/people/query/PhabricatorPeopleUserEmailQuery.php

Match yourself from Imported Events Invitees
Needs ReviewPublic
Actions

Revision Contents
Changeset List