Fix auth if localStorage is missing or corrupted (#7674)

* fix(auth): correctly redirect to logout page if localstorage is corrupted

* fix(auth): do not break site if localStorage has invalid JSON

* fix(karma): use $window instead of windo

* disable failing karma tests

* fix(tests): Provide mockwindow for tests

* fix(tests): Call habitrpgShared without $window

test/spec/controllers/copyMessageModalControllerSpec.js

@@ -4,7 +4,11 @@ describe("CopyMessageModal controller", function() {
   var scope, ctrl, user, Notification, $rootScope, $controller;
 
   beforeEach(function() {
-    module(function($provide) {});
+    module(function($provide) {
+      var mockWindow = {href: '', alert: sandbox.spy(), location: {search: '', pathname: '', href: ''}};
+
+      $provide.value('$window', mockWindow);
+    });
 
     inject(function($rootScope, _$controller_, _Notification_, User){
       user = specHelper.newUser();

test/spec/controllers/filtersCtrlSpec.js

@@ -3,16 +3,24 @@
 describe('Filters Controller', function() {
   var scope, user, userService;
 
-  beforeEach(inject(function($rootScope, $controller, Shared, User) {
-    user = specHelper.newUser();
-    Shared.wrap(user);
-    scope = $rootScope.$new();
-    // user.filters = {};
-    User.setUser(user);
-    User.user.filters = {};
-    userService = User;
-    $controller('FiltersCtrl', {$scope: scope, User: User});
-  }));
+  beforeEach(function () {
+    module(function($provide) {
+      var mockWindow = {href: '', alert: sandbox.spy(), location: {search: '', pathname: '', href: ''}};
+
+      $provide.value('$window', mockWindow);
+    });
+
+    inject(function($rootScope, $controller, Shared, User) {
+      user = specHelper.newUser();
+      Shared.wrap(user);
+      scope = $rootScope.$new();
+      // user.filters = {};
+      User.setUser(user);
+      User.user.filters = {};
+      userService = User;
+      $controller('FiltersCtrl', {$scope: scope, User: User});
+    })
+  });
 
   describe('tags', function(){
     it('creates a tag', function(){

test/spec/controllers/inventoryCtrlSpec.js

@@ -4,7 +4,16 @@ describe('Inventory Controller', function() {
   var scope, ctrl, user, rootScope;
 
   beforeEach(function() {
-    module(function($provide) {});
+    module(function($provide) {
+      var mockWindow = {
+        confirm: function(msg) {
+          return true;
+        },
+        location: {search: '', pathname: '', href: ''},
+      };
+
+      $provide.value('$window', mockWindow);
+    });
 
     inject(function($rootScope, $controller, Shared, User, $location, $window) {
       user = specHelper.newUser({
@@ -23,11 +32,6 @@ describe('Inventory Controller', function() {
       });
 
       Shared.wrap(user);
-      var mockWindow = {
-        confirm: function(msg) {
-          return true;
-        },
-      };
 
       scope = $rootScope.$new();
       rootScope = $rootScope;
@@ -36,9 +40,9 @@ describe('Inventory Controller', function() {
       User.setUser(user);
 
       // Load RootCtrl to ensure shared behaviors are loaded
-      $controller('RootCtrl',  {$scope: scope, User: User, $window: mockWindow});
+      $controller('RootCtrl',  {$scope: scope, User: User});
 
-      ctrl = $controller('InventoryCtrl', {$scope: scope, User: User, $window: mockWindow});
+      ctrl = $controller('InventoryCtrl', {$scope: scope, User: User});
     });
   });
 

test/spec/services/userServicesSpec.js

@@ -7,7 +7,7 @@ describe('userServices', function() {
 
   beforeEach(function(){
     module(function($provide){
-      $window = {href: '', alert: sandbox.spy(), location: {search: '', pathname: ''}};
+      $window = {href: '', alert: sandbox.spy(), location: {search: '', pathname: '', href: ''}};
       $provide.value('$window', $window);
     });
 

website/client/js/app.js

@@ -330,13 +330,19 @@ window.habitrpg = angular.module('habitrpg',
           title: env.t('titleSettings')
         });
 
-      var settings = JSON.parse(localStorage.getItem(STORAGE_SETTINGS_ID));
+      var settings;
 
-      if (settings && settings.auth) {
-        $httpProvider.defaults.headers.common['Content-Type'] = 'application/json;charset=utf-8';
+      try {
+        settings = JSON.parse(localStorage.getItem(STORAGE_SETTINGS_ID));
+      } catch (e) {
+        settings = {};
+      }
+
+      if (settings && settings.auth && settings.auth.apiId && settings.auth.apiToken) {
         $httpProvider.defaults.headers.common['x-api-user'] = settings.auth.apiId;
         $httpProvider.defaults.headers.common['x-api-key'] = settings.auth.apiToken;
       }
 
+      $httpProvider.defaults.headers.common['Content-Type'] = 'application/json;charset=utf-8';
       $httpProvider.defaults.headers.common['x-client'] = 'habitica-web';
   }]);

website/client/js/services/userServices.js

@@ -83,7 +83,7 @@ angular.module('habitrpg')
           if (!user.filters) {
             user.filters = {};
           }
-          
+
           if (!user._wrapped) {
             // This wraps user with `ops`, which are functions shared both on client and mobile. When performed on client,
             // they update the user in the browser and then send the request to the server, where the same operation is
@@ -129,7 +129,7 @@ angular.module('habitrpg')
           }
 
           args.push(opData);
-          clientResponse = $window.habitrpgShared.ops[opName].apply(null, args);
+          clientResponse = habitrpgShared.ops[opName].apply(null, args);
         } catch (err) {
           Notification.text(err.message);
           return;
@@ -600,9 +600,17 @@ angular.module('habitrpg')
       };
 
       //load settings if we have them
-      if (localStorage.getItem(STORAGE_SETTINGS_ID)) {
+      var storedSettings;
+      try {
+        storedSettings = localStorage.getItem(STORAGE_SETTINGS_ID) || {};
+        storedSettings = JSON.parse(storedSettings);
+      } catch (e) {
+        storedSettings = {};
+      }
+
+      if (storedSettings.auth && storedSettings.auth.apiId && storedSettings.auth.apiToken) {
         //use extend here to make sure we keep object reference in other angular controllers
-        _.extend(settings, JSON.parse(localStorage.getItem(STORAGE_SETTINGS_ID)));
+        _.extend(settings, storedSettings);
 
         //if settings were saved while fetch was in process reset the flag.
         settings.fetching = false;
@@ -613,7 +621,7 @@ angular.module('habitrpg')
       }
 
       //If user does not have ApiID that forward him to settings.
-      if (!settings.auth.apiId || !settings.auth.apiToken) {
+      if (!settings || !settings.auth || !settings.auth.apiId || !settings.auth.apiToken) {
         //var search = $location.search(); // FIXME this should be working, but it's returning an empty object when at a root url /?_id=...
         var search = $location.search($window.location.search.substring(1)).$$search; // so we use this fugly hack instead
         if (search.err) return alert(search.err);
@@ -625,7 +633,7 @@ angular.module('habitrpg')
           var isStaticOrSocial = $window.location.pathname.match(/^\/(static|social)/);
           if (!isStaticOrSocial){
             localStorage.clear();
-            $location.path('/logout');
+            $window.location.href = '/logout';
           }
         }
       } else {