fix(api): Add NotFound exception on passing an invalid roleId while inviting user in workspace

[PriyobrotoKar]

Description

Added validation to ensure that If a workspace role is provided in the roleIds array doesn't exist, a NotFoundException is thrown.

Fixes #405

Dependencies

No new dependencies were added.

Future Improvements

Mention any improvements to be done in future related to any file/feature

Mentions

Mention and tag the people

Screenshots of relevant screens

Add screenshots of relevant screens

Developer's checklist

• [x] My PR follows the style guidelines of this project
• [x] I have performed a self-check on my work

If changes are made in the code:

• [x] I have followed the coding guidelines
• [x] My changes in code generate no new warnings
• [ ] My changes are breaking another fix/feature of the project
• [ ] I have added test cases to show that my feature works
• [ ] I have added relevant screenshots in my PR
• [ ] There are no UI/UX issues

Documentation Update

• [ ] This PR requires an update to the documentation at docs.keyshade.xyz
• [ ] I have made the necessary updates to the documentation, or no documentation changes are required.

[codiumai-pr-agent-free[bot]]

PR Reviewer Guide 🔍

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Key issues to review Code Duplication The code for checking valid roles is duplicated in two different methods. Consider extracting this logic into a separate method to improve maintainability. Performance Concern The role validation involves multiple database queries and array operations. For large workspaces with many roles, this could potentially impact performance.

[codiumai-pr-agent-free[bot]]

PR Code Suggestions ✨

Category	Suggestion	Score
Enhancement	✅ Extract role validation logic into a separate method for better code organization and reusability ___ Suggestion Impact: The role validation logic was extracted into a separate method named `findInvalidWorkspaceRoles`, improving code reusability and maintainability. code diff: ```diff + const invalidRoles = await this.findInvalidWorkspaceRoles( + workspace.id, + roleIds ) if (invalidRoles.length > 0) { @@ -1000,18 +991,9 @@ ) } - //Check if the member has valid roles - const roles = await this.prisma.workspaceRole.findMany({ - where: { - id: { - in: member.roleIds - }, - workspaceId: workspace.id - } - }) - - const invalidRoles = member.roleIds.filter( - (id) => !roles.map((role) => role.id).includes(id) + const invalidRoles = await this.findInvalidWorkspaceRoles( + workspace.id, + member.roleIds ) if (invalidRoles.length > 0) { @@ -1085,6 +1067,26 @@ } } + private async findInvalidWorkspaceRoles( + workspaceId: string, + roleIds: string[] + ) { + const roles = await this.prisma.workspaceRole.findMany({ + where: { + id: { + in: roleIds + }, + workspaceId: workspaceId + } + }) + + const roleIdSet = new Set(roles.map((role) => role.id)) + + const invalidRoles = roleIds.filter((id) => !roleIdSet.has(id)) + + return invalidRoles ``` ___ **Consider extracting the role validation logic into a separate method to improve code reusability and maintainability. This method can be used in both the update and invite scenarios.** [apps/api/src/workspace/service/workspace.service.ts [418-435]](https://github.com/keyshade-xyz/keyshade/pull/408/files#diff-a92613f23cf2834d52eaedb87c4e8c460d48b6fca8df4ccc968b2a8af19a9c3fR418-R435) ```diff -const roles = await this.prisma.workspaceRole.findMany({ - where: { - id: { - in: roleIds - }, - workspaceId: workspace.id +await this.validateWorkspaceRoles(workspace, roleIds); + +// New method to be added to the class: +private async validateWorkspaceRoles(workspace: Workspace, roleIds: string[]): Promise { + const roles = await this.prisma.workspaceRole.findMany({ + where: { + id: { in: roleIds }, + workspaceId: workspace.id + } + }); + + const invalidRoles = roleIds.filter((id) => !roles.map((role) => role.id).includes(id)); + + if (invalidRoles.length > 0) { + throw new NotFoundException( + `Workspace ${workspace.name} (${workspace.id}) does not have roles ${invalidRoles.join(', ')}` + ); } -}) - -const invalidRoles = roleIds.filter( - (id) => !roles.map((role) => role.id).includes(id) -) - -if (invalidRoles.length > 0) { - throw new NotFoundException( - `Workspace ${workspace.name} (${workspace.id}) does not have roles ${invalidRoles.join(', ')}` - ) } ``` - [ ] **Apply this suggestion** Suggestion importance[1-10]: 7 Why:	7
Performance	✅ Use Set for more efficient role ID comparison ___ **Use Set data structure for more efficient role ID comparison instead of using Array.prototype.includes().** [apps/api/src/workspace/service/workspace.service.ts [427-429]](https://github.com/keyshade-xyz/keyshade/pull/408/files#diff-a92613f23cf2834d52eaedb87c4e8c460d48b6fca8df4ccc968b2a8af19a9c3fR427-R429) ```diff -const invalidRoles = roleIds.filter( - (id) => !roles.map((role) => role.id).includes(id) -) +const roleIdSet = new Set(roles.map((role) => role.id)); +const invalidRoles = roleIds.filter((id) => !roleIdSet.has(id)); ``` `[Suggestion has been applied]` Suggestion importance[1-10]: 7 Why:	7
Best practice	Use a more appropriate exception type for invalid role IDs ___ **Consider using a more specific error type like BadRequestException instead of NotFoundException when invalid role IDs are provided, as it's more appropriate for this scenario.** [apps/api/src/workspace/service/workspace.service.ts [431-435]](https://github.com/keyshade-xyz/keyshade/pull/408/files#diff-a92613f23cf2834d52eaedb87c4e8c460d48b6fca8df4ccc968b2a8af19a9c3fR431-R435) ```diff if (invalidRoles.length > 0) { - throw new NotFoundException( - `Workspace ${workspace.name} (${workspace.id}) does not have roles ${invalidRoles.join(', ')}` + throw new BadRequestException( + `Invalid role IDs provided for workspace ${workspace.name} (${workspace.id}): ${invalidRoles.join(', ')}` ) } ``` - [ ] **Apply this suggestion** Suggestion importance[1-10]: 7 Why:	7
Security	Add a limit to the number of role IDs that can be processed at once ___ **Consider adding a limit to the number of role IDs that can be processed at once to prevent potential performance issues or abuse.** [apps/api/src/workspace/service/workspace.service.ts [418-425]](https://github.com/keyshade-xyz/keyshade/pull/408/files#diff-a92613f23cf2834d52eaedb87c4e8c460d48b6fca8df4ccc968b2a8af19a9c3fR418-R425) ```diff +const MAX_ROLES = 50; // Adjust this value as needed +if (roleIds.length > MAX_ROLES) { + throw new BadRequestException(`Cannot process more than ${MAX_ROLES} roles at once.`); +} const roles = await this.prisma.workspaceRole.findMany({ where: { id: { in: roleIds }, workspaceId: workspace.id } }) ``` - [ ] **Apply this suggestion** Suggestion importance[1-10]: 7 Why:	7

[PriyobrotoKar]

@rajdip-b I have made those changes.