Skip to content

Commit

Permalink
fix: avoid failure when inserting another credential for a user
Browse files Browse the repository at this point in the history
resolves ExpressGateway#856
insertCredential will call dao's getAllCredentials() which can handle
key-auth and jwt type credentials that were affected by the bug
some extra error handling were also added
  • Loading branch information
Z authored and XVincentX committed Jan 3, 2019
1 parent a64ca42 commit 210f480
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 4 deletions.
13 changes: 12 additions & 1 deletion lib/services/credentials/credential.dao.js
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,18 @@ dao.insertCredential = function (id, type, credentialObj) {
};

dao.getCredential = function (id, type) {
return db.hgetall(buildIdKey(type, id)).then(credential => {
const key = buildIdKey(type, id);
return db.type(key).then(type => {
if (type === 'hash') {
return db.hgetall(key)
} else if (type === 'set') {
throw Error(`Implementation error: ${key} is a set, therefore getAllCredentials() should be called instead of getCredential()`);
} else if (type === 'none') {
return null;
} else {
throw Error(`Corrupt data found at ${key}`);
}
}).then(credential => {
if (!credential || Object.keys(credential).length === 0) return null;
credential.isActive = credential.isActive === 'true'; // Redis has no bool type, manual conversion
credential.type = type;
Expand Down
20 changes: 17 additions & 3 deletions lib/services/credentials/credential.service.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,24 @@ 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
return credentialDao.getAllCredentials(id) // 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
if (Array.isArray(cred)) {
cred.forEach((c) => {
if (c && typeof c.isActive !== 'boolean') {
throw new Error('Failed to get existing credentials'); // TODO: replace with validation error
}
if (c && c.isActive) {
throw new Error(`Credential ${c.keyId} already exists and is active`); // TODO: replace with validation error
}
});
} else {
if (cred && typeof cred.isActive !== 'boolean') {
throw new Error('Failed to get existing credentials'); // TODO: replace with validation error
}
if (cred && cred.isActive) {
throw new Error(`Credential ${cred.keyId} already exists and is active`); // TODO: replace with validation error
}
}
return dereferencePromise;
})
Expand Down

0 comments on commit 210f480

Please sign in to comment.