Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closes Issue #2153 - Limit how a user becomes an admin #2153 #2239

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions config/export/firebase-export-metadata.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did this file, and the other config/export/* files, get added?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This and the change to development.yml add a single user to the emulator up its bootup. This user as the isAdmin flag set to true, as without this we'd have no way to make a user an admin during testing.

The problem stems from having no way of creating an admin without first having an admin to create an admin.

"version": "9.10.2",
"firestore": {
"version": "1.11.15",
"path": "firestore_export",
"metadata_file": "firestore_export/firestore_export.overall_export_metadata"
}
}
Binary file not shown.
Binary file not shown.
Binary file not shown.
3 changes: 2 additions & 1 deletion docker/development.yml
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,8 @@ services:
volumes:
# Copy firebase.config into the container so we get proper ip/port binding
- ../config/firebase.json:/home/node/firebase.json
command: firebase emulators:start --project telescope --only firestore
- ../config/export:/export
command: firebase emulators:start --project telescope --only firestore --import=/export
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this change?

ports:
# Emulator Suite UI
- '4000:4000'
Expand Down
41 changes: 41 additions & 0 deletions src/api/users/src/models/schema.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
const { createError, hash } = require('@senecacdot/satellite');
const { celebrate, Joi, Segments } = require('celebrate');
const { User } = require('../models/user');
const { db } = require('../services/firestore');

const validatePagingParams = () =>
celebrate({
Expand All @@ -16,6 +18,42 @@ const validateId = () =>
},
});

const validateAdminStatus = () =>
celebrate({
[Segments.PARAMS]: {
id: Joi.string().hex().required(),
adminStatus: Joi.boolean().required(),
},
});

// no user can be created as an admin by default
const validateUserRights = () => (req, res, next) => {
req.body.isAdmin = false;
next();
};

const updateUser = (makeAdmin = false) => async (req, res, next) => {
const { id, adminStatus } = req.params;
const { body } = req;

try {
const userRef = db.doc(id);
const doc = await userRef.get();

if (!doc.exists) {
next(createError(404, `user ${id} not found.`));
} else {
// if makeAdmin is true turn the user into an admin, if not use body to update user.
const user = new User(makeAdmin ? { ...body, isAdmin: adminStatus } : body);
// NOTE: doc().update() doesn't use the converter, we have to make a plain object.
await db.doc(id).update(user.toJSON());
res.status(200).json({ msg: `Updated user ${id}` });
}
} catch (err) {
next(err);
}
};

// Generate a display name if none given
const generateDisplayName = (parent) => `${parent.firstName} ${parent.lastName}`;

Expand Down Expand Up @@ -52,5 +90,8 @@ const validateEmailHash = () => (req, res, next) => {

exports.validateUser = validateUser;
exports.validateId = validateId;
exports.validateAdminStatus = validateAdminStatus;
exports.validateUserRights = validateUserRights;
exports.validateEmailHash = validateEmailHash;
exports.validatePagingParams = validatePagingParams;
exports.updateUser = updateUser;
38 changes: 19 additions & 19 deletions src/api/users/src/routes/users.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,11 @@ const { errors } = require('celebrate');
const {
validatePagingParams,
validateId,
validateAdminStatus,
validateUser,
validateEmailHash,
validateUserRights,
updateUser,
} = require('../models/schema');
const { User } = require('../models/user');
const { db, documentId } = require('../services/firestore');
Expand Down Expand Up @@ -90,6 +93,7 @@ router.post(
isAuthenticated(),
validateId(),
validateUser(),
validateUserRights(),
validateEmailHash(),
isAuthorized((req, user) => {
// A user can add their own data (signup)
Expand Down Expand Up @@ -128,6 +132,7 @@ router.put(
validateId(),
validateUser(),
validateEmailHash(),
validateUserRights(),
isAuthorized((req, user) => {
// A user can update their own data
if (user.sub === req.params.id) {
Expand All @@ -136,26 +141,21 @@ router.put(
// Or an admin, or another authorized microservice
return user.roles.includes('admin') || user.roles.includes('service');
}),
async (req, res, next) => {
const { id } = req.params;
const { body } = req;

try {
const userRef = db.doc(id);
const doc = await userRef.get();
updateUser()
);

if (!doc.exists) {
next(createError(404, `user ${id} not found.`));
} else {
const user = new User(body);
// NOTE: doc().update() doesn't use the converter, we have to make a plain object.
await db.doc(id).update(user.toJSON());
res.status(200).json({ msg: `Updated user ${id}` });
}
} catch (err) {
next(err);
}
}
// put (update) a user with a supplied id, validated by the schema
// rejected if a user could not be found, otherwise user updated
// this route is only accessible by administrators
// it allows the modification of the isAdmin property
router.put(
'/:id/admin/:adminStatus',
isAuthenticated(),
validateAdminStatus(),
validateUser(),
validateEmailHash(),
isAuthorized((req, user) => user.roles.includes('admin')),
updateUser(true)
);

// delete a user with a supplied id, validated by the schema
Expand Down