-
Notifications
You must be signed in to change notification settings - Fork 1
OUT-2847 | Segregate tasks services for public and web #1079
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
7025082 to
89c2439
Compare
| async canCreateSubTask(taskId: string): Promise<boolean> { | ||
| const parentPath = await this.getPathOfTask(taskId) | ||
| if (!parentPath) { | ||
| throw new APIError(httpStatus.NOT_FOUND, 'The requested parent task was not found') | ||
| } | ||
| const uuidLength = parentPath.split('.').length | ||
| if (!uuidLength) return true | ||
| return uuidLength <= maxSubTaskDepth | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We seem to be missing policy-level authorization here
| async getPathOfTask(id: string) { | ||
| return ( | ||
| await this.db.$queryRaw<{ path: string }[] | null>` | ||
| SELECT "path" | ||
| FROM "Tasks" | ||
| WHERE id::text = ${id} | ||
| AND "workspaceId" = ${this.user.workspaceId} | ||
| ` | ||
| )?.[0]?.path | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Assuming we use this method directly from controller, we are missing policy service authorization
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are not used directly from the controller. These are utilities used by the create task method from public and web api. The policy service authorization is already used in the main method calling these utilities. However, I modified the access scope of these methods to make them more secure. Thanks!
rrojan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🏆
- created a separate service file name public.service for tasks - created a separate abstract service which inherits base service and extended by PublicTasksService and TasksService. This file contains all the utilities shared by both of the services. - changed methods in public.controller for tasks - refactored tasks service
…rom both services using the template pattern
…pe of utility methods
Changes