From c7062ed3a396db8d68f473de76f6f37dcbb8800e Mon Sep 17 00:00:00 2001 From: Vincenzo Chianese Date: Thu, 26 Oct 2017 09:13:47 +0200 Subject: [PATCH 01/18] Always use the consumer Id when looking for credentials --- lib/rest/routes/credentials.js | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/lib/rest/routes/credentials.js b/lib/rest/routes/credentials.js index fb2f70751..9a3cfb4ed 100644 --- a/lib/rest/routes/credentials.js +++ b/lib/rest/routes/credentials.js @@ -13,16 +13,11 @@ module.exports = function () { return res.status(422).json(new Error('Consumer Not Found: id:' + req.body.consumerId)); } - return credentialSrv.insertCredential(req.body.consumerId, req.body.type, req.body.credential) - .then(data => { - res.json(data); - }); + return credentialSrv + .insertCredential(consumer.id, req.body.type, req.body.credential) + .then((data) => res.json(data)); }) - .catch(err => { - if (!res.headersSent) { // no need to send error if 422 sent - next(err); - } - }); + .catch(next); }); router.put('/:type/:id/status', function (req, res, next) { @@ -83,10 +78,17 @@ module.exports = function () { }); router.get('/:consumerId', function (req, res, next) { - credentialSrv - .getCredentials(req.params.consumerId) - .then(credentials => res.json({ credentials })) - .catch(next); + findConsumer(req.params.consumerId) + .then((consumer) => { + if (!consumer) { + return res.status(422).json(new Error('Consumer Not Found: id:' + req.body.consumerId)); + } + + return credentialSrv + .getCredentials(consumer.id) + .then(credentials => res.json({ credentials })) + .catch(next); + }); }); return router; From 961a66f1e24a132844f97a60a38c78b581dd3576 Mon Sep 17 00:00:00 2001 From: Vincenzo Chianese Date: Thu, 26 Oct 2017 09:45:42 +0200 Subject: [PATCH 02/18] jsonl->json --- bin/generators/apps/create.js | 48 ++++++++++++++-------------- bin/generators/credentials/create.js | 38 +++++++++++----------- bin/generators/users/create.js | 20 ++++++------ 3 files changed, 53 insertions(+), 53 deletions(-) diff --git a/bin/generators/apps/create.js b/bin/generators/apps/create.js index f9d9b797a..58fad33b9 100644 --- a/bin/generators/apps/create.js +++ b/bin/generators/apps/create.js @@ -14,10 +14,10 @@ module.exports = class extends eg.Generator { .usage(`Usage: $0 ${process.argv[2]} create [options]`) .example(`$0 ${process.argv[2]} create -u jdoe`) .example(`$0 ${process.argv[2]} create -u jdoe -p 'name=mobile-app' ` + - '-p \'redirectUri=http://localhost/cb\'') + '-p \'redirectUri=http://localhost/cb\'') .example('echo \'{"user":"jdoe","name":"mobile-app"}\'' + - ` | $0 ${process.argv[2]} create --stdin`) - .example(`cat all_apps.jsonl | $0 ${process.argv[2]} create --stdin`) + ` | $0 ${process.argv[2]} create --stdin`) + .example(`cat all_apps.json | $0 ${process.argv[2]} create --stdin`) .string(['p', 'u']) .boolean(['stdin']) .describe('u', 'User ID or username associated with the app') @@ -79,17 +79,17 @@ module.exports = class extends eg.Generator { } return this._insert(app, { user: argv.user }) - .then(newApp => { - if (!argv.q) { - this.log.ok(`Created ${newApp.id}`); - this.stdout(JSON.stringify(newApp, null, 2)); - } else { - this.stdout(newApp.id); - } - }) - .catch(err => { - this.log.error((err.response && err.response.error && err.response.error.text) || err.message); - }); + .then(newApp => { + if (!argv.q) { + this.log.ok(`Created ${newApp.id}`); + this.stdout(JSON.stringify(newApp, null, 2)); + } else { + this.stdout(newApp.id); + } + }) + .catch(err => { + this.log.error((err.response && err.response.error && err.response.error.text) || err.message); + }); }; _createFromStdin () { @@ -136,9 +136,9 @@ module.exports = class extends eg.Generator { } } }) - .catch(err => { - this.log.error((err.response && err.response.error && err.response.error.text) || err.message); - }); + .catch(err => { + this.log.error((err.response && err.response.error && err.response.error.text) || err.message); + }); }); const p = Promise.all(promises); @@ -175,24 +175,24 @@ module.exports = class extends eg.Generator { if (shouldPrompt) { questions = missingProperties.map(p => { const required = p.descriptor.isRequired - ? ' [required]' - : ''; + ? ' [required]' + : ''; return { name: p.name, message: `Enter ${chalk.yellow(p.name)}${chalk.green(required)}:`, default: p.defaultValue, validate: input => !p.descriptor.isRequired || - (!!input && p.descriptor.isRequired) + (!!input && p.descriptor.isRequired) }; }); } } return this.prompt(questions) - .then(answers => { - app = Object.assign(app, answers); - return this.admin.apps.create(options.user, app); - }); + .then(answers => { + app = Object.assign(app, answers); + return this.admin.apps.create(options.user, app); + }); }; }; diff --git a/bin/generators/credentials/create.js b/bin/generators/credentials/create.js index a9597e72f..ef90e71a0 100644 --- a/bin/generators/credentials/create.js +++ b/bin/generators/credentials/create.js @@ -14,10 +14,10 @@ module.exports = class extends eg.Generator { .usage(`Usage: $0 ${process.argv[2]} create [options]`) .example(`$0 ${process.argv[2]} create -c jdoe -t key-auth`) .example(`echo '{"consumer":"jdoe", "type": "key-auth"}'` + - `| $0 ${process.argv[2]} create --stdin`) + `| $0 ${process.argv[2]} create --stdin`) .example(`echo '{"consumer":"jdoe", "type": "key-auth", "scopes":["existingScope"]}'` + - `| $0 ${process.argv[2]} create --stdin`) - .example(`cat all_apps.jsonl | $0 ${process.argv[2]} create --stdin`) + `| $0 ${process.argv[2]} create --stdin`) + .example(`cat all_apps.json | $0 ${process.argv[2]} create --stdin`) .example(`$0 ${process.argv[2]} create -u jdoe -p 'scopes=existingScope'`) .string(['p', 'c', 't']) .boolean(['stdin']) @@ -89,12 +89,12 @@ module.exports = class extends eg.Generator { } return this._insert(credential, { consumer: argv.consumer, type: argv.type }) - .then(newCredential => { - this._output(newCredential); - }) - .catch(err => { - this.log.error((err.response && err.response.error && err.response.error.text) || err.message); - }); + .then(newCredential => { + this._output(newCredential); + }) + .catch(err => { + this.log.error((err.response && err.response.error && err.response.error.text) || err.message); + }); }; _createFromStdin () { @@ -143,9 +143,9 @@ module.exports = class extends eg.Generator { .then(newCredential => { this._output(newCredential); }) - .catch(err => { - this.log.error((err.response && err.response.error && err.response.error.text) || err.message); - }); + .catch(err => { + this.log.error((err.response && err.response.error && err.response.error.text) || err.message); + }); }); const p = Promise.all(promises); @@ -196,23 +196,23 @@ module.exports = class extends eg.Generator { if (shouldPrompt) { questions = missingProperties.map(p => { const required = p.descriptor.isRequired - ? ' [required]' - : ''; + ? ' [required]' + : ''; return { name: p.name, message: `Enter ${chalk.yellow(p.name)}${chalk.green(required)}:`, default: p.defaultValue, validate: input => !p.descriptor.isRequired || - (!!input && p.descriptor.isRequired) + (!!input && p.descriptor.isRequired) }; }); } } return this.prompt(questions) - .then(answers => { - credential = Object.assign(credential, answers); - return this.admin.credentials.create(options.consumer, options.type, credential); - }); + .then(answers => { + credential = Object.assign(credential, answers); + return this.admin.credentials.create(options.consumer, options.type, credential); + }); }; }; diff --git a/bin/generators/users/create.js b/bin/generators/users/create.js index b604e2a80..2fa94a74e 100644 --- a/bin/generators/users/create.js +++ b/bin/generators/users/create.js @@ -14,10 +14,10 @@ module.exports = class extends eg.Generator { .usage(`Usage: $0 ${process.argv[2]} create [options]`) .example(`$0 ${process.argv[2]} create`) .example(`$0 ${process.argv[2]} create -p 'username=jdoe' ` + - '-p \'firstname=Jane\' -p \'lastname=Doe\'') + '-p \'firstname=Jane\' -p \'lastname=Doe\'') .example('echo \'{"username":"jdoe","firstname":"Jane"}\'' + - ` | $0 ${process.argv[2]} create --stdin`) - .example(`cat all_users.jsonl | $0 ${process.argv[2]} create --stdin`) + ` | $0 ${process.argv[2]} create --stdin`) + .example(`cat all_users.json | $0 ${process.argv[2]} create --stdin`) .string('p') .boolean(['stdin']) .describe('p', 'User property in the form [-p \'foo=bar\']') @@ -69,9 +69,9 @@ module.exports = class extends eg.Generator { } } }) - .catch(err => { - this.log.error((err.response && err.response.error && err.response.error.text) || err.message); - }); + .catch(err => { + this.log.error((err.response && err.response.error && err.response.error.text) || err.message); + }); }); const p = Promise.all(promises); @@ -136,7 +136,7 @@ module.exports = class extends eg.Generator { const missingProperties = []; const configProperties = Object.assign({ username: { isRequired: true } }, - models.users.properties); + models.users.properties); Object.keys(configProperties).forEach(prop => { const descriptor = configProperties[prop]; @@ -152,15 +152,15 @@ module.exports = class extends eg.Generator { if (shouldPrompt) { questions = missingProperties.map(p => { const required = p.descriptor.isRequired - ? ' [required]' - : ''; + ? ' [required]' + : ''; return { name: p.name, message: `Enter ${chalk.yellow(p.name)}${chalk.green(required)}:`, default: p.defaultValue, validate: input => !p.descriptor.isRequired || - (!!input && p.descriptor.isRequired) + (!!input && p.descriptor.isRequired) }; }); } From 0e954808ad0957bfa5dfec47a571428ba81ca5b2 Mon Sep 17 00:00:00 2001 From: Vincenzo Chianese Date: Thu, 26 Oct 2017 09:54:22 +0200 Subject: [PATCH 03/18] Fix credential creations test --- test/cli/credentials/create.test.js | 52 ++++++++++++++--------------- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/test/cli/credentials/create.test.js b/test/cli/credentials/create.test.js index 60e41be4a..e2ba74f97 100644 --- a/test/cli/credentials/create.test.js +++ b/test/cli/credentials/create.test.js @@ -22,9 +22,9 @@ describe('eg credentials create', () => { firstname: 'La', lastname: 'Deeda' }) - .then(createdUser => { - user = createdUser; - }); + .then(createdUser => { + user = createdUser; + }); }); afterEach(() => { @@ -55,13 +55,13 @@ describe('eg credentials create', () => { assert.equal(text, 'Created ' + loggedCred.keyId); return adminHelper.admin.credentials.info(loggedCred.keyId, 'key-auth') - .then(cred => { - assert.ok(cred.keyId); - assert.ok(cred.keySecret); - assert.ok(cred.isActive); - assert.equal(cred.consumerId, user.username); - done(); - }).catch(done); + .then(cred => { + assert.ok(cred.keyId); + assert.ok(cred.keySecret); + assert.ok(cred.isActive); + assert.equal(cred.consumerId, user.id); + done(); + }).catch(done); }); }); @@ -95,13 +95,13 @@ describe('eg credentials create', () => { const loggedCred = JSON.parse(output); assert.equal(text, 'Created ' + loggedCred.keyId); return adminHelper.admin.credentials.info(loggedCred.keyId, 'key-auth') - .then(cred => { - assert.ok(cred.keyId); - assert.ok(cred.keySecret); - assert.ok(cred.isActive); - assert.equal(cred.consumerId, user.username); - done(); - }).catch(done); + .then(cred => { + assert.ok(cred.keyId); + assert.ok(cred.keySecret); + assert.ok(cred.isActive); + assert.equal(cred.consumerId, user.id); + done(); + }).catch(done); }); }); @@ -124,15 +124,15 @@ describe('eg credentials create', () => { generator.once('end', () => { const loggedCred = output.split(':')[0]; return adminHelper.admin.credentials.info(loggedCred, 'key-auth') - .then(cred => { - assert.ok(cred.keyId); - assert.ok(cred.keySecret); - assert.ok(cred.isActive); - assert.equal(cred.consumerId, user.username); - assert.equal(output, cred.keyId + ':' + cred.keySecret); - - done(); - }).catch(done); + .then(cred => { + assert.ok(cred.keyId); + assert.ok(cred.keySecret); + assert.ok(cred.isActive); + assert.equal(cred.consumerId, user.id); + assert.equal(output, cred.keyId + ':' + cred.keySecret); + + done(); + }).catch(done); }); }); From 734be8df7c28a44762dc9c8c736d5d6382e56132 Mon Sep 17 00:00:00 2001 From: Vincenzo Chianese Date: Thu, 26 Oct 2017 22:03:43 +0200 Subject: [PATCH 04/18] Avoid deprecated send method --- test/e2e/oauth2-authorization-code.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/e2e/oauth2-authorization-code.js b/test/e2e/oauth2-authorization-code.js index 2d47fdc08..4e4b5d75f 100644 --- a/test/e2e/oauth2-authorization-code.js +++ b/test/e2e/oauth2-authorization-code.js @@ -1,5 +1,5 @@ const { fork } = require('child_process'); -const {runCLICommand} = require('../common/cli.helper'); +const { runCLICommand } = require('../common/cli.helper'); const fs = require('fs'); const path = require('path'); const url = require('url'); @@ -197,7 +197,7 @@ describe('oauth2 authorization code grant type', () => { testGatewayConfigData.admin.port = adminPort; testGatewayConfigData.serviceEndpoints.backend.url = - `http://localhost:${backendPort}`; + `http://localhost:${backendPort}`; return generateBackendServer(backendPort); }) @@ -270,7 +270,7 @@ describe('oauth2 authorization code grant type', () => { app.get('/cb', (req, res) => { const parsed = url.parse(req.url, true); redirectParams = parsed.query; - res.send(200); + res.sendStatus(200); }); return new Promise((resolve) => { From 86ccd8b9711231fd1195466ba996df6752e7358a Mon Sep 17 00:00:00 2001 From: Vincenzo Chianese Date: Thu, 26 Oct 2017 22:35:13 +0200 Subject: [PATCH 05/18] Reduce indendation for authenticateCredential function --- lib/services/auth.js | 144 ++++++++++++++++++++++--------------------- 1 file changed, 73 insertions(+), 71 deletions(-) diff --git a/lib/services/auth.js b/lib/services/auth.js index bca4b903c..14fd08e84 100644 --- a/lib/services/auth.js +++ b/lib/services/auth.js @@ -16,35 +16,37 @@ s.authenticateCredential = function (id, password, type) { if (type === 'key-auth') { return credentials.getCredential(id, type, { includePassword: true }) - .then(credential => { - if (!credential || !credential.isActive || credential.keySecret !== password) { - return false; - } - return this.validateConsumer(credential.consumerId, {checkUsername: true}); - }); + .then(credential => { + if (!credential || !credential.isActive || credential.keySecret !== password) { + return false; + } + return this.validateConsumer(credential.consumerId, { checkUsername: true }); + }); } + return this.validateConsumer(id, { checkUsername: true }) - .then((consumer) => { - if (!consumer) { - return false; - } + .then((consumer) => { + if (!consumer) { + return false; + } - return credentials.getCredential(id, type, { includePassword: true }) - .then(credential => { + return Promise.all([consumer, credentials.getCredential(id, type, { includePassword: true })]); + }).then(([consumer, credential]) => { if (!credential || !credential.isActive) { return false; } - return utils.compareSaltAndHashed(password, credential[config.models.credentials[type]['passwordKey']]) - .then(authenticated => { - if (!authenticated) { - return false; - } + return Promise.all([ + consumer, + utils.compareSaltAndHashed(password, credential[config.models.credentials[type]['passwordKey']]) + ]); + }).then(([consumer, authenticated]) => { + if (!authenticated) { + return false; + } - return consumer; - }); + return consumer; }); - }); }; s.authenticateToken = function (token) { @@ -52,20 +54,20 @@ s.authenticateToken = function (token) { const tokenPassword = token.split('|')[1]; return tokens.get(token) - .then(_tokenObj => { - tokenObj = _tokenObj; - - if (!tokenObj) { - return null; - } - - return this.validateConsumer(tokenObj.consumerId); - }) - .then(consumer => { - if (!consumer || !consumer.isActive) { - return false; - } else return tokenObj.tokenDecrypted === tokenPassword ? { token: tokenObj, consumer } : false; - }); + .then(_tokenObj => { + tokenObj = _tokenObj; + + if (!tokenObj) { + return null; + } + + return this.validateConsumer(tokenObj.consumerId); + }) + .then(consumer => { + if (!consumer || !consumer.isActive) { + return false; + } else return tokenObj.tokenDecrypted === tokenPassword ? { token: tokenObj, consumer } : false; + }); }; s.authorizeToken = function (_token, authType, scopes) { @@ -73,20 +75,20 @@ s.authorizeToken = function (_token, authType, scopes) { return Promise.resolve(true); } - scopes = Array.isArray(scopes) ? scopes : [ scopes ]; + scopes = Array.isArray(scopes) ? scopes : [scopes]; return tokens.get(_token) - .then(token => { - if (!token) { - return false; - } + .then(token => { + if (!token) { + return false; + } - if (scopes && scopes.length && !token.scopes) { - return false; - } + if (scopes && scopes.length && !token.scopes) { + return false; + } - return scopes.every(scope => token.scopes.indexOf(scope) !== -1); - }); + return scopes.every(scope => token.scopes.indexOf(scope) !== -1); + }); }; s.authorizeCredential = function (id, authType, scopes) { @@ -95,42 +97,42 @@ s.authorizeCredential = function (id, authType, scopes) { } return credentials.getCredential(id, authType) - .then(credential => { - if (credential) { - if (!credential.scopes) { - return false; + .then(credential => { + if (credential) { + if (!credential.scopes) { + return false; + } + return scopes.every(scope => credential.scopes.indexOf(scope) !== -1); } - return scopes.every(scope => credential.scopes.indexOf(scope) !== -1); - } - }); + }); }; s.validateConsumer = function (id, options = {}) { return applications.get(id) - .then(app => { - if (app && app.isActive) { - return createApplicationObject(app); - } - - return users.get(id) - .then(_user => { - if (_user && _user.isActive) { - return createUserObject(_user); + .then(app => { + if (app && app.isActive) { + return createApplicationObject(app); } - if (options.checkUsername) { - const username = id; - return users.find(username) - .then(user => { - if (user && user.isActive) { - return createUserObject(user); - } else return null; + return users.get(id) + .then(_user => { + if (_user && _user.isActive) { + return createUserObject(_user); + } + + if (options.checkUsername) { + const username = id; + return users.find(username) + .then(user => { + if (user && user.isActive) { + return createUserObject(user); + } else return null; + }); + } + + return null; }); - } - - return null; }); - }); }; function createUserObject (user) { From f293ded1eccd0f155be12d31de4ddf361241a694 Mon Sep 17 00:00:00 2001 From: Vincenzo Chianese Date: Fri, 27 Oct 2017 00:16:02 +0200 Subject: [PATCH 06/18] Make crendential ID = Consumer ID --- lib/policies/basic-auth/auth.js | 3 +-- lib/policies/key-auth/keyauth.js | 2 +- lib/services/auth.js | 4 +--- lib/services/credentials/credential.service.js | 3 +-- 4 files changed, 4 insertions(+), 8 deletions(-) diff --git a/lib/policies/basic-auth/auth.js b/lib/policies/basic-auth/auth.js index ba4154fda..f568ee086 100644 --- a/lib/policies/basic-auth/auth.js +++ b/lib/policies/basic-auth/auth.js @@ -21,13 +21,12 @@ function authenticateBasic (req, clientId, clientSecret, done) { requestedScopes = req.body.scope.split(' '); } } - return authService.authenticateCredential(clientId, clientSecret, credentialType) .then(consumer => { if (!consumer) { return done(null, false); } - return authService.authorizeCredential(clientId, credentialType, endpointScopes || requestedScopes) + return authService.authorizeCredential(consumer.id, credentialType, endpointScopes || requestedScopes) .then(authorized => { if (!authorized) { return done(null, false); diff --git a/lib/policies/key-auth/keyauth.js b/lib/policies/key-auth/keyauth.js index 346aeba48..7fdd946fa 100644 --- a/lib/policies/key-auth/keyauth.js +++ b/lib/policies/key-auth/keyauth.js @@ -4,7 +4,7 @@ const services = require('../../services/index'); const logger = require('../../logger').policy; const authService = services.auth; const credentialType = 'key-auth'; -passport.use(new LocalAPIKeyStrategy({passReqToCallback: true}, (req, apikey, done) => { +passport.use(new LocalAPIKeyStrategy({ passReqToCallback: true }, (req, apikey, done) => { // key will look like "h1243h1kl23h4kjh:asfasqwerqw" if (!apikey) { return done(null, false); diff --git a/lib/services/auth.js b/lib/services/auth.js index 14fd08e84..e4d6a8801 100644 --- a/lib/services/auth.js +++ b/lib/services/auth.js @@ -1,5 +1,3 @@ -'use strict'; - const credentials = require('./credentials/credential.service.js'); const users = require('./consumers/user.service.js'); const applications = require('./consumers/application.service.js'); @@ -30,7 +28,7 @@ s.authenticateCredential = function (id, password, type) { return false; } - return Promise.all([consumer, credentials.getCredential(id, type, { includePassword: true })]); + return Promise.all([consumer, credentials.getCredential(consumer.id, type, { includePassword: true })]); }).then(([consumer, credential]) => { if (!credential || !credential.isActive) { return false; diff --git a/lib/services/credentials/credential.service.js b/lib/services/credentials/credential.service.js index b0fcf3317..b9c1cfaff 100644 --- a/lib/services/credentials/credential.service.js +++ b/lib/services/credentials/credential.service.js @@ -64,13 +64,12 @@ s.insertCredential = function (id, type, credentialDetails) { validateNewCredentialProperties(credentialConfig, credentialDetails) ]).then(([scopes, credentialProps]) => { Object.assign(newCredential, credentialProps); - newCredential.keyId = uuid62.v4(); newCredential.keySecret = uuid62.v4(); newCredential.scopes = JSON.stringify(scopes); newCredential.consumerId = id; return Promise.all([ - credentialDao.insertCredential(newCredential.keyId, type, newCredential), + credentialDao.insertCredential(id, type, newCredential), credentialDao.associateCredentialWithScopes(id, type, scopes) ]); }).then(() => { From 1192496b953507da9374b71e9dc6640d1bc89135 Mon Sep 17 00:00:00 2001 From: Vincenzo Chianese Date: Fri, 27 Oct 2017 09:58:54 +0200 Subject: [PATCH 07/18] keyId is required --- lib/services/credentials/credential.dao.js | 2 +- lib/services/credentials/credential.service.js | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/services/credentials/credential.dao.js b/lib/services/credentials/credential.dao.js index 322bfc96c..138a2a127 100644 --- a/lib/services/credentials/credential.dao.js +++ b/lib/services/credentials/credential.dao.js @@ -118,7 +118,7 @@ dao.insertCredential = function (id, type, credentialObj) { db.saddAsync(buildIdKey(type, credentialObj.consumerId), id), // store key-auth keyid -> credentialObj db.hmsetAsync(buildIdKey(type, id), credentialObj) - ]); + ]).catch((e) => console.log(e)); } return db.hmsetAsync(buildIdKey(type, id), credentialObj); }; diff --git a/lib/services/credentials/credential.service.js b/lib/services/credentials/credential.service.js index b9c1cfaff..d3b04d987 100644 --- a/lib/services/credentials/credential.service.js +++ b/lib/services/credentials/credential.service.js @@ -64,13 +64,14 @@ s.insertCredential = function (id, type, credentialDetails) { validateNewCredentialProperties(credentialConfig, credentialDetails) ]).then(([scopes, credentialProps]) => { Object.assign(newCredential, credentialProps); + newCredential.keyId = uuid62.v4(); newCredential.keySecret = uuid62.v4(); newCredential.scopes = JSON.stringify(scopes); newCredential.consumerId = id; return Promise.all([ - credentialDao.insertCredential(id, type, newCredential), - credentialDao.associateCredentialWithScopes(id, type, scopes) + credentialDao.insertCredential(newCredential.keyId, type, newCredential), + credentialDao.associateCredentialWithScopes(newCredential.keyId, type, scopes) ]); }).then(() => { if (newCredential.scopes && newCredential.scopes.length > 0) { From c7a391981aa3c2127029ce4543b1a61bd0f17d9d Mon Sep 17 00:00:00 2001 From: Vincenzo Chianese Date: Fri, 27 Oct 2017 10:17:50 +0200 Subject: [PATCH 08/18] Promise result might not be an object --- lib/services/auth.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/services/auth.js b/lib/services/auth.js index e4d6a8801..d7cb67c29 100644 --- a/lib/services/auth.js +++ b/lib/services/auth.js @@ -27,7 +27,6 @@ s.authenticateCredential = function (id, password, type) { if (!consumer) { return false; } - return Promise.all([consumer, credentials.getCredential(consumer.id, type, { includePassword: true })]); }).then(([consumer, credential]) => { if (!credential || !credential.isActive) { @@ -38,11 +37,11 @@ s.authenticateCredential = function (id, password, type) { consumer, utils.compareSaltAndHashed(password, credential[config.models.credentials[type]['passwordKey']]) ]); - }).then(([consumer, authenticated]) => { - if (!authenticated) { + }).then((credentialResult) => { + if (!credentialResult) { return false; } - + const [consumer] = credentialResult; return consumer; }); }; From 3b34da1bb6f4d9fed8d0b5ea9c677918568881fd Mon Sep 17 00:00:00 2001 From: Vincenzo Chianese Date: Fri, 27 Oct 2017 10:41:13 +0200 Subject: [PATCH 09/18] Credentials should always go with a consumer id --- lib/policies/oauth2/oauth2-server.js | 156 ++++----- ...q-oauth2-expression-log-ratelimit-proxy.js | 2 +- test/oauth/authorizationcode.test.js | 322 +++++++++--------- test/oauth/implicit.test.js | 278 +++++++-------- test/oauth/password.test.js | 228 ++++++------- test/policies/basic-auth-policy.test.js | 10 +- test/policies/keyauth/keyauth.test.js | 2 +- .../oauth/consumer-and-token-headers.test.js | 14 +- test/services/auth.test.js | 4 +- .../credential-service/credentials.test.js | 10 +- 10 files changed, 512 insertions(+), 514 deletions(-) diff --git a/lib/policies/oauth2/oauth2-server.js b/lib/policies/oauth2/oauth2-server.js index d83c0cba3..26e3be654 100644 --- a/lib/policies/oauth2/oauth2-server.js +++ b/lib/policies/oauth2/oauth2-server.js @@ -32,11 +32,11 @@ server.deserializeClient((consumer, done) => { const id = consumer.id; return authService.validateConsumer(id) - .then(foundConsumer => { - if (!foundConsumer) return done(null, false); - return done(null, consumer); - }) - .catch(err => done(err)); + .then(foundConsumer => { + if (!foundConsumer) return done(null, false); + return done(null, consumer); + }) + .catch(err => done(err)); }); // Register supported grant types. @@ -63,10 +63,10 @@ server.grant(oauth2orize.grant.code((consumer, redirectUri, user, ares, done) => if (consumer.authorizedScopes) code.scopes = consumer.authorizedScopes; return authCodeService.save(code) - .then((codeObj) => { - return done(null, codeObj.id); - }) - .catch(err => done(err)); + .then((codeObj) => { + return done(null, codeObj.id); + }) + .catch(err => done(err)); })); // Grant implicit authorization. The callback takes the `client` requesting @@ -86,10 +86,10 @@ server.grant(oauth2orize.grant.token((consumer, authenticatedUser, ares, done) = if (consumer.authorizedScopes) tokenCriteria.scopes = consumer.authorizedScopes; return tokenService.findOrSave(tokenCriteria) - .then(token => { - return done(null, token.access_token); - }) - .catch(err => done(err)); + .then(token => { + return done(null, token.access_token); + }) + .catch(err => done(err)); })); // Exchange authorization codes for access tokens. The callback accepts the @@ -106,25 +106,25 @@ server.exchange(oauth2orize.exchange.code((consumer, code, redirectUri, done) => }; authCodeService.find(codeCriteria) - .then(codeObj => { - if (!codeObj) { - return done(null, false); - } + .then(codeObj => { + if (!codeObj) { + return done(null, false); + } - const tokenCriteria = { - consumerId: consumer.id, - authenticatedUserId: codeObj.userId, - authType: 'oauth2' - }; + const tokenCriteria = { + consumerId: consumer.id, + authenticatedUserId: codeObj.userId, + authType: 'oauth2' + }; - if (codeObj.scopes) tokenCriteria.scopes = codeObj.scopes; + if (codeObj.scopes) tokenCriteria.scopes = codeObj.scopes; - return tokenService.findOrSave(tokenCriteria, { includeRefreshToken: true }) - .then(token => { - return done(null, token.access_token, token.refresh_token); - }); - }) - .catch(err => done(err)); + return tokenService.findOrSave(tokenCriteria, { includeRefreshToken: true }) + .then(token => { + return done(null, token.access_token, token.refresh_token); + }); + }) + .catch(err => done(err)); })); // Exchange user id and password for access tokens. The callback accepts the @@ -135,38 +135,38 @@ server.exchange(oauth2orize.exchange.code((consumer, code, redirectUri, done) => server.exchange(oauth2orize.exchange.password((consumer, username, password, scopes, done) => { // Validate the consumer return authService.validateConsumer(consumer.id) - .then(consumer => { - if (!consumer) return done(null, false); + .then(consumer => { + if (!consumer) return done(null, false); - return authService.authenticateCredential(username, password, 'oauth2') - .then(user => { - let scopeAuthorizationPromise; + return authService.authenticateCredential(username, password, 'oauth2') + .then(user => { + let scopeAuthorizationPromise; - if (!user) return done(null, false); + if (!user) return done(null, false); - if (scopes) { - scopeAuthorizationPromise = authService.authorizeCredential(consumer.id, 'oauth2', scopes); - } else scopeAuthorizationPromise = Promise.resolve(true); + if (scopes) { + scopeAuthorizationPromise = authService.authorizeCredential(consumer.id, 'oauth2', scopes); + } else scopeAuthorizationPromise = Promise.resolve(true); - return scopeAuthorizationPromise - .then(authorized => { - if (!authorized) return done(null, false); + return scopeAuthorizationPromise + .then(authorized => { + if (!authorized) return done(null, false); - const tokenCriteria = { - consumerId: consumer.id, - authenticatedUser: user.id - }; + const tokenCriteria = { + consumerId: consumer.id, + authenticatedUser: user.id + }; - if (scopes) tokenCriteria.scopes = scopes; + if (scopes) tokenCriteria.scopes = scopes; - return tokenService.findOrSave(tokenCriteria, { includeRefreshToken: true }) - .then(token => { - return done(null, token.access_token, token.refresh_token); - }); - }); - }) - .catch(err => done(err)); - }); + return tokenService.findOrSave(tokenCriteria, { includeRefreshToken: true }) + .then(token => { + return done(null, token.access_token, token.refresh_token); + }); + }); + }) + .catch(err => done(err)); + }); })); // Exchange the client id and password/secret for an access token. The callback accepts the @@ -187,21 +187,21 @@ server.exchange(oauth2orize.exchange.clientCredentials((consumer, scopes, done) } else scopeAuthorizationPromise = Promise.resolve(true); return scopeAuthorizationPromise - .then(authorized => { - if (!authorized) return done(null, false); + .then(authorized => { + if (!authorized) return done(null, false); - const tokenCriteria = { - consumerId: consumer.id, - authType: 'oauth2' - }; + const tokenCriteria = { + consumerId: consumer.id, + authType: 'oauth2' + }; - if (scopes) tokenCriteria.scopes = scopes; + if (scopes) tokenCriteria.scopes = scopes; - return tokenService.findOrSave(tokenCriteria) - .then(token => { - return done(null, token.access_token); + return tokenService.findOrSave(tokenCriteria) + .then(token => { + return done(null, token.access_token); + }); }); - }); }) .catch(err => done(err)); })); @@ -248,24 +248,24 @@ module.exports.authorization = [ login.ensureLoggedIn(), server.authorization((areq, done) => { return authService.validateConsumer(areq.clientID) - .then(consumer => { - if (!consumer || consumer.redirectUri !== areq.redirectURI) return done(null, false); - - if (!areq.scope) { - return done(null, consumer, areq.redirectURI); - } + .then(consumer => { + if (!consumer || consumer.redirectUri !== areq.redirectURI) return done(null, false); - return authService.authorizeCredential(areq.clientID, 'oauth2', areq.scope) - .then(authorized => { - if (!authorized) { - return done(null, false); + if (!areq.scope) { + return done(null, consumer, areq.redirectURI); } - consumer.authorizedScopes = areq.scope; + return authService.authorizeCredential(areq.clientID, 'oauth2', areq.scope) + .then(authorized => { + if (!authorized) { + return done(null, false); + } - return done(null, consumer, areq.redirectURI); + consumer.authorizedScopes = areq.scope; + + return done(null, consumer, areq.redirectURI); + }); }); - }); }), (request, response) => { response.set('transaction_id', request.oauth2.transactionID); diff --git a/test/e2e/policy-seq-oauth2-expression-log-ratelimit-proxy.js b/test/e2e/policy-seq-oauth2-expression-log-ratelimit-proxy.js index cf4a32fd2..e4995f1f8 100644 --- a/test/e2e/policy-seq-oauth2-expression-log-ratelimit-proxy.js +++ b/test/e2e/policy-seq-oauth2-expression-log-ratelimit-proxy.js @@ -148,7 +148,7 @@ describe('E2E: oauth2, proxy, log, expression, rate-limit policies', () => { }) .then(() => Promise.all([credentialService.insertCredential(application.id, 'oauth2', { secret: 'app-secret', scopes: ['authorizedScope'] }), - credentialService.insertCredential(user.username, 'basic-auth', { password: 'password', scopes: ['authorizedScope'] })]) + credentialService.insertCredential(user.id, 'basic-auth', { password: 'password', scopes: ['authorizedScope'] })]) ) .then(res => { should.exist(res); diff --git a/test/oauth/authorizationcode.test.js b/test/oauth/authorizationcode.test.js index 8a0d22b0b..966b83aa0 100644 --- a/test/oauth/authorizationcode.test.js +++ b/test/oauth/authorizationcode.test.js @@ -40,64 +40,64 @@ describe('Functional Test Authorization Code grant', function () { }; config.models.users.properties = { - firstname: {isRequired: true, isMutable: true}, - lastname: {isRequired: true, isMutable: true}, - email: {isRequired: false, isMutable: true} + firstname: { isRequired: true, isMutable: true }, + lastname: { isRequired: true, isMutable: true }, + email: { isRequired: false, isMutable: true } }; db.flushdbAsync() - .then(function (didSucceed) { - if (!didSucceed) { - console.log('Failed to flush the database'); - } - const user1 = { - username: 'irfanbaqui', - firstname: 'irfan', - lastname: 'baqui', - email: 'irfan@eg.com' - }; + .then(function (didSucceed) { + if (!didSucceed) { + console.log('Failed to flush the database'); + } + const user1 = { + username: 'irfanbaqui', + firstname: 'irfan', + lastname: 'baqui', + email: 'irfan@eg.com' + }; - const user2 = { - username: 'somejoe', - firstname: 'joe', - lastname: 'smith', - email: 'joe@eg.com' - }; + const user2 = { + username: 'somejoe', + firstname: 'joe', + lastname: 'smith', + email: 'joe@eg.com' + }; - Promise.all([userService.insert(user1), userService.insert(user2)]) - .then(([_fromDbUser1, _fromDbUser2]) => { - should.exist(_fromDbUser1); - should.exist(_fromDbUser2); + Promise.all([userService.insert(user1), userService.insert(user2)]) + .then(([_fromDbUser1, _fromDbUser2]) => { + should.exist(_fromDbUser1); + should.exist(_fromDbUser2); - fromDbUser1 = _fromDbUser1; + fromDbUser1 = _fromDbUser1; - const app1 = { - name: 'irfan_app', - redirectUri: 'https://some.host.com/some/route' - }; + const app1 = { + name: 'irfan_app', + redirectUri: 'https://some.host.com/some/route' + }; - applicationService.insert(app1, fromDbUser1.id) - .then(_fromDbApp => { - should.exist(_fromDbApp); - fromDbApp = _fromDbApp; + applicationService.insert(app1, fromDbUser1.id) + .then(_fromDbApp => { + should.exist(_fromDbApp); + fromDbApp = _fromDbApp; - return credentialService.insertScopes('someScope') - .then(() => { - return Promise.all([ credentialService.insertCredential(fromDbUser1.username, 'basic-auth', { password: 'user-secret' }), - credentialService.insertCredential(fromDbApp.id, 'oauth2', { secret: 'app-secret', scopes: ['someScope'] }) ]) - .then(([userRes, appRes]) => { - should.exist(userRes); - should.exist(appRes); - done(); + return credentialService.insertScopes('someScope') + .then(() => { + return Promise.all([credentialService.insertCredential(fromDbUser1.id, 'basic-auth', { password: 'user-secret' }), + credentialService.insertCredential(fromDbApp.id, 'oauth2', { secret: 'app-secret', scopes: ['someScope'] })]) + .then(([userRes, appRes]) => { + should.exist(userRes); + should.exist(appRes); + done(); + }); + }); }); }); - }); + }) + .catch(function (err) { + should.not.exist(err); + done(); }); - }) - .catch(function (err) { - should.not.exist(err); - done(); - }); }); after((done) => { @@ -123,61 +123,61 @@ describe('Functional Test Authorization Code grant', function () { res.redirects.length.should.equal(1); res.redirects[0].should.containEql('/login'); request - .post('/login') - .query({ - username: 'irfanbaqui', - password: 'user-secret' - }) - .expect(302) - .end(function (err, res) { - should.not.exist(err); - should.exist(res.headers.location); - res.headers.location.should.containEql('/oauth2/authorize'); - request - .get('/oauth2/authorize') + .post('/login') .query({ - redirect_uri: fromDbApp.redirectUri, - response_type: 'code', - client_id: fromDbApp.id + username: 'irfanbaqui', + password: 'user-secret' }) - .expect(200) + .expect(302) .end(function (err, res) { should.not.exist(err); + should.exist(res.headers.location); + res.headers.location.should.containEql('/oauth2/authorize'); request - .post('/oauth2/authorize/decision') - .query({ - transaction_id: res.headers.transaction_id - }) - .expect(302) - .end(function (err, res) { - should.not.exist(err); - should.exist(res.headers.location); - res.headers.location.should.containEql(fromDbApp.redirectUri); - const params = qs.parse(url.parse(res.headers.location).search.slice(1)); - should.exist(params.code); - request - .post('/oauth2/token') - .send({ - grant_type: 'authorization_code', + .get('/oauth2/authorize') + .query({ redirect_uri: fromDbApp.redirectUri, - client_id: fromDbApp.id, - client_secret: 'app-secret', - code: params.code + response_type: 'code', + client_id: fromDbApp.id }) .expect(200) .end(function (err, res) { should.not.exist(err); - should.exist(res.body.access_token); - should.exist(res.body.refresh_token); - res.body.access_token.length.should.be.greaterThan(15); - res.body.refresh_token.length.should.be.greaterThan(15); - should.exist(res.body.token_type); - res.body.token_type.should.eql('Bearer'); - done(); + request + .post('/oauth2/authorize/decision') + .query({ + transaction_id: res.headers.transaction_id + }) + .expect(302) + .end(function (err, res) { + should.not.exist(err); + should.exist(res.headers.location); + res.headers.location.should.containEql(fromDbApp.redirectUri); + const params = qs.parse(url.parse(res.headers.location).search.slice(1)); + should.exist(params.code); + request + .post('/oauth2/token') + .send({ + grant_type: 'authorization_code', + redirect_uri: fromDbApp.redirectUri, + client_id: fromDbApp.id, + client_secret: 'app-secret', + code: params.code + }) + .expect(200) + .end(function (err, res) { + should.not.exist(err); + should.exist(res.body.access_token); + should.exist(res.body.refresh_token); + res.body.access_token.length.should.be.greaterThan(15); + res.body.refresh_token.length.should.be.greaterThan(15); + should.exist(res.body.token_type); + res.body.token_type.should.eql('Bearer'); + done(); + }); + }); }); - }); }); - }); }); }); @@ -197,69 +197,69 @@ describe('Functional Test Authorization Code grant', function () { res.redirects.length.should.equal(1); res.redirects[0].should.containEql('/login'); request - .post('/login') - .query({ - username: 'irfanbaqui', - password: 'user-secret' - }) - .expect(302) - .end(function (err, res) { - should.not.exist(err); - should.exist(res.headers.location); - res.headers.location.should.containEql('/oauth2/authorize'); - request - .get('/oauth2/authorize') + .post('/login') .query({ - redirect_uri: fromDbApp.redirectUri, - response_type: 'code', - client_id: fromDbApp.id, - scope: 'someScope' + username: 'irfanbaqui', + password: 'user-secret' }) - .expect(200) + .expect(302) .end(function (err, res) { should.not.exist(err); + should.exist(res.headers.location); + res.headers.location.should.containEql('/oauth2/authorize'); request - .post('/oauth2/authorize/decision') - .query({ - transaction_id: res.headers.transaction_id - }) - .expect(302) - .end(function (err, res) { - should.not.exist(err); - should.exist(res.headers.location); - res.headers.location.should.containEql(fromDbApp.redirectUri); - const params = qs.parse(url.parse(res.headers.location).search.slice(1)); - should.exist(params.code); - request - .post('/oauth2/token') - .send({ - grant_type: 'authorization_code', + .get('/oauth2/authorize') + .query({ redirect_uri: fromDbApp.redirectUri, + response_type: 'code', client_id: fromDbApp.id, - client_secret: 'app-secret', - code: params.code + scope: 'someScope' }) .expect(200) .end(function (err, res) { should.not.exist(err); - should.exist(res.body.access_token); - should.exist(res.body.refresh_token); - res.body.access_token.length.should.be.greaterThan(15); - res.body.refresh_token.length.should.be.greaterThan(15); - should.exist(res.body.token_type); - res.body.token_type.should.eql('Bearer'); - refreshToken = res.body.refresh_token; - tokenService.get(res.body.access_token) - .then(token => { - should.exist(token); - token.scopes.should.eql([ 'someScope' ]); - [ token.id, token.tokenDecrypted ].should.eql(res.body.access_token.split('|')); - done(); - }); + request + .post('/oauth2/authorize/decision') + .query({ + transaction_id: res.headers.transaction_id + }) + .expect(302) + .end(function (err, res) { + should.not.exist(err); + should.exist(res.headers.location); + res.headers.location.should.containEql(fromDbApp.redirectUri); + const params = qs.parse(url.parse(res.headers.location).search.slice(1)); + should.exist(params.code); + request + .post('/oauth2/token') + .send({ + grant_type: 'authorization_code', + redirect_uri: fromDbApp.redirectUri, + client_id: fromDbApp.id, + client_secret: 'app-secret', + code: params.code + }) + .expect(200) + .end(function (err, res) { + should.not.exist(err); + should.exist(res.body.access_token); + should.exist(res.body.refresh_token); + res.body.access_token.length.should.be.greaterThan(15); + res.body.refresh_token.length.should.be.greaterThan(15); + should.exist(res.body.token_type); + res.body.token_type.should.eql('Bearer'); + refreshToken = res.body.refresh_token; + tokenService.get(res.body.access_token) + .then(token => { + should.exist(token); + token.scopes.should.eql(['someScope']); + [token.id, token.tokenDecrypted].should.eql(res.body.access_token.split('|')); + done(); + }); + }); + }); }); - }); }); - }); }); }); @@ -285,8 +285,8 @@ describe('Functional Test Authorization Code grant', function () { tokenService.get(res.body.access_token) .then(token => { should.exist(token); - token.scopes.should.eql([ 'someScope' ]); - [ token.id, token.tokenDecrypted ].should.eql(res.body.access_token.split('|')); + token.scopes.should.eql(['someScope']); + [token.id, token.tokenDecrypted].should.eql(res.body.access_token.split('|')); done(); }); }); @@ -308,30 +308,30 @@ describe('Functional Test Authorization Code grant', function () { res.redirects.length.should.equal(1); res.redirects[0].should.containEql('/login'); request - .post('/login') - .query({ - username: 'irfanbaqui', - password: 'user-secret' - }) - .expect(302) - .end(function (err, res) { - should.not.exist(err); - should.exist(res.headers.location); - res.headers.location.should.containEql('/oauth2/authorize'); - request - .get('/oauth2/authorize') + .post('/login') .query({ - redirect_uri: fromDbApp.redirectUri, - response_type: 'code', - client_id: fromDbApp.id, - scope: 'someScope, unauthorizedScope' + username: 'irfanbaqui', + password: 'user-secret' }) - .expect(403) - .end(function (err) { + .expect(302) + .end(function (err, res) { should.not.exist(err); - done(); + should.exist(res.headers.location); + res.headers.location.should.containEql('/oauth2/authorize'); + request + .get('/oauth2/authorize') + .query({ + redirect_uri: fromDbApp.redirectUri, + response_type: 'code', + client_id: fromDbApp.id, + scope: 'someScope, unauthorizedScope' + }) + .expect(403) + .end(function (err) { + should.not.exist(err); + done(); + }); }); - }); }); }); }); diff --git a/test/oauth/implicit.test.js b/test/oauth/implicit.test.js index b45d999fb..5f5c4b4ca 100644 --- a/test/oauth/implicit.test.js +++ b/test/oauth/implicit.test.js @@ -40,64 +40,64 @@ describe('Functional Test Implicit grant', function () { }; config.models.users.properties = { - firstname: {isRequired: true, isMutable: true}, - lastname: {isRequired: true, isMutable: true}, - email: {isRequired: false, isMutable: true} + firstname: { isRequired: true, isMutable: true }, + lastname: { isRequired: true, isMutable: true }, + email: { isRequired: false, isMutable: true } }; db.flushdbAsync() - .then(function (didSucceed) { - if (!didSucceed) { - console.log('Failed to flush the database'); - } - const user1 = { - username: 'irfanbaqui', - firstname: 'irfan', - lastname: 'baqui', - email: 'irfan@eg.com' - }; + .then(function (didSucceed) { + if (!didSucceed) { + console.log('Failed to flush the database'); + } + const user1 = { + username: 'irfanbaqui', + firstname: 'irfan', + lastname: 'baqui', + email: 'irfan@eg.com' + }; - const user2 = { - username: 'somejoe', - firstname: 'joe', - lastname: 'smith', - email: 'joe@eg.com' - }; + const user2 = { + username: 'somejoe', + firstname: 'joe', + lastname: 'smith', + email: 'joe@eg.com' + }; - Promise.all([userService.insert(user1), userService.insert(user2)]) - .then(([_fromDbUser1, _fromDbUser2]) => { - should.exist(_fromDbUser1); - should.exist(_fromDbUser2); + Promise.all([userService.insert(user1), userService.insert(user2)]) + .then(([_fromDbUser1, _fromDbUser2]) => { + should.exist(_fromDbUser1); + should.exist(_fromDbUser2); - fromDbUser1 = _fromDbUser1; + fromDbUser1 = _fromDbUser1; - const app1 = { - name: 'irfan_app', - redirectUri: 'https://some.host.com/some/route' - }; + const app1 = { + name: 'irfan_app', + redirectUri: 'https://some.host.com/some/route' + }; - applicationService.insert(app1, fromDbUser1.id) - .then(_fromDbApp => { - should.exist(_fromDbApp); - fromDbApp = _fromDbApp; + applicationService.insert(app1, fromDbUser1.id) + .then(_fromDbApp => { + should.exist(_fromDbApp); + fromDbApp = _fromDbApp; - return credentialService.insertScopes('someScope') - .then(() => { - return Promise.all([ credentialService.insertCredential(fromDbUser1.username, 'basic-auth', { password: 'user-secret' }), - credentialService.insertCredential(fromDbApp.id, 'oauth2', { secret: 'app-secret', scopes: ['someScope'] }) ]) - .then(([userRes, appRes]) => { - should.exist(userRes); - should.exist(appRes); - done(); + return credentialService.insertScopes('someScope') + .then(() => { + return Promise.all([credentialService.insertCredential(fromDbUser1.id, 'basic-auth', { password: 'user-secret' }), + credentialService.insertCredential(fromDbApp.id, 'oauth2', { secret: 'app-secret', scopes: ['someScope'] })]) + .then(([userRes, appRes]) => { + should.exist(userRes); + should.exist(appRes); + done(); + }); + }); }); }); - }); + }) + .catch(function (err) { + should.not.exist(err); + done(); }); - }) - .catch(function (err) { - should.not.exist(err); - done(); - }); }); after((done) => { @@ -123,50 +123,50 @@ describe('Functional Test Implicit grant', function () { res.redirects.length.should.equal(1); res.redirects[0].should.containEql('/login'); request - .post('/login') - .query({ - username: 'irfanbaqui', - password: 'user-secret' - }) - .expect(302) - .end(function (err, res) { - should.not.exist(err); - should.exist(res.headers.location); - res.headers.location.should.containEql('/oauth2/authorize'); - request - .get('/oauth2/authorize') + .post('/login') .query({ - redirect_uri: fromDbApp.redirectUri, - response_type: 'token', - client_id: fromDbApp.id + username: 'irfanbaqui', + password: 'user-secret' }) - .expect(200) + .expect(302) .end(function (err, res) { should.not.exist(err); + should.exist(res.headers.location); + res.headers.location.should.containEql('/oauth2/authorize'); request - .post('/oauth2/authorize/decision') - .query({ - transaction_id: res.headers.transaction_id - }) - .expect(302) - .end(function (err, res) { - should.not.exist(err); - should.exist(res.headers.location); - res.headers.location.should.containEql(fromDbApp.redirectUri); - const params = qs.parse(url.parse(res.headers.location).hash.slice(1)); - should.exist(params.access_token); - should.exist(params.token_type); + .get('/oauth2/authorize') + .query({ + redirect_uri: fromDbApp.redirectUri, + response_type: 'token', + client_id: fromDbApp.id + }) + .expect(200) + .end(function (err, res) { + should.not.exist(err); + request + .post('/oauth2/authorize/decision') + .query({ + transaction_id: res.headers.transaction_id + }) + .expect(302) + .end(function (err, res) { + should.not.exist(err); + should.exist(res.headers.location); + res.headers.location.should.containEql(fromDbApp.redirectUri); + const params = qs.parse(url.parse(res.headers.location).hash.slice(1)); + should.exist(params.access_token); + should.exist(params.token_type); - tokenService.get(params.access_token) - .then(token => { - should.exist(token); - should.not.exist(token.scopes); - [ token.id, token.tokenDecrypted ].should.eql(params.access_token.split('|')); - done(); + tokenService.get(params.access_token) + .then(token => { + should.exist(token); + should.not.exist(token.scopes); + [token.id, token.tokenDecrypted].should.eql(params.access_token.split('|')); + done(); + }); + }); }); - }); }); - }); }); }); @@ -186,50 +186,50 @@ describe('Functional Test Implicit grant', function () { res.redirects.length.should.equal(1); res.redirects[0].should.containEql('/login'); request - .post('/login') - .query({ - username: 'irfanbaqui', - password: 'user-secret' - }) - .expect(302) - .end(function (err, res) { - should.not.exist(err); - should.exist(res.headers.location); - res.headers.location.should.containEql('/oauth2/authorize'); - request - .get('/oauth2/authorize') + .post('/login') .query({ - redirect_uri: fromDbApp.redirectUri, - response_type: 'token', - client_id: fromDbApp.id, - scope: 'someScope' + username: 'irfanbaqui', + password: 'user-secret' }) - .expect(200) + .expect(302) .end(function (err, res) { should.not.exist(err); + should.exist(res.headers.location); + res.headers.location.should.containEql('/oauth2/authorize'); request - .post('/oauth2/authorize/decision') - .query({ - transaction_id: res.headers.transaction_id - }) - .expect(302) - .end(function (err, res) { - should.not.exist(err); - should.exist(res.headers.location); - res.headers.location.should.containEql(fromDbApp.redirectUri); - const params = qs.parse(url.parse(res.headers.location).hash.slice(1)); - should.exist(params.access_token); - should.exist(params.token_type); - tokenService.get(params.access_token) - .then(token => { - should.exist(token); - token.scopes.should.eql([ 'someScope' ]); - [ token.id, token.tokenDecrypted ].should.eql(params.access_token.split('|')); - done(); + .get('/oauth2/authorize') + .query({ + redirect_uri: fromDbApp.redirectUri, + response_type: 'token', + client_id: fromDbApp.id, + scope: 'someScope' + }) + .expect(200) + .end(function (err, res) { + should.not.exist(err); + request + .post('/oauth2/authorize/decision') + .query({ + transaction_id: res.headers.transaction_id + }) + .expect(302) + .end(function (err, res) { + should.not.exist(err); + should.exist(res.headers.location); + res.headers.location.should.containEql(fromDbApp.redirectUri); + const params = qs.parse(url.parse(res.headers.location).hash.slice(1)); + should.exist(params.access_token); + should.exist(params.token_type); + tokenService.get(params.access_token) + .then(token => { + should.exist(token); + token.scopes.should.eql(['someScope']); + [token.id, token.tokenDecrypted].should.eql(params.access_token.split('|')); + done(); + }); + }); }); - }); }); - }); }); }); @@ -249,30 +249,30 @@ describe('Functional Test Implicit grant', function () { res.redirects.length.should.equal(1); res.redirects[0].should.containEql('/login'); request - .post('/login') - .query({ - username: 'irfanbaqui', - password: 'user-secret' - }) - .expect(302) - .end(function (err, res) { - should.not.exist(err); - should.exist(res.headers.location); - res.headers.location.should.containEql('/oauth2/authorize'); - request - .get('/oauth2/authorize') + .post('/login') .query({ - redirect_uri: fromDbApp.redirectUri, - response_type: 'token', - client_id: fromDbApp.id, - scope: 'someScope, someUnauthorizedScope' + username: 'irfanbaqui', + password: 'user-secret' }) - .expect(403) - .end(function (err) { + .expect(302) + .end(function (err, res) { should.not.exist(err); - done(); + should.exist(res.headers.location); + res.headers.location.should.containEql('/oauth2/authorize'); + request + .get('/oauth2/authorize') + .query({ + redirect_uri: fromDbApp.redirectUri, + response_type: 'token', + client_id: fromDbApp.id, + scope: 'someScope, someUnauthorizedScope' + }) + .expect(403) + .end(function (err) { + should.not.exist(err); + done(); + }); }); - }); }); }); }); diff --git a/test/oauth/password.test.js b/test/oauth/password.test.js index b20196fa2..3f56e5d35 100644 --- a/test/oauth/password.test.js +++ b/test/oauth/password.test.js @@ -33,64 +33,64 @@ describe('Functional Test Client Password grant', function () { }; config.models.users.properties = { - firstname: {isRequired: true, isMutable: true}, - lastname: {isRequired: true, isMutable: true}, - email: {isRequired: false, isMutable: true} + firstname: { isRequired: true, isMutable: true }, + lastname: { isRequired: true, isMutable: true }, + email: { isRequired: false, isMutable: true } }; db.flushdbAsync() - .then(function (didSucceed) { - if (!didSucceed) { - console.log('Failed to flush the database'); - } - const user1 = { - username: 'irfanbaqui', - firstname: 'irfan', - lastname: 'baqui', - email: 'irfan@eg.com' - }; - - const user2 = { - username: 'somejoe', - firstname: 'joe', - lastname: 'smith', - email: 'joe@eg.com' - }; - - return Promise.all([userService.insert(user1), userService.insert(user2)]) - .then(([_fromDbUser1, _fromDbUser2]) => { - should.exist(_fromDbUser1); - should.exist(_fromDbUser2); - - fromDbUser1 = _fromDbUser1; - - const app1 = { - name: 'irfan_app', - redirectUri: 'https://some.host.com/some/route' + .then(function (didSucceed) { + if (!didSucceed) { + console.log('Failed to flush the database'); + } + const user1 = { + username: 'irfanbaqui', + firstname: 'irfan', + lastname: 'baqui', + email: 'irfan@eg.com' + }; + + const user2 = { + username: 'somejoe', + firstname: 'joe', + lastname: 'smith', + email: 'joe@eg.com' }; - applicationService.insert(app1, fromDbUser1.id) - .then(_fromDbApp => { - should.exist(_fromDbApp); - fromDbApp = _fromDbApp; - - credentialService.insertScopes('someScope') - .then(() => { - Promise.all([ credentialService.insertCredential(fromDbUser1.username, 'oauth2', { secret: 'user-secret' }), - credentialService.insertCredential(fromDbApp.id, 'oauth2', { secret: 'app-secret', scopes: [ 'someScope' ] }) ]) - .then(([userRes, appRes]) => { - should.exist(userRes); - should.exist(appRes); - done(); - }); + return Promise.all([userService.insert(user1), userService.insert(user2)]) + .then(([_fromDbUser1, _fromDbUser2]) => { + should.exist(_fromDbUser1); + should.exist(_fromDbUser2); + + fromDbUser1 = _fromDbUser1; + + const app1 = { + name: 'irfan_app', + redirectUri: 'https://some.host.com/some/route' + }; + + applicationService.insert(app1, fromDbUser1.id) + .then(_fromDbApp => { + should.exist(_fromDbApp); + fromDbApp = _fromDbApp; + + credentialService.insertScopes('someScope') + .then(() => { + Promise.all([credentialService.insertCredential(fromDbUser1.id, 'oauth2', { secret: 'user-secret' }), + credentialService.insertCredential(fromDbApp.id, 'oauth2', { secret: 'app-secret', scopes: ['someScope'] })]) + .then(([userRes, appRes]) => { + should.exist(userRes); + should.exist(appRes); + done(); + }); + }); + }); }); - }); + }) + .catch(function (err) { + should.not.exist(err); + done(); }); - }) - .catch(function (err) { - should.not.exist(err); - done(); - }); }); after((done) => { @@ -105,24 +105,24 @@ describe('Functional Test Client Password grant', function () { const credentials = Buffer.from(fromDbApp.id.concat(':app-secret')).toString('base64'); request - .post('/oauth2/token') - .set('Authorization', 'basic ' + credentials) - .set('content-type', 'application/x-www-form-urlencoded') - .type('form') - .send({ - grant_type: 'password', - username: 'irfanbaqui', - password: 'user-secret' - }) - .expect(200) - .end(function (err, res) { - should.not.exist(err); - const token = res.body; - should.exist(token); - should.exist(token.access_token); - token.token_type.should.equal('Bearer'); - done(); - }); + .post('/oauth2/token') + .set('Authorization', 'basic ' + credentials) + .set('content-type', 'application/x-www-form-urlencoded') + .type('form') + .send({ + grant_type: 'password', + username: 'irfanbaqui', + password: 'user-secret' + }) + .expect(200) + .end(function (err, res) { + should.not.exist(err); + const token = res.body; + should.exist(token); + should.exist(token.access_token); + token.token_type.should.equal('Bearer'); + done(); + }); }); it('should grant access token with authorized scopes', function (done) { @@ -130,34 +130,34 @@ describe('Functional Test Client Password grant', function () { const credentials = Buffer.from(fromDbApp.id.concat(':app-secret')).toString('base64'); request - .post('/oauth2/token') - .set('Authorization', 'basic ' + credentials) - .set('content-type', 'application/x-www-form-urlencoded') - .type('form') - .send({ - grant_type: 'password', - username: 'irfanbaqui', - password: 'user-secret', - scope: 'someScope' - }) - .expect(200) - .end(function (err, res) { - should.not.exist(err); - const token = res.body; - should.exist(token); - should.exist(token.access_token); - should.exist(token.refresh_token); - token.token_type.should.equal('Bearer'); - refreshToken = token.refresh_token; - - tokenService.get(token.access_token) - .then(fromDbToken => { - should.exist(fromDbToken); - fromDbToken.scopes.should.eql([ 'someScope' ]); - [ fromDbToken.id, fromDbToken.tokenDecrypted ].should.eql(token.access_token.split('|')); - done(); - }); - }); + .post('/oauth2/token') + .set('Authorization', 'basic ' + credentials) + .set('content-type', 'application/x-www-form-urlencoded') + .type('form') + .send({ + grant_type: 'password', + username: 'irfanbaqui', + password: 'user-secret', + scope: 'someScope' + }) + .expect(200) + .end(function (err, res) { + should.not.exist(err); + const token = res.body; + should.exist(token); + should.exist(token.access_token); + should.exist(token.refresh_token); + token.token_type.should.equal('Bearer'); + refreshToken = token.refresh_token; + + tokenService.get(token.access_token) + .then(fromDbToken => { + should.exist(fromDbToken); + fromDbToken.scopes.should.eql(['someScope']); + [fromDbToken.id, fromDbToken.tokenDecrypted].should.eql(token.access_token.split('|')); + done(); + }); + }); }); it('should grant access token in exchange of refresh token', function (done) { @@ -182,8 +182,8 @@ describe('Functional Test Client Password grant', function () { tokenService.get(res.body.access_token) .then(token => { should.exist(token); - token.scopes.should.eql([ 'someScope' ]); - [ token.id, token.tokenDecrypted ].should.eql(res.body.access_token.split('|')); + token.scopes.should.eql(['someScope']); + [token.id, token.tokenDecrypted].should.eql(res.body.access_token.split('|')); done(); }); }); @@ -194,20 +194,20 @@ describe('Functional Test Client Password grant', function () { const credentials = Buffer.from(fromDbApp.id.concat(':app-secret')).toString('base64'); request - .post('/oauth2/token') - .set('Authorization', 'basic ' + credentials) - .set('content-type', 'application/x-www-form-urlencoded') - .type('form') - .send({ - grant_type: 'password', - username: 'irfanbaqui', - password: 'user-secret', - scope: 'someScope unauthorizedScope' - }) - .expect(401) - .end(function (err) { - should.not.exist(err); - done(); - }); + .post('/oauth2/token') + .set('Authorization', 'basic ' + credentials) + .set('content-type', 'application/x-www-form-urlencoded') + .type('form') + .send({ + grant_type: 'password', + username: 'irfanbaqui', + password: 'user-secret', + scope: 'someScope unauthorizedScope' + }) + .expect(401) + .end(function (err) { + should.not.exist(err); + done(); + }); }); }); diff --git a/test/policies/basic-auth-policy.test.js b/test/policies/basic-auth-policy.test.js index 1663da82a..4a3021a79 100644 --- a/test/policies/basic-auth-policy.test.js +++ b/test/policies/basic-auth-policy.test.js @@ -34,7 +34,7 @@ describe('Functional Tests basic auth Policy', () => { authorizedEndpoint: { host: '*', paths: ['/authorizedPath'], - scopes: [ 'authorizedScope' ] + scopes: ['authorizedScope'] }, unauthorizedEndpoint: { host: '*', @@ -80,9 +80,9 @@ describe('Functional Tests basic auth Policy', () => { }; userModelConfig.properties = { - firstname: {isRequired: true, isMutable: true}, - lastname: {isRequired: true, isMutable: true}, - email: {isRequired: false, isMutable: true} + firstname: { isRequired: true, isMutable: true }, + lastname: { isRequired: true, isMutable: true }, + email: { isRequired: false, isMutable: true } }; db.flushdbAsync() @@ -101,7 +101,7 @@ describe('Functional Tests basic auth Policy', () => { credentialService.insertScopes('authorizedScope', 'unauthorizedScope') .then(() => { - return credentialService.insertCredential(user.username, 'basic-auth', { password: 'user-secret', scopes: [ 'authorizedScope' ] }) + return credentialService.insertCredential(user.id, 'basic-auth', { password: 'user-secret', scopes: ['authorizedScope'] }) .then((userRes) => { should.exist(userRes); return serverHelper.generateBackendServer(6067) diff --git a/test/policies/keyauth/keyauth.test.js b/test/policies/keyauth/keyauth.test.js index 1132d4f4f..49dd284c9 100644 --- a/test/policies/keyauth/keyauth.test.js +++ b/test/policies/keyauth/keyauth.test.js @@ -109,7 +109,7 @@ describe('Functional Tests keyAuth Policy', () => { return credentialService.insertScopes('authorizedScope', 'unauthorizedScope') .then(() => { - return credentialService.insertCredential(_fromDbUser1.username, 'key-auth', { + return credentialService.insertCredential(_fromDbUser1.id, 'key-auth', { scopes: ['authorizedScope'] }); }) diff --git a/test/policies/oauth/consumer-and-token-headers.test.js b/test/policies/oauth/consumer-and-token-headers.test.js index b29aed961..6f82f54f2 100644 --- a/test/policies/oauth/consumer-and-token-headers.test.js +++ b/test/policies/oauth/consumer-and-token-headers.test.js @@ -40,7 +40,7 @@ describe('Request Headers with consumer and token information as part of auth po authorizedEndpoint: { host: '*', paths: ['/authorizedPath'], - scopes: [ 'authorizedScope' ] + scopes: ['authorizedScope'] } }, policies: ['oauth2', 'proxy'], @@ -49,7 +49,7 @@ describe('Request Headers with consumer and token information as part of auth po apiEndpoints: ['authorizedEndpoint'], policies: [ { oauth2: {} }, - { proxy: [ { action: { serviceEndpoint: 'backend' } } ] } + { proxy: [{ action: { serviceEndpoint: 'backend' } }] } ] } } @@ -70,9 +70,9 @@ describe('Request Headers with consumer and token information as part of auth po }; userModelConfig.properties = { - firstname: {isRequired: true, isMutable: true}, - lastname: {isRequired: true, isMutable: true}, - email: {isRequired: false, isMutable: true} + firstname: { isRequired: true, isMutable: true }, + lastname: { isRequired: true, isMutable: true }, + email: { isRequired: false, isMutable: true } }; db.flushdbAsync() @@ -101,8 +101,8 @@ describe('Request Headers with consumer and token information as part of auth po return credentialService.insertScopes(['authorizedScope']) .then(() => { - Promise.all([ credentialService.insertCredential(application.id, 'oauth2', { secret: 'app-secret', scopes: ['authorizedScope'] }), - credentialService.insertCredential(user.username, 'basic-auth', { password: 'password', scopes: ['authorizedScope'] }) ]) + Promise.all([credentialService.insertCredential(application.id, 'oauth2', { secret: 'app-secret', scopes: ['authorizedScope'] }), + credentialService.insertCredential(user.id, 'basic-auth', { password: 'password', scopes: ['authorizedScope'] })]) .then(res => { should.exist(res); diff --git a/test/services/auth.test.js b/test/services/auth.test.js index 4c0de94b4..78168b4ec 100644 --- a/test/services/auth.test.js +++ b/test/services/auth.test.js @@ -56,9 +56,7 @@ describe('Auth tests', function () { }) .then(() => { return credentialService.insertCredential(user.username, 'oauth2', _credential) - .then(function (res) { - should.exist(res); - }); + .then((res) => should.exist(res)); }); }); diff --git a/test/services/credential-service/credentials.test.js b/test/services/credential-service/credentials.test.js index b3529b44e..788615d28 100644 --- a/test/services/credential-service/credentials.test.js +++ b/test/services/credential-service/credentials.test.js @@ -155,8 +155,8 @@ describe('Credential service tests', function () { .then((newUser) => { user = newUser; return Promise.all([ - credentialService.insertCredential(user.username, 'oauth2'), - credentialService.insertCredential(user.username, 'basic-auth') + credentialService.insertCredential(user.username, 'oauth2').then((oauthCred) => should.exist(oauthCred.secret)), + credentialService.insertCredential(user.username, 'basic-auth').then((basicAuthCred) => should.exist(basicAuthCred.password)) ]); }); }); @@ -189,12 +189,12 @@ describe('Credential service tests', function () { }); it('should delete a credential', () => { - return credentialService.insertCredential(user.username, 'oauth2').then(res => { + return credentialService.insertCredential(user.id, 'oauth2').then(res => { should.exist(res); - return credentialService.removeCredential(user.username, 'oauth2'); + return credentialService.removeCredential(user.id, 'oauth2'); }).then(deleted => { should(deleted).be.equal(1); - return credentialService.getCredential(user.username, 'oauth2'); + return credentialService.getCredential(user.id, 'oauth2'); }).then(resAfterDelete => should.not.exist(resAfterDelete)); }); }); From f5b65e459420a18590b730593a3ef65249cde885 Mon Sep 17 00:00:00 2001 From: Vincenzo Chianese Date: Fri, 27 Oct 2017 11:02:57 +0200 Subject: [PATCH 10/18] Credentials remove should go by id --- lib/services/consumers/user.service.js | 156 +++++++++--------- .../credential-service/credentials.test.js | 12 +- 2 files changed, 84 insertions(+), 84 deletions(-) diff --git a/lib/services/consumers/user.service.js b/lib/services/consumers/user.service.js index 07d8f864b..f03360445 100644 --- a/lib/services/consumers/user.service.js +++ b/lib/services/consumers/user.service.js @@ -11,15 +11,15 @@ const s = {}; s.insert = function (user) { return validateAndCreateUser(user) - .then(function (newUser) { - return userDao.insert(newUser) - .then(function (success) { - if (success) { - newUser.isActive = newUser.isActive === 'true'; - return newUser; - } else return Promise.reject(new Error('insert user failed')); // TODO: replace with server error + .then(function (newUser) { + return userDao.insert(newUser) + .then(function (success) { + if (success) { + newUser.isActive = newUser.isActive === 'true'; + return newUser; + } else return Promise.reject(new Error('insert user failed')); // TODO: replace with server error + }); }); - }); }; s.get = function (userId, options) { @@ -28,18 +28,18 @@ s.get = function (userId, options) { } return userDao - .getUserById(userId) - .then(function (user) { - if (!user) { - return false; - } + .getUserById(userId) + .then(function (user) { + if (!user) { + return false; + } - user.isActive = user.isActive === 'true'; - if (!options || !options.includePassword) { - delete user.password; - } - return user; - }); + user.isActive = user.isActive === 'true'; + if (!options || !options.includePassword) { + delete user.password; + } + return user; + }); }; s.findAll = function (query) { @@ -56,10 +56,10 @@ s.find = function (username, options) { } return userDao - .find(username) - .then(userId => { - return userId ? this.get(userId, options) : false; - }); + .find(username) + .then(userId => { + return userId ? this.get(userId, options) : false; + }); }; s.update = function (userId, _props) { @@ -67,58 +67,58 @@ s.update = function (userId, _props) { return Promise.reject(new Error('invalid user id')); // TODO: replace with validation error } return this.get(userId) // validate user exists - .then(user => { - if (!user) { return false; } // user does not exist - - delete _props.username; - return validateUpdateToUserProperties(_props) - .then(function (updatedUserProperties) { - if (updatedUserProperties) { - utils.appendUpdatedAt(updatedUserProperties); - return userDao.update(userId, updatedUserProperties); - } else return true; // there are no properties to update - }) - .then(updated => { - return updated ? true : Promise.reject(new Error('user update failed')); // TODO: replace with server error + .then(user => { + if (!user) { return false; } // user does not exist + + delete _props.username; + return validateUpdateToUserProperties(_props) + .then(function (updatedUserProperties) { + if (updatedUserProperties) { + utils.appendUpdatedAt(updatedUserProperties); + return userDao.update(userId, updatedUserProperties); + } else return true; // there are no properties to update + }) + .then(updated => { + return updated ? true : Promise.reject(new Error('user update failed')); // TODO: replace with server error + }); }); - }); }; s.deactivate = function (id) { return this.get(id) // make sure user exists - .then(function () { - return userDao.deactivate(id) - .then(() => applicationService.deactivateAll(id)); // Cascade deactivate all applications associated with the user - }) - .then(() => true) - .catch(() => Promise.reject(new Error('failed to deactivate user'))); + .then(function () { + return userDao.deactivate(id) + .then(() => applicationService.deactivateAll(id)); // Cascade deactivate all applications associated with the user + }) + .then(() => true) + .catch(() => Promise.reject(new Error('failed to deactivate user'))); }; s.activate = function (id) { return this.get(id) // make sure user exists - .then(function () { - return userDao.activate(id); - }) - .then(() => true) - .catch(() => Promise.reject(new Error('failed to deactivate user'))); + .then(function () { + return userDao.activate(id); + }) + .then(() => true) + .catch(() => Promise.reject(new Error('failed to deactivate user'))); }; s.remove = function (userId) { return this.get(userId) // validate user exists - .then(function (user) { - return !user ? false // user does not exist - : userDao.remove(userId) - .then(function (userDeleted) { - if (!userDeleted) { - return Promise.reject(new Error('user delete failed')); // TODO: replace with server error - } else { - return Promise.all([ applicationService.removeAll(userId), // Cascade delete all apps associated with user - credentialService.removeAllCredentials(user.username)]) // Cascade delete all user credentials - .catch(() => Promise.reject(new Error('failed to delete user\'s applications or credentials'))) // TODO: replace with server error - .then(() => true); - } + .then(function (user) { + return !user ? false // user does not exist + : userDao.remove(userId) + .then(function (userDeleted) { + if (!userDeleted) { + return Promise.reject(new Error('user delete failed')); // TODO: replace with server error + } else { + return Promise.all([applicationService.removeAll(userId), // Cascade delete all apps associated with user + credentialService.removeAllCredentials(user.id)]) // Cascade delete all user credentials + .catch(() => Promise.reject(new Error('failed to delete user\'s applications or credentials'))) // TODO: replace with server error + .then(() => true); + } + }); }); - }); }; function validateAndCreateUser (_user) { @@ -129,23 +129,23 @@ function validateAndCreateUser (_user) { } return s.find(username) // Ensure username is unique - .then(function (exists) { - if (exists) { - throw new Error('username already exists'); - } - return validateNewUserProperties(_user); - }) - .then(function (newUser) { - const baseUserProps = { isActive: 'true', username, id: uuidv4() }; - if (newUser) { - user = Object.assign(newUser, baseUserProps); - } else user = baseUserProps; - - utils.appendCreatedAt(user); - utils.appendUpdatedAt(user); - - return user; - }); + .then(function (exists) { + if (exists) { + throw new Error('username already exists'); + } + return validateNewUserProperties(_user); + }) + .then(function (newUser) { + const baseUserProps = { isActive: 'true', username, id: uuidv4() }; + if (newUser) { + user = Object.assign(newUser, baseUserProps); + } else user = baseUserProps; + + utils.appendCreatedAt(user); + utils.appendUpdatedAt(user); + + return user; + }); } function validateUpdateToUserProperties (userProperties) { diff --git a/test/services/credential-service/credentials.test.js b/test/services/credential-service/credentials.test.js index 788615d28..d12e7087c 100644 --- a/test/services/credential-service/credentials.test.js +++ b/test/services/credential-service/credentials.test.js @@ -155,8 +155,8 @@ describe('Credential service tests', function () { .then((newUser) => { user = newUser; return Promise.all([ - credentialService.insertCredential(user.username, 'oauth2').then((oauthCred) => should.exist(oauthCred.secret)), - credentialService.insertCredential(user.username, 'basic-auth').then((basicAuthCred) => should.exist(basicAuthCred.password)) + credentialService.insertCredential(user.id, 'oauth2').then((oauthCred) => should.exist(oauthCred.secret)), + credentialService.insertCredential(user.id, 'basic-auth').then((basicAuthCred) => should.exist(basicAuthCred.password)) ]); }); }); @@ -167,8 +167,8 @@ describe('Credential service tests', function () { it('should delete all credentials associated with a user when user is deleted', () => { return Promise.all([ - credentialService.getCredential(user.username, 'oauth2'), - credentialService.getCredential(user.username, 'basic-auth') + credentialService.getCredential(user.id, 'oauth2'), + credentialService.getCredential(user.id, 'basic-auth') ]) .then(([oauthRes, basicAuthRes]) => { should.exist(oauthRes); // Check to confirm the credentials exist @@ -178,8 +178,8 @@ describe('Credential service tests', function () { .then(res => { should.exist(res); return Promise.all([ - credentialService.getCredential(user.username, 'oauth2'), - credentialService.getCredential(user.username, 'basic-auth') + credentialService.getCredential(user.id, 'oauth2'), + credentialService.getCredential(user.id, 'basic-auth') ]); }) .then(([oauthResAfterDelete, basicAuthResAfterDelete]) => { From e67b14ae59a6565f5c85763aa3095963dc84b6ea Mon Sep 17 00:00:00 2001 From: Vincenzo Chianese Date: Fri, 27 Oct 2017 12:31:11 +0200 Subject: [PATCH 11/18] Always use user-name --- test/policies/basic-auth-policy.test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/policies/basic-auth-policy.test.js b/test/policies/basic-auth-policy.test.js index 4a3021a79..484e9ea76 100644 --- a/test/policies/basic-auth-policy.test.js +++ b/test/policies/basic-auth-policy.test.js @@ -139,7 +139,7 @@ describe('Functional Tests basic auth Policy', () => { }); it('should not authenticate token for requests if requester doesn\'t have authorized scopes', function () { - const credentials = Buffer.from(user.id.concat(':user-secret')).toString('base64'); + const credentials = Buffer.from(user.username.concat(':user-secret')).toString('base64'); return request(app) .get('/unauthorizedPath') @@ -157,7 +157,7 @@ describe('Functional Tests basic auth Policy', () => { }); it('should not authenticate invalid token', function () { - const credentials = Buffer.from(user.id.concat(':wrongPassword')).toString('base64'); + const credentials = Buffer.from(user.username.concat(':wrongPassword')).toString('base64'); return request(app) .get('/authorizedPath') From ff054073afa8719a2d4657a49bedac2091a06b3a Mon Sep 17 00:00:00 2001 From: Vincenzo Chianese Date: Fri, 27 Oct 2017 13:05:59 +0200 Subject: [PATCH 12/18] Correctly decompose and evaluate authorization result --- lib/services/auth.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/services/auth.js b/lib/services/auth.js index d7cb67c29..723840075 100644 --- a/lib/services/auth.js +++ b/lib/services/auth.js @@ -41,7 +41,12 @@ s.authenticateCredential = function (id, password, type) { if (!credentialResult) { return false; } - const [consumer] = credentialResult; + const [consumer, authenticated] = credentialResult; + + if (!authenticated) { + return false; + } + return consumer; }); }; From 9a0b22b94a81abacac0d1e3d62382be8be49d52c Mon Sep 17 00:00:00 2001 From: Vincenzo Chianese Date: Fri, 27 Oct 2017 13:17:34 +0200 Subject: [PATCH 13/18] Correctly propagate user id to test --- test/services/auth.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/services/auth.test.js b/test/services/auth.test.js index 78168b4ec..5ca78c203 100644 --- a/test/services/auth.test.js +++ b/test/services/auth.test.js @@ -55,7 +55,7 @@ describe('Auth tests', function () { return credentialService.insertScopes(['someScope1', 'someScope2', 'someScope3']); }) .then(() => { - return credentialService.insertCredential(user.username, 'oauth2', _credential) + return credentialService.insertCredential(userFromDb.id, 'oauth2', _credential) .then((res) => should.exist(res)); }); }); From 89014c2fe130f9c2cfaa12e7e757c87c85776e30 Mon Sep 17 00:00:00 2001 From: Vincenzo Chianese Date: Fri, 27 Oct 2017 13:57:31 +0200 Subject: [PATCH 14/18] Correcly check for boolean in validate result --- lib/services/auth.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/services/auth.js b/lib/services/auth.js index 723840075..87a78406e 100644 --- a/lib/services/auth.js +++ b/lib/services/auth.js @@ -28,7 +28,13 @@ s.authenticateCredential = function (id, password, type) { return false; } return Promise.all([consumer, credentials.getCredential(consumer.id, type, { includePassword: true })]); - }).then(([consumer, credential]) => { + }).then((validateResult) => { + if (!validateResult) { + return false; + } + + const [consumer, credential] = validateResult; + if (!credential || !credential.isActive) { return false; } From 5d3deddc15c8737dec38e0687b433b3433661d3e Mon Sep 17 00:00:00 2001 From: Vincenzo Chianese Date: Fri, 27 Oct 2017 13:57:42 +0200 Subject: [PATCH 15/18] Use User.Ids in credentials checks --- test/services/auth.test.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/services/auth.test.js b/test/services/auth.test.js index 5ca78c203..b9cdb4618 100644 --- a/test/services/auth.test.js +++ b/test/services/auth.test.js @@ -97,7 +97,7 @@ describe('Auth tests', function () { }); it('should authorize Credential with scopes', () => { - return authService.authorizeCredential(user.username, 'oauth2', ['someScope1', 'someScope2']) + return authService.authorizeCredential(userFromDb.id, 'oauth2', ['someScope1', 'someScope2']) .then((authResponse) => { should.exist(authResponse); authResponse.should.eql(true); @@ -105,7 +105,7 @@ describe('Auth tests', function () { }); it('should not authorize Credential with invalid scopes', () => { - return authService.authorizeCredential(user.username, 'oauth2', ['otherScope', 'someScope2']) + return authService.authorizeCredential(userFromDb.id, 'oauth2', ['otherScope', 'someScope2']) .then((authResponse) => { should.exist(authResponse); authResponse.should.eql(false); @@ -114,19 +114,19 @@ describe('Auth tests', function () { it('should not authorize Credential that is inActive', () => { return credentialService - .deactivateCredential(user.username, 'oauth2') + .deactivateCredential(userFromDb.id, 'oauth2') .then(function (res) { should.exist(res); res.should.eql(true); }) - .then(() => authService.authorizeCredential(user.username, 'oauth2', ['otherScope', 'someScope2'])) + .then(() => authService.authorizeCredential(userFromDb.id, 'oauth2', ['otherScope', 'someScope2'])) .then((authResponse) => { should.exist(authResponse); authResponse.should.eql(false); // reset credential back to active status return credentialService - .activateCredential(user.username, 'oauth2') + .activateCredential(userFromDb.id, 'oauth2') .then(function (res) { should.exist(res); res.should.eql(true); From bd08c1a54f7f5ce0e2a336e1c896ac9b16470bef Mon Sep 17 00:00:00 2001 From: Vincenzo Chianese Date: Mon, 30 Oct 2017 12:29:55 +0100 Subject: [PATCH 16/18] Small test tweaks --- lib/services/credentials/credential.dao.js | 2 +- test/services/auth.test.js | 8 ++--- .../credential-service/credentials.test.js | 33 ++++++++++--------- 3 files changed, 22 insertions(+), 21 deletions(-) diff --git a/lib/services/credentials/credential.dao.js b/lib/services/credentials/credential.dao.js index 138a2a127..322bfc96c 100644 --- a/lib/services/credentials/credential.dao.js +++ b/lib/services/credentials/credential.dao.js @@ -118,7 +118,7 @@ dao.insertCredential = function (id, type, credentialObj) { db.saddAsync(buildIdKey(type, credentialObj.consumerId), id), // store key-auth keyid -> credentialObj db.hmsetAsync(buildIdKey(type, id), credentialObj) - ]).catch((e) => console.log(e)); + ]); } return db.hmsetAsync(buildIdKey(type, id), credentialObj); }; diff --git a/test/services/auth.test.js b/test/services/auth.test.js index b9cdb4618..732bfbceb 100644 --- a/test/services/auth.test.js +++ b/test/services/auth.test.js @@ -55,9 +55,9 @@ describe('Auth tests', function () { return credentialService.insertScopes(['someScope1', 'someScope2', 'someScope3']); }) .then(() => { - return credentialService.insertCredential(userFromDb.id, 'oauth2', _credential) - .then((res) => should.exist(res)); - }); + return credentialService.insertCredential(userFromDb.id, 'oauth2', _credential); + }) + .then((res) => should.exist(res)); }); after(() => { @@ -89,7 +89,7 @@ describe('Auth tests', function () { }); it('should not authenticate valid user with invalid credentials', () => { - return authService.authenticateCredential(user.username, 'invalidSecret', 'oauth2') + return authService.authenticateCredential(userFromDb.id, 'invalidSecret', 'oauth2') .then(authResponse => { should.exist(authResponse); authResponse.should.eql(false); diff --git a/test/services/credential-service/credentials.test.js b/test/services/credential-service/credentials.test.js index d12e7087c..2318fc6bb 100644 --- a/test/services/credential-service/credentials.test.js +++ b/test/services/credential-service/credentials.test.js @@ -8,8 +8,8 @@ const credentialService = services.credential; const userService = services.user; const db = require('../../../lib/db')(); -describe('Credential service tests', function () { - describe('Credential tests', function () { +describe('Credential service tests', () => { + describe('Credential tests', () => { let credential; const originalModelConfig = config.models.credentials; const username = 'someUser'; @@ -45,7 +45,7 @@ describe('Credential service tests', function () { return credentialService .insertCredential(username, 'oauth2', _credential) - .then(function (newCredential) { + .then((newCredential) => { should.exist(newCredential); credential = Object.assign(newCredential, _credential); }); @@ -65,10 +65,10 @@ describe('Credential service tests', function () { return credentialService .insertCredential('someUsername', 'oauth2', _credential) - .then(function (newCredential) { + .then((newCredential) => { should.exist(newCredential); should.exist(newCredential.secret); - newCredential.secret.length.should.greaterThanOrEqual(10); + should(newCredential.secret.length).greaterThanOrEqual(10); }); }); @@ -79,7 +79,7 @@ describe('Credential service tests', function () { return credentialService .insertCredential(username, 'basic-auth', _credential) - .then(function (newCredential) { + .then((newCredential) => { should.exist(newCredential); newCredential.isActive.should.eql(true); }); @@ -88,7 +88,7 @@ describe('Credential service tests', function () { it('should get a credential', () => { return credentialService .getCredential(username, 'oauth2') - .then(function (cred) { + .then((cred) => { should.exist(cred); should.not.exist(cred.secret); credential.isActive.should.eql(true); @@ -109,12 +109,12 @@ describe('Credential service tests', function () { it('should reactivate a credential', () => { return credentialService .activateCredential(username, 'oauth2') - .then(function (res) { + .then((res) => { should.exist(res); return credentialService.getCredential(username, 'oauth2'); }) - .then(function (cred) { + .then((cred) => { should.exist(cred); should.not.exist(cred.secret); cred.isActive.should.eql(true); @@ -236,12 +236,13 @@ describe('Credential service tests', function () { .then(() => credentialService.insertCredential(username, 'oauth2', _credential)) .then((newCredential) => { should.exist(newCredential); - newCredential.isActive.should.eql(true); should.exist(newCredential.scopes); + should.not.exist(newCredential.secret); + + newCredential.isActive.should.eql(true); newCredential.scopes.should.eql(['someScope']); newCredential.someProperty.should.eql('propVal'); newCredential.otherProperty.should.eql('someDefaultValue'); - should.not.exist(newCredential.secret); }); }); @@ -254,12 +255,12 @@ describe('Credential service tests', function () { .then((cred) => { should.exist(cred); should.exist(cred.scopes); + cred.isActive.should.eql(true); cred.scopes.should.containEql(_credential.scopes); cred.scopes.should.containEql('someScope1'); cred.scopes.should.containEql('someScope2'); cred.scopes.should.containEql('someScope3'); cred.scopes.should.containEql('someOtherOne'); - cred.isActive.should.eql(true); }); }); }); @@ -344,10 +345,10 @@ describe('Credential service tests', function () { .updateCredential(username, 'oauth2', {}) .then((newCredential) => { should.not.exist(newCredential); - return credentialService.getCredential(username, 'oauth2') - .then(credential => { - should.exist(credential); - }); + return credentialService.getCredential(username, 'oauth2'); + }) + .then(credential => { + should.exist(credential); }); }); }); From 9ac297daee3124d1d3a634ed1ed83fade7967a9d Mon Sep 17 00:00:00 2001 From: Vincenzo Chianese Date: Mon, 30 Oct 2017 17:47:11 +0100 Subject: [PATCH 17/18] Move catch block under the findConsumer main function --- lib/rest/routes/credentials.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/rest/routes/credentials.js b/lib/rest/routes/credentials.js index 9a3cfb4ed..264a12d2d 100644 --- a/lib/rest/routes/credentials.js +++ b/lib/rest/routes/credentials.js @@ -86,9 +86,8 @@ module.exports = function () { return credentialSrv .getCredentials(consumer.id) - .then(credentials => res.json({ credentials })) - .catch(next); - }); + .then(credentials => res.json({ credentials })); + }).catch(next); }); return router; From ba2d7d1a8b0b6eccf4d4a50f229b9816b4b8b26d Mon Sep 17 00:00:00 2001 From: Vincenzo Chianese Date: Tue, 31 Oct 2017 16:05:01 +0100 Subject: [PATCH 18/18] Use correct status code when a consumer is not found --- lib/rest/routes/credentials.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/rest/routes/credentials.js b/lib/rest/routes/credentials.js index 264a12d2d..0a23ee019 100644 --- a/lib/rest/routes/credentials.js +++ b/lib/rest/routes/credentials.js @@ -81,7 +81,7 @@ module.exports = function () { findConsumer(req.params.consumerId) .then((consumer) => { if (!consumer) { - return res.status(422).json(new Error('Consumer Not Found: id:' + req.body.consumerId)); + return res.status(404).json(new Error('Consumer Not Found: id:' + req.body.consumerId)); } return credentialSrv