fix(auth): check protocol before calling getRedirectResult #271
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 angular#243
jeffbcross
force-pushed
the
redirectresult
branch
from
0930330 to
d95b567
Compare
|
LGTM! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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,
Fixes #243