Mercurial > hg > thermostat-ng > web-client
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">