fix(auth): Return provider data

[jteplitz]

@jeffbcross I could be totally wrong here since I'm still catching up on the new firebase changes.
But it seems like the current auth implementation doesn't return any provider data (at least for facebook it appears to return facebook: undefined). I think we actually want to return firebase.UserInfo for all of providers and get the data from the providerData.

Let me know if this looks right or if I'm missing something. If it looks good I'll update the tests accordingly.

[jeffbcross]

Yeah, the main problem is that the provider data and credential are separate objects from Firebase, whereas before they were merged. I.e. the accessToken and displayName were together. Currently we're just setting the object to the credential returned from Firebase, ignoring the providerData. And since the credential is only persisted in memory, there is no provider data at all when the page is refreshed.

For example, if I log in with Twitter, I get this providerData:

{
  displayName: "Jeff Cross ️",
  email: null,
  photoURL: "http://pbs.twimg.com/profile_images/627356983306293249/2r0Y6aNT_normal.jpg",
  providerId: "twitter.com",
  uid: "12345"
}

and this credential:

{
  accessToken: 'abc',
  secret: 'def',
  provider: 'twitter.com'

We need to either merge credential and providerData ourselves for backwards compatibility, or have them as separate properties.

Which begs the question, if we're going to have separate properties, i.e. twitterCredential and twitterData, what value are we adding that the existing firebase.User object doesn't already have? For one, we are caching the credential in memory in re-attaching it to the authState object each time it's emitted. And having separate type definitions for different providers makes it easier to serialize auth objects over web worker without using reflection.

Since it's currently up to the developer/application to check that a user is authenticated AND has credentials, I think it would be best to split the credential into its own property, ${provider}Credential, so that it's simpler to check:

auth.subscribe(user => {
  if (user && user.githubCredential) doSomething(user);
})

as opposed to

auth.subscribe(user => {
  if (user && user.github && user.github.credential) doSomething(user);
})

We should change the existing names from the provider name to ${provider}Data. i.e. authState.github -> authState.githubData.

What do you think, @jteplitz602 ?

[jteplitz]

@jeffbcross Ah I see. I agree that merging them doesn't make much sense.

So you're proposing caching the credential ourselves and then attaching it to the user object? I like the idea of us caching the credential for developer's convenience, but I feel like adding all these properties to the user object is going to make our docs different from the firebase docs in a somewhat confusing way. How do you feel about emitting a tuple from the auth observable that includes user and credential properties?

[jeffbcross]

So you're proposing caching the credential ourselves and then attaching it to the user object?

Yeah, we already cache them in memory, and I have a proposal to make it easy to persist to disk. #230

I feel like adding all these properties to the user object is going to make our docs different from the firebase docs in a somewhat confusing way.

You're right. I wanted to keep the auth state object similar to pre-Firebase-3 so that upgrading would be simpler, but we shouldn't keep clinging to that. I think having user and credential makes sense, this is consistent with the UserCredential type returned from signInWithPopup() and getRedirectResult().

[davideast]

Addressing in #420