Bläddra i källkod

refactor: improve getManyById functionality with permissions, add it to more models

Kristian Vos 1 år sedan
förälder
incheckning
c214e5f79a

+ 4 - 1
backend/src/JobContext.ts

@@ -122,6 +122,9 @@ export default class JobContext {
 			hasPermission = permissions[permission];
 		}
 
-		if (!hasPermission) throw new Error("Insufficient permissions");
+		if (!hasPermission)
+			throw new Error(
+				`Insufficient permissions for permission ${permission}`
+			);
 	}
 }

+ 37 - 11
backend/src/modules/DataModule/DataModuleJob.ts

@@ -1,11 +1,11 @@
-import { HydratedDocument, Model, isObjectIdOrHexString } from "mongoose";
-import { forEachIn } from "@common/utils/forEachIn";
+import { HydratedDocument, Model, isValidObjectId } from "mongoose";
 import Job, { JobOptions } from "@/Job";
 import DataModule from "../DataModule";
 import { UserSchema } from "./models/users/schema";
 
 export default abstract class DataModuleJob extends Job {
 	protected static _modelName: string;
+	protected static _isBulk: boolean = false;
 
 	protected static _hasPermission:
 		| boolean
@@ -34,6 +34,14 @@ export default abstract class DataModuleJob extends Job {
 		return (this.constructor as typeof DataModuleJob)._modelName;
 	}
 
+	public static isBulk() {
+		return this._isBulk;
+	}
+
+	public isBulk() {
+		return (this.constructor as typeof DataModuleJob)._isBulk;
+	}
+
 	public static async hasPermission(
 		model: HydratedDocument<Model<any>>,
 		user: HydratedDocument<UserSchema> | null
@@ -54,22 +62,40 @@ export default abstract class DataModuleJob extends Job {
 	}
 
 	protected override async _authorize() {
-		const modelId = this._payload?._id;
-
-		if (isObjectIdOrHexString(modelId)) {
-			await this._context.assertPermission(
-				`${this.getPath()}.${modelId}`
+		const modelIds = this._payload?.modelIds ?? this._payload?._ids;
+
+		// If this job is a bulk job, and all model ids are valid object ids
+		if (this.isBulk()) {
+			if (modelIds.some((modelId: unknown) => !isValidObjectId(modelId)))
+				throw new Error(
+					`One or more model id is invalid: ${modelIds
+						.filter((modelId: unknown) => !isValidObjectId(modelId))
+						.join(", ")}`
+				);
+
+			const permissions = modelIds.map(
+				(modelId: string) => `${this.getPath()}.${modelId}`
+			);
+			await Promise.all(
+				permissions.map((permission: string) =>
+					this._context.assertPermission(permission)
+				)
 			);
 
 			return;
 		}
 
-		const modelIds = this._payload?.modelIds;
+		const modelId = this._payload?.modelId ?? this._payload?._id;
 
-		if (Array.isArray(modelIds)) {
-			await forEachIn(modelIds, async _id =>
-				this._context.assertPermission(`${this.getPath()}.${_id}`)
+		// If this job is not a bulk job, and the model id is a valid object id
+		if (!this.isBulk() && modelId) {
+			if (!isValidObjectId(modelId))
+				throw new Error(`Model id is invalid: ${modelId}`);
+			await this._context.assertPermission(
+				`${this.getPath()}.${modelId}`
 			);
+
+			return;
 		}
 
 		await this._context.assertPermission(this.getPath());

+ 2 - 0
backend/src/modules/DataModule/FindManyByIdJob.ts

@@ -3,6 +3,8 @@ import DataModule from "../DataModule";
 import DataModuleJob from "./DataModuleJob";
 
 export default abstract class FindManyByIdJob extends DataModuleJob {
+	protected static _isBulk: boolean = true;
+
 	protected override async _validate() {
 		if (typeof this._payload !== "object" || this._payload === null)
 			throw new Error("Payload must be an object");

+ 5 - 0
backend/src/modules/DataModule/models/abc/jobs/FindManyById.ts

@@ -0,0 +1,5 @@
+import FindManyByIdJob from "@/modules/DataModule/FindManyByIdJob";
+
+export default class FindManyById extends FindManyByIdJob {
+	protected static _modelName = "abc";
+}

+ 7 - 0
backend/src/modules/DataModule/models/news/jobs/FindManyById.ts

@@ -0,0 +1,7 @@
+import FindManyByIdJob from "@/modules/DataModule/FindManyByIdJob";
+
+export default class FindManyById extends FindManyByIdJob {
+	protected static _modelName = "news";
+
+	protected static _hasPermission = true;
+}

+ 5 - 0
backend/src/modules/DataModule/models/sessions/jobs/FindManyById.ts

@@ -0,0 +1,5 @@
+import FindManyByIdJob from "@/modules/DataModule/FindManyByIdJob";
+
+export default class FindManyById extends FindManyByIdJob {
+	protected static _modelName = "sessions";
+}

+ 11 - 0
backend/src/modules/DataModule/models/stations/jobs/FindManyById.ts

@@ -0,0 +1,11 @@
+import FindManyByIdJob from "@/modules/DataModule/FindManyByIdJob";
+import isDj from "@/modules/DataModule/permissions/isDj";
+import isPublic from "@/modules/DataModule/permissions/isPublic";
+import isUnlisted from "@/modules/DataModule/permissions/isUnlisted";
+import isOwner from "@/modules/DataModule/permissions/isOwner";
+
+export default class FindManyById extends FindManyByIdJob {
+	protected static _modelName = "stations";
+
+	protected static _hasPermission = [isOwner, isDj, isPublic, isUnlisted];
+}

+ 5 - 0
backend/src/modules/DataModule/models/users/jobs/FindManyById.ts

@@ -0,0 +1,5 @@
+import FindManyByIdJob from "@/modules/DataModule/FindManyByIdJob";
+
+export default class FindManyById extends FindManyByIdJob {
+	protected static _modelName = "users";
+}

+ 1 - 0
backend/src/modules/DataModule/models/users/jobs/GetModelPermissions.ts

@@ -53,6 +53,7 @@ export default class GetModelPermissions extends DataModuleJob {
 
 		if (!Model) throw new Error("Model not found");
 
+		// TODO when we have a findManyById or other bulk permission, we don't want to call this individually for each modelId
 		const model = modelId ? await Model.findById(modelId) : null;
 
 		if (modelId && !model) throw new Error("Model not found");

+ 3 - 0
backend/src/modules/DataModule/models/users/permissions.ts

@@ -23,6 +23,7 @@ const moderator = {
 	"data.news.create": true,
 	"data.news.getData": true,
 	"data.news.updateById.*": true,
+	"data.news.findManyById.*": true,
 
 	// DataModule playlists model
 	"data.playlists.addSongById.*": true,
@@ -57,6 +58,7 @@ const moderator = {
 	"data.stations.getData": true,
 	"data.stations.index.adminFilter": true,
 	"data.stations.updateById.*": true,
+	"data.stations.findManyById.*": true,
 
 	// DataModule users model
 	"data.users.banById.*": true,
@@ -65,6 +67,7 @@ const moderator = {
 	"data.users.requestPasswordResetById.*": !!config.get("mail.enabled"),
 	"data.users.resendVerifyEmailById.*": !!config.get("mail.enabled"),
 	"data.users.updateById.*": true,
+	// "data.users.findManyById.*": true,
 
 	// DataModule youtubeVideos model
 	"data.youtubeVideos.getData": true,