Skip to content

Commit f38e9d7

Browse files
jeffbcrossdavideast
authored andcommitted
fix(auth): check protocol before calling getRedirectResult (#271)
When using AngularFire in non-browser environments, such as Ionic, subscribing to the AngularFireAuth service would cause an error to be thrown. This was because getRedirectResult() is always called at the beginning of the auth observable, to determine if the page is being loaded after a redirect-based oAuth flow. However, calling this method is not supported if location.protocol is not http or https. In the case of Ionic, the protocol is file:. This change adds a check before calling getRedirectResult to make sure the protocol is http or https. BREAKING CHANGE: The AngularFireAuth class has changed the order of its constructor arguments. Since this is usually instantiated automatically via dependency injection, it shouldn't affect common usage of the library. However, if manually instantiating AngularFireAuth in tests or in an application, the order of arguments is now: `(AuthBackend, WindowLocation[, AuthConfiguration])`. Fixes #243
1 parent 5ab37bf commit f38e9d7

File tree

4 files changed

+77
-37
lines changed

4 files changed

+77
-37
lines changed

src/angularfire2.ts

+8-3
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import {
99
FirebaseObjectFactory
1010
} from './utils/firebase_object_factory';
1111
import * as utils from './utils/utils';
12-
import {FirebaseConfig, FirebaseApp} from './tokens';
12+
import { FirebaseConfig, FirebaseApp, WindowLocation } from './tokens';
1313
import { FirebaseAppConfig } from './interfaces';
1414
import {
1515
AuthBackend,
@@ -55,7 +55,11 @@ export const FIREBASE_PROVIDERS:any[] = [
5555
provide: AuthBackend,
5656
useFactory: _getAuthBackend,
5757
deps: [FirebaseApp]
58-
}
58+
},
59+
{
60+
provide: WindowLocation,
61+
useValue: window.location
62+
},
5963
];
6064

6165
function _getAuthBackend(app: firebase.app.App): FirebaseSdkAuthBackend {
@@ -86,7 +90,8 @@ export {
8690
firebaseAuthConfig,
8791
FirebaseAuthState,
8892
AuthMethods,
89-
AuthProviders
93+
AuthProviders,
94+
WindowLocation
9095
}
9196

9297
export { FirebaseConfig, FirebaseApp, FirebaseAuthConfig, FirebaseRef, FirebaseUrl } from './tokens';

src/providers/auth.spec.ts

+54-24
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,8 @@ import {
2424
AngularFireAuth,
2525
AuthMethods,
2626
firebaseAuthConfig,
27-
AuthProviders
27+
AuthProviders,
28+
WindowLocation
2829
} from '../angularfire2';
2930
import { COMMON_CONFIG } from '../test-config';
3031

@@ -99,20 +100,37 @@ describe('FirebaseAuth', () => {
99100
let backend: AuthBackend;
100101
let afAuth: AngularFireAuth;
101102
let authSpy: jasmine.Spy;
102-
var fbAuthObserver: Observer<firebase.User>;
103-
104-
beforeEachProviders(() => [
105-
FIREBASE_PROVIDERS,
106-
defaultFirebase(COMMON_CONFIG),
107-
provide(FirebaseApp, {
108-
useFactory: (config: FirebaseAppConfig) => {
109-
var app = initializeApp(config);
110-
(<any>app).auth = () => authSpy;
111-
return app;
112-
},
113-
deps: [FirebaseConfig]
114-
})
115-
]);
103+
let fbAuthObserver: Observer<firebase.User>;
104+
let windowLocation: any;
105+
106+
beforeEachProviders(() => {
107+
windowLocation = {
108+
hash: '',
109+
search: '',
110+
pathname:'/',
111+
port: '',
112+
hostname:'localhost',
113+
host:'localhost',
114+
protocol:'https:',
115+
origin:'localhost',
116+
href:'https://localhost/'
117+
};
118+
return [
119+
FIREBASE_PROVIDERS,
120+
defaultFirebase(COMMON_CONFIG),
121+
provide(FirebaseApp, {
122+
useFactory: (config: FirebaseAppConfig) => {
123+
var app = initializeApp(config);
124+
(<any>app).auth = () => authSpy;
125+
return app;
126+
},
127+
deps: [FirebaseConfig]
128+
}),
129+
provide(WindowLocation, {
130+
useValue: windowLocation
131+
})
132+
]
133+
});
116134

117135
beforeEach(() => {
118136
authSpy = jasmine.createSpyObj('auth', authMethods);
@@ -202,7 +220,7 @@ describe('FirebaseAuth', () => {
202220
let config = {
203221
method: AuthMethods.Anonymous
204222
};
205-
afAuth = new AngularFireAuth(backend, config);
223+
afAuth = new AngularFireAuth(backend, windowLocation, config);
206224
afAuth.login();
207225
expect(app.auth().signInAnonymously).toHaveBeenCalled();
208226
});
@@ -211,7 +229,7 @@ describe('FirebaseAuth', () => {
211229
let config = {
212230
method: AuthMethods.Anonymous
213231
};
214-
afAuth = new AngularFireAuth(backend, config);
232+
afAuth = new AngularFireAuth(backend, windowLocation, config);
215233
afAuth.login({
216234
method: AuthMethods.Popup,
217235
provider: AuthProviders.Google
@@ -227,7 +245,7 @@ describe('FirebaseAuth', () => {
227245
provider: AuthProviders.Google,
228246
scope: ['email']
229247
};
230-
afAuth = new AngularFireAuth(backend, config);
248+
afAuth = new AngularFireAuth(backend, windowLocation, config);
231249
afAuth.login({
232250
provider: AuthProviders.Github
233251
});
@@ -252,7 +270,7 @@ describe('FirebaseAuth', () => {
252270
let config = {
253271
method: AuthMethods.Password
254272
};
255-
let afAuth = new AngularFireAuth(backend, config);
273+
let afAuth = new AngularFireAuth(backend, windowLocation, config);
256274
afAuth.login()
257275
.then(done.fail, done);
258276
});
@@ -261,7 +279,7 @@ describe('FirebaseAuth', () => {
261279
let config = {
262280
method: AuthMethods.CustomToken
263281
};
264-
let afAuth = new AngularFireAuth(backend, config);
282+
let afAuth = new AngularFireAuth(backend, windowLocation, config);
265283
afAuth.login()
266284
.then(done.fail, done);
267285
});
@@ -270,7 +288,7 @@ describe('FirebaseAuth', () => {
270288
let config = {
271289
method: AuthMethods.OAuthToken
272290
};
273-
let afAuth = new AngularFireAuth(backend, config);
291+
let afAuth = new AngularFireAuth(backend, windowLocation, config);
274292
afAuth.login()
275293
.then(done.fail, done);
276294
});
@@ -279,7 +297,7 @@ describe('FirebaseAuth', () => {
279297
let config = {
280298
method: AuthMethods.Popup
281299
};
282-
let afAuth = new AngularFireAuth(backend, config);
300+
let afAuth = new AngularFireAuth(backend, windowLocation, config);
283301
afAuth.login()
284302
.then(done.fail, done);
285303
});
@@ -288,7 +306,7 @@ describe('FirebaseAuth', () => {
288306
let config = {
289307
method: AuthMethods.Redirect
290308
};
291-
let afAuth = new AngularFireAuth(backend, config);
309+
let afAuth = new AngularFireAuth(backend, windowLocation, config);
292310
afAuth.login()
293311
.then(done.fail, done);
294312
});
@@ -359,7 +377,7 @@ describe('FirebaseAuth', () => {
359377
360378
password: 'supersecretpassword'
361379
};
362-
let afAuth = new AngularFireAuth(backend, config);
380+
let afAuth = new AngularFireAuth(backend, windowLocation, config);
363381
afAuth.login(credentials);
364382
expect(app.auth().signInWithEmailAndPassword).toHaveBeenCalledWith(credentials.email, credentials.password);
365383
});
@@ -433,6 +451,18 @@ describe('FirebaseAuth', () => {
433451
fbAuthObserver.next(firebaseUser);
434452
});
435453
}, 10);
454+
455+
456+
it('should not call getRedirectResult() if location.protocol is not http or https', (done) => {
457+
windowLocation.protocol = 'file:';
458+
afAuth
459+
.take(1)
460+
.do(() => {
461+
expect(authSpy['getRedirectResult']).not.toHaveBeenCalled();
462+
})
463+
.subscribe(done, done.fail);
464+
fbAuthObserver.next(firebaseUser);
465+
});
436466
});
437467

438468
describe('authWithOAuthRedirect', () => {

src/providers/auth.ts

+14-10
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import 'rxjs/add/operator/concat';
99
import 'rxjs/add/operator/skip';
1010
import 'rxjs/add/observable/of';
1111

12-
import {FirebaseApp, FirebaseAuthConfig} from '../tokens';
12+
import { FirebaseApp, FirebaseAuthConfig, WindowLocation } from '../tokens';
1313
import {isPresent} from '../utils/utils';
1414
import * as utils from '../utils/utils';
1515
import {
@@ -36,23 +36,27 @@ export const firebaseAuthConfig = (config: AuthConfiguration): Provider => {
3636
export class AngularFireAuth extends ReplaySubject<FirebaseAuthState> {
3737
private _credentialCache: {[key:string]: OAuthCredential} = {};
3838
constructor(private _authBackend: AuthBackend,
39+
@Inject(WindowLocation) loc: Location,
3940
@Optional() @Inject(FirebaseAuthConfig) private _config?: AuthConfiguration) {
4041
super(kBufferSize);
4142

4243
let firstPass = true;
4344
this._authBackend.onAuth()
4445
.mergeMap((authState: FirebaseAuthState) => {
45-
// TODO: get rid of side effect
4646
if (firstPass) {
4747
firstPass = false;
48-
return this._authBackend.getRedirectResult()
49-
.map((userCredential: firebase.auth.UserCredential) => {
50-
if (userCredential && userCredential.credential) {
51-
authState = attachCredentialToAuthState(authState, userCredential.credential, userCredential.credential.provider);
52-
this._credentialCache[userCredential.credential.provider] = <OAuthCredential>userCredential.credential;
53-
}
54-
return authState;
55-
})
48+
if(['http:', 'https:'].indexOf(loc.protocol) > -1) {
49+
// Only call getRedirectResult() in a browser
50+
return this._authBackend.getRedirectResult()
51+
.map((userCredential: firebase.auth.UserCredential) => {
52+
if (userCredential && userCredential.credential) {
53+
authState = attachCredentialToAuthState(authState, userCredential.credential, userCredential.credential.provider);
54+
this._credentialCache[userCredential.credential.provider] = <OAuthCredential>userCredential.credential;
55+
}
56+
return authState;
57+
});
58+
}
59+
5660
}
5761
return Observable.of(authState);
5862
})

src/tokens.ts

+1
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import {OpaqueToken} from '@angular/core';
33
export const FirebaseConfig = new OpaqueToken('FirebaseUrl');
44
export const FirebaseApp = new OpaqueToken('FirebaseApp')
55
export const FirebaseAuthConfig = new OpaqueToken('FirebaseAuthConfig');
6+
export const WindowLocation = new OpaqueToken('WindowLocation');
67
// TODO: Deprecate
78
export const FirebaseRef = FirebaseApp;
89
export const FirebaseUrl = FirebaseConfig;

0 commit comments

Comments
 (0)