From 243a81eef2423a4dddbdf289c4167df038637f5d Mon Sep 17 00:00:00 2001 From: David East Date: Mon, 8 Aug 2016 11:13:11 -0700 Subject: [PATCH 1/2] fix(auth): providerData issue --- src/auth/auth.spec.ts | 17 +++++----- src/auth/auth.ts | 9 +++--- src/auth/auth_backend.spec.ts | 46 ++++++++++++--------------- src/auth/auth_backend.ts | 39 ++++++++--------------- src/auth/firebase_sdk_auth_backend.ts | 2 +- 5 files changed, 48 insertions(+), 65 deletions(-) diff --git a/src/auth/auth.spec.ts b/src/auth/auth.spec.ts index c02cfc557..a7c8307a9 100644 --- a/src/auth/auth.spec.ts +++ b/src/auth/auth.spec.ts @@ -1,5 +1,5 @@ import { auth, initializeApp } from 'firebase'; -import {ReflectiveInjector, provide, Provider} from '@angular/core'; +import { ReflectiveInjector, provide, Provider } from '@angular/core'; import { Observable } from 'rxjs/Observable' import { Observer } from 'rxjs/Observer'; import { @@ -23,8 +23,8 @@ import { } from '../angularfire2'; import { COMMON_CONFIG } from '../test-config'; -import {AuthBackend} from './auth_backend'; -import {FirebaseSdkAuthBackend} from './firebase_sdk_auth_backend'; +import { AuthBackend } from './auth_backend'; +import { FirebaseSdkAuthBackend } from './firebase_sdk_auth_backend'; // Set providers from firebase so no firebase.auth.GoogleProvider() necessary const { @@ -66,7 +66,6 @@ const anonymouseFirebaseUser = { const githubCredential = { credential: { - accessToken: 'ACCESS_TOKEN', provider: 'github.com' }, user: firebaseUser @@ -77,15 +76,15 @@ const googleCredential = { user: firebaseUser } -const AngularFireAuthState = { +const AngularFireAuthState = { provider: 0, auth: firebaseUser, uid: '12345', github: { - accessToken: 'GH_ACCESS_TOKEN', - provider: 'github.com' - } -}; + displayName: 'FirebaseUser', + providerId: 'github.com' + } as firebase.UserInfo +} as FirebaseAuthState; describe('Zones', () => { it('should call operators and subscriber in the same zone as when service was initialized', (done) => { diff --git a/src/auth/auth.ts b/src/auth/auth.ts index 6fc89c0fe..d4b4bd99d 100644 --- a/src/auth/auth.ts +++ b/src/auth/auth.ts @@ -10,7 +10,6 @@ import { AuthProviders, AuthMethods, EmailPasswordCredentials, - OAuthCredential, AuthConfiguration, FirebaseAuthState, stripProviderId @@ -31,7 +30,7 @@ export const firebaseAuthConfig = (config: AuthConfiguration): Provider => { @Injectable() export class AngularFireAuth extends ReplaySubject { - private _credentialCache: {[key:string]: OAuthCredential} = {}; + private _credentialCache: {[key:string]: any} = {}; constructor(private _authBackend: AuthBackend, @Inject(WindowLocation) loc: Location, @Optional() @Inject(FirebaseAuthConfig) private _config?: AuthConfiguration) { @@ -48,7 +47,7 @@ export class AngularFireAuth extends ReplaySubject { .map((userCredential: firebase.auth.UserCredential) => { if (userCredential && userCredential.credential) { authState = attachCredentialToAuthState(authState, userCredential.credential, userCredential.credential.provider); - this._credentialCache[userCredential.credential.provider] = userCredential.credential; + this._credentialCache[userCredential.credential.provider] = userCredential.credential; } return authState; }); @@ -104,8 +103,8 @@ export class AngularFireAuth extends ReplaySubject { return this._authBackend.authWithOAuthPopup(config.provider, this._scrubConfig(config)) .then((userCredential: firebase.auth.UserCredential) => { // Incorrect type information - this._credentialCache[userCredential.credential.provider] = userCredential.credential; - return authDataToAuthState(userCredential.user, (userCredential).credential); + this._credentialCache[userCredential.credential.provider] = userCredential.credential; + return authDataToAuthState(userCredential.user, (userCredential).credential); }); case AuthMethods.Redirect: // Gets around typings issue since this method doesn't resolve with a user. diff --git a/src/auth/auth_backend.spec.ts b/src/auth/auth_backend.spec.ts index c312a57ac..a0bd970a2 100644 --- a/src/auth/auth_backend.spec.ts +++ b/src/auth/auth_backend.spec.ts @@ -1,10 +1,7 @@ import { authDataToAuthState, AuthProviders, - FirebaseAuthState, - CommonOAuthCredential, - GoogleCredential, - TwitterCredential + FirebaseAuthState } from './auth_backend'; const baseFBUser = { @@ -25,26 +22,25 @@ const baseAuthState: FirebaseAuthState = { auth: baseFBUser }; -const baseGithubCredential: CommonOAuthCredential = { - accessToken: 'GH_ACCESS_TOKEN', - provider: 'github.com' -}; +const baseGithubCredential = { + providerId: 'github.com', + displayName: 'GithubAlice' +} as firebase.UserInfo; -const baseFacebookCredential: CommonOAuthCredential = { - accessToken: 'FB_ACCESS_TOKEN', - provider: 'facebook.com' -}; +const baseFacebookCredential = { + displayName: 'FacebookFranny', + providerId: 'facebook.com' +} as firebase.UserInfo; -const baseGoogleCredential: GoogleCredential = { - idToken: 'GOOGLE_ID_TOKEN', - provider: 'google.com' -}; +const baseGoogleCredential = { + displayName: 'GoogleGerry', + providerId: 'google.com' +} as firebase.UserInfo; -const baseTwitterCredential: TwitterCredential = { - accessToken: 'TWITTER_ACCESS_TOKEN', - provider: 'twitter.com', - secret: 'TWITTER_SECRET' -}; +const baseTwitterCredential = { + displayName: 'TwitterTiffany', + providerId: 'twitter.com', +} as firebase.UserInfo; describe('auth_backend', () => { describe('authDataToAuthState', () => { @@ -58,7 +54,7 @@ describe('auth_backend', () => { }); let actualAuthState = authDataToAuthState(githubUser, baseGithubCredential); - expect(actualAuthState.github.accessToken).toEqual(baseGithubCredential.accessToken); + expect(actualAuthState.github.displayName).toEqual(baseGithubCredential.displayName); }); }); @@ -72,7 +68,7 @@ describe('auth_backend', () => { }); let actualAuthState = authDataToAuthState(googleUser, baseGoogleCredential); - expect(actualAuthState.google.idToken).toEqual(baseGoogleCredential.idToken); + expect(actualAuthState.google.displayName).toEqual(baseGoogleCredential.displayName); }); it('Twitter: should return a FirebaseAuthState object with full provider data', () => { @@ -85,7 +81,7 @@ describe('auth_backend', () => { }); let actualAuthState = authDataToAuthState(twitterUser, baseTwitterCredential); - expect(actualAuthState.twitter.secret).toEqual(baseTwitterCredential.secret); + expect(actualAuthState.twitter.displayName).toEqual(baseTwitterCredential.displayName); }); it('Facebook: should return a FirebaseAuthState object with full provider data', () => { @@ -98,7 +94,7 @@ describe('auth_backend', () => { }); let actualAuthState = authDataToAuthState(facebookUser, baseFacebookCredential); - expect(actualAuthState.facebook.accessToken).toEqual(baseFacebookCredential.accessToken); + expect(actualAuthState.facebook.displayName).toEqual(baseFacebookCredential.displayName); }); diff --git a/src/auth/auth_backend.ts b/src/auth/auth_backend.ts index 18a3434c8..e8a29f41c 100644 --- a/src/auth/auth_backend.ts +++ b/src/auth/auth_backend.ts @@ -46,30 +46,14 @@ export interface FirebaseAuthState { provider: AuthProviders; auth: firebase.User; expires?: number; - github?: CommonOAuthCredential; - google?: GoogleCredential; - twitter?: TwitterCredential; - facebook?: CommonOAuthCredential; + github?: firebase.UserInfo; + google?: firebase.UserInfo; + twitter?: firebase.UserInfo; + facebook?: firebase.UserInfo; anonymous?: boolean; } -export interface CommonOAuthCredential { - accessToken: string; - provider: 'github.com' | 'google.com' | 'twitter.com' | 'facebook.com'; -} - -export interface GoogleCredential { - idToken: string; - provider: 'google.com'; -} - -export interface TwitterCredential extends CommonOAuthCredential { - secret: string; -} - -export type OAuthCredential = CommonOAuthCredential | GoogleCredential | TwitterCredential; - -export function authDataToAuthState(authData: firebase.User, providerData?: OAuthCredential): FirebaseAuthState { +export function authDataToAuthState(authData: firebase.User, providerData?: firebase.UserInfo): FirebaseAuthState { if (!authData) return null; let providerId; let { uid } = authData; @@ -79,25 +63,30 @@ export function authDataToAuthState(authData: firebase.User, providerData?: OAut authState.provider = AuthProviders.Anonymous; authState.anonymous = true; return authState; + } else if (authData.providerData[0] === undefined || authData.providerData[0] === null) { + // There is no provider data, user is likely custom + providerId = 'custom'; + authState.provider = AuthProviders.Custom; + return authState; } else { providerId = authData.providerData[0].providerId; } switch (providerId) { case 'github.com': - authState.github = providerData; + authState.github = providerData; authState.provider = AuthProviders.Github; break; case 'twitter.com': - authState.twitter = providerData; + authState.twitter = providerData; authState.provider = AuthProviders.Twitter; break; case 'facebook.com': - authState.facebook = providerData; + authState.facebook = providerData; authState.provider = AuthProviders.Facebook; break; case 'google.com': - authState.google = providerData; + authState.google = providerData; authState.provider = AuthProviders.Google; break; case 'password': diff --git a/src/auth/firebase_sdk_auth_backend.ts b/src/auth/firebase_sdk_auth_backend.ts index fa9730ac6..f11d4a2f6 100644 --- a/src/auth/firebase_sdk_auth_backend.ts +++ b/src/auth/firebase_sdk_auth_backend.ts @@ -48,7 +48,7 @@ export class FirebaseSdkAuthBackend extends AuthBackend { }) .map((user: firebase.User) => { if (!user) return null; - return authDataToAuthState(user); + return authDataToAuthState(user, user.providerData[0]); }) /** * TODO: since the auth service automatically subscribes to this before From fb95c46fd9a87c1fede724cd8e42456c08c97954 Mon Sep 17 00:00:00 2001 From: David East Date: Thu, 11 Aug 2016 17:17:29 -0700 Subject: [PATCH 2/2] fix(tests): Auth specs --- src/auth/auth_backend.spec.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/auth/auth_backend.spec.ts b/src/auth/auth_backend.spec.ts index a0bd970a2..5c29eca2e 100644 --- a/src/auth/auth_backend.spec.ts +++ b/src/auth/auth_backend.spec.ts @@ -47,7 +47,7 @@ describe('auth_backend', () => { it('Github: should return a FirebaseAuthState object with full provider data', () => { let githubUser = Object.assign({}, baseFBUser, { providerData: [{providerId: 'github.com'}] - }); + }) as firebase.User; let expectedAuthState = Object.assign({}, baseAuthState, { github: baseGithubCredential, auth: githubUser @@ -61,7 +61,7 @@ describe('auth_backend', () => { it('Google: should return a FirebaseAuthState object with full provider data', () => { let googleUser = Object.assign({}, baseFBUser, { providerData: [{providerId: 'google.com'}] - }); + }) as firebase.User; let expectedAuthState = Object.assign({}, baseAuthState, { google: baseGoogleCredential, auth: googleUser @@ -74,7 +74,7 @@ describe('auth_backend', () => { it('Twitter: should return a FirebaseAuthState object with full provider data', () => { let twitterUser = Object.assign({}, baseFBUser, { providerData: [{providerId: 'twitter.com'}] - }); + }) as firebase.User; let expectedAuthState = Object.assign({}, baseAuthState, { twitter: baseTwitterCredential, auth: twitterUser @@ -87,7 +87,7 @@ describe('auth_backend', () => { it('Facebook: should return a FirebaseAuthState object with full provider data', () => { let facebookUser = Object.assign({}, baseFBUser, { providerData: [{providerId: 'facebook.com'}] - }); + }) as firebase.User; let expectedAuthState = Object.assign({}, baseAuthState, { facebook: baseFacebookCredential, auth: facebookUser @@ -99,16 +99,16 @@ describe('auth_backend', () => { it('Anonymous: should return a FirebaseAuthState object', () => { - let anonymouseFirebaseUser = Object.assign({}, baseFBUser, { + let anonymousFirebaseUser = Object.assign({}, baseFBUser, { providerData: [], isAnonymous: true - }); + }) as firebase.User; let expectedAuthState = Object.assign({}, baseAuthState, { facebook: baseFacebookCredential, - auth: anonymouseFirebaseUser + auth: anonymousFirebaseUser }); - let actualAuthState = authDataToAuthState(anonymouseFirebaseUser); + let actualAuthState = authDataToAuthState(anonymousFirebaseUser); expect(actualAuthState.anonymous).toEqual(true); }); });