Skip to content

Commit

Permalink
fix(permissions): circular dependency (#18986)
Browse files Browse the repository at this point in the history
* fix(permissions): circular dependency

* Apply suggestions from code review

Co-authored-by: Ben Irvin <ben@innerdvations.com>

* Apply suggestions from code review

Co-authored-by: Jean-S茅bastien Herbaux <jean-sebastien.herbaux@epitech.eu>

---------

Co-authored-by: Ben Irvin <ben@innerdvations.com>
Co-authored-by: Jean-S茅bastien Herbaux <jean-sebastien.herbaux@epitech.eu>
  • Loading branch information
3 people committed Dec 6, 2023
1 parent 2951783 commit e85a23d
Show file tree
Hide file tree
Showing 6 changed files with 39 additions and 31 deletions.
13 changes: 6 additions & 7 deletions packages/core/permissions/src/engine/abilities/casl-ability.ts
Expand Up @@ -2,12 +2,11 @@ import * as sift from 'sift';
import qs from 'qs';
import { AbilityBuilder, Ability } from '@casl/ability';
import { pick, isNil, isObject } from 'lodash/fp';
// eslint-disable-next-line import/no-extraneous-dependencies, node/no-extraneous-import
import { Permissions as PermissionsTypes } from '@strapi/types';
import type { ParametrizedAction, PermissionRule } from '../../types';

export interface CustomAbilityBuilder {
can(permission: PermissionsTypes.PermissionRule): ReturnType<AbilityBuilder<Ability>['can']>;
buildParametrizedAction: (parametrizedAction: PermissionsTypes.ParametrizedAction) => string;
can(permission: PermissionRule): ReturnType<AbilityBuilder<Ability>['can']>;
buildParametrizedAction: (parametrizedAction: ParametrizedAction) => string;
build(): Ability;
}

Expand All @@ -32,7 +31,7 @@ const conditionsMatcher = (conditions: unknown) => {
return sift.createQueryTester(conditions, { operations });
};

const buildParametrizedAction = ({ name, params }: PermissionsTypes.ParametrizedAction) => {
const buildParametrizedAction = ({ name, params }: ParametrizedAction) => {
return `${name}?${qs.stringify(params)}`;
};

Expand All @@ -43,7 +42,7 @@ export const caslAbilityBuilder = (): CustomAbilityBuilder => {
const { can, build, ...rest } = new AbilityBuilder(Ability);

return {
can(permission: PermissionsTypes.PermissionRule) {
can(permission: PermissionRule) {
const { action, subject, properties = {}, condition } = permission;
const { fields } = properties;

Expand All @@ -57,7 +56,7 @@ export const caslAbilityBuilder = (): CustomAbilityBuilder => {
);
},

buildParametrizedAction({ name, params }: PermissionsTypes.ParametrizedAction) {
buildParametrizedAction({ name, params }: ParametrizedAction) {
return `${name}?${qs.stringify(params)}`;
},

Expand Down
5 changes: 2 additions & 3 deletions packages/core/permissions/src/engine/hooks.ts
@@ -1,10 +1,9 @@
import { cloneDeep, has, isArray } from 'lodash/fp';
import { hooks } from '@strapi/utils';
// eslint-disable-next-line node/no-extraneous-import
import type { Permissions as PermissionsTypes } from '@strapi/types';

import * as domain from '../domain';
import type { Permission } from '../domain/permission';
import type { PermissionRule } from '../types';

export interface PermissionEngineHooks {
'before-format::validate.permission': ReturnType<typeof hooks.createAsyncBailHook>;
Expand Down Expand Up @@ -52,7 +51,7 @@ const createBeforeEvaluateContext = (permission: Permission) => ({
});

interface WillRegisterContextParams {
permission: PermissionsTypes.PermissionRule;
permission: PermissionRule;
options: Record<string, unknown>;
}

Expand Down
11 changes: 5 additions & 6 deletions packages/core/permissions/src/engine/index.ts
Expand Up @@ -2,8 +2,6 @@ import _ from 'lodash/fp';
import qs from 'qs';
import { Ability } from '@casl/ability';
import { providerFactory } from '@strapi/utils';
// eslint-disable-next-line import/no-extraneous-dependencies, node/no-extraneous-import
import { Permissions as PermissionsTypes } from '@strapi/types';

import {
createEngineHooks,
Expand All @@ -15,6 +13,7 @@ import type { PermissionEngineHooks, HookName } from './hooks';

import * as abilities from './abilities';
import { Permission } from '../domain/permission';
import type { PermissionRule } from '../types';

export { abilities };

Expand All @@ -30,9 +29,9 @@ export interface Engine {
on(hook: HookName, handler: (...args: any[]) => any): Engine;
generateAbility(permissions: Permission[], options?: object): Promise<Ability>;
createRegisterFunction(
can: (permission: PermissionsTypes.PermissionRule) => unknown,
can: (permission: PermissionRule) => unknown,
options: Record<string, unknown>
): (permission: PermissionsTypes.PermissionRule) => Promise<unknown>;
): (permission: PermissionRule) => Promise<unknown>;
}

export interface EngineParams {
Expand All @@ -42,7 +41,7 @@ export interface EngineParams {

interface EvaluateParams {
options: Record<string, unknown>;
register: (permission: PermissionsTypes.PermissionRule) => Promise<unknown>;
register: (permission: PermissionRule) => Promise<unknown>;
permission: Permission;
}

Expand Down Expand Up @@ -179,7 +178,7 @@ const newEngine = (params: EngineParams): Engine => {
* used to register a permission in the ability builder
*/
createRegisterFunction(can, options: Record<string, unknown>) {
return async (permission: PermissionsTypes.PermissionRule) => {
return async (permission: PermissionRule) => {
const hookContext = createWillRegisterContext({ options, permission });

await state.hooks['before-register.permission'].call(hookContext);
Expand Down
19 changes: 19 additions & 0 deletions packages/core/permissions/src/types.ts
@@ -0,0 +1,19 @@
/**
* These were imported from `@strapi/types` but if we do that
* it becomes a circular dependency. This is the source of truth,
* they're re-exported from `@strapi/types` for convenience.
*/
import type { Subject } from '@casl/ability';

export interface ParametrizedAction {
name: string;
params: Record<string, unknown>;
}
export interface PermissionRule {
action: string | ParametrizedAction;
subject?: Subject | null;
properties?: {
fields?: string[];
};
condition?: Record<string, unknown>;
}
2 changes: 0 additions & 2 deletions packages/core/types/src/index.ts
Expand Up @@ -50,11 +50,9 @@ export {
};

declare global {
// @ts-expect-error - global strapi variable is also defined in the index.d.ts file
var strapi: LoadedStrapi;
namespace NodeJS {
interface Global {
// @ts-expect-error - global strapi variable is also defined in the index.d.ts file
strapi: LoadedStrapi;
}
}
Expand Down
20 changes: 7 additions & 13 deletions packages/core/types/src/types/core/permissions/index.ts
@@ -1,14 +1,8 @@
import { Subject } from '@casl/ability';
import type { engine } from '@strapi/permissions';

export interface ParametrizedAction {
name: string;
params: Record<string, unknown>;
}
export interface PermissionRule {
action: string | ParametrizedAction;
subject?: Subject | null;
properties?: {
fields?: string[];
};
condition?: Record<string, unknown>;
}
type PermissionRule = Parameters<engine.abilities.CustomAbilityBuilder['can']>[0];
type ParametrizedAction = Parameters<
engine.abilities.CustomAbilityBuilder['buildParametrizedAction']
>[0];

export type { PermissionRule, ParametrizedAction };

0 comments on commit e85a23d

Please sign in to comment.