Skip to content

Commit

Permalink
impr: optimize permissions middleware (@fehmer)
Browse files Browse the repository at this point in the history
  • Loading branch information
fehmer committed Aug 18, 2024
1 parent 46fa970 commit 8886a51
Show file tree
Hide file tree
Showing 5 changed files with 14 additions and 9 deletions.
2 changes: 1 addition & 1 deletion backend/__tests__/api/controllers/ape-key.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ const configuration = Configuration.getCachedConfiguration();
const uid = new ObjectId().toHexString();

describe("ApeKeyController", () => {
const getUserMock = vi.spyOn(UserDal, "getUser");
const getUserMock = vi.spyOn(UserDal, "getPartialUser");

beforeEach(async () => {
await enableApeKeysEndpoints(true);
Expand Down
2 changes: 1 addition & 1 deletion backend/src/api/routes/ape-keys.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ const commonMiddleware = [
},
invalidMessage: "ApeKeys are currently disabled.",
}),
checkUserPermissions({
checkUserPermissions(["canManageApeKeys"], {
criteria: (user) => {
return user.canManageApeKeys ?? true;
},
Expand Down
4 changes: 2 additions & 2 deletions backend/src/api/routes/quotes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { validateRequest } from "../../middlewares/validation";

const router = Router();

const checkIfUserIsQuoteMod = checkUserPermissions({
const checkIfUserIsQuoteMod = checkUserPermissions(["quoteMod"], {
criteria: (user) => {
return (
user.quoteMod === true ||
Expand Down Expand Up @@ -171,7 +171,7 @@ router.post(
captcha: withCustomMessages.regex(/[\w-_]+/).required(),
},
}),
checkUserPermissions({
checkUserPermissions(["canReport"], {
criteria: (user) => {
return user.canReport !== false;
},
Expand Down
2 changes: 1 addition & 1 deletion backend/src/api/routes/users.ts
Original file line number Diff line number Diff line change
Expand Up @@ -638,7 +638,7 @@ router.post(
captcha: withCustomMessages.regex(/[\w-_]+/).required(),
},
}),
checkUserPermissions({
checkUserPermissions(["canReport"], {
criteria: (user) => {
return user.canReport !== false;
},
Expand Down
13 changes: 9 additions & 4 deletions backend/src/middlewares/permission.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import _ from "lodash";
import MonkeyError from "../utils/error";
import type { Response, NextFunction, RequestHandler } from "express";
import { getUser } from "../dal/user";
import { getPartialUser } from "../dal/user";
import { isAdmin } from "../dal/admin-uids";
import type { ValidationOptions } from "./configuration";

Expand Down Expand Up @@ -34,8 +34,9 @@ export function checkIfUserIsAdmin(): RequestHandler {
* Check user permissions before handling request.
* Note that this middleware must be used after authentication in the middleware stack.
*/
export function checkUserPermissions(
options: ValidationOptions<MonkeyTypes.DBUser>
export function checkUserPermissions<K extends keyof MonkeyTypes.DBUser>(
fields: K[],
options: ValidationOptions<Pick<MonkeyTypes.DBUser, K>>
): RequestHandler {
const { criteria, invalidMessage = "You don't have permission to do this." } =
options;
Expand All @@ -48,7 +49,11 @@ export function checkUserPermissions(
try {
const { uid } = req.ctx.decodedToken;

const userData = await getUser(uid, "check user permissions");
const userData = await getPartialUser(
uid,
"check user permissions",
fields
);
const hasPermission = criteria(userData);

if (!hasPermission) {
Expand Down

0 comments on commit 8886a51

Please sign in to comment.