Skip to content

clear redirect result after signOut or getRedirectResult being called #2049

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 2, 2019
Merged

clear redirect result after signOut or getRedirectResult being called #2049

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 41 additions & 2 deletions packages/auth/src/auth.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,10 @@ fireauth.Auth = function(app) {
this.userDeleteListener_ = goog.bind(this.handleUserDelete_, this);
/** @private {!function(!Object)} The handler for user invalidation. */
this.userInvalidatedListener_ = goog.bind(this.handleUserInvalidated_, this);
/**
* @private {?fireauth.AuthEventManager} The Auth event manager instance.
*/
this.authEventManager_ = null;
// TODO: find better way to enable or disable auth event manager.
if (fireauth.AuthEventManager.ENABLED) {
// Initialize Auth event manager to handle popup and redirect operations.
Expand Down Expand Up @@ -713,8 +717,9 @@ fireauth.Auth.prototype.signInWithRedirect = function(provider) {
* redirect sign, will reject with the proper error. If no redirect operation
* called, resolves with null.
* @return {!goog.Promise<!fireauth.AuthEventManager.Result>}
* @private
*/
fireauth.Auth.prototype.getRedirectResult = function() {
fireauth.Auth.prototype.getRedirectResultWithoutClearing_ = function() {
// Check if popup and redirect are supported in this environment.
if (!fireauth.util.isPopupRedirectSupported()) {
return goog.Promise.reject(new fireauth.AuthError(
Expand All @@ -737,6 +742,29 @@ fireauth.Auth.prototype.getRedirectResult = function() {
};


/**
* In addition to returning the redirect result as in
* `getRedirectResultWithoutClearing_`, this will also clear the cached
* redirect result for security reasons.
* @return {!goog.Promise<!fireauth.AuthEventManager.Result>}
*/
fireauth.Auth.prototype.getRedirectResult = function() {
return this.getRedirectResultWithoutClearing_()
.then((result) => {
if (this.authEventManager_) {
this.authEventManager_.clearRedirectResult();
}
return result;
})
.thenCatch((error) => {
if (this.authEventManager_) {
this.authEventManager_.clearRedirectResult();
}
throw error;
});
};


/**
* Asynchronously sets the provided user as currentUser on the current Auth
* instance.
Expand Down Expand Up @@ -866,6 +894,17 @@ fireauth.Auth.prototype.signOut = function() {
// Wait for final state to be ready first, otherwise a signed out user could
// come back to life.
var p = this.redirectStateIsReady_.then(function() {
// Clear any cached redirect result on sign out, even if user is already
// signed out. For example, sign in could fail due to account conflict
// error, the error in redirect result should still be cleared. There is
// also the use case where you keep a reference to a signed out user and
// call signedOutUser.linkWithRedirect(provider). Even though the user is
// signed out, getRedirectResult() will resolve with the modified signed
// out user. This could also throw an error
// (provider already linked, etc).
if (self.authEventManager_) {
self.authEventManager_.clearRedirectResult();
}
// Ignore if already signed out.
if (!self.currentUser_()) {
return goog.Promise.resolve();
Expand Down Expand Up @@ -999,7 +1038,7 @@ fireauth.Auth.prototype.initAuthRedirectState_ = function() {
// Wait first for state to be loaded from storage.
return this.authStateLoaded_.then(function() {
// Resolve any pending redirect result.
return self.getRedirectResult();
return self.getRedirectResultWithoutClearing_();
}).thenCatch(function(error) {
// Ignore any error in the process. Redirect could be not supported.
return;
Expand Down
214 changes: 214 additions & 0 deletions packages/auth/test/auth_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6959,6 +6959,155 @@ function testAuth_getRedirectResult_unsupportedEnvironment() {
}


function testAuth_signOut_clearSuccessRedirectResult() {
// Tests getRedirectResult with success event after signOut being called.
fireauth.AuthEventManager.ENABLED = true;
const expectedCred = fireauth.GoogleAuthProvider.credential(
null, 'ACCESS_TOKEN');
// Expected sign in via redirect successful Auth event.
const expectedAuthEvent = new fireauth.AuthEvent(
fireauth.AuthEvent.Type.SIGN_IN_VIA_REDIRECT,
null,
'http://www.example.com/#response',
'SESSION_ID');
// Stub instantiateOAuthSignInHandler.
stubs.replace(
fireauth.AuthEventManager, 'instantiateOAuthSignInHandler',
function(authDomain, apiKey, appName) {
return {
'addAuthEventListener': function(handler) {
// auth1 should be subscribed.
const manager = fireauth.AuthEventManager.getManager(
config3['authDomain'], config3['apiKey'], app1.name);
assertTrue(manager.isSubscribed(auth1));
// In this case run immediately with expected redirect event.
handler(expectedAuthEvent);
asyncTestCase.signal();
},
'initializeAndWait': function() { return goog.Promise.resolve(); },
'shouldBeInitializedEarly': function() { return false; },
'hasVolatileStorage': function() { return false; }
};
});
// Simulate successful finishPopupAndRedirectSignIn.
stubs.replace(
fireauth.Auth.prototype,
'finishPopupAndRedirectSignIn',
function(requestUri, sessionId, postBody) {
assertEquals('http://www.example.com/#response', requestUri);
assertEquals('SESSION_ID', sessionId);
assertNull(postBody);
user1 = new fireauth.AuthUser(
config3, expectedTokenResponse, accountInfo);
expectedPopupResult = {
'user': user1,
'credential': expectedCred,
'additionalUserInfo': expectedAdditionalUserInfo,
'operationType': fireauth.constants.OperationType.SIGN_IN
};
// User 1 should be set here and saved to storage.
auth1.setCurrentUser_(user1);
asyncTestCase.signal();
return currentUserStorageManager.setCurrentUser(user1).then(() => {
return expectedPopupResult;
});
});
let user1, expectedPopupResult;
asyncTestCase.waitForSignals(3);
const pendingRedirectManager = new fireauth.storage.PendingRedirectManager(
config3['apiKey'] + ':' + appId1);
currentUserStorageManager = new fireauth.storage.UserManager(
config3['apiKey'] + ':' + appId1);
app1 = firebase.initializeApp(config3, appId1);
auth1 = app1.auth();
pendingRedirectManager.setPendingStatus().then(() => {
// Verify that the redirect result is resolved before signing out.
const manager = fireauth.AuthEventManager.getManager(
config3['authDomain'], config3['apiKey'], app1.name);
return manager.getRedirectResult().then((result) => {
// Expected result returned.
assertObjectEquals(expectedPopupResult, result);
return auth1.signOut();
}).then(() => {
// signOut should clear the cached redirect result.
return auth1.getRedirectResult();
}).then((resultAfterClearing) => {
fireauth.common.testHelper.assertUserCredentialResponse(
null, null, null, undefined, resultAfterClearing);
asyncTestCase.signal();
});
});
}


function testAuth_signOut_clearErrorRedirectResult() {
// Tests getRedirectResult with error event after signOut being called.
fireauth.AuthEventManager.ENABLED = true;
// The expected error.
const expectedError =
new fireauth.AuthError(fireauth.authenum.Error.INTERNAL_ERROR);
// Expected sign in via redirect error Auth event.
const expectedAuthEvent = new fireauth.AuthEvent(
fireauth.AuthEvent.Type.SIGN_IN_VIA_REDIRECT,
null,
null,
null,
expectedError);
// Stub instantiateOAuthSignInHandler.
stubs.replace(
fireauth.AuthEventManager, 'instantiateOAuthSignInHandler',
function(authDomain, apiKey, appName) {
return {
'addAuthEventListener': function(handler) {
// auth1 should be subscribed.
const manager = fireauth.AuthEventManager.getManager(
config3['authDomain'], config3['apiKey'], app1.name);
assertTrue(manager.isSubscribed(auth1));
// In this case run immediately with expected redirect event.
handler(expectedAuthEvent);
asyncTestCase.signal();
},
'initializeAndWait': function() { return goog.Promise.resolve(); },
'shouldBeInitializedEarly': function() { return false; },
'hasVolatileStorage': function() { return false; }
};
});
stubs.replace(
fireauth.Auth.prototype,
'finishPopupAndRedirectSignIn',
function(requestUri, sessionId, postBody) {
fail('finishPopupAndRedirectSignIn should not run on event error!');
});
asyncTestCase.waitForSignals(2);
const pendingRedirectManager = new fireauth.storage.PendingRedirectManager(
config3['apiKey'] + ':' + appId1);
currentUserStorageManager = new fireauth.storage.UserManager(
config3['apiKey'] + ':' + appId1);
app1 = firebase.initializeApp(config3, appId1);
auth1 = app1.auth();
pendingRedirectManager.setPendingStatus().then(() => {
// Verify that the redirect result is rejected with error before
// signing out.
const manager = fireauth.AuthEventManager.getManager(
config3['authDomain'], config3['apiKey'], app1.name);
return manager.getRedirectResult().thenCatch((error) => {
// Expected error returned.
fireauth.common.testHelper.assertErrorEquals(expectedError, error);
return auth1.signOut();
}).then(() => {
// signOut should clear the error in cached redirect result.
return auth1.getRedirectResult();
}).then((resultAfterClearing) => {
fireauth.common.testHelper.assertUserCredentialResponse(
null, null, null, undefined, resultAfterClearing);
asyncTestCase.signal();
});
}).thenCatch((error) => {
fail('Redirect result should be cleared by signOut.');
}) ;
}


function testAuth_returnFromSignInWithRedirect_success_withoutPostBody() {
// Tests the return from a successful sign in with redirect.
fireauth.AuthEventManager.ENABLED = true;
Expand Down Expand Up @@ -7025,6 +7174,11 @@ function testAuth_returnFromSignInWithRedirect_success_withoutPostBody() {
auth1.getRedirectResult().then(function(result) {
// Expected result returned.
assertObjectEquals(expectedPopupResult, result);
// Redirect result should be cleared after being returned once.
return auth1.getRedirectResult();
}).then(function(resultAfterClearing) {
fireauth.common.testHelper.assertUserCredentialResponse(
null, null, null, undefined, resultAfterClearing);
asyncTestCase.signal();
});
});
Expand Down Expand Up @@ -7114,6 +7268,11 @@ function testAuth_returnFromSignInWithRedirect_success_withPostBody() {
auth1.getRedirectResult().then(function(result) {
// Expected result returned.
assertObjectEquals(expectedPopupResult, result);
// Redirect result should be cleared after being returned once.
return auth1.getRedirectResult();
}).then(function(resultAfterClearing) {
fireauth.common.testHelper.assertUserCredentialResponse(
null, null, null, undefined, resultAfterClearing);
asyncTestCase.signal();
});
});
Expand Down Expand Up @@ -7226,6 +7385,11 @@ function testAuth_returnFromSignInWithRedirect_withExistingUser() {
// Newly signed in user.
assertEquals(user1, auth1['currentUser']);
assertObjectEquals(expectedPopupResult, result);
// Redirect result should be cleared after being returned once.
return auth1.getRedirectResult();
}).then(function(resultAfterClearing) {
fireauth.common.testHelper.assertUserCredentialResponse(
null, null, null, undefined, resultAfterClearing);
asyncTestCase.signal();
});
// State listener should fire once only with the final redirected user.
Expand Down Expand Up @@ -7296,7 +7460,14 @@ function testAuth_returnFromSignInWithRedirect_error() {
// Get redirect result should return the expected error.
auth1.getRedirectResult().thenCatch(function(error) {
fireauth.common.testHelper.assertErrorEquals(expectedError, error);
// Error in redirect result should be cleared after being returned once.
return auth1.getRedirectResult();
}).then(function(resultAfterClearing) {
fireauth.common.testHelper.assertUserCredentialResponse(
null, null, null, undefined, resultAfterClearing);
asyncTestCase.signal();
}).thenCatch(function(error) {
fail('Error in event should only be thrown once.');
});
});
// State listener should fire once only with null user.
Expand Down Expand Up @@ -7365,6 +7536,12 @@ function testAuth_returnFromSignInWithRedirect_error_webStorageNotSupported() {
pendingRedirectManager.setPendingStatus().then(function() {
// Get redirect result should return the expected error.
auth1.getRedirectResult().thenCatch(function(error) {
fireauth.common.testHelper.assertErrorEquals(expectedError, error);
// Errors not being tied to a sign-in session should not be cleared.
return auth1.getRedirectResult();
}).then(function(result) {
fail('Errors not being tied to a sign-in session should not be cleared.');
}).thenCatch(function(error) {
fireauth.common.testHelper.assertErrorEquals(expectedError, error);
asyncTestCase.signal();
});
Expand Down Expand Up @@ -7535,6 +7712,11 @@ function testAuth_returnFromLinkWithRedirect_success_withoutPostBody() {
// Get redirect result should resolve with expected user and credential.
auth1.getRedirectResult().then(function(result) {
assertObjectEquals(expectedPopupResult, result);
// Redirect result should be cleared after being returned once.
return auth1.getRedirectResult();
}).then(function(resultAfterClearing) {
fireauth.common.testHelper.assertUserCredentialResponse(
null, null, null, undefined, resultAfterClearing);
asyncTestCase.signal();
});
// Should fire once only with the final redirected user.
Expand Down Expand Up @@ -7636,6 +7818,11 @@ function testAuth_returnFromLinkWithRedirect_success_withPostBody() {
// Get redirect result should resolve with expected user and credential.
auth1.getRedirectResult().then(function(result) {
assertObjectEquals(expectedPopupResult, result);
// Redirect result should be cleared after being returned once.
return auth1.getRedirectResult();
}).then(function(resultAfterClearing) {
fireauth.common.testHelper.assertUserCredentialResponse(
null, null, null, undefined, resultAfterClearing);
asyncTestCase.signal();
});
// Should fire once only with the final redirected user.
Expand Down Expand Up @@ -7779,6 +7966,11 @@ function testAuth_returnFromLinkWithRedirect_redirectedLoggedOutUser_success() {
assertEquals('fr', result.user.getLanguageCode());
auth1.languageCode = 'de';
assertEquals('de', result.user.getLanguageCode());
// Redirect result should be cleared after being returned once.
return auth1.getRedirectResult();
}).then(function(resultAfterClearing) {
fireauth.common.testHelper.assertUserCredentialResponse(
null, null, null, undefined, resultAfterClearing);
asyncTestCase.signal();
});
// Should fire once only with the original user.
Expand Down Expand Up @@ -7889,6 +8081,11 @@ function testAuth_redirectedLoggedOutUser_differentAuthDomain() {
assertEquals('fr', result.user.getLanguageCode());
auth1.languageCode = 'de';
assertEquals('de', result.user.getLanguageCode());
// Redirect result should be cleared after being returned once.
return auth1.getRedirectResult();
}).then(function(resultAfterClearing) {
fireauth.common.testHelper.assertUserCredentialResponse(
null, null, null, undefined, resultAfterClearing);
asyncTestCase.signal();
});
// Should fire once only with null user.
Expand Down Expand Up @@ -8014,6 +8211,11 @@ function testAuth_returnFromLinkWithRedirect_noCurrentUser_redirectUser() {
assertEquals('fr', result.user.getLanguageCode());
auth1.languageCode = 'de';
assertEquals('de', result.user.getLanguageCode());
// Redirect result should be cleared after being returned once.
return auth1.getRedirectResult();
}).then(function(resultAfterClearing) {
fireauth.common.testHelper.assertUserCredentialResponse(
null, null, null, undefined, resultAfterClearing);
asyncTestCase.signal();
});
// Should fire once only with null user.
Expand Down Expand Up @@ -8217,6 +8419,11 @@ function testAuth_returnFromLinkWithRedirect_redirectedLoggedInUser_success() {
// operationType not implemented yet.
fireauth.constants.OperationType.SIGN_IN,
result);
// Redirect result should be cleared after being returned once.
return auth1.getRedirectResult();
}).then(function(resultAfterClearing) {
fireauth.common.testHelper.assertUserCredentialResponse(
null, null, null, undefined, resultAfterClearing);
asyncTestCase.signal();
});
// Should fire once only with redirected user.
Expand Down Expand Up @@ -8402,7 +8609,14 @@ function testAuth_returnFromLinkWithRedirect_error() {
// Get redirect result should contain an error.
auth1.getRedirectResult().thenCatch(function(error) {
fireauth.common.testHelper.assertErrorEquals(expectedError, error);
// Error in redirect result should be cleared after being returned once.
return auth1.getRedirectResult();
}).then(function(resultAfterClearing) {
fireauth.common.testHelper.assertUserCredentialResponse(
null, null, null, undefined, resultAfterClearing);
asyncTestCase.signal();
}).thenCatch(function(error) {
fail('Error in event should only be thrown once.');
});
// Should fire once only with original user1.
var idTokenChangeCounter = 0;
Expand Down