From 6a6b630292cb2e05d1d79c9bf233d23955e2ae17 Mon Sep 17 00:00:00 2001 From: itelo Date: Thu, 6 Oct 2016 11:34:15 -0400 Subject: [PATCH] feat(users): change username to usernameOrEmail in signin (#1545) * feat(users): change username to usernameOrEmail in signin * fix(users): toLowerCase at email in local strategy --- .../admin.article.server.routes.tests.js | 4 +- .../server/article.server.routes.tests.js | 12 +++--- .../authentication/signin.client.view.html | 8 ++-- .../users/server/config/strategies/local.js | 10 +++-- .../authentication.client.controller.tests.js | 16 +++++++- modules/users/tests/e2e/users.e2e.tests.js | 6 +-- .../tests/server/user.server.routes.tests.js | 37 ++++++++++++++----- 7 files changed, 64 insertions(+), 29 deletions(-) diff --git a/modules/articles/tests/server/admin.article.server.routes.tests.js b/modules/articles/tests/server/admin.article.server.routes.tests.js index 33bc87a742..e56119da9e 100644 --- a/modules/articles/tests/server/admin.article.server.routes.tests.js +++ b/modules/articles/tests/server/admin.article.server.routes.tests.js @@ -32,7 +32,7 @@ describe('Article Admin CRUD tests', function () { beforeEach(function (done) { // Create user credentials credentials = { - username: 'username', + usernameOrEmail: 'username', password: 'M3@n.jsI$Aw3$0m3' }; @@ -43,7 +43,7 @@ describe('Article Admin CRUD tests', function () { displayName: 'Full Name', email: 'test@test.com', roles: ['user', 'admin'], - username: credentials.username, + username: credentials.usernameOrEmail, password: credentials.password, provider: 'local' }); diff --git a/modules/articles/tests/server/article.server.routes.tests.js b/modules/articles/tests/server/article.server.routes.tests.js index 3e1cf53d09..6f35907e6a 100644 --- a/modules/articles/tests/server/article.server.routes.tests.js +++ b/modules/articles/tests/server/article.server.routes.tests.js @@ -33,7 +33,7 @@ describe('Article CRUD tests', function () { beforeEach(function (done) { // Create user credentials credentials = { - username: 'username', + usernameOrEmail: 'username', password: 'M3@n.jsI$Aw3$0m3' }; @@ -43,7 +43,7 @@ describe('Article CRUD tests', function () { lastName: 'Name', displayName: 'Full Name', email: 'test@test.com', - username: credentials.username, + username: credentials.usernameOrEmail, password: credentials.password, provider: 'local' }); @@ -216,7 +216,7 @@ describe('Article CRUD tests', function () { it('should be able to get a single article that has an orphaned user reference', function (done) { // Create orphan user creds var _creds = { - username: 'orphan', + usernameOrEmail: 'orphan', password: 'M3@n.jsI$Aw3$0m3' }; @@ -226,7 +226,7 @@ describe('Article CRUD tests', function () { lastName: 'Name', displayName: 'Full Name', email: 'orphan@test.com', - username: _creds.username, + username: _creds.usernameOrEmail, password: _creds.password, provider: 'local', roles: ['admin'] @@ -322,7 +322,7 @@ describe('Article CRUD tests', function () { it('should be able to get single article, that a different user created, if logged in & verify the "isCurrentUserOwner" field is set to "false"', function (done) { // Create temporary user creds var _creds = { - username: 'articleowner', + usernameOrEmail: 'articleowner', password: 'M3@n.jsI$Aw3$0m3' }; @@ -332,7 +332,7 @@ describe('Article CRUD tests', function () { lastName: 'Name', displayName: 'Full Name', email: 'temp@test.com', - username: _creds.username, + username: _creds.usernameOrEmail, password: _creds.password, provider: 'local', roles: ['admin', 'user'] diff --git a/modules/users/client/views/authentication/signin.client.view.html b/modules/users/client/views/authentication/signin.client.view.html index d70ab8850b..31cb11272e 100644 --- a/modules/users/client/views/authentication/signin.client.view.html +++ b/modules/users/client/views/authentication/signin.client.view.html @@ -7,10 +7,10 @@

Or with your account

- - -
-

Username is required.

+ + +
+

Username or Email is required.

diff --git a/modules/users/server/config/strategies/local.js b/modules/users/server/config/strategies/local.js index a458e8d780..eb3b4ab4a6 100644 --- a/modules/users/server/config/strategies/local.js +++ b/modules/users/server/config/strategies/local.js @@ -10,12 +10,16 @@ var passport = require('passport'), module.exports = function () { // Use local strategy passport.use(new LocalStrategy({ - usernameField: 'username', + usernameField: 'usernameOrEmail', passwordField: 'password' }, - function (username, password, done) { + function (usernameOrEmail, password, done) { User.findOne({ - username: username.toLowerCase() + $or: [{ + username: usernameOrEmail.toLowerCase() + }, { + email: usernameOrEmail.toLowerCase() + }] }, function (err, user) { if (err) { return done(err); diff --git a/modules/users/tests/client/authentication.client.controller.tests.js b/modules/users/tests/client/authentication.client.controller.tests.js index d675dbcf9e..0eafab51b1 100644 --- a/modules/users/tests/client/authentication.client.controller.tests.js +++ b/modules/users/tests/client/authentication.client.controller.tests.js @@ -48,7 +48,7 @@ })); describe('$scope.signin()', function () { - it('should login with a correct user and password', function () { + it('should login with a correct username and password', function () { // Test expected GET request $httpBackend.when('POST', 'api/auth/signin').respond(200, { username: 'Fred' }); @@ -60,6 +60,18 @@ expect($location.url()).toEqual('/'); }); + it('should login with a correct email and password', function () { + // Test expected GET request + $httpBackend.when('POST', 'api/auth/signin').respond(200, { email: 'Fred@email.com' }); + + scope.vm.signin(true); + $httpBackend.flush(); + + // Test scope value + expect(scope.vm.authentication.user.email).toEqual('Fred@email.com'); + expect($location.url()).toEqual('/'); + }); + it('should be redirected to previous state after successful login', inject(function (_$state_) { $state = _$state_; @@ -101,7 +113,7 @@ it('should fail to log in with wrong credentials', function () { // Foo/Bar combo assumed to not exist - scope.vm.authentication.user = { usersname: 'Foo' }; + scope.vm.authentication.user = { username: 'Foo' }; scope.vm.credentials = 'Bar'; // Test expected POST request diff --git a/modules/users/tests/e2e/users.e2e.tests.js b/modules/users/tests/e2e/users.e2e.tests.js index aab8cc6fcc..5e95d584c2 100644 --- a/modules/users/tests/e2e/users.e2e.tests.js +++ b/modules/users/tests/e2e/users.e2e.tests.js @@ -113,7 +113,7 @@ describe('Users E2E Tests:', function () { expect(element.all(by.css('.error-text')).get(0).getText()).toBe('Email address is invalid.'); }); - it('Should report missing username', function () { + it('Should report missing username or email', function () { browser.get('http://localhost:3001/authentication/signup'); // Enter First Name element(by.model('vm.credentials.firstName')).sendKeys(user1.firstName); @@ -306,7 +306,7 @@ describe('Users E2E Tests:', function () { // Click Submit button element(by.css('button[type="submit"]')).click(); // Username Error - expect(element.all(by.css('.error-text')).get(0).getText()).toBe('Username is required.'); + expect(element.all(by.css('.error-text')).get(0).getText()).toBe('Username or Email is required.'); // Password Error expect(element.all(by.css('.error-text')).get(1).getText()).toBe('Password is required.'); }); @@ -317,7 +317,7 @@ describe('Users E2E Tests:', function () { // Sign in browser.get('http://localhost:3001/authentication/signin'); // Enter UserName - element(by.model('vm.credentials.username')).sendKeys(user1.username); + element(by.model('vm.credentials.usernameOrEmail')).sendKeys(user1.username); // Enter Password element(by.model('vm.credentials.password')).sendKeys(user1.password); // Click Submit button diff --git a/modules/users/tests/server/user.server.routes.tests.js b/modules/users/tests/server/user.server.routes.tests.js index 4b0b450e48..851c68bc4d 100644 --- a/modules/users/tests/server/user.server.routes.tests.js +++ b/modules/users/tests/server/user.server.routes.tests.js @@ -14,6 +14,7 @@ var semver = require('semver'), var app, agent, credentials, + credentialsEmail, user, _user, admin; @@ -32,9 +33,15 @@ describe('User CRUD tests', function () { }); beforeEach(function (done) { - // Create user credentials + // Create user credentials with username credentials = { - username: 'username', + usernameOrEmail: 'username', + password: 'M3@n.jsI$Aw3$0m3' + }; + + // Create user credentials with email + credentialsEmail = { + usernameOrEmail: 'test@test.com', password: 'M3@n.jsI$Aw3$0m3' }; @@ -44,7 +51,7 @@ describe('User CRUD tests', function () { lastName: 'Name', displayName: 'Full Name', email: 'test@test.com', - username: credentials.username, + username: credentials.usernameOrEmail, password: credentials.password, provider: 'local' }; @@ -83,7 +90,8 @@ describe('User CRUD tests', function () { }); }); - it('should be able to login successfully and logout successfully', function (done) { + it('should be able to login with username and email successfully and logout successfully', function (done) { + // Login with username agent.post('/api/auth/signin') .send(credentials) .expect(200) @@ -111,7 +119,18 @@ describe('User CRUD tests', function () { signoutRes.text.should.equal('Moved Temporarily. Redirecting to /'); } - return done(); + // Login with username + agent.post('/api/auth/signin') + .send(credentials) + .expect(200) + .end(function (signinErr, signinRes) { + // Handle signin error + if (signinErr) { + return done(signinErr); + } + + return done(); + }); }); }); }); @@ -711,11 +730,11 @@ describe('User CRUD tests', function () { _user2.email = 'user2_email@test.com'; var credentials2 = { - username: 'username2', + usernameOrEmail: 'username2', password: 'M3@n.jsI$Aw3$0m3' }; - _user2.username = credentials2.username; + _user2.username = credentials2.usernameOrEmail; _user2.password = credentials2.password; var user2 = new User(_user2); @@ -763,11 +782,11 @@ describe('User CRUD tests', function () { _user2.email = 'user2_email@test.com'; var credentials2 = { - username: 'username2', + usernameOrEmail: 'username2', password: 'M3@n.jsI$Aw3$0m3' }; - _user2.username = credentials2.username; + _user2.username = credentials2.usernameOrEmail; _user2.password = credentials2.password; var user2 = new User(_user2);