Skip to content

Commit

Permalink
fix(auth): fixes various Auth bugs. (#287)
Browse files Browse the repository at this point in the history 
Fixes verifyAssertion unrecoverable errors when returnIdpCredential is set to true.
In this case, the error code is returned along with the credential in the errorMessage without any
ID token/refresh token.

Catch, suppress/handle when localStorage is null or when `localStorage.getItem`
throws a security error due to access being disabled by the browser for whatever reason.
  • Loading branch information
@bojeil-google @schmidt-sebastian
bojeil-google authored and schmidt-sebastian committed Nov 3, 2017
1 parent 413e296 commit 4dd1c27
Show file tree
Hide file tree
Showing 7 changed files with 268 additions and 20 deletions.
17 changes: 12 additions & 5 deletions packages/auth/src/authstorage.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137,13 +137,16 @@ fireauth.authStorage.Key;
* some mobile browsers. A localStorage change in the foreground window
* will not be detected in the background window via the storage event.
* This was detected in iOS 7.x mobile browsers.
* @param {boolean} webStorageSupported Whether browser web storage is
* supported.
* @constructor @struct @final
*/
fireauth.authStorage.Manager = function(
namespace,
separator,
safariLocalStorageNotSynced,
runsInBackground) {
runsInBackground,
webStorageSupported) {
/** @const @private {string} Storage namespace. */
this.namespace_ = namespace;
/** @const @private {string} Storage namespace key separator. */
Expand All @@ -159,6 +162,8 @@ fireauth.authStorage.Manager = function(
* mobile browsers.
*/
this.runsInBackground_ = runsInBackground;
/** @const @private {boolean} Whether browser web storage is supported. */
this.webStorageSupported_ = webStorageSupported;

/**
* @const @private {!Object.<string, !Array<function()>>} The storage event
Expand Down Expand Up @@ -223,7 +228,8 @@ fireauth.authStorage.Manager.getInstance = function() {
fireauth.authStorage.NAMESPACE_,
fireauth.authStorage.SEPARATOR_,
fireauth.util.isSafariLocalStorageNotSynced(),
fireauth.util.runsInBackground());
fireauth.util.runsInBackground(),
fireauth.util.isWebStorageSupported());
}
return fireauth.authStorage.Manager.instance_;
};
Expand Down Expand Up @@ -337,8 +343,7 @@ fireauth.authStorage.Manager.prototype.addListener =
function(dataKey, id, listener) {
var key = this.getKeyName_(dataKey, id);
// Initialize local map for current key if web storage is supported.
if (typeof goog.global['localStorage'] !== 'undefined' &&
typeof goog.global['localStorage']['getItem'] === 'function') {
if (this.webStorageSupported_) {
this.localMap_[key] = goog.global['localStorage']['getItem'](key);
}
if (goog.object.isEmpty(this.listeners_)) {
Expand Down Expand Up @@ -401,7 +406,9 @@ fireauth.authStorage.Manager.prototype.startListeners_ = function() {
if (!this.runsInBackground_ &&
// Add an exception for IE11 and Edge browsers, we should stick to
// indexedDB in that case.
!fireauth.util.isLocalStorageNotSynchronized()) {
!fireauth.util.isLocalStorageNotSynchronized() &&
// Confirm browser web storage is supported as polling relies on it.
this.webStorageSupported_) {
this.startManualListeners_();
}
};
Expand Down
34 changes: 34 additions & 0 deletions packages/auth/src/rpchandler.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1499,6 +1499,11 @@ fireauth.RpcHandler.validateVerifyAssertionForExistingResponse_ =
fireauth.RpcHandler.ServerError.USER_NOT_FOUND) {
// This corresponds to user-not-found.
throw new fireauth.AuthError(fireauth.authenum.Error.USER_DELETED);
} else if (response[fireauth.RpcHandler.AuthServerField.ERROR_MESSAGE]) {
// Construct developer facing error message from server code in errorMessage
// field.
throw fireauth.RpcHandler.getDeveloperErrorFromCode_(
response[fireauth.RpcHandler.AuthServerField.ERROR_MESSAGE]);
}
// Need confirmation should not be returned when do not create new user flag
// is set.
Expand Down Expand Up @@ -1543,6 +1548,11 @@ fireauth.RpcHandler.validateVerifyAssertionResponse_ = function(response) {
// owner of the account and then link to the returned credential here.
response['code'] = fireauth.authenum.Error.EMAIL_EXISTS;
error = fireauth.AuthErrorWithCredential.fromPlainObject(response);
} else if (response[fireauth.RpcHandler.AuthServerField.ERROR_MESSAGE]) {
// Construct developer facing error message from server code in errorMessage
// field.
error = fireauth.RpcHandler.getDeveloperErrorFromCode_(
response[fireauth.RpcHandler.AuthServerField.ERROR_MESSAGE]);
}
// If error found, throw it.
if (error) {
Expand Down Expand Up @@ -1979,6 +1989,30 @@ fireauth.RpcHandler.hasError_ = function(resp) {
};


/**
* Returns the developer facing error corresponding to the server code provided.
* @param {string} serverErrorCode The server error message.
* @return {!fireauth.AuthError} The corresponding error object.
* @private
*/
fireauth.RpcHandler.getDeveloperErrorFromCode_ = function(serverErrorCode) {
// Encapsulate the server error code in a typical server error response with
// the code populated within. This will convert the response to a developer
// facing one.
return fireauth.RpcHandler.getDeveloperError_({
'error': {
'errors': [
{
'message': serverErrorCode
}
],
'code': 400,
'message': serverErrorCode
}
});
};


/**
* Converts a server response with errors to a developer-facing AuthError.
* @param {!Object} response The server response.
Expand Down
6 changes: 5 additions & 1 deletion packages/auth/src/storage/indexeddb.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,11 @@ fireauth.storage.IndexedDB.prototype.initializeDbAndRun_ =
* @return {boolean} Whether indexedDB is available or not.
*/
fireauth.storage.IndexedDB.isAvailable = function() {
return !!window.indexedDB;
try {
return !!goog.global['indexedDB'];
} catch (e) {
return false;
}
};


Expand Down
15 changes: 14 additions & 1 deletion packages/auth/src/storage/localstorage.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,20 @@ fireauth.storage.LocalStorage = function() {

/** @return {?Storage|undefined} The global localStorage instance. */
fireauth.storage.LocalStorage.getGlobalStorage = function() {
return goog.global['localStorage'];
try {
var storage = goog.global['localStorage'];
// Try editing web storage. If an error is thrown, it may be disabled.
var key = fireauth.util.generateEventId();
if (storage) {
storage['setItem'](key, '1');
storage['removeItem'](key);
}
return storage;
} catch (e) {
// In some cases, browsers with web storage disabled throw an error simply
// on access.
return null;
}
};


Expand Down
15 changes: 14 additions & 1 deletion packages/auth/src/storage/sessionstorage.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,20 @@ fireauth.storage.SessionStorage = function() {

/** @return {?Storage|undefined} The global sessionStorage instance. */
fireauth.storage.SessionStorage.getGlobalStorage = function() {
return goog.global['sessionStorage'];
try {
var storage = goog.global['sessionStorage'];
// Try editing web storage. If an error is thrown, it may be disabled.
var key = fireauth.util.generateEventId();
if (storage) {
storage['setItem'](key, '1');
storage['removeItem'](key);
}
return storage;
} catch (e) {
// In some cases, browsers with web storage disabled throw an error simply
// on access.
return null;
}
};


Expand Down
72 changes: 60 additions & 12 deletions packages/auth/test/authstorage_test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ function tearDown() {
* synchronized manager instance used for testing.
*/
function getDefaultManagerInstance() {
return new fireauth.authStorage.Manager('firebase', ':', false, true);
return new fireauth.authStorage.Manager('firebase', ':', false, true, true);
}


Expand Down Expand Up @@ -404,7 +404,8 @@ function testGetSet_persistentStorage_noId() {


function testAddRemoveListeners_localStorage() {
var manager = new fireauth.authStorage.Manager('name', ':', false, true);
var manager =
new fireauth.authStorage.Manager('name', ':', false, true, true);
var listener1 = goog.testing.recordFunction();
var listener2 = goog.testing.recordFunction();
var listener3 = goog.testing.recordFunction();
Expand Down Expand Up @@ -470,7 +471,8 @@ function testAddRemoveListeners_localStorage() {


function testAddRemoveListeners_localStorage_nullKey() {
var manager = new fireauth.authStorage.Manager('name', ':', false, true);
var manager =
new fireauth.authStorage.Manager('name', ':', false, true, true);
var listener1 = goog.testing.recordFunction();
var listener2 = goog.testing.recordFunction();
var listener3 = goog.testing.recordFunction();
Expand Down Expand Up @@ -522,7 +524,8 @@ function testAddRemoveListeners_localStorage_ie10() {
function() {
return true;
});
var manager = new fireauth.authStorage.Manager('name', ':', false, true);
var manager =
new fireauth.authStorage.Manager('name', ':', false, true, true);
var listener1 = goog.testing.recordFunction();
var listener2 = goog.testing.recordFunction();
var listener3 = goog.testing.recordFunction();
Expand Down Expand Up @@ -637,7 +640,8 @@ function testAddRemoveListeners_indexeddb() {
function() {
return mockIndexeddb;
});
var manager = new fireauth.authStorage.Manager('name', ':', false, true);
var manager =
new fireauth.authStorage.Manager('name', ':', false, true, true);
var listener1 = goog.testing.recordFunction();
var listener2 = goog.testing.recordFunction();
var listener3 = goog.testing.recordFunction();
Expand Down Expand Up @@ -714,7 +718,8 @@ function testAddRemoveListeners_indexeddb_cannotRunInBackground() {
return mockIndexeddb;
});
// Cannot run in the background.
var manager = new fireauth.authStorage.Manager('name', ':', false, false);
var manager =
new fireauth.authStorage.Manager('name', ':', false, false, true);
var listener1 = goog.testing.recordFunction();
var listener2 = goog.testing.recordFunction();
var listener3 = goog.testing.recordFunction();
Expand Down Expand Up @@ -757,7 +762,7 @@ function testAddRemoveListeners_indexeddb_cannotRunInBackground() {

function testSafariLocalStorageSync_newEvent() {
var manager =
new fireauth.authStorage.Manager('firebase', ':', true, true);
new fireauth.authStorage.Manager('firebase', ':', true, true, true);
// Simulate Safari bug.
stubs.replace(
fireauth.util,
Expand Down Expand Up @@ -797,7 +802,7 @@ function testSafariLocalStorageSync_cannotRunInBackground() {
// Realistically only storage event should trigger here.
// Test when new data is added to storage.
var manager =
new fireauth.authStorage.Manager('firebase', ':', true, false);
new fireauth.authStorage.Manager('firebase', ':', true, false, true);
// Simulate Safari bug.
stubs.replace(
fireauth.util,
Expand Down Expand Up @@ -837,7 +842,7 @@ function testSafariLocalStorageSync_deletedEvent() {
// Realistically only storage event should trigger here.
// Test when old data is deleted from storage.
var manager =
new fireauth.authStorage.Manager('firebase', ':', true, true);
new fireauth.authStorage.Manager('firebase', ':', true, true, true);
var key1 = {'name': 'authEvent', 'persistent': true};
// Simulate Safari bug.
stubs.replace(
Expand Down Expand Up @@ -879,7 +884,7 @@ function testRunsInBackground_storageEventMode() {
var key = {name: 'authEvent', persistent: 'local'};
var storageKey = 'firebase:authEvent:appId1';
var manager = new fireauth.authStorage.Manager(
'firebase', ':', false, false);
'firebase', ':', false, false, true);
var listener1 = goog.testing.recordFunction();
var expectedEvent = {
type: 'signInViaPopup',
Expand Down Expand Up @@ -923,6 +928,49 @@ function testRunsInBackground_storageEventMode() {
}


function testRunsInBackground_webStorageNotSupported() {
// Test when browser does not run in the background and web storage is not
// supported. Polling should not be turned on.
var key = {name: 'authEvent', persistent: 'local'};
var storageKey = 'firebase:authEvent:appId1';
// Simulate manager doesn't support web storage and can't run in the
// background. Normally when a browser can't run in the background, polling is
// enabled.
var manager = new fireauth.authStorage.Manager(
'firebase', ':', false, false, false);
var listener1 = goog.testing.recordFunction();
var expectedEvent = {
type: 'signInViaPopup',
eventId: '1234',
callbackUrl: 'http://www.example.com/#oauthResponse',
sessionId: 'SESSION_ID'
};

// Add listener.
manager.addListener(key, appId, listener1);
// Test that polling function is not set by updating localStorage with some
// data. This should not happen realistically when web storage is disabled.
window.localStorage.setItem(storageKey, JSON.stringify(expectedEvent));
// Run clock.
clock.tick(1000);
// Listener should not trigger.
assertEquals(0, listener1.getCallCount());
// Clear storage.
window.localStorage.clear();
// Run clock.
clock.tick(1000);
// Listener should not trigger.
assertEquals(0, listener1.getCallCount());
// Save Auth event and confirm listener not triggered.
// This normally simulates polling.
window.localStorage.setItem(storageKey, JSON.stringify(expectedEvent));
// Run clock.
clock.tick(1000);
// Listener should not trigger.
assertEquals(0, listener1.getCallCount());
}


function testRunsInBackground_pollingMode() {
// Test when browser does not run in the background while another tab is in
// foreground.
Expand All @@ -932,7 +980,7 @@ function testRunsInBackground_pollingMode() {
var key = {name: 'authEvent', persistent: 'local'};
var storageKey = 'firebase:authEvent:appId1';
var manager = new fireauth.authStorage.Manager(
'firebase', ':', false, false);
'firebase', ':', false, false, true);
var listener1 = goog.testing.recordFunction();
var expectedEvent = {
type: 'signInViaPopup',
Expand Down Expand Up @@ -984,7 +1032,7 @@ function testRunsInBackground_currentTabChangesIgnored() {
var key = {name: 'authEvent', persistent: 'local'};
var storageKey = 'firebase:authEvent:appId1';
var manager = new fireauth.authStorage.Manager(
'firebase', ':', false, false);
'firebase', ':', false, false, true);
var listener1 = goog.testing.recordFunction();
var expectedEvent = {
type: 'signInViaPopup',
Expand Down
Loading

0 comments on commit 4dd1c27

Please sign in to comment.