Page MenuHomePhorge

Match yourself from Imported Events Invitees
Needs ReviewPublic

Authored by valerio.bozzolan on Jul 28 2023, 15:52.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Jul 10, 06:19
Unknown Object (File)
Wed, Jul 10, 05:47
Unknown Object (File)
Tue, Jul 9, 12:03
Unknown Object (File)
Mon, Jul 8, 18:05
Unknown Object (File)
Sun, Jul 7, 03:43
Unknown Object (File)
Thu, Jul 4, 03:58
Unknown Object (File)
Sun, Jun 30, 16:08
Unknown Object (File)
Fri, Jun 28, 13:13

Details

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
BeforeAfter
Before.png (543×948 px, 86 KB)
After.png (543×948 px, 87 KB)

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

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.

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 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:

valerio.bozzolan marked 3 inline comments as done.

Apply review tips

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

Tested again with git rebase master. Ready.

git rebase master again :) ready.

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?