1268939 - Login records should be removed if user signs out

Status: RESOLVED DUPLICATE of bug 1292565

(Firefox for iOS :: Login Management, defect)

Description

[Robin Andersen [:tecgirl]]

Since "Logins" is no longer included as an option in "Clear Private Data", it may not be obvious to the user if they want to wipe logins, that they need to do so using Logins Management. We did this to protect the user from wiping logins from their sync account. 

Issue: User Clears Private Data, and Signs Out. All login records are still there.

We should: Remove logins if the user signs out.

This is related to Bug 1162778, but more explicit to logins.

Comment 1

[Jacob White]

https://github.com/mozilla/firefox-ios/pull/1766

Added an alert that asks the user to remove logins when signing out of Firefox.

Comment 2

[Aaron Train [:aaronmt]]

Attachment: https://github.com/mozilla/firefox-ios/pull/1766

Comment 3

[Aaron Train [:aaronmt]]

Thanks for the patch Jacob, let's start with some feedback.

Comment 4

[Jacob White]

Okay. I was thinking when the user logs out, it would make sense to ask to clear private data too. But maybe showing 3 different alerts is too many?

Comment 5

[Richard Newman [:rnewman]]

This might be a bit more complicated than it sounds.

Firstly, I can't tell from Comment 0 or the bug title if you intend this to prompt the user, Robin; I hope we would, and Jacob's PR does, but…

Secondly, I think we have some logic in other CPD places that checks whether there's anything to prompt about, so we should follow that pattern here -- there's no point in prompting if there's nothing to clear.

Thirdly, along the same lines, we probably shouldn't prompt if you're not syncing passwords in the first place. Users aren't necessarily syncing passwords just because they're signed into Sync. Imagine how this user would feel:

* Use Firefox for iOS for a while. Saves a bunch of passwords.
* Sets up Sync on her desktop. Chooses to sync bookmarks and history only.
* Signs in to that FxA on her iPhone.
* Finds that she doesn't like having her history all mixed up. Signs out of her FxA.

Expected: she still has her logins on her phone.

Actual: she's lost all of her saved passwords (or she's prompted and might mis-click). They're entirely gone, and were never synced anywhere.


(This is part of Bug 1183693 and Bug 1162778, so cross-linking those.)

Comment 6

[Jacob White]

So, check for stored logins, and check for syncing of logins only prompt is both are true, and have a confirmation? Seems like a lot of alerts.

Maybe have a button below the logout button called "reset" that would logout, clear private data, and logins?

Comment 7

[Richard Newman [:rnewman]]

(In reply to Jacob White from comment #6)
> So, check for stored logins, and check for syncing of logins only prompt is
> both are true, and have a confirmation? Seems like a lot of alerts.

IMO: only show an alert at all if the user is syncing logins _and_ there are stored logins. Otherwise, just sign out.

Only one or zero alerts.

Comment 8

[Jacob White]

actuaoly, I'd think it should be the opposite, if you're syncing logins and you're logged in. There's no risk to losing logins if logging out removes logins.

Comment 9

[Stephan Leroux [:sleroux]]

Although there is no risk to the user losing their logins since they are attached to their FxA, I feel like they should be aware of the 'removal' from the local device since it might appear as a hidden side effect of signing out.

Has Logins, No FxA -> Logins stay, no prompt
Has Logins, FxA, No Password Sync -> Logins stay, no prompt
Has Logins, FxA, Password Sync -> 'Passwords will be removed from this device but still be available on other connected devices'
No Logins, *, * -> No-op

As for the code, we have a few alerts we build up from UIAlertController in an extension. If you're adding a prompt that's probably the best place to add it. For checking if the user has logins that are 'syncing', you can use the hasSyncedLogins method on the profile object. I used this within the Logins list view for determining what kind of prompt to show when deleting logins [2]. I believe you can use this check to determine if the user has both logins and if they are syncing so you don't need to do two separate checks. :rnewman would know more though if this is the right call to make.

[1] https://github.com/mozilla/firefox-ios/blob/master/Client/Extensions/UIAlertControllerExtensions.swift
[2] https://github.com/mozilla/firefox-ios/blob/master/Client/Frontend/Login%20Management/LoginListViewController.swift#L222

Comment 10

[Jacob White]

If they're not signed into a Firefox account, then there's nothing to log out of.

So there's only 3 cases to consider:

Logging out with logins and login sync -> prompt
Logging out with logins and no login sync -> prompt
Logging out with no logins -> no prompt

Also, you're suggesting notifying the user that their logins won't be deleted instead of prompting them to delete the logins?

Here's the latest changes:
https://github.com/jacobwhite/firefox-ios/commit/bbab0f0c1b59df4aabe1eb0285e8af5e4dd82680

Comment 11

[Stephan Leroux [:sleroux]]

> Logging out with logins and no login sync -> prompt

I'm not sure we need to prompt the user in this case since nothing changes. I could see us telling the user that they are safe to log out without losing their logins but since we plan on prompt for the opposite case - deleting them with password sync on - I don't think we need both.

> Also, you're suggesting notifying the user that their logins won't be deleted instead of prompting them to delete the logins?

Sorry - I meant the opposite. Prompt the user only when we're removing any logins due to signing out so they user is aware of the (potentially unaware) side effect.

> https://github.com/jacobwhite/firefox-ios/commit/bbab0f0c1b59df4aabe1eb0285e8af5e4dd82680

I've left some comments on the commit. Thanks for looking into this!