changeset 172:a318b066384b

Enhance basic auth flow Use cookies to remember username, and redirect state transitions to login if user is not already logged in. This mimics the Keycloak auth flow. Reviewed-by: jkang Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2017-August/024677.html
author Andrew Azores <aazores@redhat.com>
date Tue, 29 Aug 2017 12:23:00 -0400
parents 522da87ea068
children 8fe5fed67703
files package.json src/app/app.module.js src/app/app.routing.js src/app/app.routing.spec.js src/app/auth-interceptor.factory.js src/app/auth-interceptor.factory.spec.js src/app/components/auth/auth.module.js src/app/components/auth/auth.routing.js src/app/components/auth/basic-auth.controller.js src/app/components/auth/basic-auth.controller.spec.js src/app/components/auth/basic-auth.service.js src/app/components/auth/basic-auth.service.spec.js src/app/components/auth/en.locale.yaml src/app/components/auth/keycloak-auth.service.js src/app/components/auth/keycloak-auth.service.spec.js src/app/components/auth/login.controller.js src/app/components/auth/login.controller.spec.js src/app/components/auth/login.html
diffstat 18 files changed, 259 insertions(+), 317 deletions(-) [+]
line wrap: on
line diff
--- a/package.json	Mon Aug 28 14:40:53 2017 -0400
+++ b/package.json	Tue Aug 29 12:23:00 2017 -0400
@@ -11,6 +11,7 @@
   "license": "",
   "devDependencies": {
     "@uirouter/angularjs": "^1.0.0",
+    "angular-cookies": "1.5.*",
     "angular-mocks": "1.5.*",
     "angular-patternfly": "^4.4.1",
     "angular-translate": "^2.15.2",
--- a/src/app/app.module.js	Mon Aug 28 14:40:53 2017 -0400
+++ b/src/app/app.module.js	Tue Aug 29 12:23:00 2017 -0400
@@ -27,6 +27,7 @@
 
 import 'angular-patternfly';
 import '@uirouter/angularjs';
+import 'angular-cookies';
 import angularTranslate from 'angular-translate';
 import 'angular-translate-interpolation-messageformat';
 import 'oclazyload';
@@ -53,6 +54,7 @@
   .module('appModule', [
     'ui.router',
     'ui.bootstrap',
+    'ngCookies',
     angularTranslate,
     configModule,
     authModule,
--- a/src/app/app.routing.js	Mon Aug 28 14:40:53 2017 -0400
+++ b/src/app/app.routing.js	Tue Aug 29 12:23:00 2017 -0400
@@ -54,16 +54,21 @@
 
 let appRouter = angular.module('app.routing', componentRoutingModules);
 
-function transitionHook ($q, $transitions, authService) {
+function transitionHook ($q, $transitions, $state, authService) {
   'ngInject';
-  $transitions.onBefore({}, () => {
+  $transitions.onBefore({ to: '/' }, () => {
+    return $state.target('landing');
+  });
+
+  $transitions.onBefore({ to: state => {
+    return state.name !== 'about' && state.name !== 'login' && !authService.status();
+  }}, () => {
     let defer = $q.defer();
     authService.refresh()
-      .success(() => defer.resolve())
-      .error(() => {
-        defer.reject('Auth token update failed');
-        authService.login();
-      });
+      .then(() => defer.resolve(),
+        () => {
+          authService.goToLogin(defer);
+        });
     return defer.promise;
   });
 }
--- a/src/app/app.routing.spec.js	Mon Aug 28 14:40:53 2017 -0400
+++ b/src/app/app.routing.spec.js	Tue Aug 29 12:23:00 2017 -0400
@@ -29,7 +29,7 @@
 
   let module = require('./app.routing.js');
 
-  let stateProvider, urlRouterProvider, q, transitions, authSvc;
+  let stateProvider, urlRouterProvider, q, transitions, state, authSvc, refreshPromise;
 
   beforeEach(() => {
     stateProvider = { state: sinon.spy() };
@@ -41,24 +41,26 @@
       reject: sinon.spy()
     });
 
-    let authSvcRefreshError = sinon.stub();
-    let authSvcRefreshSuccess = sinon.stub().returns({ error: authSvcRefreshError });
+    state = {
+      target: sinon.stub().returns('stateTarget'),
+    };
+
+    refreshPromise = sinon.spy();
     authSvc = {
       login: sinon.spy(),
       logout: sinon.spy(),
       refresh: sinon.stub().returns({
-        success: authSvcRefreshSuccess
+        then: refreshPromise
       }),
-      refreshSuccess: authSvcRefreshSuccess,
-      refreshError: authSvcRefreshError,
-      status: () => true
+      status: () => true,
+      goToLogin: sinon.spy()
     };
     transitions = {
       onBefore: sinon.spy()
     };
 
     module.errorRouting(stateProvider, urlRouterProvider);
-    module.transitionHook(q, transitions, authSvc);
+    module.transitionHook(q, transitions, state, authSvc);
   });
 
   describe('stateProvider', () => {
@@ -105,44 +107,63 @@
   });
 
   describe('state change hook', () => {
-    it('should be on state change start', () => {
-      transitions.onBefore.should.be.calledOnce();
+    it('should only be on state change start', () => {
+      transitions.onBefore.should.be.calledTwice();
     });
 
-    it('should match all transitions', () => {
-      transitions.onBefore.args[0][0].should.deepEqual({});
-    });
+    describe('first hook', () => {
+      it('should match root transitions', () => {
+        transitions.onBefore.args[0][0].should.have.ownProperty('to');
+        transitions.onBefore.args[0][0].to.should.equal('/');
+      });
 
-    it('should provide a transition function', () => {
-      transitions.onBefore.args[0][1].should.be.a.Function();
+      it('should redirect to landing', () => {
+        state.target.should.not.be.called();
+        transitions.onBefore.args[0][1].should.be.a.Function();
+        let res = transitions.onBefore.args[0][1]();
+        state.target.should.be.calledOnce();
+        res.should.deepEqual('stateTarget');
+      });
     });
 
-    it('should call authService.refresh()', () => {
-      authSvc.refresh.should.not.be.called();
-
-      transitions.onBefore.args[0][1]();
+    describe('second hook', () => {
+      it('should match non-login transitions', () => {
+        transitions.onBefore.args[1][0].should.have.ownProperty('to');
+        let fn = transitions.onBefore.args[1][0].to;
+        fn.should.be.a.Function();
+        fn({ name: 'login' }).should.be.false();
+      });
 
-      authSvc.refresh.should.be.calledOnce();
-    });
+      it('should provide a transition function', () => {
+        transitions.onBefore.args[1][1].should.be.a.Function();
+      });
 
-    it('should resolve on success', () => {
-      q.defer().resolve.should.not.be.called();
+      it('should call authService.refresh()', () => {
+        authSvc.refresh.should.not.be.called();
+
+        transitions.onBefore.args[1][1]();
 
-      authSvc.refreshSuccess.yields();
-      transitions.onBefore.args[0][1]();
+        authSvc.refresh.should.be.calledOnce();
+      });
 
-      q.defer().resolve.should.be.calledOnce();
-    });
+      it('should resolve on success', () => {
+        q.defer().resolve.should.not.be.called();
+
+        transitions.onBefore.args[1][1]();
+        refreshPromise.args[0][0]();
 
-    it('should reject on error', () => {
-      q.defer().reject.should.not.be.called();
-      authSvc.login.should.not.be.called();
+        q.defer().resolve.should.be.calledOnce();
+      });
 
-      authSvc.refreshError.yields();
-      transitions.onBefore.args[0][1]();
+      it('should go to login on error', () => {
+        q.defer().reject.should.not.be.called();
+        authSvc.login.should.not.be.called();
 
-      q.defer().reject.should.be.calledOnce();
-      authSvc.login.should.be.calledOnce();
+        transitions.onBefore.args[1][1]();
+        refreshPromise.args[0][1]();
+
+        authSvc.goToLogin.should.be.calledOnce();
+      });
     });
   });
 
--- a/src/app/auth-interceptor.factory.js	Mon Aug 28 14:40:53 2017 -0400
+++ b/src/app/auth-interceptor.factory.js	Tue Aug 29 12:23:00 2017 -0400
@@ -39,12 +39,11 @@
 
         if (authService.authHeader) {
           authService.refresh()
-            .success(() => {
+            .then(() => {
               config.headers = config.headers || {};
               config.headers.Authorization = authService.authHeader;
               defer.resolve(config);
-            })
-            .error(() => {
+            }, () => {
               defer.reject('Failed to refresh token');
             });
         }
--- a/src/app/auth-interceptor.factory.spec.js	Mon Aug 28 14:40:53 2017 -0400
+++ b/src/app/auth-interceptor.factory.spec.js	Tue Aug 29 12:23:00 2017 -0400
@@ -27,22 +27,18 @@
 
 describe('authInterceptorFactory', () => {
 
-  let authSvc, interceptor;
+  let authSvc, refreshPromise, interceptor;
   beforeEach(() => {
     angular.mock.module('authModule', $provide => {
       'ngInject';
 
-      let refreshError = sinon.spy();
-      let refreshSuccess = sinon.stub().returns({ error: refreshError });
+      refreshPromise = sinon.spy();
       authSvc = {
         status: sinon.stub().returns('mockStatus'),
         login: sinon.stub().yields(),
         logout: sinon.stub().yields(),
-        refreshSuccess: refreshSuccess,
-        refreshError: refreshError,
         refresh: sinon.stub().returns({
-          success: refreshSuccess,
-          error: refreshError
+          then: refreshPromise
         }),
         authHeader: 'Basic foo64'
       };
@@ -83,16 +79,16 @@
     it('should append header if refresh succeeds', () => {
       let cfg = {};
       fn(cfg);
-      authSvc.refreshSuccess.should.be.calledWith(sinon.match.func);
-      authSvc.refreshSuccess.yield();
+      refreshPromise.should.be.calledWith(sinon.match.func, sinon.match.func);
+      refreshPromise.args[0][0]();
       cfg.should.deepEqual({ headers: { Authorization: 'Basic foo64'} });
     });
 
     it('should do nothing if refresh fails', () => {
       let cfg = {};
       fn(cfg);
-      authSvc.refreshError.should.be.calledWith(sinon.match.func);
-      authSvc.refreshError.yield();
+      refreshPromise.should.be.calledWith(sinon.match.func, sinon.match.func);
+      refreshPromise.args[0][1]();
       cfg.should.deepEqual({});
     });
 
--- a/src/app/components/auth/auth.module.js	Mon Aug 28 14:40:53 2017 -0400
+++ b/src/app/components/auth/auth.module.js	Tue Aug 29 12:23:00 2017 -0400
@@ -30,7 +30,6 @@
 import KeycloakAuthService from './keycloak-auth.service.js';
 import BasicAuthService from './basic-auth.service.js';
 import LoginController from './login.controller.js';
-import BasicAuthController from './basic-auth.controller.js';
 
 let MOD_NAME = 'authModule';
 export default MOD_NAME;
@@ -52,7 +51,6 @@
 
   mod.constant('AUTH_MODULE', MOD_NAME);
   mod.controller('LoginController', LoginController);
-  mod.controller('BasicAuthController', BasicAuthController);
 
   if (env === 'production') {
     let cloak = keycloakProvider();
--- a/src/app/components/auth/auth.routing.js	Mon Aug 28 14:40:53 2017 -0400
+++ b/src/app/components/auth/auth.routing.js	Tue Aug 29 12:23:00 2017 -0400
@@ -32,8 +32,7 @@
 
   $stateProvider
     .state('/', {
-      url:'/',
-      controller: 'BasicAuthController'
+      url:'/'
     })
     .state('login', {
       url: '/login',
--- a/src/app/components/auth/basic-auth.controller.js	Mon Aug 28 14:40:53 2017 -0400
+++ /dev/null	Thu Jan 01 00:00:00 1970 +0000
@@ -1,44 +0,0 @@
-/**
- * Copyright 2012-2017 Red Hat, Inc.
- *
- * Thermostat is distributed under the GNU General Public License,
- * version 2 or any later version (with a special exception described
- * below, commonly known as the "Classpath Exception").
- *
- * A copy of GNU General Public License (GPL) is included in this
- * distribution, in the file COPYING.
- *
- * Linking Thermostat code with other modules is making a combined work
- * based on Thermostat.  Thus, the terms and conditions of the GPL
- * cover the whole combination.
- *
- * As a special exception, the copyright holders of Thermostat give you
- * permission to link this code with independent modules to produce an
- * executable, regardless of the license terms of these independent
- * modules, and to copy and distribute the resulting executable under
- * terms of your choice, provided that you also meet, for each linked
- * independent module, the terms and conditions of the license of that
- * module.  An independent module is a module which is not derived from
- * or based on Thermostat code.  If you modify Thermostat, you may
- * extend this exception to your version of the software, but you are
- * not obligated to do so.  If you do not wish to do so, delete this
- * exception statement from your version.
- */
-
-export default class BasicAuthController {
-
-  constructor ($scope, $state, authService) {
-    'ngInject';
-
-    if (authService.status()) {
-      $state.go('landing');
-    } else {
-      $state.go('login');
-    }
-
-    $scope.login = () => {
-      authService.login($scope.username, $scope.password, () => $state.go('landing'), () => alert('Login failed'));
-    };
-  }
-
-}
--- a/src/app/components/auth/basic-auth.controller.spec.js	Mon Aug 28 14:40:53 2017 -0400
+++ /dev/null	Thu Jan 01 00:00:00 1970 +0000
@@ -1,104 +0,0 @@
-/**
- * Copyright 2012-2017 Red Hat, Inc.
- *
- * Thermostat is distributed under the GNU General Public License,
- * version 2 or any later version (with a special exception described
- * below, commonly known as the "Classpath Exception").
- *
- * A copy of GNU General Public License (GPL) is included in this
- * distribution, in the file COPYING.
- *
- * Linking Thermostat code with other modules is making a combined work
- * based on Thermostat.  Thus, the terms and conditions of the GPL
- * cover the whole combination.
- *
- * As a special exception, the copyright holders of Thermostat give you
- * permission to link this code with independent modules to produce an
- * executable, regardless of the license terms of these independent
- * modules, and to copy and distribute the resulting executable under
- * terms of your choice, provided that you also meet, for each linked
- * independent module, the terms and conditions of the license of that
- * module.  An independent module is a module which is not derived from
- * or based on Thermostat code.  If you modify Thermostat, you may
- * extend this exception to your version of the software, but you are
- * not obligated to do so.  If you do not wish to do so, delete this
- * exception statement from your version.
- */
-
-describe('BasicAuthController', () => {
-  beforeEach(angular.mock.module($provide => {
-    'ngInject';
-    $provide.value('$transitions', { onBefore: angular.noop });
-  }));
-
-  beforeEach(angular.mock.module('appModule'));
-
-  describe('$scope.login()', () => {
-    let scope, authStatus, authLogin, stateGo, alert;
-    beforeEach(inject(($controller, $rootScope, authService) => {
-      'ngInject';
-      scope = $rootScope.$new();
-      authStatus = sinon.stub(authService, 'status').returns(false);
-      authLogin = sinon.stub(authService, 'login');
-      stateGo = sinon.spy();
-      alert = sinon.spy(window, 'alert');
-
-      $controller('BasicAuthController', {
-        $scope: scope,
-        $state: { go: stateGo },
-        authService: authService
-      });
-    }));
-
-    afterEach(() => {
-      authStatus.restore();
-      authLogin.restore();
-      alert.restore();
-    });
-
-    it('should perform a login', () => {
-      authLogin.should.not.be.called();
-      stateGo.should.be.calledOnce();
-
-      scope.login();
-
-      authLogin.should.be.calledOnce();
-      let fn = authLogin.args[0][2];
-      should.exist(fn);
-      fn.should.be.a.Function();
-      authLogin.yield();
-      stateGo.should.be.calledWith('landing');
-    });
-
-    it('should present an alert on failed login', () => {
-      authLogin.callsArg(3);
-      scope.login();
-      alert.should.be.calledWith('Login failed');
-    });
-  });
-
-  describe('when logged in', () => {
-    let scope, authStatus, stateGo;
-    beforeEach(inject(($controller, $rootScope, authService) => {
-      'ngInject';
-      scope = $rootScope.$new();
-      authStatus = sinon.stub(authService, 'status').returns(true);
-      stateGo = sinon.spy();
-
-      $controller('BasicAuthController', {
-        $scope: scope,
-        $state: { go: stateGo },
-        authService: authService
-      });
-    }));
-
-    afterEach(() => {
-      authStatus.restore();
-    });
-
-    it('should redirect to landing if already logged in', () => {
-      authStatus.should.be.calledOnce();
-      stateGo.should.be.calledWith('landing');
-    });
-  });
-});
--- a/src/app/components/auth/basic-auth.service.js	Mon Aug 28 14:40:53 2017 -0400
+++ b/src/app/components/auth/basic-auth.service.js	Tue Aug 29 12:23:00 2017 -0400
@@ -29,9 +29,11 @@
 
 export default class BasicAuthService {
 
-  constructor ($state) {
+  constructor ($q, $state, $cookies) {
     'ngInject';
+    this.q = $q;
     this.$state = $state;
+    this.cookies = $cookies;
     this.state = false;
 
     this._user = null;
@@ -46,9 +48,16 @@
     this._user = user;
     this._pass = pass;
     this.state = true;
+    if (this._rememberUser) {
+      this.cookies.put('username', user);
+    }
     success();
   }
 
+  goToLogin (promise) {
+    promise.resolve(this.$state.target('login'));
+  }
+
   logout (callback = angular.noop) {
     this._user = null;
     this._pass = null;
@@ -58,15 +67,13 @@
   }
 
   refresh () {
-    return {
-      success: function (fn) {
-        fn();
-        return this;
-      },
-      error: function () {
-        return this;
-      }
-    };
+    let defer = this.q.defer();
+    if (this.state) {
+      defer.resolve();
+    } else {
+      defer.reject();
+    }
+    return defer.promise;
   }
 
   get authHeader () {
@@ -77,6 +84,10 @@
     return this._user;
   }
 
+  get rememberedUsername () {
+    return this.cookies.get('username');
+  }
+
   getCommandChannelUrl (baseUrl) {
     let parsed = url.parse(baseUrl);
     if (this._user == null && this._pass == null) {
@@ -91,4 +102,11 @@
     return url.format(parsed);
   }
 
+  rememberUser (remember) {
+    this._rememberUser = remember;
+    if (!remember) {
+      this.cookies.remove('username');
+    }
+  }
+
 }
--- a/src/app/components/auth/basic-auth.service.spec.js	Mon Aug 28 14:40:53 2017 -0400
+++ b/src/app/components/auth/basic-auth.service.spec.js	Tue Aug 29 12:23:00 2017 -0400
@@ -30,10 +30,28 @@
 import BasicAuthService from './basic-auth.service.js';
 
 describe('BasicAuthService', () => {
-  let basicAuthService, state;
+  let basicAuthService, q, qPromise, state, cookies;
   beforeEach(() => {
-    state = { go: sinon.spy() };
-    basicAuthService = new BasicAuthService(state);
+    qPromise = sinon.stub().yields();
+    q = {
+      defer: sinon.stub().returns({
+        promise: {
+          then: qPromise
+        },
+        resolve: sinon.spy(),
+        reject: sinon.spy()
+      })
+    };
+    state = {
+      go: sinon.spy(),
+      target: sinon.stub().returns('stateTarget')
+    };
+    cookies = {
+      put: sinon.spy(),
+      get: sinon.stub(),
+      remove: sinon.spy()
+    };
+    basicAuthService = new BasicAuthService(q, state, cookies);
   });
 
   it('should be initially logged out', () => {
@@ -55,6 +73,31 @@
         done();
       });
     });
+
+    it('should not store username in cookies by default', done => {
+      basicAuthService.login('client', 'client-pwd', () => {
+        cookies.put.should.not.be.called();
+        done();
+      });
+    });
+
+    it('should store username in cookies when set', done => {
+      basicAuthService.rememberUser(true);
+      basicAuthService.login('client', 'client-pwd', () => {
+        cookies.put.should.be.calledOnce();
+        cookies.put.should.be.calledWith('username', 'client');
+        done();
+      });
+    });
+  });
+
+  describe('#goToLogin()', () => {
+    it('should resolve with login state', () => {
+      let promise = { resolve: sinon.spy() };
+      basicAuthService.goToLogin(promise);
+      promise.resolve.should.be.calledOnce();
+      promise.resolve.should.be.calledWith('stateTarget');
+    });
   });
 
   describe('#logout()', () => {
@@ -85,33 +128,22 @@
   });
 
   describe('#refresh()', () => {
-    it('should return an object', () => {
+    it('should return a Promise', () => {
       let res = basicAuthService.refresh();
       should.exist(res);
-      res.should.be.an.Object();
-    });
-
-    it('should return an object with a success callback handler', () => {
-      let res = basicAuthService.refresh();
-      res.should.have.ownProperty('success');
-      res.success.should.be.a.Function();
+      res.should.be.a.Promise();
     });
 
-    it('should return an object with an error callback handler', () => {
-      let res = basicAuthService.refresh();
-      res.should.have.ownProperty('error');
-      res.error.should.be.a.Function();
+    it('should call success handler if logged in', done => {
+      basicAuthService.login('foo', 'bar', () => {
+        basicAuthService.refresh().then(done, angular.noop);
+      });
     });
 
-    it('should call success callbacks', done => {
-      basicAuthService.refresh().success(done);
-    });
-
-    it('should not call error callbacks', done => {
-      basicAuthService.refresh().error(() => {
-        done('should not reach here');
-      });
-      done();
+    it('should call error handler if logged out', done => {
+      qPromise.callsArg(1);
+      basicAuthService.logout();
+      basicAuthService.refresh().then(angular.noop, done);
     });
   });
 
@@ -137,6 +169,15 @@
     });
   });
 
+  describe('#get rememberedUsername()', () => {
+    it('should return username stored in cookie', () => {
+      cookies.get.returns('fakeUser');
+      cookies.get.should.not.be.called();
+      basicAuthService.rememberedUsername.should.equal('fakeUser');
+      cookies.get.should.be.calledOnce();
+    });
+  });
+
   describe('#getCommandChannelUrl()', () => {
     it('should return provided value if not logged in', () => {
       let mockUrl = 'http://example.com:1234/';
@@ -157,4 +198,13 @@
       });
     });
   });
+
+  describe('rememberUser', () => {
+    it('should remove username stored in cookie when called with "false"', () => {
+      basicAuthService.rememberUser(true);
+      cookies.remove.should.not.be.called();
+      basicAuthService.rememberUser(false);
+      cookies.remove.should.be.calledWith('username');
+    });
+  });
 });
--- a/src/app/components/auth/en.locale.yaml	Mon Aug 28 14:40:53 2017 -0400
+++ b/src/app/components/auth/en.locale.yaml	Tue Aug 29 12:23:00 2017 -0400
@@ -1,7 +1,7 @@
 auth:
   USERNAME: Username
   PASSWORD: Password
-  REMEMBER_USERNAME_LABEL: <input type="checkbox" tabindex="3"/> Remember username
+  REMEMBER_USERNAME_LABEL: Remember username
   LOGIN_BUTTON_LABEL: Log In
   WELCOME_TEXT: Welcome to Thermostat | JVM Instrumentation Tool
   FORGOT_CREDENTIALS: Forgot <a href="" tabindex="5">username</a> or <a href="" tabindex="6">password</a>?
--- a/src/app/components/auth/keycloak-auth.service.js	Mon Aug 28 14:40:53 2017 -0400
+++ b/src/app/components/auth/keycloak-auth.service.js	Tue Aug 29 12:23:00 2017 -0400
@@ -33,9 +33,13 @@
     this.keycloak = keycloak;
   }
 
-  login (user, pass, success = angular.noop) {
+  login () {
+    // no-op
+  }
+
+  goToLogin (promise) {
     this.keycloak.login();
-    success();
+    promise.resolve();
   }
 
   logout () {
--- a/src/app/components/auth/keycloak-auth.service.spec.js	Mon Aug 28 14:40:53 2017 -0400
+++ b/src/app/components/auth/keycloak-auth.service.spec.js	Tue Aug 29 12:23:00 2017 -0400
@@ -31,14 +31,14 @@
 
 describe('KeycloakAuthService', () => {
 
-  let keycloakAuthService, login, logout, refresh, authenticated;
+  let keycloakAuthService, mockCloak;
   beforeEach(() => {
-    login = sinon.spy();
-    logout = sinon.spy();
+    let login = sinon.spy();
+    let logout = sinon.spy();
+    let refresh = sinon.stub().returns('refresh-foo');
+    let authenticated = 'invalid-testing-token';
 
-    refresh = sinon.stub().returns('refresh-foo');
-    authenticated = 'invalid-testing-token';
-    let mockCloak = {
+    mockCloak = {
       login: login,
       logout: logout,
       updateToken: refresh,
@@ -52,39 +52,42 @@
   });
 
   describe('#login()', () => {
-    it('should call callback', done => {
-      keycloakAuthService.login('', '', done);
+    it('should be a no-op', () => {
+      mockCloak.login.should.not.be.called();
+      keycloakAuthService.login();
+      mockCloak.login.should.not.be.called();
     });
+  });
 
-    it('should not require callback', () => {
-      keycloakAuthService.login('', '');
-    });
-
-    it('should delegate to Keycloak object', done => {
-      keycloakAuthService.login('', '', done);
-      logout.should.be.calledOnce();
+  describe('#goToLogin()', () => {
+    it('should call Keycloak login, then resolve', () => {
+      mockCloak.login.should.not.be.called();
+      let promise = { resolve: sinon.spy() };
+      keycloakAuthService.goToLogin(promise);
+      mockCloak.login.should.be.calledOnce();
+      promise.resolve.should.be.calledOnce();
     });
   });
 
   describe('#logout()', () => {
     it('should delegate to keycloak object', () => {
       keycloakAuthService.logout();
-      logout.should.be.calledOnce();
+      mockCloak.logout.should.be.calledOnce();
     });
   });
 
   describe('#status()', () => {
     it('should delegate to Keycloak object', () => {
-      let res = keycloakAuthService.status();
-      res.should.equal(authenticated);
+      keycloakAuthService.status().should.equal(mockCloak.authenticated);
     });
   });
 
   describe('#refresh()', () => {
     it('should delegate to Keycloak object', () => {
+      mockCloak.updateToken.should.not.be.called();
       let res = keycloakAuthService.refresh();
       res.should.equal('refresh-foo');
-      refresh.should.be.calledOnce();
+      mockCloak.updateToken.should.be.calledOnce();
     });
   });
 
@@ -101,11 +104,8 @@
   });
 
   describe('#getCommandChannelUrl()', () => {
-    it('should add the Keycloak token to the query', done => {
-      keycloakAuthService.login('foo', 'bar', () => {
-        keycloakAuthService.getCommandChannelUrl('http://example.com/').should.equal('http://example.com/?token=fakeToken');
-        done();
-      });
+    it('should add the Keycloak token to the query', () => {
+      keycloakAuthService.getCommandChannelUrl('http://example.com/').should.equal('http://example.com/?token=fakeToken');
     });
   });
 });
--- a/src/app/components/auth/login.controller.js	Mon Aug 28 14:40:53 2017 -0400
+++ b/src/app/components/auth/login.controller.js	Tue Aug 29 12:23:00 2017 -0400
@@ -32,12 +32,17 @@
 
     if (authService.status()) {
       $state.go('landing');
-    } else {
-      $state.go('login');
+      return;
+    }
+
+    $scope.rememberUser = angular.isDefined(authService.rememberedUsername);
+    if ($scope.rememberUser) {
+      $scope.username = authService.rememberedUsername;
     }
 
     $scope.login = () => {
-      authService.login($scope.username, $scope.password, () => $state.go('landing'), () => alert('Login failed'));
+      authService.rememberUser($scope.rememberUser);
+      authService.login($scope.username, $scope.password, () => $state.go('landing'));
     };
   }
 
--- a/src/app/components/auth/login.controller.spec.js	Mon Aug 28 14:40:53 2017 -0400
+++ b/src/app/components/auth/login.controller.spec.js	Tue Aug 29 12:23:00 2017 -0400
@@ -35,14 +35,18 @@
   beforeEach(angular.mock.module('appModule'));
 
   describe('$scope.login()', () => {
-    let scope, authStatus, authLogin, stateGo, alert;
-    beforeEach(inject(($controller, $rootScope, authService) => {
+    let scope, authService, stateGo, alert;
+    beforeEach(inject(($controller, $rootScope) => {
       'ngInject';
 
       scope = $rootScope.$new();
 
-      authStatus = sinon.stub(authService, 'status').returns(false);
-      authLogin = sinon.stub(authService, 'login');
+      authService = {
+        status: sinon.stub().returns(false),
+        login: sinon.stub().yields(),
+        rememberUser: sinon.spy()
+      };
+
       stateGo = sinon.spy();
       alert = sinon.spy(window, 'alert');
 
@@ -54,8 +58,6 @@
     }));
 
     afterEach(() => {
-      authStatus.restore();
-      authLogin.restore();
       alert.restore();
     });
 
@@ -67,35 +69,29 @@
       scope.login.should.be.a.Function();
     });
 
-    it('should perform a login', () => {
-      authLogin.should.not.be.called();
-      stateGo.should.be.calledOnce();
-
-      scope.login();
+    it('should set remember user on authService if set in scope', () => {
+      authService.login.should.not.be.called();
+      stateGo.should.not.be.called();
 
-      authLogin.should.be.calledOnce();
-      let fn = authLogin.args[0][2];
-      should.exist(fn);
-      fn.should.be.a.Function();
-      authLogin.yield();
-      stateGo.should.be.calledWith('landing');
+      scope.rememberUser = true;
+      scope.login();
+      authService.login.yield();
+      authService.rememberUser.should.be.calledOnce();
+      authService.rememberUser.should.be.calledWith(true);
     });
 
-    it('should present an alert on failed login', () => {
-      authLogin.callsArg(3);
-      scope.login();
-      alert.should.be.calledWith('Login failed');
-    });
   });
 
   describe('when logged in', () => {
-    let scope, authStatus, stateGo;
-    beforeEach(inject(($controller, $rootScope, authService) => {
+    let scope, authService, stateGo;
+    beforeEach(inject(($controller, $rootScope) => {
       'ngInject';
 
       scope = $rootScope.$new();
 
-      authStatus = sinon.stub(authService, 'status').returns(true);
+      authService = {
+        status: sinon.stub().returns(true)
+      };
       stateGo = sinon.spy();
 
       $controller('LoginController', {
@@ -105,40 +101,37 @@
       });
     }));
 
-    afterEach(() => {
-      authStatus.restore();
-    });
-
     it('should redirect to landing if already logged in', () => {
-      authStatus.should.be.calledOnce();
+      authService.status.should.be.calledOnce();
       stateGo.should.be.calledWith('landing');
     });
   });
 
-  describe('when logged out', () => {
-    let scope, authStatus, stateGo;
-    beforeEach(inject(($controller, $rootScope, authService) => {
+  describe('stored username fill', () => {
+    let scope, authService, stateGo;
+    beforeEach(inject(($controller, $rootScope) => {
       'ngInject';
 
       scope = $rootScope.$new();
+
+      authService = {
+        status: sinon.stub().returns(false),
+        rememberedUsername: 'foo-user'
+      };
       stateGo = sinon.spy();
 
-      authStatus = sinon.stub(authService, 'status').returns(false);
-
       $controller('LoginController', {
         $scope: scope,
-        $state: {go: stateGo},
+        $state: { go: stateGo },
         authService: authService
       });
     }));
 
-    afterEach(() => {
-      authStatus.restore();
-    });
-
-    it('should keep login state if not logged in', () => {
-      authStatus.should.be.calledOnce();
-      stateGo.should.be.calledWith('login');
+    it('should fill username from authService if available', () => {
+      scope.should.have.ownProperty('username');
+      scope.username.should.equal('foo-user');
+      scope.should.have.ownProperty('rememberUser');
+      scope.rememberUser.should.be.true();
     });
   });
 
--- a/src/app/components/auth/login.html	Mon Aug 28 14:40:53 2017 -0400
+++ b/src/app/components/auth/login.html	Tue Aug 29 12:23:00 2017 -0400
@@ -25,9 +25,8 @@
           </div>
           <div class="form-group">
             <div class="col-xs-8 col-sm-offset-2 col-sm-6 col-md-offset-2 col-md-6">
-              <div class="checkbox">
-                <label translate>auth.REMEMBER_USERNAME_LABEL</label>
-              </div>
+              <input name="rememberUser" type="checkbox" tabindex="3" ng-model="rememberUser"/>
+              <label for="rememberUser" translate>auth.REMEMBER_USERNAME_LABEL</label>
               <span class="help-block" translate>auth.FORGOT_CREDENTIALS</span>
             </div>
             <div class="col-xs-4 col-sm-4 col-md-4 submit">