- Sanitizes all URLs passed via query parameters to prevent JS inject… (#345)

* - Sanitizes all URLs passed via query parameters to prevent JS injection via URL.
- Appends photo size for profile image hosted in Google in demo app.

PiperOrigin-RevId: 188789358
Change-Id: Ib3faa82cdb8efcd9b91edd66a0fc1df28d14d9d4

* updated readme and change log

Author: wti806

README.md

@@ -63,17 +63,17 @@ Localized versions of the widget are available through the CDN. To use a localiz
 localized JS library instead of the default library:
 
 ```html
-<script src="https://www.gstatic.com/firebasejs/ui/2.6.2/firebase-ui-auth__{LANGUAGE_CODE}.js"></script>
-<link type="text/css" rel="stylesheet" href="https://www.gstatic.com/firebasejs/ui/2.6.2/firebase-ui-auth.css" />
+<script src="https://www.gstatic.com/firebasejs/ui/2.6.3/firebase-ui-auth__{LANGUAGE_CODE}.js"></script>
+<link type="text/css" rel="stylesheet" href="https://www.gstatic.com/firebasejs/ui/2.6.3/firebase-ui-auth.css" />
 ```
 
 where `{LANGUAGE_CODE}` is replaced by the code of the language you want. For example, the French
 version of the library is available at
-`https://www.gstatic.com/firebasejs/ui/2.6.2/firebase-ui-auth__fr.js`. The list of available
+`https://www.gstatic.com/firebasejs/ui/2.6.3/firebase-ui-auth__fr.js`. The list of available
 languages and their respective language codes can be found at [LANGUAGES.md](LANGUAGES.md).
 
 Right-to-left languages also require the right-to-left version of the stylesheet, available at
-`https://www.gstatic.com/firebasejs/ui/2.6.2/firebase-ui-auth-rtl.css`, instead of the default
+`https://www.gstatic.com/firebasejs/ui/2.6.3/firebase-ui-auth-rtl.css`, instead of the default
 stylesheet. The supported right-to-left languages are Arabic (ar), Farsi (fa), and Hebrew (iw).
 
 ### Option 2: npm Module

changelog.txt

@@ -0,0 +1,2 @@
+fixed - Sanitizes all URLs passed via query parameters to prevent JS injection via URL.
+fixed - Appends photo size for profile image hosted in Google in demo app.

demo/public/app.js

@@ -112,7 +112,16 @@ var handleSignedInUser = function(user) {
   document.getElementById('email').textContent = user.email;
   document.getElementById('phone').textContent = user.phoneNumber;
   if (user.photoURL){
-    document.getElementById('photo').src = user.photoURL;
+    var photoURL = user.photoURL;
+    // Append size to the photo URL for Google hosted images to avoid requesting
+    // the image with its original resolution (using more bandwidth than needed)
+    // when it is going to be presented in smaller size.
+    if ((photoURL.indexOf('googleusercontent.com') != -1) ||
+        (photoURL.indexOf('ggpht.com') != -1)) {
+      photoURL = photoURL + '?sz=' +
+          document.getElementById('photo').clientHeight;
+    }
+    document.getElementById('photo').src = photoURL;
     document.getElementById('photo').style.display = 'block';
   } else {
     document.getElementById('photo').style.display = 'none';

javascript/utils/util.js

@@ -22,6 +22,7 @@ goog.require('goog.Promise');
 goog.require('goog.dom');
 goog.require('goog.events');
 goog.require('goog.events.EventType');
+goog.require('goog.html.SafeUrl');
 goog.require('goog.userAgent');
 goog.require('goog.window');
 
@@ -32,9 +33,21 @@ goog.require('goog.window');
  * some browsers don't allow overwriting of the native object.
  *
  * @param {string} url The target URL.
+ * @param {?Window=} opt_win Optional window whose location is to be reassigned.
  */
-firebaseui.auth.util.goTo = function(url) {
-  window.location.assign(url);
+firebaseui.auth.util.goTo = function(url, opt_win) {
+  var win = opt_win || window;
+  // Sanitize URL before redirect.
+  win.location.assign(firebaseui.auth.util.sanitizeUrl(url));
+};
+
+
+/**
+ * @param {string} url The plain unsafe URL.
+ * @return {string} The sanitized safe version of the provided URL.
+ */
+firebaseui.auth.util.sanitizeUrl = function(url) {
+  return goog.html.SafeUrl.unwrap(goog.html.SafeUrl.sanitize(url));
 };
 
 
@@ -67,9 +80,13 @@ firebaseui.auth.util.goBack = function() {
   * since some browsers don't allow to overwrite the native object.
   *
   * @param {string} url The target URL.
+  * @param {?Window=} opt_win Optional window whose parent's location is to
+  *     be reassigned.
   */
-firebaseui.auth.util.openerGoTo = function(url) {
-  window.opener.location.assign(url);
+firebaseui.auth.util.openerGoTo = function(url, opt_win) {
+  var win = opt_win || window;
+  // Sanitize URL before redirect.
+  win.opener.location.assign(firebaseui.auth.util.sanitizeUrl(url));
 };
 
 

javascript/utils/util_test.js

@@ -117,3 +117,95 @@ function testOnDomReady() {
   // Should resolve immediately without any error.
   return firebaseui.auth.util.onDomReady();
 }
+
+
+function testGoTo_safeUrl() {
+  var mockWin = {
+    location: {
+      assign: goog.testing.recordFunction()
+    }
+  };
+  stubs.reset();
+
+  // Confirm goTo redirects as expected to safe URLs.
+  firebaseui.auth.util.goTo(
+      'https://www.example.com:80/path/?a=1#b=2', mockWin);
+  assertEquals(1, mockWin.location.assign.getCallCount());
+  assertEquals(
+      'https://www.example.com:80/path/?a=1#b=2',
+      mockWin.location.assign.getLastCall().getArgument(0));
+}
+
+
+function testGoTo_unsafeUrl() {
+  var mockWin = {
+    location: {
+      assign: goog.testing.recordFunction()
+    }
+  };
+  stubs.reset();
+
+  // Confirm URLs are sanitized before redirection.
+  firebaseui.auth.util.goTo(
+      'javascript:doEvilStuff()', mockWin);
+  assertEquals(1, mockWin.location.assign.getCallCount());
+  assertEquals(
+      'about:invalid#zClosurez',
+      mockWin.location.assign.getLastCall().getArgument(0));
+}
+
+
+function testOpenerGoTo_safeUrl() {
+  var mockWin = {
+    opener: {
+      location: {
+        assign: goog.testing.recordFunction()
+      }
+    }
+  };
+  stubs.reset();
+
+  // Confirm openerGoTo redirects as expected to safe URLs.
+  firebaseui.auth.util.openerGoTo(
+      'https://www.example.com:80/path/?a=1#b=2', mockWin);
+  assertEquals(1, mockWin.opener.location.assign.getCallCount());
+  assertEquals(
+      'https://www.example.com:80/path/?a=1#b=2',
+      mockWin.opener.location.assign.getLastCall().getArgument(0));
+}
+
+
+function testOpenerGoTo_unsafeUrl() {
+  var mockWin = {
+    opener: {
+      location: {
+        assign: goog.testing.recordFunction()
+      }
+    }
+  };
+  stubs.reset();
+
+  // Confirm URLs are sanitized before redirection.
+  firebaseui.auth.util.openerGoTo(
+      'javascript:doEvilStuff()', mockWin);
+  assertEquals(1, mockWin.opener.location.assign.getCallCount());
+  assertEquals(
+      'about:invalid#zClosurez',
+      mockWin.opener.location.assign.getLastCall().getArgument(0));
+}
+
+
+function testSanitizeUrl_safeUrl() {
+  assertEquals(
+      'https://www.example.com:80/path/?a=1#b=2',
+      firebaseui.auth.util.sanitizeUrl(
+          'https://www.example.com:80/path/?a=1#b=2'));
+}
+
+
+function testSanitizeUrl_unsafeUrl() {
+  assertEquals(
+      'about:invalid#zClosurez',
+      firebaseui.auth.util.sanitizeUrl('javascript:doEvilStuff()'));
+}
+

javascript/widgets/authui.js

@@ -911,7 +911,7 @@ firebaseui.auth.AuthUI.prototype.startSignInWithEmailAndPassword =
  * Create a new email and password account.
  * @param {string} email The email to sign up with.
  * @param {string} password The password to sign up with.
- * @return {!firebase.Promise<!firebase.User>}
+ * @return {!firebase.Promise<!firebase.auth.UserCredential>}
  */
 firebaseui.auth.AuthUI.prototype.startCreateUserWithEmailAndPassword =
     function(email, password) {
@@ -925,17 +925,15 @@ firebaseui.auth.AuthUI.prototype.startCreateUserWithEmailAndPassword =
       // For anonymous user upgrade, call link with credential on the external
       // Auth user. Otherwise merge conflict will always occur when linking the
       // credential to the external anonymous user after creation.
-      return /** @type {!firebase.Promise<!firebase.User>} */ (
-          user.linkAndRetrieveDataWithCredential(credential)
-          .then(function(result) {
-            return result['user'];
-          }));
+      return /** @type {!firebase.Promise<!firebase.auth.UserCredential>} */ (
+          user.linkAndRetrieveDataWithCredential(credential));
     } else {
       // Start create user with email and password. This runs on the internal
       // Auth instance as finish sign in will sign in with that same credential
       // to developer Auth instance.
-      return /** @type {!firebase.Promise<!firebase.User>} */ (
-          self.getAuth().createUserWithEmailAndPassword(email, password));
+      return /** @type {!firebase.Promise<!firebase.auth.UserCredential>} */ (
+          self.getAuth().createUserAndRetrieveDataWithEmailAndPassword(
+              email, password));
     }
   };
   // Initialize current user if auto upgrade is enabled beforing running
@@ -1149,24 +1147,28 @@ firebaseui.auth.AuthUI.prototype.finishSignInWithCredential =
   this.checkIfDestroyed_();
   var self = this;
   var cb = function(user) {
-    // Anonymous user upgrade successful, resolve immediately with the user.
-    // No need to sign in again with the same credential on the external Auth
-    // instance.
+    // Anonymous user upgrade successful, sign out on internal instance and
+    // resolve with the user. No need to sign in again with the same credential
+    // on the external Auth instance.
     // If user is signed in on internal instance, ignore the user on external
-    // instance and finish the sign in on external instance.
+    // instance, sign out on internal instance and finish the sign in on
+    // external instance.
     if (self.currentUser_ &&
         !self.currentUser_['isAnonymous'] &&
         self.getConfig().autoUpgradeAnonymousUsers() &&
         !self.getAuth().currentUser) {
-      return (firebase.Promise || goog.Promise).resolve(self.currentUser_);
+      return self.clearTempAuthState().then(function() {
+          return self.currentUser_;
+      });
     } else if (user) {
       // TODO: optimize and fail directly as this will fail in most cases
       // with error credential already in use.
       // There are cases where this is required. For example, when email
       // mismatch occurs and the user continues with the new account.
       return /** @type {!firebase.Promise<!firebase.User>} */ (
-          user.linkWithCredential(credential)
-          .then(function() {
+          self.clearTempAuthState().then(function() {
+            return user.linkWithCredential(credential);
+          }).then(function() {
             return user;
           }, function(error) {
             // Rethrow email already in use error so it can trigger the account
@@ -1183,7 +1185,84 @@ firebaseui.auth.AuthUI.prototype.finishSignInWithCredential =
       // Finishes sign in with the supplied credential on the developer provided
       // Auth instance. On completion, this will redirect to signInSuccessUrl or
       // trigger the signInSuccess callback.
-      return self.getExternalAuth().signInWithCredential(credential);
+      return self.clearTempAuthState().then(function() {
+        return self.getExternalAuth().signInWithCredential(credential);
+      });
+    }
+  };
+  // Initialize current user if auto upgrade is enabled beforing running
+  // callback and returning result.
+  return this.initializeForAutoUpgrade_(cb);
+};
+
+
+/**
+ * Finishes FirebaseUI login with the given 3rd party credentials.
+ * @param {!firebaseui.auth.AuthResult} authResult The Auth result.
+ * @return {!firebase.Promise<!firebaseui.auth.AuthResult>}
+ */
+firebaseui.auth.AuthUI.prototype.finishSignInAndRetrieveDataWithAuthResult =
+    function(authResult) {
+  // Check if instance is already destroyed.
+  this.checkIfDestroyed_();
+  var self = this;
+  var cb = function(user) {
+    if (self.currentUser_ &&
+        !self.currentUser_['isAnonymous'] &&
+        self.getConfig().autoUpgradeAnonymousUsers() &&
+        !self.getAuth().currentUser) {
+      return self.clearTempAuthState().then(function() {
+        // Do not expose password Auth credential to signInSuccess callback.
+        if (authResult['credential']['providerId'] == 'password') {
+          authResult['credential'] = null;
+        }
+        return authResult;
+      });
+    } else if (user) {
+      // TODO: optimize and fail directly as this will fail in most cases
+      // with error credential already in use.
+      // There are cases where this is required. For example, when email
+      // mismatch occurs and the user continues with the new account.
+      return /** @type {!firebase.Promise<!firebaseui.auth.AuthResult>} */ (
+          self.clearTempAuthState().then(function() {
+            return user.linkAndRetrieveDataWithCredential(
+                authResult['credential']);
+          }).then(function(userCredential) {
+            authResult['user'] = userCredential['user'];
+            authResult['credential'] = userCredential['credential'];
+            authResult['operationType'] = userCredential['operationType'];
+            authResult['additionalUserInfo'] =
+                userCredential['additionalUserInfo'];
+            return authResult;
+          }, function(error) {
+            // Rethrow email already in use error so it can trigger the account
+            // linking flow.
+            if (error &&
+                error['code'] == 'auth/email-already-in-use' &&
+                error['email'] && error['credential']) {
+              throw error;
+            }
+            // For all other errors, run onUpgrade check.
+            return self.onUpgradeError(error, authResult['credential']);
+          }));
+    } else {
+      // Finishes sign in with the supplied credential on the developer provided
+      // Auth instance. On completion, this will redirect to signInSuccessUrl or
+      // trigger the signInSuccessWithAuthResult callback.
+      if (!authResult['credential']) {
+        throw new Error('No credential found!');
+      }
+      return self.clearTempAuthState().then(function() {
+        return self.getExternalAuth().signInAndRetrieveDataWithCredential(
+            authResult['credential']);
+      }).then(function(userCredential) {
+        authResult['user'] = userCredential['user'];
+        authResult['credential'] = userCredential['credential'];
+        authResult['operationType'] = userCredential['operationType'];
+        // AdditionalUserInfo should remain the same as isNewUser field should
+        // be the one returned in the first sign in attempt.
+        return authResult;
+      });
     }
   };
   // Initialize current user if auto upgrade is enabled beforing running