made it possible for a user to have same `sub` for multiple `azp`'s

This commit is contained in:
tigerbot 2017-11-03 16:32:09 -06:00
parent 19d153283e
commit f6fe7acaa3
3 changed files with 45 additions and 25 deletions

View File

@ -40,5 +40,28 @@ function checkIssuerToken(req, expectedSub) {
}); });
} }
async function getPrimarySub(grantStore, secondary) {
var results = await Promise.all([
grantStore.find({ sub: secondary }),
grantStore.find({ azpSub: secondary }),
]);
// Extract only the sub field from each row and reduce the duplicates so that each value
// can only appear in the final list once.
var subList = [].concat.apply([], results).map(grant => grant.sub);
subList = subList.filter((sub, ind, list) => list.indexOf(sub) === ind);
if (subList.length > 1) {
// This should not ever happen since there is a check for PPID collisions when saving
// grants, but it's probably better to have this check anyway just incase something
// happens that isn't currently accounted for.
console.error('acounts ' + JSON.stringify(subList) + ' are all associated with "'+secondary+'"');
throw new Error('PPID collision');
}
return subList[0];
}
module.exports.checkIssuerToken = checkIssuerToken; module.exports.checkIssuerToken = checkIssuerToken;
module.exports.getPrimarySub = getPrimarySub;
module.exports.makeB64UrlSafe = makeB64UrlSafe; module.exports.makeB64UrlSafe = makeB64UrlSafe;

View File

@ -56,14 +56,16 @@ function create(app) {
throw new OpErr("malformed request: 'sub' and 'scope' must be strings"); throw new OpErr("malformed request: 'sub' and 'scope' must be strings");
} }
return req.Store.find({ azpSub: req.body.sub }); return require('./common').getPrimarySub(req.Store, req.body.sub).catch(function (err) {
}).then(function (existing) { if (/collision/.test(err.message)) {
if (existing.length) { err.message = 'pre-existing PPID collision detected';
if (existing.length > 1) {
throw new OpErr("pre-existing PPID collision detected");
} else if (existing[0].sub !== req.params.sub || existing[0].azp !== req.params.azp) {
throw new OpErr("PPID collision detected, cannot save authorized party's sub");
} }
throw err;
});
}).then(function (primSub) {
if (primSub && primSub !== req.params.sub) {
console.log('account "'+req.params.sub+'" cannot use PPID "'+req.body.sub+'" already used by "'+primSub+'"');
throw new OpErr("PPID collision detected, cannot save authorized party's sub");
} }
var grant = { var grant = {

31
jwks.js
View File

@ -62,26 +62,21 @@ function create(app) {
} }
// The keys are stored by the issuer PPID, but the sub we have might be a different PPID // The keys are stored by the issuer PPID, but the sub we have might be a different PPID
// for a 3rd party. As such we need to look up the subject in the grants to get the issuer // for a 3rd party.
// PPID and to make sure there wasn't some kind of error that allowed multiple users to var issuerSub;
// somehow use the same PPID. try {
var results = await Promise.all([ issuerSub = await require('./common').getPrimarySub(store.IssuerOauth3OrgGrants, req.params.sub);
store.IssuerOauth3OrgGrants.find({ sub: req.params.sub }), } catch (err) {
store.IssuerOauth3OrgGrants.find({ azpSub: req.params.sub }), if (/collision/.test(err.message)) {
]); err.message = 'PPID collision - unable to safely retrieve keys';
var subList = [].concat.apply([], results).map(grant => grant.sub); }
subList = subList.filter((sub, ind, list) => list.indexOf(sub) === ind); throw err;
if (!subList.length) {
throw new OpErr("unknown PPID '" + req.params.sub + "'");
}
if (subList.length > 1) {
// This should not ever happen since there is a check for PPID collisions when saving
// grants, but it's probably better to have this check anyway just incase something
// happens that isn't currently accounted for.
throw new OpErr('PPID collision - unable to safely retrieve keys');
} }
return store.IssuerOauth3OrgJwks.get(subList[0] + '/' + req.params.kid); if (!issuerSub) {
throw new OpErr("unknown PPID '" + req.params.sub + "'");
}
return store.IssuerOauth3OrgJwks.get(issuerSub + '/' + req.params.kid);
} }
restful.get = function (req, res) { restful.get = function (req, res) {