Skip to content

Commit

Permalink
Merge pull request ExpressGateway#899 from nakardo/fix-856 closes Exp…
Browse files Browse the repository at this point in the history 
…ressGateway#856

fix multiple creds support for 'jwt' and 'key-auth' closes ExpressGateway#856
  • Loading branch information
@XVincentX
XVincentX authored Apr 11, 2019
2 parents 28492a5 + b4dd785 commit 4d720b4
Show file tree
Hide file tree
Showing 2 changed files with 168 additions and 136 deletions.
155 changes: 82 additions & 73 deletions lib/services/credentials/credential.service.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,84 +45,93 @@ s.insertCredential = function (id, type, credentialDetails) {
throw new Error(`Invalid credential type: ${type}`); // TODO: replace with validation error
}

return this.getCredential(id, type) // check if credential already exists
.then((cred) => {
if (cred && cred.isActive) {
throw new Error('Credential already exists and is active'); // TODO: replace with validation error
}
return dereferencePromise;
})
.then((resolvedSchema) => {
const credentialConfig = resolvedSchema.properties[type];
const newCredential = { isActive: 'true' };
utils.appendCreatedAt(newCredential);
utils.appendUpdatedAt(newCredential);
// check if credential already exists
const checkSingleCredExistence = () => {
return this.getCredential(id, type)
.then(cred => {
if (cred && cred.isActive) {
throw new Error('Credential already exists and is active'); // TODO: replace with validation error
}
});
};

if (type === 'key-auth' || type === 'jwt') { // TODO: if/else is not a good approach, new way TBD
return Promise.all([
validateNewCredentialScopes(credentialConfig, credentialDetails),
validateNewCredentialProperties(credentialConfig, credentialDetails)
]).then(([scopes, credentialProps]) => {
Object.assign(newCredential, credentialProps);
newCredential.keyId = credentialDetails.keyId || uuid62.v4();
newCredential.keySecret = credentialDetails.keySecret || uuid62.v4();
newCredential.scopes = JSON.stringify(scopes);
newCredential.consumerId = id;

return Promise.all([
credentialDao.insertCredential(newCredential.keyId, type, newCredential),
credentialDao.associateCredentialWithScopes(newCredential.keyId, type, scopes)
]);
}).then(() => {
if (newCredential.scopes && newCredential.scopes.length > 0) {
newCredential.scopes = JSON.parse(newCredential.scopes);
}

if (typeof newCredential.isActive === 'string') {
newCredential.isActive = newCredential.isActive === 'true';
}
return newCredential;
});
}
// TODO: not a good approach, new way TBD
const areMultipleCredsAllowed = ['key-auth', 'jwt'].includes(type);
const flow = areMultipleCredsAllowed
? dereferencePromise
: checkSingleCredExistence().then(() => dereferencePromise);

return flow.then(resolvedSchema => {
const credentialConfig = resolvedSchema.properties[type];
const newCredential = { isActive: 'true' };
utils.appendCreatedAt(newCredential);
utils.appendUpdatedAt(newCredential);

if (areMultipleCredsAllowed) {
return Promise.all([
validateNewCredentialScopes(credentialConfig, credentialDetails),
validateAndHashPassword(credentialConfig, credentialDetails),
validateNewCredentialProperties(credentialConfig, credentialDetails)
])
.then(([scopes, { hash, password }, credentialProps]) => {
if (scopes) {
newCredential['scopes'] = JSON.stringify(scopes);
}
newCredential[credentialConfig.properties.passwordKey.default] = hash;
delete credentialProps[credentialConfig.properties.passwordKey.default];

Object.assign(newCredential, credentialProps);

return Promise.all([
password,
credentialDao.insertCredential(id, type, newCredential),
credentialDao.associateCredentialWithScopes(id, type, scopes)
]);
}).then(([password]) => {
const credential = newCredential;
delete credential[credentialConfig.properties.passwordKey.default];
credential.id = id;

if (password) {
credential[credentialConfig.properties.passwordKey.default] = password;
}

if (credential.scopes && credential.scopes.length > 0) {
credential.scopes = JSON.parse(credential.scopes);
}

if (typeof credential.isActive === 'string') {
credential.isActive = credential.isActive === 'true';
}
return credential;
});
});
]).then(([scopes, credentialProps]) => {
Object.assign(newCredential, credentialProps);
newCredential.keyId = credentialDetails.keyId || uuid62.v4();
newCredential.keySecret = credentialDetails.keySecret || uuid62.v4();
newCredential.scopes = JSON.stringify(scopes);
newCredential.consumerId = id;

return Promise.all([
credentialDao.insertCredential(newCredential.keyId, type, newCredential),
credentialDao.associateCredentialWithScopes(newCredential.keyId, type, scopes)
]);
}).then(() => {
if (newCredential.scopes && newCredential.scopes.length > 0) {
newCredential.scopes = JSON.parse(newCredential.scopes);
}

if (typeof newCredential.isActive === 'string') {
newCredential.isActive = newCredential.isActive === 'true';
}
return newCredential;
});
}

return Promise.all([
validateNewCredentialScopes(credentialConfig, credentialDetails),
validateAndHashPassword(credentialConfig, credentialDetails),
validateNewCredentialProperties(credentialConfig, credentialDetails)
])
.then(([scopes, { hash, password }, credentialProps]) => {
if (scopes) {
newCredential['scopes'] = JSON.stringify(scopes);
}
newCredential[credentialConfig.properties.passwordKey.default] = hash;
delete credentialProps[credentialConfig.properties.passwordKey.default];

Object.assign(newCredential, credentialProps);

return Promise.all([
password,
credentialDao.insertCredential(id, type, newCredential),
credentialDao.associateCredentialWithScopes(id, type, scopes)
]);
}).then(([password]) => {
const credential = newCredential;
delete credential[credentialConfig.properties.passwordKey.default];
credential.id = id;

if (password) {
credential[credentialConfig.properties.passwordKey.default] = password;
}

if (credential.scopes && credential.scopes.length > 0) {
credential.scopes = JSON.parse(credential.scopes);
}

if (typeof credential.isActive === 'string') {
credential.isActive = credential.isActive === 'true';
}
return credential;
});
});
};

s.getCredential = function (id, type, options) {
Expand Down
149 changes: 86 additions & 63 deletions test/services/credential-service/credentials.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,97 +6,120 @@ const credentialService = services.credential;
const userService = services.user;
const db = require('../../../lib/db');

describe('Credential service tests', () => {
describe('Credential tests', () => {
let credential;
const username = 'someUser';
describe('Credential tests', () => {
const username = 'someUser';
const credential = {
secret: 'password'
};

before(() => db.flushdb());
function insertCredential(type) {
return credentialService.insertCredential(username, type, credential);
}

it('should insert a credential', () => {
const _credential = {
secret: 'password'
};
beforeEach(() => db.flushdb());

return credentialService
.insertCredential(username, 'oauth2', _credential)
.then((newCredential) => {
should.exist(newCredential);
credential = Object.assign(newCredential, _credential);
});
});
describe("'oauth2' specific cases", () => {
const type = 'oauth2';

it('should not insert a credential that already exists', () => {
const _credential = {
secret: 'password'
};

return should(credentialService.insertCredential(username, 'oauth2', _credential))
.be.rejected();
return insertCredential(type).then(newCredential => {
return should(insertCredential(type)).be.rejected();
});
});

it('should insert a credential without password specified if autoGeneratePassword is set to true', () => {
const _credential = {};

return credentialService
.insertCredential('someUsername', 'oauth2', _credential)
.then((newCredential) => {
.insertCredential(username, type, {})
.then(newCredential => {
should.exist(newCredential);
should.exist(newCredential.secret);
should(newCredential.secret.length).greaterThanOrEqual(10);
});
});

it('should insert a credential with id but different type that already exists', () => {
const _credential = {
password: 'password'
};

return credentialService
.insertCredential(username, 'basic-auth', _credential)
.then((newCredential) => {
return insertCredential(type)
.then(() => credentialService.insertCredential(username, 'basic-auth', credential))
.then(newCredential => {
should.exist(newCredential);
newCredential.isActive.should.eql(true);
});
});
});

it('should get a credential', () => {
return credentialService
.getCredential(username, 'oauth2')
.then((cred) => {
should.exist(cred);
should.not.exist(cred.secret);
credential.isActive.should.eql(true);
});
describe('support for multiple credentials', () => {
['key-auth', 'jwt'].forEach(type => {
it(type, () => {
return Promise.all([
insertCredential(type),
insertCredential(type)
])
.then(() => credentialService.getCredentials(username))
.then(results => {
results.should.be.instanceOf(Array).and.have.length(2);
});
});
});
});

it('should deactivate a credential', () => {
return credentialService
.deactivateCredential(username, 'oauth2')
.then(() => credentialService.getCredential(username, 'oauth2'))
.then((cred) => {
should.exist(cred);
should.not.exist(cred.secret);
cred.isActive.should.eql(false);
});
});
const tests = [
{ type: 'oauth2', passwordKey: 'secret' },
{ type: 'basic-auth', passwordKey: 'password' },
{ type: 'key-auth' },
{ type: 'jwt' }
];

tests.forEach(({ type, passwordKey }) => {
describe(`credential type: '${type}'`, () => {
it('should insert a credential', () => {
return insertCredential(type);
});

it('should reactivate a credential', () => {
return credentialService
.activateCredential(username, 'oauth2')
.then((res) => {
should.exist(res);
it("should get a credential and strip 'passwordKey' props", () => {
return insertCredential(type)
.then(credential => credentialService.getCredential(credential.id, type))
.then(credential => {
should.exist(credential);
if (passwordKey) {
should.not.exist(credential.passwordKey);
should.not.exist(credential[passwordKey]);
};
credential.isActive.should.eql(true);
});
});

return credentialService.getCredential(username, 'oauth2');
})
.then((cred) => {
should.exist(cred);
should.not.exist(cred.secret);
cred.isActive.should.eql(true);
});
it('should deactivate a credential', () => {
return insertCredential(type)
.then(credential => {
return credentialService.deactivateCredential(credential.id, type).then(res => {
should.exist(res);
return credentialService.getCredential(credential.id, type);
});
})
.then(credential => {
should.exist(credential);
credential.isActive.should.eql(false);
});
});

it('should reactivate a credential', () => {
return insertCredential(type)
.then(credential => {
return credentialService.activateCredential(credential.id, type).then(res => {
should.exist(res);
return credentialService.getCredential(credential.id, type);
});
})
.then(credential => {
should.exist(credential);
credential.isActive.should.eql(true);
});
});
});
});
});

describe('Credential service tests', () => {
describe('Credential Cascade Delete tests', () => {
let user;

Expand Down

0 comments on commit 4d720b4

Please sign in to comment.