From ae3142dd4e00bd5b78f3414527b358d587e6b136 Mon Sep 17 00:00:00 2001 From: Rudhra Deep Biswas <98055396+rudeUltra@users.noreply.github.com> Date: Mon, 10 Mar 2025 17:19:44 +0530 Subject: [PATCH] Permission FIxes including appId and DataSourceId (#12184) * init * remove ForwardAbility --- server/src/modules/app-git/ability/guard.ts | 4 -- server/src/modules/app-git/ability/index.ts | 40 +++++++++++-------- server/src/modules/app/ability-factory.ts | 1 - .../src/modules/app/guards/ability.guard.ts | 2 +- .../src/modules/data-sources/ability/index.ts | 28 +++++++++---- .../ability/app/guard.ts | 4 -- .../ability/data-source/guard.ts | 4 -- .../modules/licensing/constants/PlanTerms.ts | 2 +- .../src/modules/tooljet-db/ability/guard.ts | 4 -- server/src/modules/versions/ability/index.ts | 21 +++++++--- .../modules/workflows/ability/app/guard.ts | 4 -- 11 files changed, 61 insertions(+), 53 deletions(-) diff --git a/server/src/modules/app-git/ability/guard.ts b/server/src/modules/app-git/ability/guard.ts index a937a4c6d5..41c7bf9ed9 100644 --- a/server/src/modules/app-git/ability/guard.ts +++ b/server/src/modules/app-git/ability/guard.ts @@ -20,8 +20,4 @@ export class AppGitAbilityGuard extends AbilityGuard { protected getSubjectType() { return App; } - - protected forwardAbility(): boolean { - return true; - } } diff --git a/server/src/modules/app-git/ability/index.ts b/server/src/modules/app-git/ability/index.ts index 81d7bea81f..69207d6221 100644 --- a/server/src/modules/app-git/ability/index.ts +++ b/server/src/modules/app-git/ability/index.ts @@ -14,7 +14,13 @@ export class AppGitAbilityFactory extends AbilityFactory return App; } - protected defineAbilityFor(can: AbilityBuilder['can'], UserAllPermissions: UserAllPermissions): void { + protected defineAbilityFor( + can: AbilityBuilder['can'], + UserAllPermissions: UserAllPermissions, + extractedMetadata: { moduleName: string; features: string[] }, + request?: any + ): void { + const appId = request?.tj_resource_id; const { superAdmin, isAdmin, userPermission } = UserAllPermissions; const userAppGitPermissions = userPermission?.APP; @@ -35,11 +41,12 @@ export class AppGitAbilityFactory extends AbilityFactory } // READ-based features - if (isAllAppsViewable || userAppGitPermissions?.viewableAppsId?.length) { + if ( + isAllAppsViewable || + (userAppGitPermissions?.viewableAppsId?.length && appId && userAppGitPermissions?.viewableAppsId?.includes(appId)) + ) { can(FEATURE_KEY.GIT_GET_APPS, App); - can(FEATURE_KEY.GIT_GET_APP, App, { - id: { $in: isAllAppsViewable ? undefined : userAppGitPermissions?.viewableAppsId }, - }); + can(FEATURE_KEY.GIT_GET_APP, App); } // CREATE-based features @@ -48,20 +55,21 @@ export class AppGitAbilityFactory extends AbilityFactory } // UPDATE-based features - if (isAllAppsEditable || userAppGitPermissions?.editableAppsId?.length) { - can(FEATURE_KEY.GIT_UPDATE_APP, App, { - id: { $in: isAllAppsEditable ? undefined : userAppGitPermissions?.editableAppsId }, - }); - can(FEATURE_KEY.GIT_SYNC_APP, App, { - id: { $in: isAllAppsEditable ? undefined : userAppGitPermissions?.editableAppsId }, - }); + if ( + isAllAppsEditable || + (userAppGitPermissions?.editableAppsId?.length && appId && userAppGitPermissions.editableAppsId.includes(appId)) + ) { + can(FEATURE_KEY.GIT_UPDATE_APP, App); + can(FEATURE_KEY.GIT_SYNC_APP, App); } // Additional checks based on specific actions - if (userAppGitPermissions?.editableAppsId?.length) { - can(FEATURE_KEY.GIT_GET_APP_CONFIG, App, { - id: { $in: userAppGitPermissions.editableAppsId }, - }); + if ( + userAppGitPermissions?.editableAppsId?.length && + appId && + userAppGitPermissions.editableAppsId.includes(appId) + ) { + can(FEATURE_KEY.GIT_GET_APP_CONFIG, App); } } } diff --git a/server/src/modules/app/ability-factory.ts b/server/src/modules/app/ability-factory.ts index 9450f21906..d461d53788 100644 --- a/server/src/modules/app/ability-factory.ts +++ b/server/src/modules/app/ability-factory.ts @@ -35,7 +35,6 @@ export abstract class AbilityFactory { } : {}), })); - if (request) { request.tj_user_permissions = userPermission; } diff --git a/server/src/modules/app/guards/ability.guard.ts b/server/src/modules/app/guards/ability.guard.ts index d6c35de5ae..690129f81c 100644 --- a/server/src/modules/app/guards/ability.guard.ts +++ b/server/src/modules/app/guards/ability.guard.ts @@ -83,7 +83,7 @@ export abstract class AbilityGuard implements CanActivate { user, { moduleName: module, features }, resourceArray, - this.forwardAbility() ? request : undefined + request ); if (this.forwardAbility()) { diff --git a/server/src/modules/data-sources/ability/index.ts b/server/src/modules/data-sources/ability/index.ts index e1af13e5ab..a256bffabf 100644 --- a/server/src/modules/data-sources/ability/index.ts +++ b/server/src/modules/data-sources/ability/index.ts @@ -15,7 +15,12 @@ export class FeatureAbilityFactory extends AbilityFactory return DataSource; } - protected defineAbilityFor(can: AbilityBuilder['can'], UserAllPermissions: UserAllPermissions): void { + protected defineAbilityFor( + can: AbilityBuilder['can'], + UserAllPermissions: UserAllPermissions, + extractedMetadata: { moduleName: string; features: string[] }, + request?: any + ): void { // Data source permissions // EE - data source create/delete -> full access // CE - Admin - full access. builder -> use access @@ -27,6 +32,8 @@ export class FeatureAbilityFactory extends AbilityFactory const isCanDelete = userPermission.dataSourceDelete; const isAllViewable = !!resourcePermissions?.isAllUsable; + const dataSourceId = request?.tj_resource_id; + // Oauth end points available to all can(FEATURE_KEY.GET_OAUTH2_BASE_URL, DataSource); can(FEATURE_KEY.AUTHORIZE, DataSource); @@ -81,11 +88,14 @@ export class FeatureAbilityFactory extends AbilityFactory return; } - if (resourcePermissions?.configurableDataSourceId?.length) { + if ( + resourcePermissions?.configurableDataSourceId?.length && + dataSourceId && + resourcePermissions?.configurableDataSourceId?.includes(dataSourceId) + ) { can( [FEATURE_KEY.GET, FEATURE_KEY.UPDATE, FEATURE_KEY.GET_BY_ENVIRONMENT, FEATURE_KEY.TEST_CONNECTION], - DataSource, - { id: { $in: resourcePermissions.configurableDataSourceId } } + DataSource ); } @@ -93,10 +103,12 @@ export class FeatureAbilityFactory extends AbilityFactory can([FEATURE_KEY.GET, FEATURE_KEY.GET_BY_ENVIRONMENT], DataSource); return; } - if (resourcePermissions.usableDataSourcesId?.length) { - can([FEATURE_KEY.GET, FEATURE_KEY.GET_BY_ENVIRONMENT], DataSource, { - id: { $in: resourcePermissions.usableDataSourcesId }, - }); + if ( + resourcePermissions.usableDataSourcesId?.length && + dataSourceId && + resourcePermissions?.usableDataSourcesId?.includes(dataSourceId) + ) { + can([FEATURE_KEY.GET, FEATURE_KEY.GET_BY_ENVIRONMENT], DataSource); } } } diff --git a/server/src/modules/import-export-resources/ability/app/guard.ts b/server/src/modules/import-export-resources/ability/app/guard.ts index 5c0e3943bb..291e34cdd8 100644 --- a/server/src/modules/import-export-resources/ability/app/guard.ts +++ b/server/src/modules/import-export-resources/ability/app/guard.ts @@ -15,10 +15,6 @@ export class FeatureAbilityGuard extends AbilityGuard { return App; } - protected forwardAbility(): boolean { - return true; - } - protected getResource(): ResourceDetails | ResourceDetails[] { return [ { diff --git a/server/src/modules/import-export-resources/ability/data-source/guard.ts b/server/src/modules/import-export-resources/ability/data-source/guard.ts index cd858fbe45..5cb58f68f9 100644 --- a/server/src/modules/import-export-resources/ability/data-source/guard.ts +++ b/server/src/modules/import-export-resources/ability/data-source/guard.ts @@ -12,8 +12,4 @@ export class FeatureAbilityGuard extends AbilityGuard { protected getAbilityFactory() { return FeatureAbilityFactory; } - - protected forwardAbility(): boolean { - return true; - } } diff --git a/server/src/modules/licensing/constants/PlanTerms.ts b/server/src/modules/licensing/constants/PlanTerms.ts index 57b0ecb7bd..0eb05cfe6c 100644 --- a/server/src/modules/licensing/constants/PlanTerms.ts +++ b/server/src/modules/licensing/constants/PlanTerms.ts @@ -48,7 +48,7 @@ export const BASIC_PLAN_TERMS: Partial = { export const BASIC_PLAN_SETTINGS = { ALLOW_PERSONAL_WORKSPACE: { - value: 'true', + value: 'false', }, WHITE_LABEL_LOGO: { value: '', diff --git a/server/src/modules/tooljet-db/ability/guard.ts b/server/src/modules/tooljet-db/ability/guard.ts index 564f05979d..b56572a62e 100644 --- a/server/src/modules/tooljet-db/ability/guard.ts +++ b/server/src/modules/tooljet-db/ability/guard.ts @@ -12,8 +12,4 @@ export class FeatureAbilityGuard extends AbilityGuard { protected getSubjectType() { return InternalTable; } - - protected forwardAbility(): boolean { - return true; - } } diff --git a/server/src/modules/versions/ability/index.ts b/server/src/modules/versions/ability/index.ts index 551526c208..5237a97ec9 100644 --- a/server/src/modules/versions/ability/index.ts +++ b/server/src/modules/versions/ability/index.ts @@ -15,7 +15,13 @@ export class FeatureAbilityFactory extends AbilityFactory return App; } - protected defineAbilityFor(can: AbilityBuilder['can'], UserAllPermissions: UserAllPermissions): void { + protected defineAbilityFor( + can: AbilityBuilder['can'], + UserAllPermissions: UserAllPermissions, + extractedMetadata: { moduleName: string; features: string[] }, + request?: any + ): void { + const appId = request?.tj_resource_id; const { superAdmin, isAdmin, userPermission } = UserAllPermissions; const userAppPermissions = userPermission?.[MODULES.APP]; @@ -52,7 +58,7 @@ export class FeatureAbilityFactory extends AbilityFactory return; } - if (userAppPermissions?.editableAppsId?.length) { + if (userAppPermissions?.editableAppsId?.length && appId && userAppPermissions.editableAppsId.includes(appId)) { can( [ FEATURE_KEY.GET, @@ -76,16 +82,19 @@ export class FeatureAbilityFactory extends AbilityFactory FEATURE_KEY.UPDATE_EVENT, FEATURE_KEY.DELETE_EVENT, ], - App, - { id: { $in: userAppPermissions.editableAppsId } } + App ); } if (isAllAppsViewable) { // add view permissions for all apps can([FEATURE_KEY.GET_EVENTS], App); - } else if (userAppPermissions?.viewableAppsId?.length) { - can([FEATURE_KEY.GET_EVENTS], App, { id: { $in: userAppPermissions.viewableAppsId } }); + } else if ( + userAppPermissions?.viewableAppsId?.length && + appId && + userAppPermissions.viewableAppsId.includes(appId) + ) { + can([FEATURE_KEY.GET_EVENTS], App); } } } diff --git a/server/src/modules/workflows/ability/app/guard.ts b/server/src/modules/workflows/ability/app/guard.ts index 3f97520bbf..251e575a2a 100644 --- a/server/src/modules/workflows/ability/app/guard.ts +++ b/server/src/modules/workflows/ability/app/guard.ts @@ -15,10 +15,6 @@ export class FeatureAbilityGuard extends AbilityGuard { return App; } - protected forwardAbility(): boolean { - return true; - } - protected getResource(): ResourceDetails | ResourceDetails[] { return [ {