Closed Bug 1077332 Opened 10 years ago Closed 10 years ago

Contacts should not be available to guest users

Categories

(Hello (Loop) :: Client, defect, P1)

defect
Points:
1

Tracking

(firefox34 verified, firefox35 verified)

VERIFIED FIXED
mozilla35
Iteration:
35.3
Tracking Status
firefox34 --- verified
firefox35 --- verified

People

(Reporter: standard8, Assigned: mikedeboer)

References

Details

(Whiteboard: [loop-uplift])

Attachments

(2 files, 1 obsolete file)

According to the "not signed in" UI flow and confirmed with RT on irc, we shouldn't have contacts available to guest users:

https://people.mozilla.org/~dhenein/labs/loop-mvp-spec/#precall

Otherwise, signed-in users could be spammed by non-signed in users, and not know where the calls were coming from. See also bug 1077328.
Iteration: --- → 35.3
Points: --- → 1
Flags: qe-verify+
Flags: firefox-backlog+
The problem I see with the design as it stands:

If I logged in, set up my contacts, then let someone else sign in to my browser, they are going to see all my contacts - that's probably fine.

However, if we disable contacts when they are signed out, then they might think, "well I don't have contacts when I'm signed out, so why can my friend access all my contacts"?

So we might need to think about this before doing it.
Flags: needinfo?(rtestard)
Flags: needinfo?(dhenein)
Iteration: 35.3 → ---
So I think we have 3 issues:

(1) We shouldn't show contacts if we aren't logged into FxA.  Addressing that is pretty easy (p=1).

(2) If we have more than user using the same browser and same profile, should they be able to see one another's contacts?  

e.g.  User A logs into FxA, add his/her contacts, and logs out.   User B logs into FxA, should he/she be able to see User A's contacts?   I believe the answer needs to be "no".  However, the code currently doesn't store them separately based on FxA login, and the work to do this is around a p=5 (roughly half a week of work).

(3) Do we need to encrypt the contacts that are stored locally?  If we don't, are there privacy issues?

I think (3) needs to be a separate bug, but I wanted to mention this as part of the analysis of what I think we need for contacts.
(In reply to Maire Reavy [:mreavy] (Plz needinfo me) from comment #2)
> So I think we have 3 issues:
> 
> (1) We shouldn't show contacts if we aren't logged into FxA.  Addressing
> that is pretty easy (p=1).
Agreed.
> 
> (2) If we have more than user using the same browser and same profile,
> should they be able to see one another's contacts?  
This should behave like any other buddy list -- if I am not signed in, I see no contacts (in fact the contacts tab is not even supposed to be visible when not signed in (as per https://people.mozilla.org/~dhenein/labs/loop-mvp-spec/#precall-signedin, "Contacts and History tabs show up when signed in"). If I sign in with my email address, I should see my contacts. If someone else signs in with their email address, they should not see my contacts, and should see their contacts.
Flags: needinfo?(dhenein)
Assignee: nobody → mdeboer
Flags: needinfo?(mmucci)
Added to IT 35.3
Status: NEW → ASSIGNED
Iteration: --- → 35.3
Flags: needinfo?(mmucci)
I'm an advocate for positive naming, but React makes that harder than it could be. Thus, going for `hidden`
Attachment #8499563 - Flags: review?(paolo.mozmail)
Clearing NI as Darrin answered
Flags: needinfo?(rtestard)
Comment on attachment 8499563 [details] [diff] [review]
Patch v1: hide tabs when not logged in with FxA

Waiting for the next version as discussed:
- Avoid rendering instead of hiding
- Reset the active tab on logout
Attachment #8499563 - Flags: review?(paolo.mozmail)
Attachment #8499715 - Flags: review?(paolo.mozmail)
Attachment #8499563 - Attachment is obsolete: true
Attachment #8499715 - Flags: review?(paolo.mozmail) → review+
And https://treeherder.mozilla.org/ui/logviewer.html#?job_id=816741&repo=fx-team

Please verify that you're green on Try before pushing again...
(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #11)
> And
> https://treeherder.mozilla.org/ui/logviewer.html#?job_id=816741&repo=fx-team

Sorry, this was from bug 1015988.
Attachment #8500384 - Flags: review?(paolo.mozmail) → review+
Blocks: 1078309
Comment on attachment 8499715 [details] [diff] [review]
Patch v2: hide tabs when not logged in with FxA

Approval Request Comment
[Feature/regressing bug #]: Loop
[User impact if declined]: Contacts will be visible to users that are not logged in. This is not the expected UX.
[Describe test coverage new/current, TBPL]: Landed on m-c
[Risks and why]: minor.
[String/UUID change made/needed]: n/a
Attachment #8499715 - Flags: approval-mozilla-aurora?
Comment on attachment 8500384 [details] [diff] [review]
Follow-up: fix tests to always show the contacts tab

Approval Request Comment
[Feature/regressing bug #]: Loop
[User impact if declined]: The unit tests will fail if the aforementioned patch is uplifted without this one accompanying it.
[Describe test coverage new/current, TBPL]: landed on m-c
[Risks and why]: minor.
[String/UUID change made/needed]: n/a
Attachment #8500384 - Flags: approval-mozilla-aurora?
FYI:  This will be part of the second uplift to Aurora that I am planning for this Thursday.  

For Sheriffs or anyone doing Aurora uplifts -- Please ping me (mreavy on irc) before doing any Loop bug uplifts to Aurora.  Thanks.
I've checked this a Windows 7, Mac OS X 10.9, and Ubuntu 14.04 with today's Nightly. I confirm that I don't get a Contacts tab in the Hello panel until after I've logged in. When I log out the Contacts tab disappears.

Marking this bug verified fixed.
Status: RESOLVED → VERIFIED
Flags: needinfo?(paul.silaghi)
Verified fixed FF 34b1 OS X 10.9.5, Win 7
Flags: needinfo?(paul.silaghi)
Comment on attachment 8499715 [details] [diff] [review]
Patch v2: hide tabs when not logged in with FxA

Already landed in aurora, approving for posterity
Attachment #8499715 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment on attachment 8500384 [details] [diff] [review]
Follow-up: fix tests to always show the contacts tab

Already landed in aurora, approving for posterity
Attachment #8500384 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.