Skip to content

Commit

Permalink
fix(app): fix issue with FirebaseApp extending INTERNAL (#38)
Browse files Browse the repository at this point in the history
* fix(app): fix issue with FirebaseApp extending INTERNAL
FirebaseAppImpl should not extend prototype.INTERNAL as this will apply to
all Firebase app instances.
To reproduce, do the following:
var app1 = firebase.initializeApp(config);
var app2 = firebase.initializeApp(config, 'app2');
console.log(app1.INTERNAL === app2.internal);
The above incorrectly resolves to true.
This means that if I sign in with app1.auth() and then call app1.INTERNAL.getToken(),
it will resolve with null as it is getting app2.INTERNAL.getToken.
The last initialized instance will basically overwrite all existing ones.

This is currently breaking all FirebaseUI-web single page applications
using Firebase real-time database with JS version >= 4.1.0:
firebase/firebaseui-web#47 (comment)
This should also be breaking any application using Auth and Database with
multiple app instances.

* fix(app): removed extra spaces and commented code
  • Loading branch information
bojeil-google authored and jshcrowthe committed Jun 7, 2017
1 parent 8a90518 commit cb46f75
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 15 deletions.
29 changes: 14 additions & 15 deletions src/app/firebase_app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,18 @@ class FirebaseAppImpl implements FirebaseApp {
private firebase_: FirebaseNamespace) {
this.name_ = name;
this.options_ = deepCopy<FirebaseOptions>(options);
this.INTERNAL = {
'getUid': () => null,
'getToken': () => LocalPromise.resolve(null),
'addAuthTokenListener': (callback: (token: string|null) => void) => {
tokenListeners.push(callback);
// Make sure callback is called, asynchronously, in the absence of the auth module
setTimeout(() => callback(null), 0);
},
'removeAuthTokenListener': (callback) => {
tokenListeners = tokenListeners.filter(listener => listener !== callback);
},
};
}

get name(): string {
Expand Down Expand Up @@ -321,7 +333,7 @@ class FirebaseAppImpl implements FirebaseApp {
*/
private extendApp(props: {[name: string]: any}): void {
// Copy the object onto the FirebaseAppImpl prototype
deepExtend(FirebaseAppImpl.prototype, props);
deepExtend(this, props);

/**
* If the app has overwritten the addAuthTokenListener stub, forward
Expand Down Expand Up @@ -351,19 +363,6 @@ class FirebaseAppImpl implements FirebaseApp {
}
};

FirebaseAppImpl.prototype.INTERNAL = {
'getUid': () => null,
'getToken': () => LocalPromise.resolve(null),
'addAuthTokenListener': (callback: (token: string|null) => void) => {
tokenListeners.push(callback);
// Make sure callback is called, asynchronously, in the absence of the auth module
setTimeout(() => callback(null), 0);
},
'removeAuthTokenListener': (callback) => {
tokenListeners = tokenListeners.filter(listener => listener !== callback);
},
}

// Prevent dead-code elimination of these methods w/o invalid property
// copying.
FirebaseAppImpl.prototype.name &&
Expand Down Expand Up @@ -603,4 +602,4 @@ let errors: {[code: string]: string} = {
'Firebase App instance.'
};

let appErrors = new ErrorFactory<AppError>('app', 'Firebase', errors);
let appErrors = new ErrorFactory<AppError>('app', 'Firebase', errors);
35 changes: 35 additions & 0 deletions tests/app/unit/firebase_app.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,41 @@ describe("Firebase App Class", () => {
const service = (firebase.app() as any).testService();
});

it(`Should extend INTERNAL per app instance`, () => {
let counter: number = 0;
firebase.INTERNAL.registerService(
'test',
(app: FirebaseApp, extendApp: any) => {
const service = new TestService(app);
(service as any).token = 'tokenFor' + counter++;
extendApp({
'INTERNAL': {
getToken: () => {
return Promise.resolve({
accessToken: (service as any).token,
});
},
},
});
return service;
});
// Initialize 2 apps and their corresponding services.
const app = firebase.initializeApp({});
(app as any).test();
const app2 = firebase.initializeApp({}, 'app2');
(app2 as any).test();
// Confirm extended INTERNAL getToken resolve with the corresponding
// service's value.
return app.INTERNAL.getToken()
.then(token => {
assert.equal('tokenFor0', token.accessToken);
return app2.INTERNAL.getToken();
})
.then(token => {
assert.equal('tokenFor1', token.accessToken);
});
});

describe("Check for bad app names", () => {
let tests = ["", 123, false, null];
for (let data of tests) {
Expand Down

0 comments on commit cb46f75

Please sign in to comment.