Skip to content

Commit f527a44

Browse files
committed
Additional restrictions for non-elevated users in privacy mode
1 parent 5049071 commit f527a44

File tree

9 files changed

+135
-15
lines changed

9 files changed

+135
-15
lines changed

app/src/components/constants.js

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,14 @@ module.exports = Object.freeze({
2424
NONE: 'NONE'
2525
},
2626

27+
/**
28+
* In COMS 'strict mode', users specific with token idp's (eg: idir)
29+
* are given additional permissions.
30+
*/
31+
ElevatedIdps: [
32+
'idir'
33+
],
34+
2735
/** Default CORS settings used across the entire application */
2836
DEFAULTCORS: {
2937
/** Tells browsers to cache preflight requests for Access-Control-Max-Age seconds */

app/src/controllers/bucket.js

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -194,11 +194,24 @@ const controller = {
194194
await controller._validateCredentials(childBucket);
195195
childBucket.userId = await userService.getCurrentUserId(getCurrentIdentity(req.currentUser, SYSTEM_USER));
196196

197-
// assign all permissions
198-
childBucket.permCodes = Object.values(Permissions);
197+
198+
// if non-elevated user, exclude MANAGE and DELETE permission
199+
// ---------------------
200+
// childBucket.permCodes = Object.values(Permissions);
199201

200202
// Create child bucket
201203
const response = await bucketService.create(childBucket);
204+
205+
206+
/**
207+
* copy all permissions from parent to child
208+
* exclude MANAGE and DELETE of external users
209+
*
210+
* we also need a migration to give top-level bucket creator, if idir, the MANAGE permission on all sub-folders.
211+
* or use the recursive sync flow
212+
*/
213+
214+
202215
res.status(201).json(redactSecrets(response, secretFields));
203216
} catch (e) {
204217
next(errorToProblem(SERVICE, e));

app/src/middleware/authorization.js

Lines changed: 59 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
const Problem = require('api-problem');
22

33
const log = require('../components/log')(module.filename);
4-
const { AuthMode, AuthType, Permissions } = require('../components/constants');
4+
const { AuthMode, AuthType, Permissions, ElevatedIdps } = require('../components/constants');
55
const {
66
getAppAuthMode,
77
getCurrentIdentity,
@@ -223,7 +223,7 @@ const restrictNonIdirUserSearch = async (req, _res, next) => {
223223
try {
224224
if (getConfigBoolean('server.privacyMask') &&
225225
req.currentUser.authType === AuthType.BEARER &&
226-
req.currentUser.tokenPayload.identity_provider !== 'idir' &&
226+
!ElevatedIdps.includes(req.currentUser.tokenPayload.identity_provider) &&
227227
!hasOnlyPermittedKeys(req.query, ['email', 'userId', 'identityId'])
228228
) {
229229
throw new Error('User lacks permission to complete this action');
@@ -243,10 +243,66 @@ const restrictNonIdirUserSearch = async (req, _res, next) => {
243243

244244
next();
245245
};
246+
247+
/**
248+
* If privacyMask (soon to be renamed as 'strictMode' to support additional feature restroctions)
249+
* is true and request is from a non-idir user, throw a permission error
250+
*/
251+
const checkElevatedUser = async (req, _res, next) => {
252+
try {
253+
if (getConfigBoolean('server.privacyMask') &&
254+
req.currentUser.authType === AuthType.BEARER &&
255+
!ElevatedIdps.includes(req.currentUser.tokenPayload.identity_provider)) {
256+
throw new Error('User lacks permission to complete this action');
257+
}
258+
}
259+
catch (err) {
260+
log.verbose(err.message, { function: 'checkElevatedUser' });
261+
return next(new Problem(403, {
262+
detail: err.message,
263+
instance: req.originalUrl
264+
}));
265+
}
246266
next();
247267
};
248268

269+
/**
270+
* If 'strict mode' is enabled
271+
* deny non-elevated user's the MANAGE and DELETE permission
272+
* probably not going to bother with this. just block making folder public
273+
*/
274+
// const checkGrantingPermittedPermissions = async (req, _res, next) => {
275+
// try {
276+
// if (getConfigBoolean('server.privacyMask')) {
277+
// // check if subject is a non-elevated user
278+
// const managePerms = req.body
279+
// .map(p => ({ ...p, permCode: p.permCode.toUpperCase().trim() }))
280+
// .filter(obj => obj.permCode === Permissions.MANAGE || obj.permCode === Permissions.DELETE);
281+
// for (const perm of managePerms) {
282+
// const user = await userService.readUser(perm.userId);
283+
// if (!ElevatedIdps.includes(user.idp)) {
284+
// throw new Error(`Subject is not permitted to have ${perm.permCode} permission`);
285+
// }
286+
// }
287+
// }
288+
// }
289+
// catch (err) {
290+
// log.verbose(err.message, { function: 'checkGrantingPermittedPermissions' });
291+
// return next(new Problem(403, {
292+
// detail: err.message,
293+
// instance: req.originalUrl
294+
// }));
295+
// }
296+
// next();
297+
// };
298+
249299
module.exports = {
300+
_checkPermission,
301+
checkAppMode,
302+
checkS3BasicAccess,
303+
currentObject,
304+
hasPermission,
250305
restrictNonIdirUserSearch,
251-
isElevatedUser
306+
checkElevatedUser,
307+
// checkGrantingPermittedPermissions
252308
};

app/src/routes/v1/bucket.js

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,12 @@ const { isTruthy } = require('../../components/utils');
66
const { bucketController, syncController } = require('../../controllers');
77
const { bucketValidator } = require('../../validators');
88
const { requireSomeAuth } = require('../../middleware/featureToggle');
9-
const { checkAppMode, hasPermission, checkS3BasicAccess } = require('../../middleware/authorization');
9+
const {
10+
checkAppMode,
11+
hasPermission,
12+
checkS3BasicAccess,
13+
checkElevatedUser
14+
} = require('../../middleware/authorization');
1015

1116
router.use(checkAppMode);
1217
router.use(requireSomeAuth);
@@ -16,6 +21,7 @@ router.put('/',
1621
express.json(),
1722
bucketValidator.createBucket,
1823
checkS3BasicAccess,
24+
checkElevatedUser,
1925
(req, res, next) => {
2026
bucketController.createBucket(req, res, next);
2127
});
@@ -55,6 +61,7 @@ router.patch('/:bucketId',
5561
express.json(),
5662
bucketValidator.updateBucket,
5763
checkS3BasicAccess,
64+
checkElevatedUser,
5865
hasPermission(Permissions.MANAGE),
5966
(req, res, next) => {
6067
bucketController.updateBucket(req, res, next);
@@ -65,17 +72,22 @@ router.patch('/:bucketId',
6572
router.delete('/:bucketId',
6673
bucketValidator.deleteBucket,
6774
checkS3BasicAccess,
75+
checkElevatedUser,
6876
hasPermission(Permissions.DELETE),
6977
(req, res, next) => {
7078
bucketController.deleteBucket(req, res, next);
7179
});
7280

73-
/** Creates a child bucket */
81+
/**
82+
* Creates a child bucket
83+
* Note: operation requires CREATE permission on parent
84+
* */
7485
router.put('/:bucketId/child',
7586
express.json(),
7687
bucketValidator.createBucketChild,
7788
checkS3BasicAccess,
78-
hasPermission(Permissions.MANAGE),
89+
checkElevatedUser,
90+
hasPermission(Permissions.CREATE),
7991
(req, res, next) => {
8092
bucketController.createBucketChild(req, res, next);
8193
}
@@ -94,6 +106,7 @@ router.get('/:bucketId/sync',
94106
if (isTruthy(req.query.recursive)) next();
95107
else next('route');
96108
},
109+
checkElevatedUser,
97110
hasPermission(Permissions.MANAGE),
98111
(req, res, next) => syncController.syncBucketRecursive(req, res, next));
99112

app/src/routes/v1/object.js

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,13 @@ const helmet = require('helmet');
44
const { Permissions } = require('../../components/constants');
55
const { objectController, syncController } = require('../../controllers');
66
const { objectValidator } = require('../../validators');
7-
const { checkAppMode, checkS3BasicAccess, currentObject, hasPermission } = require('../../middleware/authorization');
7+
const {
8+
checkAppMode,
9+
checkS3BasicAccess,
10+
currentObject,
11+
hasPermission,
12+
checkElevatedUser
13+
} = require('../../middleware/authorization');
814
const { requireSomeAuth } = require('../../middleware/featureToggle');
915
const { currentUpload } = require('../../middleware/upload');
1016

@@ -110,6 +116,7 @@ router.get('/:objectId/version',
110116

111117
/** creates a new version of an object using either a specified version or latest as the source */
112118
router.put('/:objectId/version',
119+
requireSomeAuth,
113120
objectValidator.copyVersion,
114121
currentObject,
115122
checkS3BasicAccess,
@@ -124,6 +131,7 @@ router.patch('/:objectId/public',
124131
objectValidator.togglePublic,
125132
currentObject,
126133
checkS3BasicAccess,
134+
checkElevatedUser,
127135
hasPermission(Permissions.MANAGE),
128136
(req, res, next) => {
129137
objectController.togglePublic(req, res, next);
@@ -202,7 +210,7 @@ router.put('/:objectId/tagging',
202210
}
203211
);
204212

205-
/** Add tags to an object */
213+
/** Delete tags from an object */
206214
router.delete('/:objectId/tagging',
207215
requireSomeAuth,
208216
objectValidator.deleteTags,

app/src/routes/v1/permission/bucketPermission.js

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,14 @@ const router = express.Router();
44
const { Permissions } = require('../../../components/constants');
55
const { bucketPermissionController } = require('../../../controllers');
66
const { bucketPermissionValidator } = require('../../../validators');
7-
const { checkAppMode, currentObject, checkS3BasicAccess, hasPermission } = require('../../../middleware/authorization');
7+
const {
8+
checkAppMode,
9+
currentObject,
10+
checkS3BasicAccess,
11+
hasPermission,
12+
checkElevatedUser,
13+
// checkGrantingPermittedPermissions
14+
} = require('../../../middleware/authorization');
815
const { requireSomeAuth } = require('../../../middleware/featureToggle');
916

1017
router.use(checkAppMode);
@@ -35,6 +42,9 @@ router.put('/:bucketId',
3542
bucketPermissionValidator.addPermissions,
3643
currentObject,
3744
checkS3BasicAccess,
45+
checkElevatedUser,
46+
// probably not going to bother with this. just block making folder public
47+
// checkGrantingPermittedPermissions,
3848
hasPermission(Permissions.MANAGE),
3949
(req, res, next) => {
4050
bucketPermissionController.addPermissions(req, res, next);
@@ -46,6 +56,7 @@ router.delete('/:bucketId',
4656
bucketPermissionValidator.removePermissions,
4757
currentObject,
4858
checkS3BasicAccess,
59+
checkElevatedUser,
4960
hasPermission(Permissions.MANAGE),
5061
(req, res, next) => {
5162
bucketPermissionController.removePermissions(req, res, next);

app/src/routes/v1/permission/invite.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ const router = express.Router();
33

44
const { inviteController } = require('../../../controllers');
55
const { inviteValidator } = require('../../../validators');
6-
const { checkS3BasicAccess } = require('../../../middleware/authorization');
6+
const { checkS3BasicAccess, checkElevatedUser } = require('../../../middleware/authorization');
77
const { requireBearerAuth, requireSomeAuth } = require('../../../middleware/featureToggle');
88

99
router.use(requireSomeAuth);
@@ -13,6 +13,7 @@ router.post('/',
1313
express.json(),
1414
inviteValidator.createInvite,
1515
checkS3BasicAccess,
16+
checkElevatedUser,
1617
(req, res, next) => {
1718
inviteController.createInvite(req, res, next);
1819
});

app/src/routes/v1/permission/objectPermission.js

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,14 @@ const router = express.Router();
44
const { Permissions } = require('../../../components/constants');
55
const { objectPermissionController } = require('../../../controllers');
66
const { objectPermissionValidator } = require('../../../validators');
7-
const { checkAppMode, currentObject, checkS3BasicAccess, hasPermission } = require('../../../middleware/authorization');
7+
const {
8+
checkAppMode,
9+
currentObject,
10+
checkS3BasicAccess,
11+
hasPermission,
12+
checkElevatedUser,
13+
// checkGrantingPermittedPermissions
14+
} = require('../../../middleware/authorization');
815
const { requireSomeAuth } = require('../../../middleware/featureToggle');
916

1017
router.use(checkAppMode);
@@ -35,6 +42,8 @@ router.put('/:objectId',
3542
objectPermissionValidator.addPermissions,
3643
currentObject,
3744
checkS3BasicAccess,
45+
checkElevatedUser,
46+
// checkGrantingPermittedPermissions,
3847
hasPermission(Permissions.MANAGE),
3948
(req, res, next) => {
4049
objectPermissionController.addPermissions(req, res, next);
@@ -46,6 +55,7 @@ router.delete('/:objectId',
4655
objectPermissionValidator.removePermissions,
4756
currentObject,
4857
checkS3BasicAccess,
58+
checkElevatedUser,
4959
hasPermission(Permissions.MANAGE),
5060
(req, res, next) => {
5161
objectPermissionController.removePermissions(req, res, next);

app/src/services/bucketPermission.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,12 +33,12 @@ const service = {
3333
const currentPerms = await service.searchPermissions({ bucketId });
3434
const obj = data
3535
// Ensure all codes are upper cased
36-
.map(p => ({ ...p, code: p.permCode.toUpperCase().trim() }))
36+
.map(p => ({ ...p, permCode: p.permCode.toUpperCase().trim() }))
3737
// Filter out any invalid code values
3838
.filter(p => Object.values(Permissions).some(perm => perm === p.permCode))
3939
// Filter entry tuples that already exist
4040
.filter(p => !currentPerms.some(cp => cp.userId === p.userId && cp.permCode === p.permCode))
41-
// Create DB buckets to insert
41+
// Create DB records to insert
4242
.map(p => ({
4343
id: uuidv4(),
4444
userId: p.userId,

0 commit comments

Comments
 (0)