feat(api): Added Project Level Access

[rayaanoidPrime]

User description

Description

• Added schema for enum ProjectLevelAccess for GLOBAL , INTERNAL, PRIVATE access level types
• Modified logic for checkAuthorityOverProject method allow access to projects based on differed access levels
• Added check-user-role-associations.ts in the src/common folder for checking the set of roles associated
  with the workspace member for project access authorization
• Added relevant tests

Fixes #154

Dependencies

None

Future Improvements

Mentions

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:

• I have followed the coding guidelines
• 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.

---

PR Type

enhancement, bug_fix

---

Description

• Introduced new project access levels (GLOBAL, INTERNAL, PRIVATE) replacing the boolean isPublic.
• Updated project service, DTOs, and various test specs to use the new accessLevel field.
• Added authority checking logic for different access levels in authority-checker.service.ts.
• Implemented a new function checkUserRoleAssociations to validate user roles for project access.
• Updated Prisma schema and added a SQL migration for these changes.

---

Changes walkthrough 📝

	Relevant files
Enhancement	9 files project.e2e.spec.ts Enhance Project Controller Tests with Access Level Checks apps/api/src/project/project.e2e.spec.ts Added new test cases for different project access levels: GLOBAL, INTERNAL, PRIVATE. Modified existing test cases to use accessLevel instead of isPublic. Introduced new project instances globalProject, internalProject, and privateProject for testing. +190/-2  authority-checker.service.ts Refactor Authority Checker to Support New Access Levels    apps/api/src/common/authority-checker.service.ts Refactored authority checking logic to accommodate new project access levels. Added handling for GLOBAL, INTERNAL, and PRIVATE access levels. Integrated checkUserRoleAssociations function to validate user roles for PRIVATE projects. +51/-15  check-user-role-associations.ts Implement User Role Association Check for Project Access  apps/api/src/common/check-user-role-associations.ts New function to check user role associations for project access. Determines if a user has the required authority based on their role in the workspace. +55/-0    project.service.ts Update Project Service to Handle Access Levels                      apps/api/src/project/service/project.service.ts Updated methods to set accessLevel instead of isPublic in project creation and update. +2/-2      create.project.ts Refactor CreateProject DTO to Use Access Level Enum            apps/api/src/project/dto/create.project/create.project.ts Replaced isPublic boolean with accessLevel enum in project DTO. +4/-2      approval.types.ts Update Approval Types to Reflect Access Level Changes        apps/api/src/approval/approval.types.ts Updated UpdateProjectMetadata to use accessLevel instead of isPublic. +1/-1      workspace.service.ts Update Workspace Service to Support Access Level Queries  apps/api/src/workspace/service/workspace.service.ts Updated workspace service to handle accessLevel property in project queries. +1/-1      schema.prisma Introduce Project Access Level Enum in Prisma Schema          apps/api/src/prisma/schema.prisma Introduced ProjectAccessLevel enum with values 'GLOBAL', 'INTERNAL', 'PRIVATE'. Replaced isPublic with accessLevel in the Project model. +7/-1      migration.sql SQL Migration for Project Access Level Changes                      apps/api/src/prisma/migrations/20240513155428_add_project_level_access/migration.sql SQL migration to replace isPublic column with accessLevel in the Project table. Creation of ProjectAccessLevel enum type in the database. +12/-0
Tests	4 files secret.e2e.spec.ts Update Secret Controller Tests to Use Access Level              apps/api/src/secret/secret.e2e.spec.ts Updated test cases to use accessLevel instead of isPublic. +3/-3      variable.e2e.spec.ts Update Variable Controller Tests to Use Access Level          apps/api/src/variable/variable.e2e.spec.ts Updated test cases to use accessLevel instead of isPublic. +3/-3      event.e2e.spec.ts Update Event Controller Tests for Global Access Level        apps/api/src/event/event.e2e.spec.ts Updated test setup to use accessLevel 'GLOBAL' for a project. +1/-1      environment.e2e.spec.ts Update Environment Controller Tests for Private Access Level apps/api/src/environment/environment.e2e.spec.ts Updated test setup to use accessLevel 'PRIVATE' for a project. +1/-1

---

💡 PR-Agent usage:
Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

[codiumai-pr-agent-free]

PR Description updated to latest commit (eb0d7ff)

[codiumai-pr-agent-free]

PR Review 🔍

(Review updated until commit 1488ff1)

⏱️ Estimated effort to review [1-5]	4, because the PR involves multiple changes across various files including backend logic, database schema updates, and test modifications. The complexity of access control logic and its integration with existing services increases the review effort.
🧪 Relevant tests	Yes
⚡ Possible issues	Possible Bug: The logic in authority-checker.service.ts might not correctly handle cases where a user has multiple roles that could conflict or overlap in authority, potentially leading to incorrect access control decisions.
	Data Loss Concern: The migration script drops the isPublic column which could lead to loss of data. Consider a migration strategy that maps existing data to the new accessLevel field.
🔒 Security concerns	No

Code feedback:

---

relevant file	apps/api/src/common/authority-checker.service.ts
suggestion	Consider adding detailed logging for each case in the switch statement handling different project access levels. This will help in debugging issues related to access control and understanding the flow of authority checks. [important]
relevant line	switch (projectAccessLevel) {

---

relevant file	apps/api/src/prisma/migrations/20240513155428_add_project_level_access/migration.sql
suggestion	Instead of dropping the isPublic column immediately, consider using a temporary data migration strategy where you first populate the new accessLevel column based on the values of isPublic before dropping it. This prevents data loss and ensures a smoother transition. [important]
relevant line	ALTER TABLE "Project" DROP COLUMN "isPublic",

---

relevant file	apps/api/src/project/project.e2e.spec.ts
suggestion	Add more comprehensive tests for edge cases, such as users with multiple roles or conflicting authorities, to ensure the access level logic is robust and handles all expected scenarios. [important]
relevant line	describe('Project Controller tests for access levels', () => {

---

relevant file	apps/api/src/common/authority-checker.service.ts
suggestion	Refactor the authority checking logic to a separate method or class that handles different access levels. This can improve modularity and make the code easier to manage and test. [medium]
relevant line	const projectAccessLevel = project.accessLevel

---

[codiumai-pr-agent-free]

PR Code Suggestions ✨

Category	Suggestion	Score
Best practice	Use a transaction to ensure atomic operations during the schema update Consider using a transaction to ensure that the operations of dropping a column and adding a new column with a default value are atomic. This will prevent partial updates to the database schema in case of an error during the migration. apps/api/src/prisma/migrations/20240513155428_add_project_level_access/migration.sql [11-12] -ALTER TABLE "Project" DROP COLUMN "isPublic", -ADD COLUMN "accessLevel" "ProjectAccessLevel" NOT NULL DEFAULT 'PRIVATE'; +BEGIN; +ALTER TABLE "Project" DROP COLUMN "isPublic"; +ALTER TABLE "Project" ADD COLUMN "accessLevel" "ProjectAccessLevel" NOT NULL DEFAULT 'PRIVATE'; +COMMIT; Suggestion importance[1-10]: 9 Why: The suggestion to use transactions is crucial for maintaining database integrity during schema changes, preventing partial updates in case of errors.	9
Performance	Optimize the role checking logic by improving the database query Optimize the role checking logic by directly querying for the specific role and project ID in the database query, rather than fetching all roles and filtering in application logic. apps/api/src/common/check-user-role-associations.ts [23-42] -const workSpaceMembership = await prisma.workspaceMember.findUnique({ +const hasAccess = await prisma.workspaceMember.findFirst({ where: { - workspaceId_userId: { - workspaceId: project.workspaceId, - userId - } - }, - select: { + workspaceId: project.workspaceId, + userId, roles: { - include: { + some: { role: { - select: { - authorities: true, - projects: true + authorities: { + has: authority + }, + projects: { + some: { + id: project.id + } } } } } } -}); +}) !== null; Suggestion importance[1-10]: 8 Why: The suggestion to optimize the database query is highly relevant as it directly enhances performance by reducing unnecessary data fetching and processing.	8
Enhancement	Add a data migration script to map old column values to the new column Consider adding a script to migrate existing data from the isPublic column to the new accessLevel column before dropping isPublic. This would prevent data loss and provide a clear migration path for existing records. apps/api/src/prisma/migrations/20240513155428_add_project_level_access/migration.sql [11-12] -ALTER TABLE "Project" DROP COLUMN "isPublic", -ADD COLUMN "accessLevel" "ProjectAccessLevel" NOT NULL DEFAULT 'PRIVATE'; +-- Script to migrate existing data +UPDATE "Project" SET "accessLevel" = CASE WHEN "isPublic" = TRUE THEN 'GLOBAL' ELSE 'PRIVATE' END; +ALTER TABLE "Project" DROP COLUMN "isPublic"; +ALTER TABLE "Project" ADD COLUMN "accessLevel" "ProjectAccessLevel" NOT NULL DEFAULT 'PRIVATE'; Suggestion importance[1-10]: 8 Why: This suggestion is highly relevant as it addresses potential data loss during the migration by proposing a script to map old values to the new column.	8
	Review and possibly extend the ENUM values to cover all future access scenarios Ensure that the ENUM type ProjectAccessLevel includes all necessary access levels or consider adding additional levels if the business requirements might include more granular access controls in the future. apps/api/src/prisma/migrations/20240513155428_add_project_level_access/migration.sql [8] -CREATE TYPE "ProjectAccessLevel" AS ENUM ('GLOBAL', 'INTERNAL', 'PRIVATE'); +CREATE TYPE "ProjectAccessLevel" AS ENUM ('GLOBAL', 'INTERNAL', 'PRIVATE', 'RESTRICTED', 'CUSTOM'); Suggestion importance[1-10]: 5 Why: While considering future requirements is good practice, the suggestion lacks specific context or evidence that more ENUM values are currently needed.	5
Possible issue	Add error handling for asynchronous operations in the beforeEach block Consider adding error handling for the asynchronous operations within the beforeEach block to ensure that any issues during project creation are properly managed and do not lead to unhandled promise rejections. apps/api/src/project/project.e2e.spec.ts [708-717] -globalProject = (await projectService.createProject( - user1, - workspace1.id, - { - name: 'Global Project', - description: 'Global Project description', - storePrivateKey: true, - accessLevel: 'GLOBAL' - } -)) as Project +try { + globalProject = (await projectService.createProject( + user1, + workspace1.id, + { + name: 'Global Project', + description: 'Global Project description', + storePrivateKey: true, + accessLevel: 'GLOBAL' + } + )) as Project +} catch (error) { + console.error('Failed to create global project:', error); +} Suggestion importance[1-10]: 7 Why: Adding error handling for asynchronous operations is crucial to prevent unhandled promise rejections and improve the robustness of the test setup.	7
Maintainability	Refactor repeated project creation logic into a utility function to improve maintainability Refactor the repeated project creation logic in the test cases to a utility function to avoid code duplication and improve test maintainability. apps/api/src/project/project.e2e.spec.ts [737-745] -privateProject = (await projectService.createProject( - user1, - workspace1.id, - { - name: 'Private Project', - description: 'Private Project description', - storePrivateKey: true, - accessLevel: 'PRIVATE' - } -)) as Project +privateProject = await createTestProject('Private Project', 'Private Project description', 'PRIVATE'); Suggestion importance[1-10]: 7 Why: Refactoring repeated code into a utility function is a good practice for reducing duplication and improving maintainability, especially in test suites where similar setups are common.	7
	Refactor the switch-case block into a separate function to improve code readability Refactor the switch-case block to a separate function to improve code readability and maintainability. This function can handle the logic for determining if a user has access based on the project access level. apps/api/src/common/authority-checker.service.ts [138-171] -switch (projectAccessLevel) { - case ProjectAccessLevel.GLOBAL: - break; - case ProjectAccessLevel.INTERNAL: - if (!permittedAuthorities.has(Authority.READ_PROJECT) && !permittedAuthorities.has(authority)) { - throw new UnauthorizedException( - `User with id ${userId} does not have the authority in the project with id ${entity?.id}` - ) - } - break; - case ProjectAccessLevel.PRIVATE: - const canAccessPrivateProject = await checkUserRoleAssociations( - userId, - project, - authority, - prisma - ) - if (!canAccessPrivateProject || !permittedAuthorities.has(authority)) { - throw new UnauthorizedException( - `User with id ${userId} does not have the authority in the project with id ${entity?.id}` - ) - } - break; -} +handleProjectAccessControl(projectAccessLevel, permittedAuthorities, userId, entity, project, authority, prisma); Suggestion importance[1-10]: 6 Why: Refactoring the switch-case block into a separate function would indeed improve readability and maintainability, but the suggestion lacks specific implementation details.	6
	Document the rationale behind the default value for the new column Add a comment explaining the choice of 'PRIVATE' as the default value for the new accessLevel column. This helps maintainers understand the rationale behind defaulting to the most restrictive access level. apps/api/src/prisma/migrations/20240513155428_add_project_level_access/migration.sql [12] +-- Defaulting to 'PRIVATE' to ensure all projects are explicitly made more accessible ADD COLUMN "accessLevel" "ProjectAccessLevel" NOT NULL DEFAULT 'PRIVATE'; Suggestion importance[1-10]: 6 Why: Adding a comment to explain the default value choice improves code maintainability and understanding, though it's not critical for functionality.	6

[rajdip-b]

Okei I've added quite a few reviews. Do go through them. And, when you make the changes and push it, do resolve the threads one by one.

[sonarcloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.1% Duplication on New Code

See analysis details on SonarCloud

[rajdip-b]

/review

[codiumai-pr-agent-free]

Persistent review updated to latest commit 1488ff1

[rajdip-b]

🎉 This PR is included in version 1.4.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀