From 4ee2a30d48f8348bceb1ad3aca40db5c204b27e9 Mon Sep 17 00:00:00 2001 From: Lukasz Bandzarewicz Date: Tue, 18 Oct 2016 19:38:26 +0200 Subject: [PATCH 01/15] Add express http logging --- package.json | 1 + server/app.js | 2 ++ 2 files changed, 3 insertions(+) diff --git a/package.json b/package.json index ccebd6a..91a6d30 100644 --- a/package.json +++ b/package.json @@ -58,6 +58,7 @@ "karma-webpack": "^1.8.0", "minimist": "^1.2.0", "mocha": "^3.1.2", + "morgan": "^1.7.0", "ng-annotate-loader": "^0.2.0", "node-sass": "^3.10.1", "nodemon": "^1.11.0", diff --git a/server/app.js b/server/app.js index 0cbea13..a4a5594 100644 --- a/server/app.js +++ b/server/app.js @@ -1,10 +1,12 @@ const express = require('express'); +const morgan = require('morgan'); const path = require('path'); const app = express(); app.use(express.static(path.join(__dirname, '..', 'build'))); app.use(require('body-parser').json()); +app.use(morgan('short')); if (process.env.NODE_ENV !== 'production') { app.use('/api/seed', require('./api/seed')); From c8dad2fd97210f329256ae3fdec2d6f9d3e25334 Mon Sep 17 00:00:00 2001 From: Lukasz Bandzarewicz Date: Wed, 19 Oct 2016 09:16:05 +0200 Subject: [PATCH 02/15] Simple API endpoint for authentication --- package.json | 5 ++- server.js | 4 +- server/api/authentication.js | 37 +++++++++++++++++ server/api/authentication.spec.js | 61 ++++++++++++++++++++++++++++ server/api/contacts.spec.js | 4 -- server/db.js | 67 +++++++++++++++++++++---------- server/db.spec.js | 18 +++++++-- 7 files changed, 162 insertions(+), 34 deletions(-) create mode 100644 server/api/authentication.js create mode 100644 server/api/authentication.spec.js diff --git a/package.json b/package.json index 91a6d30..ebe49aa 100644 --- a/package.json +++ b/package.json @@ -86,12 +86,15 @@ "angular-resource": "^1.5.8", "angular-toastr": "^2.1.1", "angular-ui-router": "^1.0.0-beta.3", + "bcrypt": "^0.8.7", "bluebird": "^3.4.6", "body-parser": "^1.15.2", "bootstrap-sass": "^3.3.7", "express": "^4.14.0", "faker": "^3.1.0", "jquery": "^3.1.1", - "lodash": "^4.16.4" + "jsonwebtoken": "^7.1.9", + "lodash": "^4.16.4", + "morgan": "^1.7.0" } } diff --git a/server.js b/server.js index b22c5e1..02b3267 100644 --- a/server.js +++ b/server.js @@ -1,9 +1,7 @@ const app = require('./server/app'); const db = require('./server/db'); -db.seed().then((contacts) => { - console.log(`Database populated with ${contacts.length} contacts`); - +db.seed().then(() => { const port = process.env.PORT || 9090; app.listen(port, () => { console.log('Server is running on port', port); diff --git a/server/api/authentication.js b/server/api/authentication.js new file mode 100644 index 0000000..3c1aaa9 --- /dev/null +++ b/server/api/authentication.js @@ -0,0 +1,37 @@ +const Promise = require('bluebird'); +const bcrypt = require('bcrypt'); +const jwt = require('jsonwebtoken'); +const router = require('express').Router(); +const _ = require('lodash'); + +const db = require('../db'); + +// TODO store secret somewhere +const secret = 'secret'; + +function checkPassword(password, passwordHash) { + const compare = Promise.promisify(bcrypt.compare); + return compare(password, passwordHash); +} + +router.post('/', (req, res) => { + const { email, password } = req.body; + + db.users.findOne({ email }).then((user) => { + return checkPassword(password, user.passwordHash).then((success) => { + if (success) { + res.json({ + token: jwt.sign(_.pick(user, ['id', 'email']), secret, { + expiresIn: '7 days' + }) + }); + } else { + return Promise.reject('invalid password'); + } + }); + }).catch(() => { + res.sendStatus(422); + }); +}); + +module.exports = router; diff --git a/server/api/authentication.spec.js b/server/api/authentication.spec.js new file mode 100644 index 0000000..38d406c --- /dev/null +++ b/server/api/authentication.spec.js @@ -0,0 +1,61 @@ +const expect = require('chai').expect; +const jwt = require('jsonwebtoken'); +const request = require('supertest'); + +const app = require('../app'); +const db = require('../db'); + +describe('app', () => { + + beforeEach(() => { + return db.seed(); + }); + + describe('POST /api/authentication', () => { + + describe('with valid credentials', () => { + + const email = 'demo@email.com'; + const password = 'password'; + + it('creates a contact', (done) => { + request(app) + .post('/api/authentication') + .set('Accept', 'application/json') + .send({ email, password }) + .expect('Content-Type', /json/) + .expect(200) + .expect((res) => { + const { token } = res.body; + const payload = jwt.verify(token, 'secret'); + + expect(payload).to.have.property('id'); + expect(payload).to.have.property('email', email); + expect(payload).to.not.have.property('passwordHash'); + expect(payload).to.have.property('iat'); + expect(payload).to.have.property('exp'); + }) + .end(done); + }); + + }); + + describe('with invalid credentials', () => { + + const email = 'demo@email.com'; + const password = 'wrong'; + + it('creates a contact', (done) => { + request(app) + .post('/api/authentication') + .set('Accept', 'application/json') + .send({ email, password }) + .expect(422) + .end(done); + }); + + }); + + }); + +}); diff --git a/server/api/contacts.spec.js b/server/api/contacts.spec.js index 9d329e9..e8a6de4 100644 --- a/server/api/contacts.spec.js +++ b/server/api/contacts.spec.js @@ -19,10 +19,6 @@ describe('app', () => { return db.seed(); }); - afterEach(() => { - return db.drop(); - }); - describe('GET /api/contacts', () => { it('respond with json', (done) => { diff --git a/server/db.js b/server/db.js index b12aec2..e02cb36 100644 --- a/server/db.js +++ b/server/db.js @@ -1,4 +1,5 @@ const Promise = require('bluebird'); +const bcrypt = require('bcrypt'); const faker = require('faker'); const _ = require('lodash'); @@ -7,37 +8,59 @@ const Collection = require('./collection'); class Db { constructor() { + this.users = new Collection(); this.contacts = new Collection(); } - seed(n = 20) { + seed() { faker.seed(667); return this.drop().then(() => { - return Promise.all(_.times(n, () => { - const address = { - country: faker.address.country(), - town: faker.address.city(), - zipCode: faker.address.zipCode(), - street: faker.address.streetAddress(), - location: { - lon: faker.address.longitude(), - lat: faker.address.latitude() - } - }; - - return this.contacts.insertOne({ - favourite: faker.random.boolean(), - firstName: faker.name.firstName(), - lastName: faker.name.lastName(), - email: faker.internet.email(), - phone: faker.phone.phoneNumber(), - address - }); - })); + return Promise.all([ + this._seedUsers(), + this._seedContacts() + ]); }); } + _seedUsers() { + const hashPassword = (password) => { + const salt = bcrypt.genSaltSync(10); + return Promise.promisify(bcrypt.hash)(password, salt); + }; + + return hashPassword('password').then((passwordHash) => { + return this.users.insertOne({ + email: 'demo@email.com', + passwordHash + }); + }); + } + + _seedContacts(n = 20) { + return Promise.all(_.times(n, () => { + const address = { + country: faker.address.country(), + town: faker.address.city(), + zipCode: faker.address.zipCode(), + street: faker.address.streetAddress(), + location: { + lon: faker.address.longitude(), + lat: faker.address.latitude() + } + }; + + return this.contacts.insertOne({ + favourite: faker.random.boolean(), + firstName: faker.name.firstName(), + lastName: faker.name.lastName(), + email: faker.internet.email(), + phone: faker.phone.phoneNumber(), + address + }); + })); + } + drop() { return Promise.all([ this.contacts.drop() diff --git a/server/db.spec.js b/server/db.spec.js index 99db570..97ad468 100644 --- a/server/db.spec.js +++ b/server/db.spec.js @@ -5,10 +5,12 @@ describe('db', () => { describe('seed', () => { - it('seed the fake database', () => { - return db.seed().then(() => { - return db.contacts.find(); - }).then((contacts) => { + beforeEach(function() { + return db.seed(); + }); + + it('seeds contacts', () => { + return db.contacts.find().then((contacts) => { expect(contacts).to.have.length(20); expect(contacts[0]).to.have.property('id', 1); @@ -17,6 +19,14 @@ describe('db', () => { }); }); + it('seeds users', () => { + return db.users.findOne({ email: 'demo@email.com' }).then((user) => { + expect(user).to.have.property('id'); + expect(user).to.have.property('email', 'demo@email.com'); + expect(user).to.have.property('passwordHash'); + }); + }); + }); describe('.drop', () =>{ From ab6d20c59621f8e4b7e113ef0e8498de884cd353 Mon Sep 17 00:00:00 2001 From: Lukasz Bandzarewicz Date: Wed, 19 Oct 2016 09:42:51 +0200 Subject: [PATCH 03/15] Simple server app configuration --- gulpfile.js | 2 ++ server/api/authentication.js | 6 ++---- server/api/authentication.spec.js | 3 ++- server/app.js | 9 +++++++-- server/config.js | 4 ++++ 5 files changed, 17 insertions(+), 7 deletions(-) create mode 100644 server/config.js diff --git a/gulpfile.js b/gulpfile.js index 4fd4389..24e2b61 100644 --- a/gulpfile.js +++ b/gulpfile.js @@ -74,6 +74,8 @@ gulp.task('tdd', () => { }); gulp.task('server:test', (done) => { + process.env.NODE_ENV = 'test'; + const istanbul = require('gulp-istanbul'); const mocha = require('gulp-mocha'); diff --git a/server/api/authentication.js b/server/api/authentication.js index 3c1aaa9..fc4e2e0 100644 --- a/server/api/authentication.js +++ b/server/api/authentication.js @@ -4,11 +4,9 @@ const jwt = require('jsonwebtoken'); const router = require('express').Router(); const _ = require('lodash'); +const config = require('../config'); const db = require('../db'); -// TODO store secret somewhere -const secret = 'secret'; - function checkPassword(password, passwordHash) { const compare = Promise.promisify(bcrypt.compare); return compare(password, passwordHash); @@ -21,7 +19,7 @@ router.post('/', (req, res) => { return checkPassword(password, user.passwordHash).then((success) => { if (success) { res.json({ - token: jwt.sign(_.pick(user, ['id', 'email']), secret, { + token: jwt.sign(_.pick(user, ['id', 'email']), config.secret, { expiresIn: '7 days' }) }); diff --git a/server/api/authentication.spec.js b/server/api/authentication.spec.js index 38d406c..fed96a9 100644 --- a/server/api/authentication.spec.js +++ b/server/api/authentication.spec.js @@ -3,6 +3,7 @@ const jwt = require('jsonwebtoken'); const request = require('supertest'); const app = require('../app'); +const config = require('../config'); const db = require('../db'); describe('app', () => { @@ -27,7 +28,7 @@ describe('app', () => { .expect(200) .expect((res) => { const { token } = res.body; - const payload = jwt.verify(token, 'secret'); + const payload = jwt.verify(token, config.secret); expect(payload).to.have.property('id'); expect(payload).to.have.property('email', email); diff --git a/server/app.js b/server/app.js index a4a5594..0fbde3f 100644 --- a/server/app.js +++ b/server/app.js @@ -2,16 +2,21 @@ const express = require('express'); const morgan = require('morgan'); const path = require('path'); +const config = require('./config'); const app = express(); app.use(express.static(path.join(__dirname, '..', 'build'))); app.use(require('body-parser').json()); -app.use(morgan('short')); -if (process.env.NODE_ENV !== 'production') { +if (config.env !== 'test') { + app.use(morgan('short')); +} + +if (config.env !== 'production') { app.use('/api/seed', require('./api/seed')); } +app.use('/api/authentication', require('./api/authentication')); app.use('/api/contacts', require('./api/contacts')); app.all('*', (req, res) => { diff --git a/server/config.js b/server/config.js new file mode 100644 index 0000000..9bbdda5 --- /dev/null +++ b/server/config.js @@ -0,0 +1,4 @@ +module.exports = Object.freeze({ + env: process.env.NODE_ENV || 'development', + secret: process.env.SECRET || 'I am a vegan' +}); From 0dc4f3ebf9a900bc97bd413423563c57bb0b5463 Mon Sep 17 00:00:00 2001 From: Lukasz Bandzarewicz Date: Wed, 19 Oct 2016 09:47:06 +0200 Subject: [PATCH 04/15] More strict ESLint rules --- .eslintrc.json | 15 +++++++++++++++ karma.conf.js | 2 +- package.json | 3 +-- server/db.spec.js | 8 ++++---- 4 files changed, 21 insertions(+), 7 deletions(-) diff --git a/.eslintrc.json b/.eslintrc.json index 86aed71..c91f86c 100644 --- a/.eslintrc.json +++ b/.eslintrc.json @@ -14,12 +14,21 @@ "sourceType": "commonjs" }, "rules": { + "arrow-spacing": "error", "comma-dangle": "error", "comma-style": ["error", "last"], + "comma-spacing": ["error", { + "before": false, + "after": true + }], "complexity": "warn", "curly": "error", "eol-last": "error", "indent": ["error", 2], + "key-spacing": ["error", { + "beforeColon": false, + "afterColon": true + }], "keyword-spacing": "error", "max-len": ["error", 120], "no-console": "off", @@ -28,11 +37,17 @@ "no-trailing-spaces": "error", "no-var": "error", "no-warning-comments": "warn", + "object-curly-spacing": ["error", "always"], "object-shorthand": "error", "prefer-arrow-callback": "error", "prefer-const": "warn", "semi": "error", + "semi-spacing": ["error", { + "before": false, + "after": true + }], "space-before-blocks": "error", + "template-curly-spacing": ["error", "never"], "quotes": ["error", "single"] } } diff --git a/karma.conf.js b/karma.conf.js index e70709a..4263a3d 100644 --- a/karma.conf.js +++ b/karma.conf.js @@ -46,7 +46,7 @@ module.exports = function(config) { coverageReporter: { - dir : 'artifacts/client/coverage', + dir: 'artifacts/client/coverage', subdir: (browser) => { return browser.toLowerCase().split(/[ /-]/)[0]; }, diff --git a/package.json b/package.json index ebe49aa..412f541 100644 --- a/package.json +++ b/package.json @@ -32,7 +32,7 @@ "eslint-plugin-angular": "^1.4.1", "eslint-plugin-jasmine": "^1.8.1", "eslint-plugin-mocha": "^4.7.0", - "eslint-plugin-promise": "^3.1.0", + "eslint-plugin-promise": "^3.3.0", "eslint-plugin-protractor": "^1.28.0", "extract-text-webpack-plugin": "^1.0.1", "favicons-webpack-plugin": "0.0.7", @@ -58,7 +58,6 @@ "karma-webpack": "^1.8.0", "minimist": "^1.2.0", "mocha": "^3.1.2", - "morgan": "^1.7.0", "ng-annotate-loader": "^0.2.0", "node-sass": "^3.10.1", "nodemon": "^1.11.0", diff --git a/server/db.spec.js b/server/db.spec.js index 97ad468..bb90dbc 100644 --- a/server/db.spec.js +++ b/server/db.spec.js @@ -5,7 +5,7 @@ describe('db', () => { describe('seed', () => { - beforeEach(function() { + beforeEach(() => { return db.seed(); }); @@ -29,13 +29,13 @@ describe('db', () => { }); - describe('.drop', () =>{ + describe('.drop', () => { - beforeEach(() =>{ + beforeEach(() => { return db.seed(); }); - it('removes all collections', () =>{ + it('removes all collections', () => { return db.drop().then(() => { return db.contacts.find(); }) From 0fbe674688da0218b38e2386606552f59779d14d Mon Sep 17 00:00:00 2001 From: Lukasz Bandzarewicz Date: Thu, 20 Oct 2016 10:38:54 +0200 Subject: [PATCH 05/15] Remove unused lodash --- webpack.config.js | 1 - 1 file changed, 1 deletion(-) diff --git a/webpack.config.js b/webpack.config.js index b62574b..bc03fd1 100644 --- a/webpack.config.js +++ b/webpack.config.js @@ -12,7 +12,6 @@ module.exports = { entry: { vendor: [ 'jquery', - 'lodash', 'angular', 'angular-animate', 'angular-messages', From 2f52dd1b9eee387d7dbb8c148be0e3389e2ffe9e Mon Sep 17 00:00:00 2001 From: Lukasz Bandzarewicz Date: Thu, 20 Oct 2016 17:47:06 +0200 Subject: [PATCH 06/15] Require authentication for /api/contacts --- .babelrc.karma | 2 +- build_signature_loader.js | 10 +- gulpfile.js | 3 +- package.json | 2 +- server/api/authentication.js | 28 +-- server/api/authentication.spec.js | 5 +- server/api/contacts.js | 4 + server/api/contacts.spec.js | 79 +++++--- server/api/seed.spec.js | 2 +- server/app.spec.js | 288 ------------------------------ server/db.js | 2 + server/middlewares.js | 25 +++ server/middlewares.spec.js | 0 webpack.config.js | 2 +- 14 files changed, 117 insertions(+), 335 deletions(-) delete mode 100644 server/app.spec.js create mode 100644 server/middlewares.js create mode 100644 server/middlewares.spec.js diff --git a/.babelrc.karma b/.babelrc.karma index 9083ccf..9c5d378 100644 --- a/.babelrc.karma +++ b/.babelrc.karma @@ -1,5 +1,5 @@ { - "presets": [ "es2015" ], + "presets": ["es2015"], "plugins": [ ["istanbul", { "exclude": [ diff --git a/build_signature_loader.js b/build_signature_loader.js index 8b7a9b4..3270684 100644 --- a/build_signature_loader.js +++ b/build_signature_loader.js @@ -5,12 +5,12 @@ const path = require('path'); const cwd = path.join(__dirname, '.'); const exec = Promise.promisify(require('child_process').exec); -function inspectRepository() { - function revParse(args) { - return exec(`git rev-parse ${args}`, { cwd }) - .then((result) => result.trim()); - } +function revParse(args) { + return exec(`git rev-parse ${args}`, { cwd }) + .then((result) => result.trim()); +} +function inspectRepository() { return Promise.all([ revParse('--abbrev-ref HEAD'), revParse('HEAD') diff --git a/gulpfile.js b/gulpfile.js index 24e2b61..6761584 100644 --- a/gulpfile.js +++ b/gulpfile.js @@ -86,7 +86,8 @@ gulp.task('server:test', (done) => { gulp.src(['server/**/*.spec.js'], { read: false }) .pipe(mocha({ ui: 'bdd', - reporter: 'dot' + reporter: 'dot', + timeout: 250 })) .pipe(istanbul.writeReports({ dir: 'artifacts/server/coverage', diff --git a/package.json b/package.json index 412f541..8caa737 100644 --- a/package.json +++ b/package.json @@ -69,7 +69,7 @@ "sass-loader": "^4.0.2", "sinon": "^1.17.6", "style-loader": "^0.13.1", - "supertest": "^2.0.0", + "supertest": "^2.0.1", "url-loader": "^0.5.7", "webdriver-manager": "^10.2.5", "webpack": "^1.13.2", diff --git a/server/api/authentication.js b/server/api/authentication.js index fc4e2e0..f129de8 100644 --- a/server/api/authentication.js +++ b/server/api/authentication.js @@ -9,23 +9,29 @@ const db = require('../db'); function checkPassword(password, passwordHash) { const compare = Promise.promisify(bcrypt.compare); - return compare(password, passwordHash); + return compare(password, passwordHash).then((success) => { + if (success) { + return Promise.resolve(true); + } else { + return Promise.reject(true); + } + }); } router.post('/', (req, res) => { const { email, password } = req.body; db.users.findOne({ email }).then((user) => { - return checkPassword(password, user.passwordHash).then((success) => { - if (success) { - res.json({ - token: jwt.sign(_.pick(user, ['id', 'email']), config.secret, { - expiresIn: '7 days' - }) - }); - } else { - return Promise.reject('invalid password'); - } + const { passwordHash } = user; + + return checkPassword(password, passwordHash).then(() => { + const data = _.pick(user, ['id', 'email']); + + const token = jwt.sign(data, config.secret, { + expiresIn: '7 days' + }); + + res.json({ token }); }); }).catch(() => { res.sendStatus(422); diff --git a/server/api/authentication.spec.js b/server/api/authentication.spec.js index fed96a9..68ed9f7 100644 --- a/server/api/authentication.spec.js +++ b/server/api/authentication.spec.js @@ -6,7 +6,7 @@ const app = require('../app'); const config = require('../config'); const db = require('../db'); -describe('app', () => { +describe('authentication API', () => { beforeEach(() => { return db.seed(); @@ -22,9 +22,7 @@ describe('app', () => { it('creates a contact', (done) => { request(app) .post('/api/authentication') - .set('Accept', 'application/json') .send({ email, password }) - .expect('Content-Type', /json/) .expect(200) .expect((res) => { const { token } = res.body; @@ -49,7 +47,6 @@ describe('app', () => { it('creates a contact', (done) => { request(app) .post('/api/authentication') - .set('Accept', 'application/json') .send({ email, password }) .expect(422) .end(done); diff --git a/server/api/contacts.js b/server/api/contacts.js index 8bb550a..bc4f09d 100644 --- a/server/api/contacts.js +++ b/server/api/contacts.js @@ -1,5 +1,9 @@ const router = require('express').Router(); + const db = require('../db'); +const { requireAuthorization } = require('../middlewares'); + +router.use(requireAuthorization); router.param('id', (req, res, next, id) => { id = parseInt(id); diff --git a/server/api/contacts.spec.js b/server/api/contacts.spec.js index e8a6de4..935884e 100644 --- a/server/api/contacts.spec.js +++ b/server/api/contacts.spec.js @@ -1,22 +1,50 @@ +const _ = require('lodash'); const expect = require('chai').expect; +const jwt = require('jsonwebtoken'); const request = require('supertest'); const app = require('../app'); +const config = require('../config'); const db = require('../db'); -function itRespondsWith404(cb) { - it('responds with 404', (done) => { - cb(request(app)) - .set('Accept', 'application/json') - .expect(404) - .end(done); +describe('contacts API', () => { + + beforeEach(() => { + return db.seed(); }); -} -describe('app', () => { + let token; + // Authenticate beforeEach(() => { - return db.seed(); + return db.users.findOne({ email: 'demo@email.com' }).then((user) => { + const data = _.pick(user, ['id', 'email']); + + token = jwt.sign(data, config.secret, { + expiresIn: '1 minute' + }); + }); + }); + + function itRespondsWith404(cb) { + it('responds with 404', (done) => { + cb(request(app)) + .set('x-access-token', token) + .expect(404) + .end(done); + }); + } + + describe('with invalid token', () => { + + it('responds with `401` error', (done) => { + request(app) + .get('/api/contacts') + .set('x-access-token', 'invalid token') + .expect(401) + .end(done); + }); + }); describe('GET /api/contacts', () => { @@ -24,8 +52,7 @@ describe('app', () => { it('respond with json', (done) => { request(app) .get('/api/contacts') - .set('Accept', 'application/json') - .expect('Content-Type', /json/) + .set('x-access-token', token) .expect(200) .expect((res) => { const { contacts } = res.body; @@ -49,9 +76,8 @@ describe('app', () => { request(app) .post('/api/contacts') - .set('Accept', 'application/json') + .set('x-access-token', token) .send({ firstName, lastName, email }) - .expect('Content-Type', /json/) .expect(200) .expect((res) => { const { body: contact } = res; @@ -72,9 +98,13 @@ describe('app', () => { describe('when an email is not taken', () => { + const email = 'luke@rebel.org'; + it('responds with `{ taken: false }`', (done) => { request(app) - .get('/api/contacts/validate-email?email=luke@rebel.org') + .get('/api/contacts/validate-email') + .query({ email }) + .set('x-access-token', token) .expect(200) .expect((res) => { expect(res.body).to.have.property('taken', false); @@ -94,7 +124,9 @@ describe('app', () => { it('responds with `{ taken: true }`', (done) => { request(app) - .get(`/api/contacts/validate-email?email=${email}`) + .get('/api/contacts/validate-email') + .query({ email }) + .set('x-access-token', token) .expect(200) .expect((res) => { expect(res.body).to.have.property('taken', true); @@ -121,7 +153,9 @@ describe('app', () => { it('responds with `{ taken: false }`', (done) => { request(app) - .get(`/api/contacts/validate-email?id=${id}email=${email}`) + .get('/api/contacts/validate-email') + .query({ id, email }) + .set('x-access-token', token) .expect(200) .expect((res) => { expect(res.body).to.have.property('taken', false); @@ -131,7 +165,9 @@ describe('app', () => { it('responds with `{ taken: false }`', (done) => { request(app) - .get(`/api/contacts/validate-email?id=${id}&email=tarkin@empire.com`) + .get('/api/contacts/validate-email') + .query({ id, email: 'tarkin@empire.com' }) + .set('x-access-token', token) .expect(200) .expect((res) => { expect(res.body).to.have.property('taken', false); @@ -152,6 +188,7 @@ describe('app', () => { it('responds with `{ taken: true }`', (done) => { request(app) .get(`/api/contacts/validate-email?id=${id}&email=${otherEmail}`) + .set('x-access-token', token) .expect(200) .expect((res) => { expect(res.body).to.have.property('taken', true); @@ -172,8 +209,7 @@ describe('app', () => { it('respond with json', (done) => { request(app) .get('/api/contacts/3') - .set('Accept', 'application/json') - .expect('Content-Type', /json/) + .set('x-access-token', token) .expect(200) .expect((res) => { const { body: contact } = res; @@ -213,9 +249,8 @@ describe('app', () => { it('updates the contact', (done) => { request(app) .put(`/api/contacts/${id}`) - .set('Accept', 'application/json') + .set('x-access-token', token) .send({ firstName: 'Luke' }) - .expect('Content-Type', /json/) .expect(200) .expect((res) => { const { body: contact } = res; @@ -256,7 +291,7 @@ describe('app', () => { request(app) .delete(`/api/contacts/${id}`) - .set('Accept', 'application/json') + .set('x-access-token', token) .expect(200) .end(() => { db.contacts.findOne({ id }).catch(() => { diff --git a/server/api/seed.spec.js b/server/api/seed.spec.js index 8bb7373..c9b0b24 100644 --- a/server/api/seed.spec.js +++ b/server/api/seed.spec.js @@ -4,7 +4,7 @@ const request = require('supertest'); const app = require('../app'); const db = require('../db'); -describe('app', () => { +describe('seed API', () => { describe('POST /api/seed', () => { diff --git a/server/app.spec.js b/server/app.spec.js deleted file mode 100644 index cc04de6..0000000 --- a/server/app.spec.js +++ /dev/null @@ -1,288 +0,0 @@ -const expect = require('chai').expect; -const request = require('supertest'); - -const app = require('./app'); -const db = require('./db'); - -describe('app', () => { - - beforeEach(() => { - return db.seed(); - }); - - afterEach(() => { - return db.drop(); - }); - - describe('GET /api/contacts', () => { - - it('respond with json', (done) => { - request(app) - .get('/api/contacts') - .set('Accept', 'application/json') - .expect('Content-Type', /json/) - .expect(200) - .expect((res) => { - const { contacts } = res.body; - expect(contacts).to.have.length(20); - - expect(contacts[0]).to.have.property('firstName', 'Wallace'); - expect(contacts[0]).to.have.property('lastName', 'Rath'); - expect(contacts[0]).to.have.property('email', 'Tessie_Carter16@gmail.com'); - }) - .end(done); - }); - - }); - - describe('GET /api/contacts/validate-email', () => { - - describe('for non persisted contact', () => { - - describe('when an email is not taken', () => { - - it('responds with `{ taken: false }`', (done) => { - request(app) - .get('/api/contacts/validate-email?email=luke@rebel.org') - .expect(200) - .expect((res) => { - expect(res.body).to.have.property('taken', false); - }) - .end(done); - }); - - }); - - describe('when an email is already taken', () => { - - const email = 'anakin@republic.org'; - - beforeEach(() => { - return db.contacts.insertOne({ email }); - }); - - it('responds with `{ taken: true }`', (done) => { - request(app) - .get(`/api/contacts/validate-email?email=${email}`) - .expect(200) - .expect((res) => { - expect(res.body).to.have.property('taken', true); - }) - .end(done); - }); - - }); - - }); - - describe('for persisted contact', () => { - - let id; - const email = 'vader@empire.com'; - - beforeEach(() => { - return db.contacts.insertOne({ email }).then((contact) => { - id = contact.id; - }); - }); - - describe('when an email is not taken', () => { - - it('responds with `{ taken: false }`', (done) => { - request(app) - .get(`/api/contacts/validate-email?id=${id}email=${email}`) - .expect(200) - .expect((res) => { - expect(res.body).to.have.property('taken', false); - }) - .end(done); - }); - - it('responds with `{ taken: false }`', (done) => { - request(app) - .get(`/api/contacts/validate-email?id=${id}&email=tarkin@empire.com`) - .expect(200) - .expect((res) => { - expect(res.body).to.have.property('taken', false); - }) - .end(done); - }); - - }); - - describe('when an email is taken', () => { - - const otherEmail = 'tarkin@empire.com'; - - beforeEach(() => { - return db.contacts.insertOne({ email: otherEmail }); - }); - - it('responds with `{ taken: true }`', (done) => { - request(app) - .get(`/api/contacts/validate-email?id=${id}&email=${otherEmail}`) - .expect(200) - .expect((res) => { - expect(res.body).to.have.property('taken', true); - }) - .end(done); - }); - - }); - - }); - - }); - - describe('POST /api/contacts', () => { - - it('creates a contact', (done) => { - const firstName = 'Luke'; - const lastName = 'Skywalker'; - const email = 'luke@rebel.org'; - - request(app) - .post('/api/contacts') - .set('Accept', 'application/json') - .send({ firstName, lastName, email }) - .expect('Content-Type', /json/) - .expect(200) - .expect((res) => { - const { body: contact } = res; - - expect(contact).to.have.property('id', 21); - expect(contact).to.have.property('firstName', firstName); - expect(contact).to.have.property('lastName', lastName); - expect(contact).to.have.property('email', email); - }) - .end(done); - }); - - }); - - describe('GET /api/contacts/:id', () => { - - describe('when a contact can be found', () => { - - it('respond with json', (done) => { - request(app) - .get('/api/contacts/3') - .set('Accept', 'application/json') - .expect('Content-Type', /json/) - .expect(200) - .expect((res) => { - const { body: contact } = res; - - expect(contact).to.have.property('id', 3); - expect(contact).to.have.property('firstName', 'Caterina'); - expect(contact).to.have.property('lastName', 'Hackett'); - expect(contact).to.have.property('email', 'Destin.Kassulke80@hotmail.com'); - }) - .end(done); - }); - - }); - - describe('when a contact cannot be found', () => { - - it('responds with 404', (done) => { - request(app) - .get('/api/contacts/21') - .set('Accept', 'application/json') - .expect(404) - .end(done); - }); - - }); - - }); - - describe('PUT /api/contacts/:id', () => { - - describe('when a contact can be found', () => { - - let id; - - beforeEach(() => { - return db.contacts.insertOne({ firstName: 'Anakin', lastName: 'Skywalker' }).then((contact) => { - id = contact.id; - }); - }); - - it('updates the contact', (done) => { - request(app) - .put(`/api/contacts/${id}`) - .set('Accept', 'application/json') - .send({ firstName: 'Luke' }) - .expect('Content-Type', /json/) - .expect(200) - .expect((res) => { - const { body: contact } = res; - - expect(contact).to.have.property('id', id); - expect(contact).to.have.property('firstName', 'Luke'); - expect(contact).to.have.property('lastName', 'Skywalker'); - }) - .end(done); - }); - - }); - - describe('when a contact cannot be found', () => { - - it('responds with 404', (done) => { - request(app) - .put('/api/contacts/21') - .send({}) - .set('Accept', 'application/json') - .expect(404) - .end(done); - }); - - }); - - }); - - describe('DELETE /api/contacts/:id', () => { - - describe('when a contact can be found', () => { - - let contact; - - beforeEach(() => { - return db.contacts.insertOne({ firstName: 'Luke' }).then((createdContact) => { - contact = createdContact; - }); - }); - - it('deletes the contact', (done) => { - const { id } = contact; - - request(app) - .delete(`/api/contacts/${id}`) - .set('Accept', 'application/json') - .expect(200) - .end(() => { - db.contacts.findOne({ id }).catch(() => { - done(); - }); - }); - }); - - }); - - describe('when a contact cannot be found', () => { - - it('responds with 404', (done) => { - request(app) - .delete('/api/contacts/21') - .set('Accept', 'application/json') - .expect(404) - .end(done); - }); - - }); - - }); - -}); diff --git a/server/db.js b/server/db.js index e02cb36..de5d858 100644 --- a/server/db.js +++ b/server/db.js @@ -31,6 +31,8 @@ class Db { return hashPassword('password').then((passwordHash) => { return this.users.insertOne({ + firstName: 'Admin', + lastName: 'Adminowsky', email: 'demo@email.com', passwordHash }); diff --git a/server/middlewares.js b/server/middlewares.js new file mode 100644 index 0000000..207381f --- /dev/null +++ b/server/middlewares.js @@ -0,0 +1,25 @@ +const Promise = require('bluebird'); +const jwt = require('jsonwebtoken'); + +const config = require('./config'); +const db = require('./db'); + +function verityToken(token) { + return Promise.promisify(jwt.verify)(token, config.secret) + .then((decoded) => { + const { id, email } = decoded; + return db.users.findOne({ id, email }); + }); +} + +// TODO write decent test for this middleware +module.exports.requireAuthorization = function(req, res, next) { + const token = req.headers['x-access-token']; + + verityToken(token).then((user) => { + req.currentUser = user; + next(); + }).catch(() => { + res.sendStatus(401); + }); +}; diff --git a/server/middlewares.spec.js b/server/middlewares.spec.js new file mode 100644 index 0000000..e69de29 diff --git a/webpack.config.js b/webpack.config.js index bc03fd1..19b4d33 100644 --- a/webpack.config.js +++ b/webpack.config.js @@ -108,5 +108,5 @@ module.exports = { } }, - devtool: 'eval-source-map ' + devtool: 'eval-source-map' }; From dcc0b5ad43f379f2a3e4a46ac0cb635c40b87d1b Mon Sep 17 00:00:00 2001 From: Lukasz Bandzarewicz Date: Thu, 20 Oct 2016 17:58:57 +0200 Subject: [PATCH 07/15] Reorganize db --- server/config.js | 6 ++++-- server/db.js | 2 +- server/{ => db}/collection.js | 0 server/{ => db}/collection.spec.js | 0 4 files changed, 5 insertions(+), 3 deletions(-) rename server/{ => db}/collection.js (100%) rename server/{ => db}/collection.spec.js (100%) diff --git a/server/config.js b/server/config.js index 9bbdda5..e501061 100644 --- a/server/config.js +++ b/server/config.js @@ -1,4 +1,6 @@ +const { env } = process; + module.exports = Object.freeze({ - env: process.env.NODE_ENV || 'development', - secret: process.env.SECRET || 'I am a vegan' + env: env['NODE_ENV'] || 'development', + secret: env['SECRET'] || 'I am a vegan' }); diff --git a/server/db.js b/server/db.js index de5d858..c5618f0 100644 --- a/server/db.js +++ b/server/db.js @@ -3,7 +3,7 @@ const bcrypt = require('bcrypt'); const faker = require('faker'); const _ = require('lodash'); -const Collection = require('./collection'); +const Collection = require('./db/collection'); class Db { diff --git a/server/collection.js b/server/db/collection.js similarity index 100% rename from server/collection.js rename to server/db/collection.js diff --git a/server/collection.spec.js b/server/db/collection.spec.js similarity index 100% rename from server/collection.spec.js rename to server/db/collection.spec.js From 55687cfc44da082de57ea13eb9716b9839f719cf Mon Sep 17 00:00:00 2001 From: Lukasz Bandzarewicz Date: Fri, 21 Oct 2016 15:51:48 +0200 Subject: [PATCH 08/15] wip --- server/app.js | 8 +- server/middlewares.js | 4 +- server/middlewares.spec.js | 0 src/app/commons/breadcrumb.template.html | 6 - src/app/commons/commons.module.js | 14 +-- .../navigation/navigation.component.html | 10 ++ .../navigation/navigation.component.js | 24 +++- src/app/commons/services/auth/auth.service.js | 23 ++++ .../services/auth/auth.service.spec.js | 119 ++++++++++++++++++ src/app/commons/services/services.config.js | 4 + 10 files changed, 192 insertions(+), 20 deletions(-) delete mode 100644 server/middlewares.spec.js delete mode 100644 src/app/commons/breadcrumb.template.html create mode 100644 src/app/commons/services/auth/auth.service.js create mode 100644 src/app/commons/services/auth/auth.service.spec.js diff --git a/server/app.js b/server/app.js index 0fbde3f..8950bbc 100644 --- a/server/app.js +++ b/server/app.js @@ -1,15 +1,17 @@ const express = require('express'); -const morgan = require('morgan'); +const bodyParser = require('body-parser'); +const logger = require('morgan'); const path = require('path'); const config = require('./config'); const app = express(); app.use(express.static(path.join(__dirname, '..', 'build'))); -app.use(require('body-parser').json()); +app.use(bodyParser.json()); +// TODO use `app.get('env')` if (config.env !== 'test') { - app.use(morgan('short')); + app.use(logger('short')); } if (config.env !== 'production') { diff --git a/server/middlewares.js b/server/middlewares.js index 207381f..d00bebf 100644 --- a/server/middlewares.js +++ b/server/middlewares.js @@ -12,7 +12,7 @@ function verityToken(token) { }); } -// TODO write decent test for this middleware +// TODO write decent tests for this middleware module.exports.requireAuthorization = function(req, res, next) { const token = req.headers['x-access-token']; @@ -20,6 +20,6 @@ module.exports.requireAuthorization = function(req, res, next) { req.currentUser = user; next(); }).catch(() => { - res.sendStatus(401); + res.status(401).send({}); }); }; diff --git a/server/middlewares.spec.js b/server/middlewares.spec.js deleted file mode 100644 index e69de29..0000000 diff --git a/src/app/commons/breadcrumb.template.html b/src/app/commons/breadcrumb.template.html deleted file mode 100644 index d6c0911..0000000 --- a/src/app/commons/breadcrumb.template.html +++ /dev/null @@ -1,6 +0,0 @@ - diff --git a/src/app/commons/commons.module.js b/src/app/commons/commons.module.js index e54f33f..3c3a122 100644 --- a/src/app/commons/commons.module.js +++ b/src/app/commons/commons.module.js @@ -1,7 +1,8 @@ import 'angular-breadcrumb'; -import breadcrumbTemplate from './breadcrumb.template.html'; +import breadcrumb from './config/breadcrumb.config'; import components from './components/components.config'; import filters from './filters/filters.config'; +import httpAuthInterceptor from './config/http-auth-interceptor'; import services from './services/services.config'; import toastr from 'angular-toastr'; @@ -9,16 +10,13 @@ export default angular.module('app.commons', [ toastr, 'ncy-angular-breadcrumb' ]) + .service('localStorage', ($window) => $window.localStorage) + .config(components) .config(services) .config(filters) - .config(($breadcrumbProvider) => { - 'ngInject'; - - $breadcrumbProvider.setOptions({ - template: breadcrumbTemplate - }); - }) + .config(breadcrumb) + .config(httpAuthInterceptor) .name; diff --git a/src/app/commons/components/navigation/navigation.component.html b/src/app/commons/components/navigation/navigation.component.html index 84affee..846edd0 100644 --- a/src/app/commons/components/navigation/navigation.component.html +++ b/src/app/commons/components/navigation/navigation.component.html @@ -17,6 +17,16 @@ About + + diff --git a/src/app/commons/components/navigation/navigation.component.js b/src/app/commons/components/navigation/navigation.component.js index 6ddb0cd..c04081f 100644 --- a/src/app/commons/components/navigation/navigation.component.js +++ b/src/app/commons/components/navigation/navigation.component.js @@ -1,5 +1,27 @@ import template from './navigation.component.html'; +class Controller { + + constructor(auth) { + 'ngInject'; + this.auth = auth; + } + + isAuthenticated() { + return this.auth.isAuthenticated(); + } + + login() { + this.auth.authenticate('demo@email.com', 'password'); + } + + logout() { + this.auth.logout(); + } + +} + export default { - template + template, + controller: Controller }; diff --git a/src/app/commons/services/auth/auth.service.js b/src/app/commons/services/auth/auth.service.js new file mode 100644 index 0000000..5e87436 --- /dev/null +++ b/src/app/commons/services/auth/auth.service.js @@ -0,0 +1,23 @@ +export default function($http, session) { + 'ngInject'; + + return { + + isAuthenticated() { + return Boolean(session.getToken()); + }, + + authenticate(email, password) { + return $http.post('/api/authentication', { email, password }).then((response) => { + const { token } = response.data; + session.setToken(token); + }); + }, + + logout() { + session.removeToken(); + } + + }; + +} diff --git a/src/app/commons/services/auth/auth.service.spec.js b/src/app/commons/services/auth/auth.service.spec.js new file mode 100644 index 0000000..58a2789 --- /dev/null +++ b/src/app/commons/services/auth/auth.service.spec.js @@ -0,0 +1,119 @@ +import appCommonsModule from '../../commons.module'; +import { expect } from 'chai'; +import sinon from 'sinon'; + +describe(`module: ${appCommonsModule}`, () => { + + beforeEach(() => { + angular.mock.module(appCommonsModule, ($provide) => { + $provide.decorator('session', ($delegate) => { + 'ngInject'; + return sinon.stub($delegate); + }); + }); + }); + + describe('service: auth', () => { + + let auth; + + beforeEach(inject(($injector) => { + auth = $injector.get('auth'); + })); + + describe('.isAuthenticated', () => { + + describe('when the token is present', () => { + + beforeEach(inject((session) => { + session.getToken.returns('the token'); + })); + + it('returns true', () => { + expect(auth.isAuthenticated()).to.be.true; + }); + + }); + + describe('when the token is not present', () => { + + beforeEach(inject((session) => { + session.getToken.returns(null); + })); + + it('returns false', () => { + expect(auth.isAuthenticated()).to.be.false; + }); + + }); + + }); + + describe('.authenticate', () => { + + const email = 'test@email.com'; + const password = 'passowrd'; + + let request; + + beforeEach(inject(($httpBackend) => { + request = $httpBackend.expectPOST('/api/authentication', { email, password }); + })); + + it('returns a promise', () => { + expect(auth.authenticate(email, password)).to.be.a.promise; + }); + + describe('on success', () => { + + beforeEach(() => { + request.respond(200, { token: 'the token' }); + }); + + it('stores the token in the session', inject(($httpBackend, session) => { + // When + auth.authenticate(email, password); + $httpBackend.flush(); + + // Then + expect(session.setToken.calledWith('the token')) + .to.be.true; + })); + + }); + + describe('on error', () => { + + beforeEach(() => { + request.respond(422); + }); + + it('does not store the token in the session', inject(($httpBackend, session) => { + // When + auth.authenticate(email, password); + $httpBackend.flush(); + + // Then + expect(session.setToken.calledWith('the token')) + .to.be.false; + })); + + }); + + }); + + describe('.logout', () => { + + it('removes a token from the session', inject((session) => { + // When + auth.logout(); + + // Then + expect(session.removeToken.called).to.be.true; + })); + + }); + + }); + +}); diff --git a/src/app/commons/services/services.config.js b/src/app/commons/services/services.config.js index 98ad361..599c5c3 100644 --- a/src/app/commons/services/services.config.js +++ b/src/app/commons/services/services.config.js @@ -1,9 +1,13 @@ import alert from './alert/alert.service'; +import auth from './auth/auth.service'; import confirm from './confirm/confirm.service'; +import session from './session/session.service'; export default function($provide) { 'ngInject'; $provide.service('alert', alert); + $provide.service('auth', auth); $provide.service('confirm', confirm); + $provide.service('session', session); } From f94994ada28729b10fff2abe2224b3f5115df7c0 Mon Sep 17 00:00:00 2001 From: Lukasz Bandzarewicz Date: Thu, 27 Oct 2016 14:54:57 +0200 Subject: [PATCH 09/15] Simple authentication logic --- src/app/commons/commons.module.js | 4 +- .../config/breadcrumb/breadcrumb.config.js | 9 ++ .../breadcrumb/breadcrumb.template.html | 6 + .../http-auth-interceptor.config.js | 35 ++++++ .../http-auth-interceptor.config.spec.js | 115 ++++++++++++++++++ .../services/session/session.service.js | 20 +++ .../services/session/session.service.spec.js | 64 ++++++++++ 7 files changed, 251 insertions(+), 2 deletions(-) create mode 100644 src/app/commons/config/breadcrumb/breadcrumb.config.js create mode 100644 src/app/commons/config/breadcrumb/breadcrumb.template.html create mode 100644 src/app/commons/config/http-auth-interceptor/http-auth-interceptor.config.js create mode 100644 src/app/commons/config/http-auth-interceptor/http-auth-interceptor.config.spec.js create mode 100644 src/app/commons/services/session/session.service.js create mode 100644 src/app/commons/services/session/session.service.spec.js diff --git a/src/app/commons/commons.module.js b/src/app/commons/commons.module.js index 3c3a122..e136a06 100644 --- a/src/app/commons/commons.module.js +++ b/src/app/commons/commons.module.js @@ -1,8 +1,8 @@ import 'angular-breadcrumb'; -import breadcrumb from './config/breadcrumb.config'; +import breadcrumb from './config/breadcrumb/breadcrumb.config'; import components from './components/components.config'; import filters from './filters/filters.config'; -import httpAuthInterceptor from './config/http-auth-interceptor'; +import httpAuthInterceptor from './config/http-auth-interceptor/http-auth-interceptor.config'; import services from './services/services.config'; import toastr from 'angular-toastr'; diff --git a/src/app/commons/config/breadcrumb/breadcrumb.config.js b/src/app/commons/config/breadcrumb/breadcrumb.config.js new file mode 100644 index 0000000..56c8dd4 --- /dev/null +++ b/src/app/commons/config/breadcrumb/breadcrumb.config.js @@ -0,0 +1,9 @@ +import template from './breadcrumb.template.html'; + +export default function($breadcrumbProvider) { + 'ngInject'; + + $breadcrumbProvider.setOptions({ + template + }); +} diff --git a/src/app/commons/config/breadcrumb/breadcrumb.template.html b/src/app/commons/config/breadcrumb/breadcrumb.template.html new file mode 100644 index 0000000..d6c0911 --- /dev/null +++ b/src/app/commons/config/breadcrumb/breadcrumb.template.html @@ -0,0 +1,6 @@ + diff --git a/src/app/commons/config/http-auth-interceptor/http-auth-interceptor.config.js b/src/app/commons/config/http-auth-interceptor/http-auth-interceptor.config.js new file mode 100644 index 0000000..70eac8e --- /dev/null +++ b/src/app/commons/config/http-auth-interceptor/http-auth-interceptor.config.js @@ -0,0 +1,35 @@ +// TODO better naming convention for interceptors +export default function($httpProvider) { + 'ngInject'; + + const HTTP_UNAUTHORIZED = 401; + + $httpProvider.interceptors.push(($injector, $q, session) => { + return { + request(config) { + const token = session.getToken(); + + if (token) { + angular.extend(config.headers, { + 'x-access-token': token + }); + } + + return config; + }, + + responseError(rejection) { + if (rejection.status === HTTP_UNAUTHORIZED) { + // Workaround for circular dependency (toastr <- $http) + const toastr = $injector.get('toastr'); + toastr.error('Unauthorized'); + + session.removeToken(); + } + + return $q.reject(rejection); + } + }; + }); + +} diff --git a/src/app/commons/config/http-auth-interceptor/http-auth-interceptor.config.spec.js b/src/app/commons/config/http-auth-interceptor/http-auth-interceptor.config.spec.js new file mode 100644 index 0000000..aecd885 --- /dev/null +++ b/src/app/commons/config/http-auth-interceptor/http-auth-interceptor.config.spec.js @@ -0,0 +1,115 @@ +import commonsModule from '../../commons.module'; +import { expect } from 'chai'; +import sinon from 'sinon'; + +describe(`module: ${commonsModule}`, () => { + + beforeEach(angular.mock.module(commonsModule, ($provide) => { + $provide.value('session', { + getToken: sinon.stub(), + removeToken: sinon.stub() + }); + })); + + let $http, $httpBackend; + + beforeEach(inject(($injector) => { + $http = $injector.get('$http'); + $httpBackend = $injector.get('$httpBackend'); + })); + + describe('request interceptor', () => { + + describe('when the token is set', () => { + + beforeEach(inject((session) => { + session.getToken.returns('the token'); + })); + + it('sends http with valid header', (done) => { + $httpBackend + .expectGET('/api/test', (headers) => { + expect(headers).to.have.property('x-access-token', 'the token'); + done(); + }) + .respond(200); + + $http.get('/api/test'); + + $httpBackend.flush(); + }); + + }); + + describe('when the token is not set', () => { + + beforeEach(inject((session) => { + session.getToken.returns(undefined); + })); + + it('sends http with valid header', (done) => { + $httpBackend + .expectGET('/api/test', (headers) => { + expect(headers).to.not.have.property('x-access-token'); + done(); + }) + .respond(200); + + $http.get('/api/test'); + + $httpBackend.flush(); + }); + + }); + + }); + + describe('response interceptor', () => { + + describe('on `401` http error', () => { + + beforeEach(inject((toastr) => { + // Given + sinon.stub(toastr, 'error'); + + $httpBackend + .expectGET('/api/test') + .respond(401); + + // When + $http.get('/api/test'); + $httpBackend.flush(); + })); + + it('removes a token from the session', inject((session) => { + expect(session.removeToken.called).to.be.true; + })); + + it('displays an error', inject((toastr) => { + expect(toastr.error.calledWith('Unauthorized')).to.be.true; + })); + + }); + + describe('on non `401` http error', () => { + + beforeEach(() => { + // Given + $httpBackend + .expectGET('/api/test') + .respond(422); + + // When + $http.get('/api/test'); + $httpBackend.flush(); + }); + + it('does not remove a token from the session', inject((session) => { + expect(session.removeToken.called).to.be.false; + })); + + }); + + }); + +}); diff --git a/src/app/commons/services/session/session.service.js b/src/app/commons/services/session/session.service.js new file mode 100644 index 0000000..23b38a5 --- /dev/null +++ b/src/app/commons/services/session/session.service.js @@ -0,0 +1,20 @@ +export default function(localStorage) { + 'ngInject'; + + const TOKEN = 'token'; + + return { + setToken(token) { + localStorage.setItem(TOKEN, token); + }, + + getToken() { + return localStorage.getItem(TOKEN); + }, + + removeToken() { + localStorage.removeItem(TOKEN); + } + }; + +} diff --git a/src/app/commons/services/session/session.service.spec.js b/src/app/commons/services/session/session.service.spec.js new file mode 100644 index 0000000..d4f8f11 --- /dev/null +++ b/src/app/commons/services/session/session.service.spec.js @@ -0,0 +1,64 @@ +import commonsModule from '../../commons.module'; +import { expect } from 'chai'; +import sinon from 'sinon'; + +describe(`module: ${commonsModule}`, () => { + + beforeEach(angular.mock.module(commonsModule)); + + describe('service: session', () => { + + let session; + + beforeEach(inject(($injector) => { + session = $injector.get('session'); + })); + + describe('.setToken', () => { + + it('sets a token in the local storage', inject((localStorage) => { + // Given + sinon.stub(localStorage, 'setItem'); + + // When + session.setToken('the token'); + + // Then + expect(localStorage.setItem.calledWith('token', 'the token')).to.be.true; + })); + + }); + + describe('.getToken', () => { + + it('retrieves a token from the local storage', inject((localStorage) => { + // Given + sinon.stub(localStorage, 'getItem').returns('the token'); + + // When + expect(session.getToken()).to.eq('the token'); + + // Then + expect(localStorage.getItem.calledWith('token')).to.be.true; + })); + + }); + + describe('.removeToken', () => { + + it('removes a token from the local storage', inject((localStorage) => { + // Given + sinon.stub(localStorage, 'removeItem'); + + // When + session.removeToken(); + + // Then + expect(localStorage.removeItem.calledWith('token')).to.be.true; + })); + + }); + + }); + +}); From 24dc890cbb1617c8f26e1b9e573d5dde9f22b73d Mon Sep 17 00:00:00 2001 From: Lukasz Bandzarewicz Date: Thu, 27 Oct 2016 15:03:11 +0200 Subject: [PATCH 10/15] Reorganize interceptors --- src/app/commons/commons.module.js | 6 +- .../access-token-interceptor.js | 20 ++++++ .../access-token-interceptor.spec.js | 67 +++++++++++++++++++ .../unauthorized-access-interceptor.js} | 13 ---- .../unauthorized-access-interceptor.spec.js} | 48 +------------ 5 files changed, 92 insertions(+), 62 deletions(-) create mode 100644 src/app/commons/config/http-interceptors/access-token-interceptor.js create mode 100644 src/app/commons/config/http-interceptors/access-token-interceptor.spec.js rename src/app/commons/config/{http-auth-interceptor/http-auth-interceptor.config.js => http-interceptors/unauthorized-access-interceptor.js} (65%) rename src/app/commons/config/{http-auth-interceptor/http-auth-interceptor.config.spec.js => http-interceptors/unauthorized-access-interceptor.spec.js} (58%) diff --git a/src/app/commons/commons.module.js b/src/app/commons/commons.module.js index e136a06..3db92f3 100644 --- a/src/app/commons/commons.module.js +++ b/src/app/commons/commons.module.js @@ -1,10 +1,11 @@ import 'angular-breadcrumb'; +import accessTokenInterceptor from './config/http-interceptors/access-token-interceptor'; import breadcrumb from './config/breadcrumb/breadcrumb.config'; import components from './components/components.config'; import filters from './filters/filters.config'; -import httpAuthInterceptor from './config/http-auth-interceptor/http-auth-interceptor.config'; import services from './services/services.config'; import toastr from 'angular-toastr'; +import unauthorizedAccessInterceptor from './config/http-interceptors/unauthorized-access-interceptor'; export default angular.module('app.commons', [ toastr, @@ -17,6 +18,7 @@ export default angular.module('app.commons', [ .config(filters) .config(breadcrumb) - .config(httpAuthInterceptor) + .config(accessTokenInterceptor) + .config(unauthorizedAccessInterceptor) .name; diff --git a/src/app/commons/config/http-interceptors/access-token-interceptor.js b/src/app/commons/config/http-interceptors/access-token-interceptor.js new file mode 100644 index 0000000..2d54c1b --- /dev/null +++ b/src/app/commons/config/http-interceptors/access-token-interceptor.js @@ -0,0 +1,20 @@ +export default function($httpProvider) { + 'ngInject'; + + $httpProvider.interceptors.push(($injector, $q, session) => { + return { + request(config) { + const token = session.getToken(); + + if (token) { + angular.extend(config.headers, { + 'x-access-token': token + }); + } + + return config; + } + }; + }); + +} diff --git a/src/app/commons/config/http-interceptors/access-token-interceptor.spec.js b/src/app/commons/config/http-interceptors/access-token-interceptor.spec.js new file mode 100644 index 0000000..7a58f2a --- /dev/null +++ b/src/app/commons/config/http-interceptors/access-token-interceptor.spec.js @@ -0,0 +1,67 @@ +import commonsModule from '../../commons.module'; +import { expect } from 'chai'; +import sinon from 'sinon'; + +describe(`module: ${commonsModule}`, () => { + + beforeEach(angular.mock.module(commonsModule, ($provide) => { + $provide.value('session', { + getToken: sinon.stub(), + removeToken: sinon.stub() + }); + })); + + let $http, $httpBackend; + + beforeEach(inject(($injector) => { + $http = $injector.get('$http'); + $httpBackend = $injector.get('$httpBackend'); + })); + + describe('access token interceptor', () => { + + describe('when the token is set', () => { + + beforeEach(inject((session) => { + session.getToken.returns('the token'); + })); + + it('sends http with valid header', (done) => { + $httpBackend + .expectGET('/api/test', (headers) => { + expect(headers).to.have.property('x-access-token', 'the token'); + done(); + }) + .respond(200); + + $http.get('/api/test'); + + $httpBackend.flush(); + }); + + }); + + describe('when the token is not set', () => { + + beforeEach(inject((session) => { + session.getToken.returns(undefined); + })); + + it('sends http with valid header', (done) => { + $httpBackend + .expectGET('/api/test', (headers) => { + expect(headers).to.not.have.property('x-access-token'); + done(); + }) + .respond(200); + + $http.get('/api/test'); + + $httpBackend.flush(); + }); + + }); + + }); + +}); diff --git a/src/app/commons/config/http-auth-interceptor/http-auth-interceptor.config.js b/src/app/commons/config/http-interceptors/unauthorized-access-interceptor.js similarity index 65% rename from src/app/commons/config/http-auth-interceptor/http-auth-interceptor.config.js rename to src/app/commons/config/http-interceptors/unauthorized-access-interceptor.js index 70eac8e..6c6da67 100644 --- a/src/app/commons/config/http-auth-interceptor/http-auth-interceptor.config.js +++ b/src/app/commons/config/http-interceptors/unauthorized-access-interceptor.js @@ -1,4 +1,3 @@ -// TODO better naming convention for interceptors export default function($httpProvider) { 'ngInject'; @@ -6,18 +5,6 @@ export default function($httpProvider) { $httpProvider.interceptors.push(($injector, $q, session) => { return { - request(config) { - const token = session.getToken(); - - if (token) { - angular.extend(config.headers, { - 'x-access-token': token - }); - } - - return config; - }, - responseError(rejection) { if (rejection.status === HTTP_UNAUTHORIZED) { // Workaround for circular dependency (toastr <- $http) diff --git a/src/app/commons/config/http-auth-interceptor/http-auth-interceptor.config.spec.js b/src/app/commons/config/http-interceptors/unauthorized-access-interceptor.spec.js similarity index 58% rename from src/app/commons/config/http-auth-interceptor/http-auth-interceptor.config.spec.js rename to src/app/commons/config/http-interceptors/unauthorized-access-interceptor.spec.js index aecd885..01f2d1e 100644 --- a/src/app/commons/config/http-auth-interceptor/http-auth-interceptor.config.spec.js +++ b/src/app/commons/config/http-interceptors/unauthorized-access-interceptor.spec.js @@ -18,53 +18,7 @@ describe(`module: ${commonsModule}`, () => { $httpBackend = $injector.get('$httpBackend'); })); - describe('request interceptor', () => { - - describe('when the token is set', () => { - - beforeEach(inject((session) => { - session.getToken.returns('the token'); - })); - - it('sends http with valid header', (done) => { - $httpBackend - .expectGET('/api/test', (headers) => { - expect(headers).to.have.property('x-access-token', 'the token'); - done(); - }) - .respond(200); - - $http.get('/api/test'); - - $httpBackend.flush(); - }); - - }); - - describe('when the token is not set', () => { - - beforeEach(inject((session) => { - session.getToken.returns(undefined); - })); - - it('sends http with valid header', (done) => { - $httpBackend - .expectGET('/api/test', (headers) => { - expect(headers).to.not.have.property('x-access-token'); - done(); - }) - .respond(200); - - $http.get('/api/test'); - - $httpBackend.flush(); - }); - - }); - - }); - - describe('response interceptor', () => { + describe('unauthorized access interceptor', () => { describe('on `401` http error', () => { From 6ab0aa8e688c15ead35c0cc5d72b6d35609ee759 Mon Sep 17 00:00:00 2001 From: Lukasz Bandzarewicz Date: Thu, 27 Oct 2016 15:26:59 +0200 Subject: [PATCH 11/15] Basic login page --- src/app/app.module.js | 2 + .../authentication/authentication.config.js | 8 +++ .../authentication/authentication.module.js | 10 +++ .../authentication/login/login.controller.js | 21 ++++++ .../login/login.controller.spec.js | 72 +++++++++++++++++++ src/app/authentication/login/login.state.html | 27 +++++++ src/app/authentication/login/login.state.js | 12 ++++ .../authentication/login/login.state.spec.js | 25 +++++++ .../navigation/navigation.component.html | 2 +- .../navigation/navigation.component.js | 4 -- 10 files changed, 178 insertions(+), 5 deletions(-) create mode 100644 src/app/authentication/authentication.config.js create mode 100644 src/app/authentication/authentication.module.js create mode 100644 src/app/authentication/login/login.controller.js create mode 100644 src/app/authentication/login/login.controller.spec.js create mode 100644 src/app/authentication/login/login.state.html create mode 100644 src/app/authentication/login/login.state.js create mode 100644 src/app/authentication/login/login.state.spec.js diff --git a/src/app/app.module.js b/src/app/app.module.js index df200f3..cf86e7c 100644 --- a/src/app/app.module.js +++ b/src/app/app.module.js @@ -2,6 +2,7 @@ import { anchorScroll, html5Mode, notFoundState, router } from './app.config'; import angularAnimate from 'angular-animate'; import angularLoadingBar from 'angular-loading-bar'; import appAboutModule from './about/about.module'; +import appAuthenticationModule from './authentication/authentication.module'; import appCommonsModule from './commons/commons.module'; import appContactsModule from './contacts/contacts.module'; import appHomeModule from './home/home.module'; @@ -28,6 +29,7 @@ export default angular.module('app', [ angularLoadingBar, appCommonsModule, + appAuthenticationModule, appHomeModule, appContactsModule, appAboutModule diff --git a/src/app/authentication/authentication.config.js b/src/app/authentication/authentication.config.js new file mode 100644 index 0000000..5cf50c8 --- /dev/null +++ b/src/app/authentication/authentication.config.js @@ -0,0 +1,8 @@ +import loginState from './login/login.state'; + +export function states($stateProvider) { + 'ngInject'; + + $stateProvider + .state(loginState); +} diff --git a/src/app/authentication/authentication.module.js b/src/app/authentication/authentication.module.js new file mode 100644 index 0000000..542f395 --- /dev/null +++ b/src/app/authentication/authentication.module.js @@ -0,0 +1,10 @@ +import appCommonsModule from '../commons/commons.module'; +import { states } from './authentication.config'; +import uiRouter from 'angular-ui-router'; + +export default angular.module('app.authentication', [ + uiRouter, + appCommonsModule +]) + .config(states) + .name; diff --git a/src/app/authentication/login/login.controller.js b/src/app/authentication/login/login.controller.js new file mode 100644 index 0000000..90b802d --- /dev/null +++ b/src/app/authentication/login/login.controller.js @@ -0,0 +1,21 @@ +export default class { + + constructor($state, auth) { + 'ngInject'; + + this.$state = $state; + this.auth = auth; + + this.credentials = { + email: 'demo@email.com', + password: 'password' + }; + } + + login() { + return this.auth.authenticate('demo@email.com', 'password').then(() => { + return this.$state.go('home'); + }); + } + +} diff --git a/src/app/authentication/login/login.controller.spec.js b/src/app/authentication/login/login.controller.spec.js new file mode 100644 index 0000000..04cfc96 --- /dev/null +++ b/src/app/authentication/login/login.controller.spec.js @@ -0,0 +1,72 @@ +import appContactsModule from '../authentication.module'; +import { expect } from 'chai'; +import { name } from './login.state'; +import sinon from 'sinon'; + +describe(`module: ${appContactsModule}`, () => { + + beforeEach(() => { + angular.mock.module(appContactsModule); + }); + + describe(`controller: ${name}`, () => { + + let ctrl; + + beforeEach(inject(($controller, $state, auth) => { + const Controller = $state.get(name).controller; + + ctrl = $controller(Controller, { + $state: sinon.stub($state), + auth: sinon.stub(auth) + }); + })); + + it('is defined', () => { + expect(ctrl).to.not.be.undefined; + }); + + it('has predefined credentials', () => { + expect(ctrl.credentials).to.have.property('email'); + expect(ctrl.credentials).to.have.property('password'); + }); + + describe('.login', () => { + + describe('on success', () => { + + beforeEach(inject(($q) => { + ctrl.auth.authenticate.returns($q.resolve()); + })); + + it('redirects to the `home` page', inject(($rootScope) => { + ctrl.login(); + $rootScope.$digest(); + + expect(ctrl.auth.authenticate.calledWith('demo@email.com', 'password')) + .to.be.true; + expect(ctrl.$state.go.calledWith('home')).to.be.true; + })); + + }); + + describe('on error', () => { + + beforeEach(inject(($q) => { + ctrl.auth.authenticate.returns($q.reject()); + })); + + it('does not redirect', inject(($rootScope) => { + ctrl.login(); + $rootScope.$digest(); + + expect(ctrl.$state.go.calledWith('home')).to.be.false; + })); + + }); + + }); + + }); + +}); diff --git a/src/app/authentication/login/login.state.html b/src/app/authentication/login/login.state.html new file mode 100644 index 0000000..136094e --- /dev/null +++ b/src/app/authentication/login/login.state.html @@ -0,0 +1,27 @@ +
+
+
+

+ Please login to the demo app +

+ +
+
+ + +
+ +
+ + +
+ + +
+
+
+
diff --git a/src/app/authentication/login/login.state.js b/src/app/authentication/login/login.state.js new file mode 100644 index 0000000..00466f5 --- /dev/null +++ b/src/app/authentication/login/login.state.js @@ -0,0 +1,12 @@ +import controller from './login.controller'; +import template from './login.state.html'; + +export const name = 'login'; + +export default { + name, + url: '/login', + template, + controller, + controllerAs: 'ctrl' +}; diff --git a/src/app/authentication/login/login.state.spec.js b/src/app/authentication/login/login.state.spec.js new file mode 100644 index 0000000..e7f2441 --- /dev/null +++ b/src/app/authentication/login/login.state.spec.js @@ -0,0 +1,25 @@ +import appAuthenticationModule from '../authentication.module'; +import { expect } from 'chai'; +import { name } from './login.state'; + +describe(`module: ${appAuthenticationModule}`, () => { + + beforeEach(() => { + angular.mock.module(appAuthenticationModule); + }); + + describe(`state: ${name}`, () => { + + let state; + + beforeEach(inject(($state) => { + state = $state.get(name); + })); + + it('has valid url', inject(($state) => { + expect($state.href(state)).to.eq('#/login'); + })); + + }); + +}); diff --git a/src/app/commons/components/navigation/navigation.component.html b/src/app/commons/components/navigation/navigation.component.html index 846edd0..53d15d7 100644 --- a/src/app/commons/components/navigation/navigation.component.html +++ b/src/app/commons/components/navigation/navigation.component.html @@ -20,7 +20,7 @@ diff --git a/src/app/commons/components/navigation/navigation.component.js b/src/app/commons/components/navigation/navigation.component.js index 1252572..c3f005b 100644 --- a/src/app/commons/components/navigation/navigation.component.js +++ b/src/app/commons/components/navigation/navigation.component.js @@ -2,17 +2,16 @@ import template from './navigation.component.html'; class Controller { - constructor(auth) { + constructor($state, auth) { 'ngInject'; - this.auth = auth; - } - isAuthenticated() { - return this.auth.isAuthenticated(); + this.$state = $state; + this.auth = auth; } logout() { this.auth.logout(); + this.$state.go('login'); } } diff --git a/src/app/commons/config/http-interceptors/unauthorized-access-interceptor.js b/src/app/commons/config/http-interceptors/unauthorized-access-interceptor.js index 6c6da67..eaa4a09 100644 --- a/src/app/commons/config/http-interceptors/unauthorized-access-interceptor.js +++ b/src/app/commons/config/http-interceptors/unauthorized-access-interceptor.js @@ -3,7 +3,7 @@ export default function($httpProvider) { const HTTP_UNAUTHORIZED = 401; - $httpProvider.interceptors.push(($injector, $q, session) => { + $httpProvider.interceptors.push(($injector, $q, $state, session) => { return { responseError(rejection) { if (rejection.status === HTTP_UNAUTHORIZED) { @@ -12,6 +12,7 @@ export default function($httpProvider) { toastr.error('Unauthorized'); session.removeToken(); + $state.go('login'); } return $q.reject(rejection); diff --git a/src/app/contacts/contacts.config.js b/src/app/contacts/contacts.config.js index cd860b2..79c29e4 100644 --- a/src/app/contacts/contacts.config.js +++ b/src/app/contacts/contacts.config.js @@ -12,6 +12,7 @@ export function states($stateProvider) { $stateProvider .state({ + parent: 'app', name: 'contacts', abstract: true, url: '/contacts', diff --git a/src/app/home/index/index.state.js b/src/app/home/index/index.state.js index eac659c..c679b4c 100644 --- a/src/app/home/index/index.state.js +++ b/src/app/home/index/index.state.js @@ -4,6 +4,8 @@ import template from './index.state.html'; export const name = 'home'; export default { + parent: 'app', + name, url: '/', template, diff --git a/src/index.html b/src/index.html index 68323c0..173f5da 100644 --- a/src/index.html +++ b/src/index.html @@ -2,8 +2,8 @@ - - + + <body> - <app-navigation></app-navigation> - - <div class="container"> - <div ncy-breadcrumb></div> - <div ui-view autoscroll="true"></div> - </div> - - <app-footer></app-footer> +<div ui-view autoscroll="true"></div> </body> </html> From 996f41df2e69ff145edcba48d901376f0b2d1717 Mon Sep 17 00:00:00 2001 From: Lukasz Bandzarewicz <lucassus@gmail.com> Date: Thu, 27 Oct 2016 18:45:53 +0200 Subject: [PATCH 13/15] Bump dependencies --- package.json | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/package.json b/package.json index 8caa737..225dbf3 100644 --- a/package.json +++ b/package.json @@ -18,7 +18,7 @@ "devDependencies": { "angular-mocks": "^1.5.8", "babel-core": "^6.18.0", - "babel-loader": "^6.2.5", + "babel-loader": "^6.2.7", "babel-plugin-istanbul": "^2.0.3", "babel-preset-es2015": "^6.18.0", "chai": "^3.5.0", @@ -63,7 +63,7 @@ "nodemon": "^1.11.0", "null-loader": "^0.1.1", "progress-bar-webpack-plugin": "^1.9.0", - "protractor": "^4.0.9", + "protractor": "^4.0.10", "request": "^2.76.0", "run-sequence": "^1.2.2", "sass-loader": "^4.0.2", @@ -71,8 +71,8 @@ "style-loader": "^0.13.1", "supertest": "^2.0.1", "url-loader": "^0.5.7", - "webdriver-manager": "^10.2.5", - "webpack": "^1.13.2", + "webdriver-manager": "^10.2.6", + "webpack": "^1.13.3", "webpack-combine-loaders": "^2.0.0", "webpack-dev-server": "^1.16.2" }, From fad81cf1a9d72865cff5270c9df2e2ab5b6de4ec Mon Sep 17 00:00:00 2001 From: Lukasz Bandzarewicz <lucassus@gmail.com> Date: Thu, 27 Oct 2016 19:15:47 +0200 Subject: [PATCH 14/15] Use `$transitions` to protect states --- src/app/app.module.js | 16 ++++++++++++++++ src/app/authentication/authentication.module.js | 10 ++++++++++ src/app/commons/app/app.state.js | 14 +++----------- 3 files changed, 29 insertions(+), 11 deletions(-) diff --git a/src/app/app.module.js b/src/app/app.module.js index cf86e7c..7cc980f 100644 --- a/src/app/app.module.js +++ b/src/app/app.module.js @@ -1,4 +1,5 @@ import { anchorScroll, html5Mode, notFoundState, router } from './app.config'; +import _ from 'lodash'; import angularAnimate from 'angular-animate'; import angularLoadingBar from 'angular-loading-bar'; import appAboutModule from './about/about.module'; @@ -38,6 +39,21 @@ export default angular.module('app', [ .config(router) .config(anchorScroll) .config(notFoundState) + .run(stateErrorsHandler) .run(logBuildSignature) + + // TODO figure out how to test it + .run(($state, $transitions, auth) => { + const to = (state) => { + return _.get(state, 'data.requiresAuthentication'); + }; + + $transitions.onBefore({ to }, () => { + if (!auth.isAuthenticated()) { + return $state.target('login'); + } + }); + }) + .name; diff --git a/src/app/authentication/authentication.module.js b/src/app/authentication/authentication.module.js index 542f395..f7dbe06 100644 --- a/src/app/authentication/authentication.module.js +++ b/src/app/authentication/authentication.module.js @@ -7,4 +7,14 @@ export default angular.module('app.authentication', [ appCommonsModule ]) .config(states) + + // TODO write specs + .run(($state, $transitions, auth) => { + $transitions.onBefore({ to: 'login' }, () => { + if (auth.isAuthenticated()) { + return $state.target('home'); + } + }); + }) + .name; diff --git a/src/app/commons/app/app.state.js b/src/app/commons/app/app.state.js index 02ba936..77a386d 100644 --- a/src/app/commons/app/app.state.js +++ b/src/app/commons/app/app.state.js @@ -2,21 +2,13 @@ import template from './app.state.html'; export const name = 'app'; +// TODO find better place / solution / research 1.0.0 API export default { name, template, abstract: true, - // TODO does not work on non reload - resolve: { - isAuthenticated: ($state, auth) => { - 'ngInject'; - - if (auth.isAuthenticated()) { - return true; - } else { - return $state.go('login'); - } - } + data: { + requiresAuthentication: true } }; From 8e69645fe0a2ebde2224bf7069acf0ad281ed4c2 Mon Sep 17 00:00:00 2001 From: Lukasz Bandzarewicz <lucassus@gmail.com> Date: Mon, 31 Oct 2016 14:27:27 +0100 Subject: [PATCH 15/15] Better protected states --- src/app/about/index/index.state.spec.js | 4 +- src/app/app.module.js | 17 +++- src/app/app.module.spec.js | 80 ++++++++++++++++++- .../authentication/authentication.module.js | 10 --- .../authentication/login/login.controller.js | 2 +- src/app/authentication/login/login.state.js | 6 +- .../authentication/login/login.state.spec.js | 4 +- src/app/commons/app/app.state.js | 2 +- .../navigation/navigation.component.spec.js | 55 +++++++++++++ .../unique-email.directive.spec.js | 4 +- .../access-token-interceptor.spec.js | 6 +- .../unauthorized-access-interceptor.spec.js | 6 +- .../checkmark/checkmark.filter.spec.js | 4 +- .../services/alert/alert.service.spec.js | 8 +- src/app/commons/services/auth/auth.service.js | 2 +- .../services/auth/auth.service.spec.js | 26 +++--- .../services/confirm/confirm.service.spec.js | 8 +- .../services/session/session.service.spec.js | 6 +- .../contact-form.component.spec.js | 4 +- .../favourite-button.component.spec.js | 4 +- src/app/contacts/list/list.controller.spec.js | 4 +- src/app/contacts/list/list.state.spec.js | 4 +- src/app/contacts/new/new.state.spec.js | 4 +- .../one/address/address.controller.spec.js | 4 +- .../one/address/edit/edit.state.spec.js | 4 +- .../one/address/show/show.state.spec.js | 4 +- src/app/contacts/one/edit/edit.state.spec.js | 4 +- src/app/contacts/one/one.state.spec.js | 4 +- src/app/contacts/one/show/show.state.spec.js | 4 +- .../services/contact/contact.factory.spec.js | 4 +- src/app/home/index/index.controller.spec.js | 4 +- src/app/home/index/index.state.spec.js | 4 +- 32 files changed, 199 insertions(+), 107 deletions(-) create mode 100644 src/app/commons/components/navigation/navigation.component.spec.js diff --git a/src/app/about/index/index.state.spec.js b/src/app/about/index/index.state.spec.js index 556e480..e248d32 100644 --- a/src/app/about/index/index.state.spec.js +++ b/src/app/about/index/index.state.spec.js @@ -4,9 +4,7 @@ import { name } from './index.state'; describe(`module: ${appAboutModule}`, () => { - beforeEach(() => { - angular.mock.module(appAboutModule); - }); + beforeEach(angular.mock.module(appAboutModule)); describe(`state: ${name}`, () => { diff --git a/src/app/app.module.js b/src/app/app.module.js index 7cc980f..3b51186 100644 --- a/src/app/app.module.js +++ b/src/app/app.module.js @@ -43,13 +43,22 @@ export default angular.module('app', [ .run(stateErrorsHandler) .run(logBuildSignature) - // TODO figure out how to test it .run(($state, $transitions, auth) => { - const to = (state) => { - return _.get(state, 'data.requiresAuthentication'); + const publicState = (state) => { + return _.get(state, 'data.publicState'); }; - $transitions.onBefore({ to }, () => { + const nonPublicState = (state) => { + return !_.get(state, 'data.publicState'); + }; + + $transitions.onBefore({ to: publicState }, () => { + if (auth.isAuthenticated()) { + return $state.target('home'); + } + }); + + $transitions.onBefore({ to: nonPublicState }, () => { if (!auth.isAuthenticated()) { return $state.target('login'); } diff --git a/src/app/app.module.spec.js b/src/app/app.module.spec.js index 29c4e4b..ccdb9ce 100644 --- a/src/app/app.module.spec.js +++ b/src/app/app.module.spec.js @@ -4,12 +4,18 @@ import sinon from 'sinon'; describe(`module: ${appModule}`, () => { - beforeEach(() => { - angular.mock.module(appModule); - }); + beforeEach(angular.mock.module(appModule, ($provide) => { + $provide.value('auth', { + isAuthenticated: sinon.stub() + }); + })); describe('navigating to unknown url', () => { + beforeEach(inject((auth) => { + auth.isAuthenticated.returns(true); + })); + it('changes the state to `404`', inject(($location, $rootScope, $state) => { $location.url('/unknown/url'); $rootScope.$digest(); @@ -38,4 +44,72 @@ describe(`module: ${appModule}`, () => { }); + describe('$transitions', () => { + + describe('to public state', () => { + + describe('when authenticated', () => { + + beforeEach(inject((auth) => { + auth.isAuthenticated.returns(true); + })); + + it('redirects to `home`', inject(($rootScope, $state) => { + $state.go('login'); + $rootScope.$digest(); + expect($state.current.name).to.eq('home'); + })); + + }); + + describe('when not authenticated', () => { + + beforeEach(inject((auth) => { + auth.isAuthenticated.returns(false); + })); + + it('does nothing', inject(($rootScope, $state) => { + $state.go('login'); + $rootScope.$digest(); + expect($state.current.name).to.eq('login'); + })); + + }); + + }); + + describe('to the protected state', () => { + + describe('when authenticated', () => { + + beforeEach(inject((auth) => { + auth.isAuthenticated.returns(true); + })); + + it('does nothing', inject(($rootScope, $state) => { + $state.go('home'); + $rootScope.$digest(); + expect($state.current.name).to.eq('home'); + })); + + }); + + describe('when not authenticated', () => { + + beforeEach(inject((auth) => { + auth.isAuthenticated.returns(false); + })); + + it('redirects to `login`', inject(($rootScope, $state) => { + $state.go('home'); + $rootScope.$digest(); + expect($state.current.name).to.eq('login'); + })); + + }); + + }); + + }); + }); diff --git a/src/app/authentication/authentication.module.js b/src/app/authentication/authentication.module.js index f7dbe06..542f395 100644 --- a/src/app/authentication/authentication.module.js +++ b/src/app/authentication/authentication.module.js @@ -7,14 +7,4 @@ export default angular.module('app.authentication', [ appCommonsModule ]) .config(states) - - // TODO write specs - .run(($state, $transitions, auth) => { - $transitions.onBefore({ to: 'login' }, () => { - if (auth.isAuthenticated()) { - return $state.target('home'); - } - }); - }) - .name; diff --git a/src/app/authentication/login/login.controller.js b/src/app/authentication/login/login.controller.js index 90b802d..41536d2 100644 --- a/src/app/authentication/login/login.controller.js +++ b/src/app/authentication/login/login.controller.js @@ -13,7 +13,7 @@ export default class { } login() { - return this.auth.authenticate('demo@email.com', 'password').then(() => { + return this.auth.authenticate(this.credentials).then(() => { return this.$state.go('home'); }); } diff --git a/src/app/authentication/login/login.state.js b/src/app/authentication/login/login.state.js index 00466f5..6ca1bee 100644 --- a/src/app/authentication/login/login.state.js +++ b/src/app/authentication/login/login.state.js @@ -8,5 +8,9 @@ export default { url: '/login', template, controller, - controllerAs: 'ctrl' + controllerAs: 'ctrl', + + data: { + publicState: true + } }; diff --git a/src/app/authentication/login/login.state.spec.js b/src/app/authentication/login/login.state.spec.js index e7f2441..8862178 100644 --- a/src/app/authentication/login/login.state.spec.js +++ b/src/app/authentication/login/login.state.spec.js @@ -4,9 +4,7 @@ import { name } from './login.state'; describe(`module: ${appAuthenticationModule}`, () => { - beforeEach(() => { - angular.mock.module(appAuthenticationModule); - }); + beforeEach(angular.mock.module(appAuthenticationModule)); describe(`state: ${name}`, () => { diff --git a/src/app/commons/app/app.state.js b/src/app/commons/app/app.state.js index 77a386d..db6560f 100644 --- a/src/app/commons/app/app.state.js +++ b/src/app/commons/app/app.state.js @@ -9,6 +9,6 @@ export default { abstract: true, data: { - requiresAuthentication: true + publicState: false } }; diff --git a/src/app/commons/components/navigation/navigation.component.spec.js b/src/app/commons/components/navigation/navigation.component.spec.js new file mode 100644 index 0000000..6b31594 --- /dev/null +++ b/src/app/commons/components/navigation/navigation.component.spec.js @@ -0,0 +1,55 @@ +import appCommonsModule from '../../commons.module'; +import { expect } from 'chai'; +import sinon from 'sinon'; + +describe(`module: ${appCommonsModule}`, () => { + + beforeEach(angular.mock.module(appCommonsModule)); + + describe('component: favouriteButton', () => { + + let element, scope; + + beforeEach(inject(($compile, $rootScope) => { + scope = $rootScope.$new(); + + element = angular.element(` + <app-navigation></app-navigation> + `); + + $compile(element)(scope); + $rootScope.$digest(); + })); + + }); + + describe('controller: appNavigation', () => { + + let ctrl; + + beforeEach(inject(($componentController) => { + ctrl = $componentController('appNavigation', { + $state: { go: sinon.stub() }, + auth: { logout: sinon.stub() } + }); + })); + + describe('.logout', () => { + + beforeEach(() => { + ctrl.logout(); + }); + + it('log out a user', () => { + expect(ctrl.auth.logout.called).to.be.true; + }); + + it('redirects to the `login` page', () => { + expect(ctrl.$state.go.calledWith('login')).to.be.true; + }); + + }); + + }); + +}); diff --git a/src/app/commons/components/unique-email/unique-email.directive.spec.js b/src/app/commons/components/unique-email/unique-email.directive.spec.js index 1905945..2246a0f 100644 --- a/src/app/commons/components/unique-email/unique-email.directive.spec.js +++ b/src/app/commons/components/unique-email/unique-email.directive.spec.js @@ -3,9 +3,7 @@ import { expect } from 'chai'; describe(`module: ${appCommonsModule}`, () => { - beforeEach(() => { - angular.mock.module(appCommonsModule); - }); + beforeEach(angular.mock.module(appCommonsModule)); describe('directive: appUniqueEmail', () => { diff --git a/src/app/commons/config/http-interceptors/access-token-interceptor.spec.js b/src/app/commons/config/http-interceptors/access-token-interceptor.spec.js index 7a58f2a..d17a90f 100644 --- a/src/app/commons/config/http-interceptors/access-token-interceptor.spec.js +++ b/src/app/commons/config/http-interceptors/access-token-interceptor.spec.js @@ -1,10 +1,10 @@ -import commonsModule from '../../commons.module'; +import appCommonsModule from '../../commons.module'; import { expect } from 'chai'; import sinon from 'sinon'; -describe(`module: ${commonsModule}`, () => { +describe(`module: ${appCommonsModule}`, () => { - beforeEach(angular.mock.module(commonsModule, ($provide) => { + beforeEach(angular.mock.module(appCommonsModule, ($provide) => { $provide.value('session', { getToken: sinon.stub(), removeToken: sinon.stub() diff --git a/src/app/commons/config/http-interceptors/unauthorized-access-interceptor.spec.js b/src/app/commons/config/http-interceptors/unauthorized-access-interceptor.spec.js index 01f2d1e..56fc7ae 100644 --- a/src/app/commons/config/http-interceptors/unauthorized-access-interceptor.spec.js +++ b/src/app/commons/config/http-interceptors/unauthorized-access-interceptor.spec.js @@ -1,10 +1,10 @@ -import commonsModule from '../../commons.module'; +import appCommonsModule from '../../commons.module'; import { expect } from 'chai'; import sinon from 'sinon'; -describe(`module: ${commonsModule}`, () => { +describe(`module: ${appCommonsModule}`, () => { - beforeEach(angular.mock.module(commonsModule, ($provide) => { + beforeEach(angular.mock.module(appCommonsModule, ($provide) => { $provide.value('session', { getToken: sinon.stub(), removeToken: sinon.stub() diff --git a/src/app/commons/filters/checkmark/checkmark.filter.spec.js b/src/app/commons/filters/checkmark/checkmark.filter.spec.js index 188efb6..7934741 100644 --- a/src/app/commons/filters/checkmark/checkmark.filter.spec.js +++ b/src/app/commons/filters/checkmark/checkmark.filter.spec.js @@ -3,9 +3,7 @@ import { expect } from 'chai'; describe(`module: ${appCommonsModule}`, () => { - beforeEach(() => { - angular.mock.module(appCommonsModule); - }); + beforeEach(angular.mock.module(appCommonsModule)); describe('filter: appCheckmark', () => { diff --git a/src/app/commons/services/alert/alert.service.spec.js b/src/app/commons/services/alert/alert.service.spec.js index 78ab875..451dd2b 100644 --- a/src/app/commons/services/alert/alert.service.spec.js +++ b/src/app/commons/services/alert/alert.service.spec.js @@ -4,11 +4,9 @@ import sinon from 'sinon'; describe(`module: ${appCommonsModule}`, () => { - beforeEach(() => { - angular.mock.module(appCommonsModule, ($provide) => { - $provide.value('$window', { alert: sinon.stub() }); - }); - }); + beforeEach(angular.mock.module(appCommonsModule, ($provide) => { + $provide.value('$window', { alert: sinon.stub() }); + })); describe('service: alert', () => { diff --git a/src/app/commons/services/auth/auth.service.js b/src/app/commons/services/auth/auth.service.js index 5e87436..2f9e417 100644 --- a/src/app/commons/services/auth/auth.service.js +++ b/src/app/commons/services/auth/auth.service.js @@ -7,7 +7,7 @@ export default function($http, session) { return Boolean(session.getToken()); }, - authenticate(email, password) { + authenticate({ email, password }) { return $http.post('/api/authentication', { email, password }).then((response) => { const { token } = response.data; session.setToken(token); diff --git a/src/app/commons/services/auth/auth.service.spec.js b/src/app/commons/services/auth/auth.service.spec.js index 58a2789..a9de4e3 100644 --- a/src/app/commons/services/auth/auth.service.spec.js +++ b/src/app/commons/services/auth/auth.service.spec.js @@ -4,14 +4,12 @@ import sinon from 'sinon'; describe(`module: ${appCommonsModule}`, () => { - beforeEach(() => { - angular.mock.module(appCommonsModule, ($provide) => { - $provide.decorator('session', ($delegate) => { - 'ngInject'; - return sinon.stub($delegate); - }); + beforeEach(angular.mock.module(appCommonsModule, ($provide) => { + $provide.decorator('session', ($delegate) => { + 'ngInject'; + return sinon.stub($delegate); }); - }); + })); describe('service: auth', () => { @@ -51,17 +49,19 @@ describe(`module: ${appCommonsModule}`, () => { describe('.authenticate', () => { - const email = 'test@email.com'; - const password = 'passowrd'; + const credentials = { + email: 'test@email.com', + password: 'password' + }; let request; beforeEach(inject(($httpBackend) => { - request = $httpBackend.expectPOST('/api/authentication', { email, password }); + request = $httpBackend.expectPOST('/api/authentication', credentials); })); it('returns a promise', () => { - expect(auth.authenticate(email, password)).to.be.a.promise; + expect(auth.authenticate(credentials)).to.be.a.promise; }); describe('on success', () => { @@ -72,7 +72,7 @@ describe(`module: ${appCommonsModule}`, () => { it('stores the token in the session', inject(($httpBackend, session) => { // When - auth.authenticate(email, password); + auth.authenticate(credentials); $httpBackend.flush(); // Then @@ -90,7 +90,7 @@ describe(`module: ${appCommonsModule}`, () => { it('does not store the token in the session', inject(($httpBackend, session) => { // When - auth.authenticate(email, password); + auth.authenticate(credentials); $httpBackend.flush(); // Then diff --git a/src/app/commons/services/confirm/confirm.service.spec.js b/src/app/commons/services/confirm/confirm.service.spec.js index 8b626af..41a2e18 100644 --- a/src/app/commons/services/confirm/confirm.service.spec.js +++ b/src/app/commons/services/confirm/confirm.service.spec.js @@ -4,11 +4,9 @@ import sinon from 'sinon'; describe(`module: ${appCommonsModule}`, () => { - beforeEach(() => { - angular.mock.module(appCommonsModule, ($provide) => { - $provide.value('$window', { confirm: sinon.stub() }); - }); - }); + beforeEach(angular.mock.module(appCommonsModule, ($provide) => { + $provide.value('$window', { confirm: sinon.stub() }); + })); describe('service: confirm', () => { diff --git a/src/app/commons/services/session/session.service.spec.js b/src/app/commons/services/session/session.service.spec.js index d4f8f11..350a0ad 100644 --- a/src/app/commons/services/session/session.service.spec.js +++ b/src/app/commons/services/session/session.service.spec.js @@ -1,10 +1,10 @@ -import commonsModule from '../../commons.module'; +import appCommonsModule from '../../commons.module'; import { expect } from 'chai'; import sinon from 'sinon'; -describe(`module: ${commonsModule}`, () => { +describe(`module: ${appCommonsModule}`, () => { - beforeEach(angular.mock.module(commonsModule)); + beforeEach(angular.mock.module(appCommonsModule)); describe('service: session', () => { diff --git a/src/app/contacts/components/contact-form/contact-form.component.spec.js b/src/app/contacts/components/contact-form/contact-form.component.spec.js index 60795fd..fce96c0 100644 --- a/src/app/contacts/components/contact-form/contact-form.component.spec.js +++ b/src/app/contacts/components/contact-form/contact-form.component.spec.js @@ -4,9 +4,7 @@ import sinon from 'sinon'; describe(`module: ${appContactsModule}`, () => { - beforeEach(() => { - angular.mock.module(appContactsModule); - }); + beforeEach(angular.mock.module(appContactsModule)); describe('component: contactForm', () => { diff --git a/src/app/contacts/components/favourite-button/favourite-button.component.spec.js b/src/app/contacts/components/favourite-button/favourite-button.component.spec.js index e6bbb45..8c6bc9e 100644 --- a/src/app/contacts/components/favourite-button/favourite-button.component.spec.js +++ b/src/app/contacts/components/favourite-button/favourite-button.component.spec.js @@ -4,9 +4,7 @@ import sinon from 'sinon'; describe(`module: ${appContactsModule}`, () => { - beforeEach(() => { - angular.mock.module(appContactsModule); - }); + beforeEach(angular.mock.module(appContactsModule)); describe('component: favouriteButton', () => { diff --git a/src/app/contacts/list/list.controller.spec.js b/src/app/contacts/list/list.controller.spec.js index f8c1258..b4aee9d 100644 --- a/src/app/contacts/list/list.controller.spec.js +++ b/src/app/contacts/list/list.controller.spec.js @@ -4,9 +4,7 @@ import { name } from './list.state'; describe(`module: ${appContactsModule}`, () => { - beforeEach(() => { - angular.mock.module(appContactsModule); - }); + beforeEach(angular.mock.module(appContactsModule)); describe(`controller: ${name}`, () => { diff --git a/src/app/contacts/list/list.state.spec.js b/src/app/contacts/list/list.state.spec.js index 091c00e..30adfdf 100644 --- a/src/app/contacts/list/list.state.spec.js +++ b/src/app/contacts/list/list.state.spec.js @@ -5,9 +5,7 @@ import sinon from 'sinon'; describe(`module: ${appContactsModule}`, () => { - beforeEach(() => { - angular.mock.module(appContactsModule); - }); + beforeEach(angular.mock.module(appContactsModule)); describe(`state: ${name}`, () => { diff --git a/src/app/contacts/new/new.state.spec.js b/src/app/contacts/new/new.state.spec.js index 5da3792..06fff73 100644 --- a/src/app/contacts/new/new.state.spec.js +++ b/src/app/contacts/new/new.state.spec.js @@ -4,9 +4,7 @@ import { name } from './new.state'; describe(`module: ${appContactsModule}`, () => { - beforeEach(() => { - angular.mock.module(appContactsModule); - }); + beforeEach(angular.mock.module(appContactsModule)); describe(`state: ${name}`, () => { diff --git a/src/app/contacts/one/address/address.controller.spec.js b/src/app/contacts/one/address/address.controller.spec.js index 58b7e2d..e6df079 100644 --- a/src/app/contacts/one/address/address.controller.spec.js +++ b/src/app/contacts/one/address/address.controller.spec.js @@ -4,9 +4,7 @@ import { name } from './address.state'; describe(`module: ${appContactsModule}`, () => { - beforeEach(() => { - angular.mock.module(appContactsModule); - }); + beforeEach(angular.mock.module(appContactsModule)); describe(`controller: ${name}`, () => { diff --git a/src/app/contacts/one/address/edit/edit.state.spec.js b/src/app/contacts/one/address/edit/edit.state.spec.js index f0c5ec7..0ebe98f 100644 --- a/src/app/contacts/one/address/edit/edit.state.spec.js +++ b/src/app/contacts/one/address/edit/edit.state.spec.js @@ -4,9 +4,7 @@ import { name } from './edit.state'; describe(`module: ${appContactsModule}`, () => { - beforeEach(() => { - angular.mock.module(appContactsModule); - }); + beforeEach(angular.mock.module(appContactsModule)); describe(`state: ${name}`, () => { diff --git a/src/app/contacts/one/address/show/show.state.spec.js b/src/app/contacts/one/address/show/show.state.spec.js index db949ae..02951d0 100644 --- a/src/app/contacts/one/address/show/show.state.spec.js +++ b/src/app/contacts/one/address/show/show.state.spec.js @@ -4,9 +4,7 @@ import { name } from './show.state'; describe(`module: ${appContactsModule}`, () => { - beforeEach(() => { - angular.mock.module(appContactsModule); - }); + beforeEach(angular.mock.module(appContactsModule)); describe(`state: ${name}`, () => { diff --git a/src/app/contacts/one/edit/edit.state.spec.js b/src/app/contacts/one/edit/edit.state.spec.js index 147664e..f8f830b 100644 --- a/src/app/contacts/one/edit/edit.state.spec.js +++ b/src/app/contacts/one/edit/edit.state.spec.js @@ -4,9 +4,7 @@ import { name } from './edit.state'; describe(`module: ${appContactsModule}`, () => { - beforeEach(() => { - angular.mock.module(appContactsModule); - }); + beforeEach(angular.mock.module(appContactsModule)); describe(`state: ${name}`, () => { diff --git a/src/app/contacts/one/one.state.spec.js b/src/app/contacts/one/one.state.spec.js index 2d4cc9f..8624a82 100644 --- a/src/app/contacts/one/one.state.spec.js +++ b/src/app/contacts/one/one.state.spec.js @@ -5,9 +5,7 @@ import sinon from 'sinon'; describe(`module: ${appContactsModule}`, () => { - beforeEach(() => { - angular.mock.module(appContactsModule); - }); + beforeEach(angular.mock.module(appContactsModule)); describe(`state: ${name}`, () => { diff --git a/src/app/contacts/one/show/show.state.spec.js b/src/app/contacts/one/show/show.state.spec.js index d9551d1..2f216ac 100644 --- a/src/app/contacts/one/show/show.state.spec.js +++ b/src/app/contacts/one/show/show.state.spec.js @@ -4,9 +4,7 @@ import { name } from './show.state'; describe(`module: ${appContactsModule}`, () => { - beforeEach(() => { - angular.mock.module(appContactsModule); - }); + beforeEach(angular.mock.module(appContactsModule)); describe(`state: ${name}`, () => { diff --git a/src/app/contacts/services/contact/contact.factory.spec.js b/src/app/contacts/services/contact/contact.factory.spec.js index ca88e6b..6ff9c35 100644 --- a/src/app/contacts/services/contact/contact.factory.spec.js +++ b/src/app/contacts/services/contact/contact.factory.spec.js @@ -3,9 +3,7 @@ import { expect } from 'chai'; describe(`module: ${appContactsModule}`, () => { - beforeEach(() => { - angular.mock.module(appContactsModule); - }); + beforeEach(angular.mock.module(appContactsModule)); describe('resource: Contact', () => { diff --git a/src/app/home/index/index.controller.spec.js b/src/app/home/index/index.controller.spec.js index fd1fa92..bccda47 100644 --- a/src/app/home/index/index.controller.spec.js +++ b/src/app/home/index/index.controller.spec.js @@ -5,9 +5,7 @@ import sinon from 'sinon'; describe(`module: ${appHomeModule}`, () => { - beforeEach(() => { - angular.mock.module(appHomeModule); - }); + beforeEach(angular.mock.module(appHomeModule)); describe(`controller: ${name}`, () => { diff --git a/src/app/home/index/index.state.spec.js b/src/app/home/index/index.state.spec.js index aebde4d..a18b5f9 100644 --- a/src/app/home/index/index.state.spec.js +++ b/src/app/home/index/index.state.spec.js @@ -4,9 +4,7 @@ import { name } from './index.state'; describe(`module: ${appHomeModule}`, () => { - beforeEach(() => { - angular.mock.module(appHomeModule); - }); + beforeEach(angular.mock.module(appHomeModule)); describe(`state: ${name}`, () => {