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

Yashwanth_Backend_for_permission_log_table #1149

Merged
82 changes: 82 additions & 0 deletions src/controllers/logUserPermissionController.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
const moment = require('moment-timezone');
const UserProfile = require('../models/userProfile');
const UserPermissionChangeLog = require('../models/userPermissionChangeLog');
const PermissionChangeLog = require('../models/permissionChangeLog');

const logUserPermissionController = function () {
const logPermissionChanges = async function (req, res) {
try {
const {
actualUserProfile,
authUser,
userId,
existingPermissions,
addedPermissions,
removedPermissions,
Copy link
Contributor

Choose a reason for hiding this comment

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

I would avoid doing the added/removed/existing permission logic on the frontend. Only requesting newPermissions from the frontend should be enough to derive the info on the backend.

Can find 'existingPermissions'/'oldPermissions' from the DB to ensure the info is accurate.

addedPermissions = newPermissions.filter(x=> !oldPermissions.contains(x));
removedPermissions = oldPermissions.filter(x => !newPermissions.contains(x));

} = req.body;

const dateTime = moment().tz('America/Los_Angeles').format();

const logEntry = new UserPermissionChangeLog({
logDateTime: dateTime,
userId,
individualName: `${actualUserProfile.firstName} ${actualUserProfile.lastName}`,
permissions: existingPermissions,
permissionsAdded: addedPermissions,
permissionsRemoved: removedPermissions,
requestorRole: authUser.role,
requestorEmail: authUser.email,
});

await logEntry.save();
res.status(200).json({ message: 'Permission changes logged successfully' });
} catch (error) {
console.error('Error logging permission change:', error);
res.status(500).json({ error: 'Failed to log permission change' });
}
};

const getPermissionChangeLogs = async function (req, res) {
try {
const userProfile = await UserProfile.findOne({ _id: req.params.userId }).exec();

if (userProfile) {
if (userProfile.role !== 'Owner') {
res.status(204).send([]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this checking for permission to view the history? I think Admins might also want to see the history? not sure.
Could use await hasPermission(req.body.requestor, 'getPermissionHistory') or something (could use an existing permission or create a new one). There shouldn't be hardcoded references to specific roles if avoidable.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see that's just copied from getPermissionChangeLogs... I guess it is only Owners then. Would've been easier to find if this was modifying that file instead of creating a new file.

} else {
// Fetch logs from both collections
const userChangeLogs = await UserPermissionChangeLog.find();
const rolePermissionChangeLogs = await PermissionChangeLog.find();

const formattedUserChangeLogs = userChangeLogs.map(log => ({
...log.toObject(),
name: log.individualName,
}));

const formattedRolePermissionChangeLogs = rolePermissionChangeLogs.map(log => ({
...log.toObject(),
name: log.roleName,
}));

const mergedLogs = [...formattedUserChangeLogs, ...formattedRolePermissionChangeLogs].sort(
(a, b) => new Date(b.logDateTime) - new Date(a.logDateTime)
);

res.status(200).json(mergedLogs);
}
} else {
res.status(403).send(`User (${req.params.userId}) not found.`);
}
} catch (error) {
console.error('Error fetching permission change logs:', error);
res.status(500).json({ error: 'Failed to fetch permission change logs' });
}
};

return {
logPermissionChanges,
getPermissionChangeLogs,
};
};

module.exports = logUserPermissionController;
23 changes: 23 additions & 0 deletions src/models/userPermissionChangeLog.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
const mongoose = require('mongoose');

const { Schema } = mongoose;

const User = require('./userProfile');


const UserPermissionChangeLog = new Schema({
logDateTime: { type: String, required: true },
userId: {
type: mongoose.Types.ObjectId,
ref: User,
required: true,
},
individualName: { type: String },
permissions: { type: [String], required: true },
permissionsAdded: { type: [String], default: [] },
permissionsRemoved: { type: [String], default: [] },
requestorRole: { type: String },
requestorEmail: { type: String, required: true },
});

module.exports = mongoose.model('UserPermissionChangeLog', UserPermissionChangeLog, 'UserPermissionChangeLogs');
5 changes: 5 additions & 0 deletions src/routes/userProfileRouter.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ const { body } = require('express-validator');

const express = require('express');
const { ValidationError } = require('../utilities/errorHandling/customError');
const logUserPermissionController = require('../controllers/logUserPermissionController')();

const routes = function (userProfile, project) {
const controller = require('../controllers/userProfileController')(userProfile, project);
Expand Down Expand Up @@ -116,6 +117,10 @@ const routes = function (userProfile, project) {

userProfileRouter.route('/userProfile/teamCode/list').get(controller.getAllTeamCode);

userProfileRouter.post('/logPermissionChanges', logUserPermissionController.logPermissionChanges);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
userProfileRouter.post('/logPermissionChanges', logUserPermissionController.logPermissionChanges);

You shouldn't be able to edit history without making the edits. Instead, call logPermissionChanges from wherever the permissions are changed (putUserProfile() I think?). That unites both API calls into one and can guarantee that the updates are atomic.

Uniting the API calls makes it so devs testing with Postman don't have to make multiple API calls to make the history accurate and ensures that they can't make API calls to give false update history. You currently don't check for requestor permissions in logPermissionChanges, so anyone could add false information with this API using something like Postman.

userProfileRouter.get('/logPermissionChanges/:userId', logUserPermissionController.getPermissionChangeLogs);


return userProfileRouter;
};

Expand Down
Loading