Skip to content

Commit 16bbe6c

Browse files
wti806wti806
authored
clear redirect result after signOut or getRedirectResult being called (#2049)
1 parent 386fd24 commit 16bbe6c

File tree

2 files changed

+255
-2
lines changed

2 files changed

+255
-2
lines changed

‎packages/auth/src/auth.js

Lines changed: 41 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,10 @@ fireauth.Auth = function(app) {
155155
this.userDeleteListener_ = goog.bind(this.handleUserDelete_, this);
156156
/** @private {!function(!Object)} The handler for user invalidation. */
157157
this.userInvalidatedListener_ = goog.bind(this.handleUserInvalidated_, this);
158+
/**
159+
* @private {?fireauth.AuthEventManager} The Auth event manager instance.
160+
*/
161+
this.authEventManager_ = null;
158162
// TODO: find better way to enable or disable auth event manager.
159163
if (fireauth.AuthEventManager.ENABLED) {
160164
// Initialize Auth event manager to handle popup and redirect operations.
@@ -713,8 +717,9 @@ fireauth.Auth.prototype.signInWithRedirect = function(provider) {
713717
* redirect sign, will reject with the proper error. If no redirect operation
714718
* called, resolves with null.
715719
* @return {!goog.Promise<!fireauth.AuthEventManager.Result>}
720+
* @private
716721
*/
717-
fireauth.Auth.prototype.getRedirectResult = function() {
722+
fireauth.Auth.prototype.getRedirectResultWithoutClearing_ = function() {
718723
// Check if popup and redirect are supported in this environment.
719724
if (!fireauth.util.isPopupRedirectSupported()) {
720725
return goog.Promise.reject(new fireauth.AuthError(
@@ -737,6 +742,29 @@ fireauth.Auth.prototype.getRedirectResult = function() {
737742
};
738743

739744

745+
/**
746+
* In addition to returning the redirect result as in
747+
* `getRedirectResultWithoutClearing_`, this will also clear the cached
748+
* redirect result for security reasons.
749+
* @return {!goog.Promise<!fireauth.AuthEventManager.Result>}
750+
*/
751+
fireauth.Auth.prototype.getRedirectResult = function() {
752+
return this.getRedirectResultWithoutClearing_()
753+
.then((result) => {
754+
if (this.authEventManager_) {
755+
this.authEventManager_.clearRedirectResult();
756+
}
757+
return result;
758+
})
759+
.thenCatch((error) => {
760+
if (this.authEventManager_) {
761+
this.authEventManager_.clearRedirectResult();
762+
}
763+
throw error;
764+
});
765+
};
766+
767+
740768
/**
741769
* Asynchronously sets the provided user as currentUser on the current Auth
742770
* instance.
@@ -866,6 +894,17 @@ fireauth.Auth.prototype.signOut = function() {
866894
// Wait for final state to be ready first, otherwise a signed out user could
867895
// come back to life.
868896
var p = this.redirectStateIsReady_.then(function() {
897+
// Clear any cached redirect result on sign out, even if user is already
898+
// signed out. For example, sign in could fail due to account conflict
899+
// error, the error in redirect result should still be cleared. There is
900+
// also the use case where you keep a reference to a signed out user and
901+
// call signedOutUser.linkWithRedirect(provider). Even though the user is
902+
// signed out, getRedirectResult() will resolve with the modified signed
903+
// out user. This could also throw an error
904+
// (provider already linked, etc).
905+
if (self.authEventManager_) {
906+
self.authEventManager_.clearRedirectResult();
907+
}
869908
// Ignore if already signed out.
870909
if (!self.currentUser_()) {
871910
return goog.Promise.resolve();
@@ -999,7 +1038,7 @@ fireauth.Auth.prototype.initAuthRedirectState_ = function() {
9991038
// Wait first for state to be loaded from storage.
10001039
return this.authStateLoaded_.then(function() {
10011040
// Resolve any pending redirect result.
1002-
return self.getRedirectResult();
1041+
return self.getRedirectResultWithoutClearing_();
10031042
}).thenCatch(function(error) {
10041043
// Ignore any error in the process. Redirect could be not supported.
10051044
return;

‎packages/auth/test/auth_test.js

Lines changed: 214 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6959,6 +6959,155 @@ function testAuth_getRedirectResult_unsupportedEnvironment() {
69596959
}
69606960

69616961

6962+
function testAuth_signOut_clearSuccessRedirectResult() {
6963+
// Tests getRedirectResult with success event after signOut being called.
6964+
fireauth.AuthEventManager.ENABLED = true;
6965+
const expectedCred = fireauth.GoogleAuthProvider.credential(
6966+
null, 'ACCESS_TOKEN');
6967+
// Expected sign in via redirect successful Auth event.
6968+
const expectedAuthEvent = new fireauth.AuthEvent(
6969+
fireauth.AuthEvent.Type.SIGN_IN_VIA_REDIRECT,
6970+
null,
6971+
'http://www.example.com/#response',
6972+
'SESSION_ID');
6973+
// Stub instantiateOAuthSignInHandler.
6974+
stubs.replace(
6975+
fireauth.AuthEventManager, 'instantiateOAuthSignInHandler',
6976+
function(authDomain, apiKey, appName) {
6977+
return {
6978+
'addAuthEventListener': function(handler) {
6979+
// auth1 should be subscribed.
6980+
const manager = fireauth.AuthEventManager.getManager(
6981+
config3['authDomain'], config3['apiKey'], app1.name);
6982+
assertTrue(manager.isSubscribed(auth1));
6983+
// In this case run immediately with expected redirect event.
6984+
handler(expectedAuthEvent);
6985+
asyncTestCase.signal();
6986+
},
6987+
'initializeAndWait': function() { return goog.Promise.resolve(); },
6988+
'shouldBeInitializedEarly': function() { return false; },
6989+
'hasVolatileStorage': function() { return false; }
6990+
};
6991+
});
6992+
// Simulate successful finishPopupAndRedirectSignIn.
6993+
stubs.replace(
6994+
fireauth.Auth.prototype,
6995+
'finishPopupAndRedirectSignIn',
6996+
function(requestUri, sessionId, postBody) {
6997+
assertEquals('http://www.example.com/#response', requestUri);
6998+
assertEquals('SESSION_ID', sessionId);
6999+
assertNull(postBody);
7000+
user1 = new fireauth.AuthUser(
7001+
config3, expectedTokenResponse, accountInfo);
7002+
expectedPopupResult = {
7003+
'user': user1,
7004+
'credential': expectedCred,
7005+
'additionalUserInfo': expectedAdditionalUserInfo,
7006+
'operationType': fireauth.constants.OperationType.SIGN_IN
7007+
};
7008+
// User 1 should be set here and saved to storage.
7009+
auth1.setCurrentUser_(user1);
7010+
asyncTestCase.signal();
7011+
return currentUserStorageManager.setCurrentUser(user1).then(() => {
7012+
return expectedPopupResult;
7013+
});
7014+
});
7015+
let user1, expectedPopupResult;
7016+
asyncTestCase.waitForSignals(3);
7017+
const pendingRedirectManager = new fireauth.storage.PendingRedirectManager(
7018+
config3['apiKey'] + ':' + appId1);
7019+
currentUserStorageManager = new fireauth.storage.UserManager(
7020+
config3['apiKey'] + ':' + appId1);
7021+
app1 = firebase.initializeApp(config3, appId1);
7022+
auth1 = app1.auth();
7023+
pendingRedirectManager.setPendingStatus().then(() => {
7024+
// Verify that the redirect result is resolved before signing out.
7025+
const manager = fireauth.AuthEventManager.getManager(
7026+
config3['authDomain'], config3['apiKey'], app1.name);
7027+
return manager.getRedirectResult().then((result) => {
7028+
// Expected result returned.
7029+
assertObjectEquals(expectedPopupResult, result);
7030+
return auth1.signOut();
7031+
}).then(() => {
7032+
// signOut should clear the cached redirect result.
7033+
return auth1.getRedirectResult();
7034+
}).then((resultAfterClearing) => {
7035+
fireauth.common.testHelper.assertUserCredentialResponse(
7036+
null, null, null, undefined, resultAfterClearing);
7037+
asyncTestCase.signal();
7038+
});
7039+
});
7040+
}
7041+
7042+
7043+
function testAuth_signOut_clearErrorRedirectResult() {
7044+
// Tests getRedirectResult with error event after signOut being called.
7045+
fireauth.AuthEventManager.ENABLED = true;
7046+
// The expected error.
7047+
const expectedError =
7048+
new fireauth.AuthError(fireauth.authenum.Error.INTERNAL_ERROR);
7049+
// Expected sign in via redirect error Auth event.
7050+
const expectedAuthEvent = new fireauth.AuthEvent(
7051+
fireauth.AuthEvent.Type.SIGN_IN_VIA_REDIRECT,
7052+
null,
7053+
null,
7054+
null,
7055+
expectedError);
7056+
// Stub instantiateOAuthSignInHandler.
7057+
stubs.replace(
7058+
fireauth.AuthEventManager, 'instantiateOAuthSignInHandler',
7059+
function(authDomain, apiKey, appName) {
7060+
return {
7061+
'addAuthEventListener': function(handler) {
7062+
// auth1 should be subscribed.
7063+
const manager = fireauth.AuthEventManager.getManager(
7064+
config3['authDomain'], config3['apiKey'], app1.name);
7065+
assertTrue(manager.isSubscribed(auth1));
7066+
// In this case run immediately with expected redirect event.
7067+
handler(expectedAuthEvent);
7068+
asyncTestCase.signal();
7069+
},
7070+
'initializeAndWait': function() { return goog.Promise.resolve(); },
7071+
'shouldBeInitializedEarly': function() { return false; },
7072+
'hasVolatileStorage': function() { return false; }
7073+
};
7074+
});
7075+
stubs.replace(
7076+
fireauth.Auth.prototype,
7077+
'finishPopupAndRedirectSignIn',
7078+
function(requestUri, sessionId, postBody) {
7079+
fail('finishPopupAndRedirectSignIn should not run on event error!');
7080+
});
7081+
asyncTestCase.waitForSignals(2);
7082+
const pendingRedirectManager = new fireauth.storage.PendingRedirectManager(
7083+
config3['apiKey'] + ':' + appId1);
7084+
currentUserStorageManager = new fireauth.storage.UserManager(
7085+
config3['apiKey'] + ':' + appId1);
7086+
app1 = firebase.initializeApp(config3, appId1);
7087+
auth1 = app1.auth();
7088+
pendingRedirectManager.setPendingStatus().then(() => {
7089+
// Verify that the redirect result is rejected with error before
7090+
// signing out.
7091+
const manager = fireauth.AuthEventManager.getManager(
7092+
config3['authDomain'], config3['apiKey'], app1.name);
7093+
return manager.getRedirectResult().thenCatch((error) => {
7094+
// Expected error returned.
7095+
fireauth.common.testHelper.assertErrorEquals(expectedError, error);
7096+
return auth1.signOut();
7097+
}).then(() => {
7098+
// signOut should clear the error in cached redirect result.
7099+
return auth1.getRedirectResult();
7100+
}).then((resultAfterClearing) => {
7101+
fireauth.common.testHelper.assertUserCredentialResponse(
7102+
null, null, null, undefined, resultAfterClearing);
7103+
asyncTestCase.signal();
7104+
});
7105+
}).thenCatch((error) => {
7106+
fail('Redirect result should be cleared by signOut.');
7107+
}) ;
7108+
}
7109+
7110+
69627111
function testAuth_returnFromSignInWithRedirect_success_withoutPostBody() {
69637112
// Tests the return from a successful sign in with redirect.
69647113
fireauth.AuthEventManager.ENABLED = true;
@@ -7025,6 +7174,11 @@ function testAuth_returnFromSignInWithRedirect_success_withoutPostBody() {
70257174
auth1.getRedirectResult().then(function(result) {
70267175
// Expected result returned.
70277176
assertObjectEquals(expectedPopupResult, result);
7177+
// Redirect result should be cleared after being returned once.
7178+
return auth1.getRedirectResult();
7179+
}).then(function(resultAfterClearing) {
7180+
fireauth.common.testHelper.assertUserCredentialResponse(
7181+
null, null, null, undefined, resultAfterClearing);
70287182
asyncTestCase.signal();
70297183
});
70307184
});
@@ -7114,6 +7268,11 @@ function testAuth_returnFromSignInWithRedirect_success_withPostBody() {
71147268
auth1.getRedirectResult().then(function(result) {
71157269
// Expected result returned.
71167270
assertObjectEquals(expectedPopupResult, result);
7271+
// Redirect result should be cleared after being returned once.
7272+
return auth1.getRedirectResult();
7273+
}).then(function(resultAfterClearing) {
7274+
fireauth.common.testHelper.assertUserCredentialResponse(
7275+
null, null, null, undefined, resultAfterClearing);
71177276
asyncTestCase.signal();
71187277
});
71197278
});
@@ -7226,6 +7385,11 @@ function testAuth_returnFromSignInWithRedirect_withExistingUser() {
72267385
// Newly signed in user.
72277386
assertEquals(user1, auth1['currentUser']);
72287387
assertObjectEquals(expectedPopupResult, result);
7388+
// Redirect result should be cleared after being returned once.
7389+
return auth1.getRedirectResult();
7390+
}).then(function(resultAfterClearing) {
7391+
fireauth.common.testHelper.assertUserCredentialResponse(
7392+
null, null, null, undefined, resultAfterClearing);
72297393
asyncTestCase.signal();
72307394
});
72317395
// State listener should fire once only with the final redirected user.
@@ -7296,7 +7460,14 @@ function testAuth_returnFromSignInWithRedirect_error() {
72967460
// Get redirect result should return the expected error.
72977461
auth1.getRedirectResult().thenCatch(function(error) {
72987462
fireauth.common.testHelper.assertErrorEquals(expectedError, error);
7463+
// Error in redirect result should be cleared after being returned once.
7464+
return auth1.getRedirectResult();
7465+
}).then(function(resultAfterClearing) {
7466+
fireauth.common.testHelper.assertUserCredentialResponse(
7467+
null, null, null, undefined, resultAfterClearing);
72997468
asyncTestCase.signal();
7469+
}).thenCatch(function(error) {
7470+
fail('Error in event should only be thrown once.');
73007471
});
73017472
});
73027473
// State listener should fire once only with null user.
@@ -7365,6 +7536,12 @@ function testAuth_returnFromSignInWithRedirect_error_webStorageNotSupported() {
73657536
pendingRedirectManager.setPendingStatus().then(function() {
73667537
// Get redirect result should return the expected error.
73677538
auth1.getRedirectResult().thenCatch(function(error) {
7539+
fireauth.common.testHelper.assertErrorEquals(expectedError, error);
7540+
// Errors not being tied to a sign-in session should not be cleared.
7541+
return auth1.getRedirectResult();
7542+
}).then(function(result) {
7543+
fail('Errors not being tied to a sign-in session should not be cleared.');
7544+
}).thenCatch(function(error) {
73687545
fireauth.common.testHelper.assertErrorEquals(expectedError, error);
73697546
asyncTestCase.signal();
73707547
});
@@ -7535,6 +7712,11 @@ function testAuth_returnFromLinkWithRedirect_success_withoutPostBody() {
75357712
// Get redirect result should resolve with expected user and credential.
75367713
auth1.getRedirectResult().then(function(result) {
75377714
assertObjectEquals(expectedPopupResult, result);
7715+
// Redirect result should be cleared after being returned once.
7716+
return auth1.getRedirectResult();
7717+
}).then(function(resultAfterClearing) {
7718+
fireauth.common.testHelper.assertUserCredentialResponse(
7719+
null, null, null, undefined, resultAfterClearing);
75387720
asyncTestCase.signal();
75397721
});
75407722
// Should fire once only with the final redirected user.
@@ -7636,6 +7818,11 @@ function testAuth_returnFromLinkWithRedirect_success_withPostBody() {
76367818
// Get redirect result should resolve with expected user and credential.
76377819
auth1.getRedirectResult().then(function(result) {
76387820
assertObjectEquals(expectedPopupResult, result);
7821+
// Redirect result should be cleared after being returned once.
7822+
return auth1.getRedirectResult();
7823+
}).then(function(resultAfterClearing) {
7824+
fireauth.common.testHelper.assertUserCredentialResponse(
7825+
null, null, null, undefined, resultAfterClearing);
76397826
asyncTestCase.signal();
76407827
});
76417828
// Should fire once only with the final redirected user.
@@ -7779,6 +7966,11 @@ function testAuth_returnFromLinkWithRedirect_redirectedLoggedOutUser_success() {
77797966
assertEquals('fr', result.user.getLanguageCode());
77807967
auth1.languageCode = 'de';
77817968
assertEquals('de', result.user.getLanguageCode());
7969+
// Redirect result should be cleared after being returned once.
7970+
return auth1.getRedirectResult();
7971+
}).then(function(resultAfterClearing) {
7972+
fireauth.common.testHelper.assertUserCredentialResponse(
7973+
null, null, null, undefined, resultAfterClearing);
77827974
asyncTestCase.signal();
77837975
});
77847976
// Should fire once only with the original user.
@@ -7889,6 +8081,11 @@ function testAuth_redirectedLoggedOutUser_differentAuthDomain() {
78898081
assertEquals('fr', result.user.getLanguageCode());
78908082
auth1.languageCode = 'de';
78918083
assertEquals('de', result.user.getLanguageCode());
8084+
// Redirect result should be cleared after being returned once.
8085+
return auth1.getRedirectResult();
8086+
}).then(function(resultAfterClearing) {
8087+
fireauth.common.testHelper.assertUserCredentialResponse(
8088+
null, null, null, undefined, resultAfterClearing);
78928089
asyncTestCase.signal();
78938090
});
78948091
// Should fire once only with null user.
@@ -8014,6 +8211,11 @@ function testAuth_returnFromLinkWithRedirect_noCurrentUser_redirectUser() {
80148211
assertEquals('fr', result.user.getLanguageCode());
80158212
auth1.languageCode = 'de';
80168213
assertEquals('de', result.user.getLanguageCode());
8214+
// Redirect result should be cleared after being returned once.
8215+
return auth1.getRedirectResult();
8216+
}).then(function(resultAfterClearing) {
8217+
fireauth.common.testHelper.assertUserCredentialResponse(
8218+
null, null, null, undefined, resultAfterClearing);
80178219
asyncTestCase.signal();
80188220
});
80198221
// Should fire once only with null user.
@@ -8217,6 +8419,11 @@ function testAuth_returnFromLinkWithRedirect_redirectedLoggedInUser_success() {
82178419
// operationType not implemented yet.
82188420
fireauth.constants.OperationType.SIGN_IN,
82198421
result);
8422+
// Redirect result should be cleared after being returned once.
8423+
return auth1.getRedirectResult();
8424+
}).then(function(resultAfterClearing) {
8425+
fireauth.common.testHelper.assertUserCredentialResponse(
8426+
null, null, null, undefined, resultAfterClearing);
82208427
asyncTestCase.signal();
82218428
});
82228429
// Should fire once only with redirected user.
@@ -8402,7 +8609,14 @@ function testAuth_returnFromLinkWithRedirect_error() {
84028609
// Get redirect result should contain an error.
84038610
auth1.getRedirectResult().thenCatch(function(error) {
84048611
fireauth.common.testHelper.assertErrorEquals(expectedError, error);
8612+
// Error in redirect result should be cleared after being returned once.
8613+
return auth1.getRedirectResult();
8614+
}).then(function(resultAfterClearing) {
8615+
fireauth.common.testHelper.assertUserCredentialResponse(
8616+
null, null, null, undefined, resultAfterClearing);
84058617
asyncTestCase.signal();
8618+
}).thenCatch(function(error) {
8619+
fail('Error in event should only be thrown once.');
84068620
});
84078621
// Should fire once only with original user1.
84088622
var idTokenChangeCounter = 0;

0 commit comments

Comments
 (0)