Support multi-user login

[chanseokoh]

Adds AccountRoaster as a member of GoogleLoginState. It's basically a list of currently logged-in users (Accounts).

This PR isn't that large, if we exclude unit tests.

The gist of the implementation isn't that complex either, in the sense that it is now a list of Accounts instead of a single Account; logIn() adds a new Account to the list, and logOut() clears the list.

Some minor details:

• queryEmail() now throws an exception if a query fails to retrieve an email address. In this case, we force it to be a failed login. (Previously, an email address could be null while logged in in this error case.)
• Account vs. AccoutInfo: Account is an internal class holding all necessary information about a logged-in account. AccountInfo is a public-facing record containing minimal information only relevant to UI presentation.

To do after this PR:

• Support persisting the entire list of logged-in accounts. (Currently, only one is persisted.) (#23)
• Query real names and avatar images. (#24)

[chanseokoh]

@akerekes @briandealwis @elharo @nbashirbello ptal

[elharo]

@briandealwis This library needs to remain independent of SWT and other Eclipse classes. That said, we don't necessarily need to use AWT either. What do you suggest as an alternative to java.awt.Image for avatars?

[chanseokoh]

@elharo Looking at the IntelliJ code, the first thing we get from a server is an URL for an image. I think returning an URL is the simplest way which sounds like a good start.

[chanseokoh]

ptal.

[chanseokoh]

And does it make sense for GoogleLoginState to be for a single appId + secret + scopes? What happens if we need more scopes — do we create a new GoogleLoginState?

There had been discussions and proposals to improve UX for having incrementally changing scopes for different services/operations. (Some of such proposals had a radically different UI and flow.) However, nothing concrete has come out yet to tackle this. For now, we will assume that appId + secret + scopes is fixed. It will probably take a lot of time and further discussions to finalize supporting changing scopes (and it's not happening), so I think it's out of scope for us at this point.

Should we be using the clientId in the preference name when persisting the active account, or does the last one win?

We will eventually persist all credentials (#26), so I don't think we need to use clientId for the preference name.

[chanseokoh]

I'm pretty strongly opposed to GoogleLoginState only exposing a single active user. I see this as UI state, and not something that should be baked into this service.

This makes sense. There is no reason not to return all accounts. I just limited it to an active account, and I can lift that restriction.

[chanseokoh]

I don't think it hurts for this service to maintain the currently active user, though again I think that's something that's better handled in the UI layer (writes out the email of the active user).

My intention was to liberate UI from maintaining any state, as it provides some benefits. I imagined that as soon as UI maintains some state about login, the UI will have to deal with the sync issue between UI and GoogleLoginState in general. No need to think about any state is also handy from the perspective of the library client, including UI.

However, this is something we can certainly consider going forward. I do get what you feel about the current GoogleLoginState; you're thinking marking which account is active in GoogleLoginState is a UI thing and is a responsibility of UI.

For now, I suggest we continue to maintain the active account state in GoogleLoginState for simplicity. (I think it's much more simple as we don't have to deal with the out-of-sync state issue between UI and GoogleLoginState.)

Creating multiple instances of GoogleLoginState is what we can consider too, but I think we'd want something set up and working instead of waiting until we implement everything in a future proof and perfect manner.

[chanseokoh]

For example: I have a AppEngine log viewer and Cloud SQL view open, using my normal account. I now want to deploy to a production instance, which requires me to switch accounts. What happens to the log and trace views? What happens if I forget to switch back to the original user? (I don't think these kind of usecases really mattered in GPE as the IAM framework wasn't really there AFAIK.)

Good point. I also think our current login model (which is the one in Android Studio and IntelliJ) doesn't fit well in this regard.

I am sure this login model was originally proposed in the context of providing personalized IDE experience in Android Studio (e.g., saving/loading IDE settings in the Cloud and having the same IDE settings on every device ubiquitously), something like a Firefox browser login with an arbitrary email; it was purely for real humans. On the other hand, I think the login model we need is in an orthogonal context, where we authorize resources for operations on GCP. For example, this is somewhat like we open multiple GCP dev console tabs in a Firefox browser logged in with different Google users. Personally, I think we need to make the distinction between these two contexts (although I'm sure not everyone will agree on me). That didn't happen in IntelliJ (and Android Studio if I'm not mistaken). For now, I'm basically following what IntelliJ did, but I do hope our team will have a chance to contemplate and revisit this login model in IDEs for GCP operations.

[chanseokoh]

I will start working on adding a method to return all existing credentials (such as getCredentials()). I am thinking of returning copies, so I don't think I will return a list of Accounts directly — I'd rather return (email, Credential) pairs.

[chanseokoh]

On second thought, returning all credentials is YAGNI at the moment. I created #26.

I could also move logics from GoogleLoginState into Account, but I like that we achieve the functionality we want while making minimal changes to GoogleLoginState in this PR. (I created #27.) It also makes reviewing very easy — no significant structural changes in the class, so the diff is pleasant to your eyes. @briandealwis are you good on this, or you still want that these are addressed here? Let me know.

[chanseokoh]

Talked with @briandealwis. We have a change of plans. I sent email with a high-level overview about the new login model to the team. Will start working on this soon. Please wait until then.

[chanseokoh]

@akerekes @briandealwis @nbashirbello PTAL. It's best to see the diff comparing with the original code, rather than looking at individual commit changes.

Note: I have removed code to refresh access tokens, since it is not being used by anyone including Eclipse and even if it did, I think it needs to be fixed.

[chanseokoh]

Fixes #26.

[chanseokoh]

I've addressed all the review comments (if I didn't miss anything). PTAL.