OneBusAway / onebusaway-android

The official Android app for OneBusAway
http://www.onebusaway.org/
Other
475 stars 291 forks source link

ES - When Google account is removed, user should be logged out of OBA ES #834

Closed barbeau closed 2 weeks ago

barbeau commented 6 years ago

Summary:

I just added a 2nd Google account to a device, logged into ES in OBA using that 2nd Google account, and then removed the 2nd Google account from the device. When I re-opened OBA, it still showed me as logged in using that 2nd Google account.

@acrown-msft Do you think that OBA should log the user out of ES in this case? Or is the current behavior correct?

Steps to reproduce:

  1. Add a 2nd Google account to a device (on a Galaxy S8+ you can do this via System "Settings -> Cloud and Accounts->Accounts->Add account->Google"- note that this is NOT the same thing as adding another user to the device, which is only supported on certain Nexus devices)
  2. Log into ES in OBA using that 2nd Google account
  3. Removed the 2nd Google account from the device
  4. Open OBA and open navigation drawer

Expected behavior:

I should be logged out of OBA ES (?)

Observed behavior:

OBA ES still shows me as logged in

Device and Android version:

Nexus 5 w/ Android 6.0

barbeau commented 6 years ago

Per our discussion, when the user logs out of a Google account on a device, OBA should invalidate the current login session and prompt the user to log in again the next time they use the app.

barbeau commented 6 years ago

@acrown-msft Here are the steps to add a new Google account on a Galaxy S8+ - System "Settings -> Cloud and Accounts->Accounts->Add account->Google". I added this to the original description as well.

acrown-msft commented 6 years ago

@barbeau I’ve been working on this issue for a bit and looked at a couple of apps to see how they handled this. I’ve looked at Yelp and Wunderlist and both apps keep the user logged in after removing the google account used for login. Do you know of an app that I could install to use as a model for this functionality?

As I understand the Android GET_ACCOUNTS permission, we can request the list of emails associated with the device. On app start, we can compare that list against the google account used to authenticate with ES and implement the appropriate logic.

The current flow is as follows:

Is the account currently associated with the device? 
    Yes: Store that this account is associated with the device
    No: Was the account associated with the device last time?
        Yes: Sign the user out
        No: No op

Does this sound reasonable?

barbeau commented 6 years ago

Yes, this sounds reasonable to me. Unfortunately I don't know of other apps that do this.

barbeau commented 6 years ago

Per our discussion today, the fix in https://github.com/OneBusAway/onebusaway-android/commit/14f4f531cdc405c228737e039fa94041f4b92cb8 requires a new permission "Find accounts on the device" that a user must accept on install of the update (we're still using install-time permissions) - here's what you see when updating from v2.2.5 (production) to v2.3.4 (current beta):

image

We are going to roll-back the current fix so we can roll out ES release to production without requiring any new install-time permissions, so I'll re-open this issue when that is merged (it requires a change in ES SDK). The rolled-back behavior is the same as other apps that @acrown-msft tested.

Then, as part of a subsequent release we will re-implement the same fix but move to runtime permissions, which is ticketed in: https://github.com/OneBusAway/onebusaway-android/issues/330

This will allow us to roll out the fix in a future release without requiring any install-time permissions from the user. Instead, they will be prompted for this permission when/if they decide to use ES.

barbeau commented 6 years ago

PR https://github.com/OneBusAway/onebusaway-android/pull/866 reverts the fix as discussed above, so I'm re-opening this issue.