[permissions] Remove raw queries and restrict its usage #12360
Conversation
…atasource for control
| @@ -3,11 +3,11 @@ import { DataSource } from 'typeorm'; | |||
| const tableName = 'billingSubscription'; | |||
|
|
|||
| export const seedBillingSubscriptions = async ( | |||
| workspaceDataSource: DataSource, | |||
| dataSource: DataSource, | |||
There was a problem hiding this comment.
clarifying name here and in other files as Datasource is not equal to WorkspaceDataSource, and does not require shouldBypassPermissionCheck indication.
For workspaceDataSource we have implemented permissions and overriden createQueryBuilder(). DataSource is not restricted to a workspace and can perform operations on core + on any workspace. Its usage is restricted to "system" operations.
|
|
||
| return isDefined(recordWithMinPosition?.position) | ||
| ? recordWithMinPosition.position - index - 1 | ||
| const recordWithMinPosition = await this.findMinPosition( |
|
🚀 Preview Environment Ready! Your preview environment is available at: http://bore.pub:24796 This environment will automatically shut down when the PR is closed or after 5 hours. |
ijreilly
changed the title
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
This PR implements a significant security improvement by removing raw SQL queries and enforcing repository usage with proper permission checks throughout the codebase. The changes affect multiple modules and services across the application.
- Replaces
WorkspaceDataSourceServicewithTwentyORMGlobalManagerin multiple modules, enforcing permission checks through repositories - Removes
WorkspaceMemberWorkspaceEntityfrom several modules, suggesting workspace member access is now handled through dedicated repositories - Adds
shouldBypassPermissionChecksflag requirement for any remaining raw queries in migration/seed files - Replaces
SelectQueryBuilderwithWorkspaceSelectQueryBuilderto ensure proper permission handling in GraphQL queries - Removes deprecated utility files like
build-record-position-query.util.tsin favor of repository-based implementations
46 file(s) reviewed, 13 comment(s)
Edit PR Review Bot Settings | Greptile
...tabase/commands/upgrade-version-command/0-43/0-43-migrate-rich-text-content-patch.command.ts
Show resolved
Hide resolved
...tabase/commands/upgrade-version-command/0-43/0-43-migrate-rich-text-content-patch.command.ts
Show resolved
Hide resolved
...er/src/engine/api/graphql/graphql-query-runner/graphql-query-parsers/graphql-query.parser.ts
Show resolved
Hide resolved
| findOneBy: jest.fn(), | ||
| update: jest.fn(), | ||
| minimum: jest.fn().mockResolvedValue(1), | ||
| maximum: jest.fn().mockResolvedValue(1), | ||
| }; |
There was a problem hiding this comment.
style: Mock repository methods should verify input parameters and return types match the actual repository interface
...r/src/engine/core-modules/record-position/services/__tests__/record-position.service.spec.ts
Outdated
Show resolved
Hide resolved
packages/twenty-server/src/engine/workspace-datasource/workspace-datasource.service.ts
Show resolved
Hide resolved
.../src/modules/blocklist/blocklist-validation-manager/services/blocklist-validation.service.ts
Show resolved
Hide resolved
packages/twenty-server/src/modules/timeline/services/timeline-activity.service.ts
Outdated
Show resolved
Hide resolved
packages/twenty-server/src/modules/timeline/services/timeline-activity.service.ts
Outdated
Show resolved
Hide resolved
.../twenty-server/src/modules/timeline/jobs/upsert-timeline-activity-from-internal-event.job.ts
Show resolved
Hide resolved
Closes twentyhq/core-team-issues#748
In the frame of the work on permissions we