Improve lastAttemptedUrl handling on logout and login

lastAttemptedUrl is now handled similarly for both login and logout. Observed behavior in an application with expiration support showed that the logout logic would sometimes overwrite the lastAttemptedUrl value due to the user getting bounced from target -> logoutTarget -> login -> logoutTarget. If logoutTarget was a protected route then that route replaces the lastAttemptedUrl value. Now the target is preserved during the hops and lastAttemptedUrl is cleared whenever it is used so it cannot create a side-effect.
justinsa · Oct 21, 2017 · d3a901b · d3a901b
1 parent 4c0a6c5
commit d3a901b
Show file tree

Hide file tree

Showing 3 changed files with 59 additions and 13 deletions.
diff --git a/README.md b/README.md
@@ -112,7 +112,10 @@ $authentication.isProfileExpired();
 
 ### loginConfirmed(data)
 ```JAVASCRIPT
-// Store the profile (data) in local storage, notify all listeners of login, and redirect to onLoginRedirectUrl if defined.
+// Store the profile (data) in local storage, notify all listeners of login, and redirect to:
+//   1. lastAttemptedUrl if defined and trackLastAttemptedUrl is true
+//   2. onLoginRedirectUrl if defined
+//   3. do not redirect
 $authentication.loginConfirmed({ ... });
 ```
 Broadcast via: ```event:auth-loginConfirmed``` with the ```data``` parameter as an argument.
@@ -132,7 +135,10 @@ Broadcast via: ```event:auth-loginRequired```.
 
 ### logoutConfirmed()
 ```JAVASCRIPT
-// Remove any existing profile from local storage, notify all listeners of logout, and redirect to onLogoutRedirectUrl if defined.
+// Remove any existing profile from local storage, notify all listeners of logout, and redirect to:
+//   1. lastAttemptedUrl if defined and trackLastAttemptedUrl is true
+//   2. onLogoutRedirectUrl if defined
+//   3. do not redirect
 $authentication.logoutConfirmed();
 ```
 Broadcast via: ```event:auth-logoutConfirmed```.
@@ -204,6 +210,12 @@ $authentication.getConfiguration();
 $authentication.getLastAttemptedUrl();
 ```
 
+### clearLastAttemptedUrl()
+```JAVASCRIPT
+// Return the last attempted url value, or fallback if tracking is disabled, and clear the value from storage.
+$authentication.clearLastAttemptedUrl();
+```
+
 ### store()
 ```JAVASCRIPT
 // Returns the configured storage service.

diff --git a/ng-authentication-service.js b/ng-authentication-service.js
@@ -168,15 +168,12 @@
          * example if you need to pass through details of the user that was logged in.
          */
         loginConfirmed: function (data) {
-          var lastAttemptedUrl = this.getLastAttemptedUrl(),
-              loginRedirectUrl = configuration.onLoginRedirectUrl;
+          var targetUrl = this.clearLastAttemptedUrl(configuration.onLoginRedirectUrl);
           storageService().set(configuration.profileStorageKey, data);
           configuration.reauthentication.timer = setInterval(configuration.reauthentication.fn, configuration.reauthentication.timeout);
           $rootScope.$broadcast(configuration.events.loginConfirmed, data);
-          if (configuration.trackLastAttemptedUrl === true && _.isString(lastAttemptedUrl)) {
-            $location.url(lastAttemptedUrl);
-          } else if (_.isString(loginRedirectUrl)) {
-            $location.url(loginRedirectUrl);
+          if (_.isString(targetUrl)) {
+            $location.url(targetUrl);
           }
         },
 
@@ -202,14 +199,13 @@
          * call this function to indicate that unauthentication is required.
          */
         logoutConfirmed: function () {
-          var logoutRedirectUrl = configuration.onLogoutRedirectUrl;
+          var targetUrl = this.clearLastAttemptedUrl(configuration.onLogoutRedirectUrl);
           storageService().remove(configuration.profileStorageKey);
-          storageService().remove(configuration.lastAttemptedUrlStorageKey);
           $window.clearInterval(configuration.reauthentication.timer);
           configuration.reauthentication.timer = undefined;
           $rootScope.$broadcast(configuration.events.logoutConfirmed);
-          if (_.isString(logoutRedirectUrl)) {
-            $location.url(logoutRedirectUrl);
+          if (_.isString(targetUrl)) {
+            $location.url(targetUrl);
           }
         },
 
@@ -297,12 +293,24 @@
         },
 
         /**
-         * returns the last attempted url` value.
+         * returns the last attempted url value.
          */
         getLastAttemptedUrl: function () {
           return storageService().get(configuration.lastAttemptedUrlStorageKey);
         },
 
+        /**
+         * returns the last attempted url value, or fallback if tracking is disabled, and clears the value from storage.
+         */
+        clearLastAttemptedUrl: function (fallback) {
+          var value = this.getLastAttemptedUrl();
+          if (configuration.trackLastAttemptedUrl !== true || !_.isString(value)) {
+            value = fallback;
+          }
+          storageService().remove(configuration.lastAttemptedUrlStorageKey);
+          return value;
+        },
+
         /**
          * returns the configured storage service.
          */

diff --git a/tests.js b/tests.js
@@ -30,6 +30,7 @@ describe('$authentication', function () {
         'permit',
         'getConfiguration',
         'getLastAttemptedUrl',
+        'clearLastAttemptedUrl',
         'reauthenticate',
         '$onLoginConfirmed',
         '$onLoginRequired',
@@ -407,6 +408,31 @@ describe('$authentication', function () {
         $location.url().should.match('/home');
       })
     );
+
+    it('should navigate to the lastAttemptedUrl, if set, when the user logs out', 
+      inject(function ($authentication, $location) {
+        $location.url('/home');
+        $location.url().should.match('/home');
+        $authentication.profile({ roles: ['a', 'b', 'c'] });
+        $authentication.store().set($authentication.getConfiguration().lastAttemptedUrlStorageKey, '/last-attempted-url');
+        $authentication.getLastAttemptedUrl().should.equal('/last-attempted-url');
+        $authentication.logoutConfirmed();
+        $location.url().should.match('/last-attempted-url');
+      })
+    );
+
+    it('should navigate to the onLogoutRedirectUrl if trackLastAttemptedUrl is false when the user logs out', 
+      inject(function ($authentication, $location) {
+        $location.url('/dashboard');
+        $location.url().should.match('/dashboard');
+        $authentication.profile({ roles: ['a', 'b', 'c'] });
+        $authentication.store().set($authentication.getConfiguration().lastAttemptedUrlStorageKey, '/last-attempted-url');
+        $authentication.getLastAttemptedUrl().should.equal('/last-attempted-url');
+        $authentication.getConfiguration().trackLastAttemptedUrl = false;
+        $authentication.logoutConfirmed();
+        $location.url().should.match('/home');
+      })
+    );
   });
 
   describe('profile()', function () {