From 4ebc128ffb06cb6929657b4670e190b5edea08dd Mon Sep 17 00:00:00 2001 From: Convly Date: Fri, 29 Jul 2022 10:17:06 +0200 Subject: [PATCH 1/6] Make users-permissions auth strategy use the content API permission engine --- .../server/strategies/__tests__/admin.test.js | 8 ++- .../core/admin/server/strategies/admin.js | 8 ++- .../core/strapi/lib/services/auth/index.js | 5 +- .../strapi/lib/types/core/strapi/index.d.ts | 5 ++ .../server/services/index.js | 2 + .../server/services/permission.js | 55 +++++++++++++++++++ .../server/services/users-permissions.js | 7 +++ .../server/strategies/users-permissions.js | 52 ++++++++++++------ .../users-permissions/server/utils/index.d.ts | 2 + 9 files changed, 122 insertions(+), 22 deletions(-) create mode 100644 packages/plugins/users-permissions/server/services/permission.js diff --git a/packages/core/admin/server/strategies/__tests__/admin.test.js b/packages/core/admin/server/strategies/__tests__/admin.test.js index aeeb26f67a..4a6667a08d 100644 --- a/packages/core/admin/server/strategies/__tests__/admin.test.js +++ b/packages/core/admin/server/strategies/__tests__/admin.test.js @@ -16,7 +16,7 @@ describe('Admin Auth Strategy', () => { const ctx = createContext({}, { request, state: {} }); const user = { id: 1, isActive: true }; const findOne = jest.fn(() => user); - const generateUserAbility = jest.fn(); + const generateUserAbility = jest.fn(() => 'ability'); global.strapi = { admin: { @@ -32,7 +32,11 @@ describe('Admin Auth Strategy', () => { expect(decodeJwtToken).toHaveBeenCalledWith('admin_tests-jwt-token'); expect(findOne).toHaveBeenCalledWith({ where: { id: 1 }, populate: ['roles'] }); - expect(response).toStrictEqual({ authenticated: true, credentials: user }); + expect(response).toStrictEqual({ + authenticated: true, + credentials: user, + ability: 'ability', + }); }); test('Fails to authenticate if the authorization header is missing', async () => { diff --git a/packages/core/admin/server/strategies/admin.js b/packages/core/admin/server/strategies/admin.js index 7d1d7d37dc..1492229323 100644 --- a/packages/core/admin/server/strategies/admin.js +++ b/packages/core/admin/server/strategies/admin.js @@ -33,10 +33,16 @@ const authenticate = async ctx => { const userAbility = await getService('permission').engine.generateUserAbility(user); + // TODO: use the ability from ctx.state.auth instead of + // ctx.state.userAbility, and remove the assign below ctx.state.userAbility = userAbility; ctx.state.user = user; - return { authenticated: true, credentials: user }; + return { + authenticated: true, + credentials: user, + ability: userAbility, + }; }; /** @type {import('.').AuthStrategy} */ diff --git a/packages/core/strapi/lib/services/auth/index.js b/packages/core/strapi/lib/services/auth/index.js index ccf96614af..f463084669 100644 --- a/packages/core/strapi/lib/services/auth/index.js +++ b/packages/core/strapi/lib/services/auth/index.js @@ -32,6 +32,7 @@ const createAuthentication = () => { return this; }, + async authenticate(ctx, next) { const { route } = ctx.state; @@ -47,7 +48,7 @@ const createAuthentication = () => { for (const strategy of strategiesToUse) { const result = await strategy.authenticate(ctx); - const { authenticated = false, error = null, credentials } = result || {}; + const { authenticated = false, credentials, ability = null, error = null } = result || {}; if (error !== null) { return ctx.unauthorized(error); @@ -58,6 +59,7 @@ const createAuthentication = () => { ctx.state.auth = { strategy, credentials, + ability, }; return next(); @@ -66,6 +68,7 @@ const createAuthentication = () => { return ctx.unauthorized('Missing or invalid credentials'); }, + async verify(auth, config = {}) { if (config === false) { return; diff --git a/packages/core/strapi/lib/types/core/strapi/index.d.ts b/packages/core/strapi/lib/types/core/strapi/index.d.ts index 5e07ae1705..4a632d4dee 100644 --- a/packages/core/strapi/lib/types/core/strapi/index.d.ts +++ b/packages/core/strapi/lib/types/core/strapi/index.d.ts @@ -23,6 +23,11 @@ export interface Strapi { */ readonly auth: any; + /** + * Getter for the Strapi content API container + */ + readonly contentAPI: any; + /** * Getter for the Strapi sanitizers container */ diff --git a/packages/plugins/users-permissions/server/services/index.js b/packages/plugins/users-permissions/server/services/index.js index 53589d442f..3df8e27da2 100644 --- a/packages/plugins/users-permissions/server/services/index.js +++ b/packages/plugins/users-permissions/server/services/index.js @@ -6,6 +6,7 @@ const user = require('./user'); const role = require('./role'); const usersPermissions = require('./users-permissions'); const providersRegistry = require('./providers-registry'); +const permission = require('./permission'); module.exports = { jwt, @@ -14,4 +15,5 @@ module.exports = { role, user, 'users-permissions': usersPermissions, + permission, }; diff --git a/packages/plugins/users-permissions/server/services/permission.js b/packages/plugins/users-permissions/server/services/permission.js new file mode 100644 index 0000000000..b3f70e9805 --- /dev/null +++ b/packages/plugins/users-permissions/server/services/permission.js @@ -0,0 +1,55 @@ +'use strict'; + +const PUBLIC_ROLE_FILTER = { role: { type: 'public' } }; + +module.exports = ({ strapi }) => ({ + /** + * Find permissions associated to a specific role ID + * + * @param {number} roleID + * + * @return {object[]} + */ + async findRolePermissions(roleID) { + return strapi.entityService.load( + 'plugin::users-permissions.role', + { id: roleID }, + 'permissions' + ); + }, + + /** + * Find permissions for the public role + * + * @return {object[]} + */ + async findPublicPermissions() { + return strapi.entityService.findMany('plugin::users-permissions.permission', { + where: PUBLIC_ROLE_FILTER, + }); + }, + + /** + * Transform a Users-Permissions' permission into a content API one + * + * @example + * const upPermission = { action: 'api::foo.foo.find' }; + * + * const permission = toContentAPIPermission(upPermission); + * // ^? { action: 'find', subject: 'api::foo.foo' } + * + * @param {object} permission + * @param {string} permission.action + * + * @return {{ action: string, subject: string }} + */ + toContentAPIPermission(permission) { + const { action } = permission; + const actionIndex = action.lastIndexOf('.'); + + return { + action: action.slice(actionIndex + 1), + subject: action.slice(0, actionIndex), + }; + }, +}); diff --git a/packages/plugins/users-permissions/server/services/users-permissions.js b/packages/plugins/users-permissions/server/services/users-permissions.js index 46c82b491c..d6b0fd42a4 100644 --- a/packages/plugins/users-permissions/server/services/users-permissions.js +++ b/packages/plugins/users-permissions/server/services/users-permissions.js @@ -170,6 +170,13 @@ module.exports = ({ strapi }) => ({ const toDelete = _.difference(permissionsFoundInDB, allActions); + // Register actions into the content API action provider + // TODO: do this in the content API bootstrap phase instead + allActions + // 'api::foo.foo.find' => { action: 'find', subject: 'api.foo.foo' } => 'find'; + .map(action => getService('permission').toContentAPIPermission({ action }).action) + .forEach(action => strapi.contentAPI.permissions.providers.action.register(action)); + await Promise.all( toDelete.map(action => { return strapi.query('plugin::users-permissions.permission').delete({ where: { action } }); diff --git a/packages/plugins/users-permissions/server/strategies/users-permissions.js b/packages/plugins/users-permissions/server/strategies/users-permissions.js index 88b556fce9..873aef0737 100644 --- a/packages/plugins/users-permissions/server/strategies/users-permissions.js +++ b/packages/plugins/users-permissions/server/strategies/users-permissions.js @@ -1,6 +1,6 @@ 'use strict'; -const { castArray, map } = require('lodash/fp'); +const { castArray, map, every, pipe } = require('lodash/fp'); const { ForbiddenError, UnauthorizedError } = require('@strapi/utils').errors; const { getService } = require('../utils'); @@ -16,48 +16,61 @@ const authenticate = async ctx => { if (token) { const { id } = token; + // Invalid token if (id === undefined) { return { authenticated: false }; } - // fetch authenticated user const user = await getService('user').fetchAuthenticatedUser(id); + // No user associated to the token if (!user) { return { error: 'Invalid credentials' }; } const advancedSettings = await getAdvancedSettings(); + // User not confirmed if (advancedSettings.email_confirmation && !user.confirmed) { return { error: 'Invalid credentials' }; } + // User blocked if (user.blocked) { return { error: 'Invalid credentials' }; } + // Fetch user's permissions + const permissions = await Promise.resolve(user.role.id) + .then(getService('permission').findRolePermissions) + .then(map(getService('permission').toContentAPIPermission)); + + // Generate an ability (content API engine) based on the given permissions + const ability = await strapi.contentAPI.permissions.engine.generateAbility(permissions); + ctx.state.user = user; return { authenticated: true, credentials: user, + ability, }; } - const publicPermissions = await strapi.query('plugin::users-permissions.permission').findMany({ - where: { - role: { type: 'public' }, - }, - }); + const publicPermissions = await getService('permission') + .findPublicPermissions() + .then(map(getService('permission').toContentAPIPermission)); if (publicPermissions.length === 0) { return { authenticated: false }; } + const ability = await strapi.contentAPI.permissions.engine.generateAbility(publicPermissions); + return { authenticated: true, credentials: null, + ability, }; } catch (err) { return { authenticated: false }; @@ -65,7 +78,7 @@ const authenticate = async ctx => { }; const verify = async (auth, config) => { - const { credentials: user } = auth; + const { credentials: user, ability } = auth; if (!config.scope) { if (!user) { @@ -77,18 +90,21 @@ const verify = async (auth, config) => { } } - let allowedActions = auth.allowedActions; - - if (!allowedActions) { - const permissions = await strapi.query('plugin::users-permissions.permission').findMany({ - where: { role: user ? user.role.id : { type: 'public' } }, - }); - - allowedActions = map('action', permissions); - auth.allowedActions = allowedActions; + // If no ability have been generated, then consider auth is missing + if (!ability) { + throw new UnauthorizedError(); } - const isAllowed = castArray(config.scope).every(scope => allowedActions.includes(scope)); + const isAllowed = pipe( + // Make sure we're dealing with an array + castArray, + // Transform the scope array into an action array + map(scope => ({ action: scope })), + // Map the users-permissions permissions into content API permissions + map(getService('permission').toContentAPIPermission), + // Check that every required scope is allowed by the ability + every(({ action, subject }) => ability.can(action, subject)) + )(config.scope); if (!isAllowed) { throw new ForbiddenError(); diff --git a/packages/plugins/users-permissions/server/utils/index.d.ts b/packages/plugins/users-permissions/server/utils/index.d.ts index b66a6db410..b9d7e20883 100644 --- a/packages/plugins/users-permissions/server/utils/index.d.ts +++ b/packages/plugins/users-permissions/server/utils/index.d.ts @@ -3,6 +3,7 @@ import * as user from '../services/user'; import * as role from '../services/role'; import * as jwt from '../services/jwt'; import * as providers from '../services/providers'; +import * as permission from '../services/permission'; type S = { ['users-permissions']: typeof usersPermissions; @@ -11,6 +12,7 @@ type S = { jwt: typeof jwt; providers: typeof providers; ['providers-registry']: typeof providers; + permission: typeof permission; }; export function getService(name: T): ReturnType; From 72c721e8e28277590f859f5d47f3585300323a91 Mon Sep 17 00:00:00 2001 From: Convly Date: Mon, 1 Aug 2022 09:31:11 +0200 Subject: [PATCH 2/6] Remove old engine test --- .../__tests__/permissions.engine.test.js | 443 ------------------ 1 file changed, 443 deletions(-) delete mode 100644 packages/core/admin/server/services/__tests__/permissions.engine.test.js diff --git a/packages/core/admin/server/services/__tests__/permissions.engine.test.js b/packages/core/admin/server/services/__tests__/permissions.engine.test.js deleted file mode 100644 index 2f7b51ac47..0000000000 --- a/packages/core/admin/server/services/__tests__/permissions.engine.test.js +++ /dev/null @@ -1,443 +0,0 @@ -'use strict'; - -const _ = require('lodash'); -const { subject } = require('@casl/ability'); -const createConditionProvider = require('../../domain/condition/provider'); -const createPermissionsEngine = require('../permission/engine'); - -describe('Permissions Engine', () => { - let conditionProvider; - let engine; - - const localTestData = { - users: { - bob: { - firstname: 'Bob', - title: 'guest', - roles: [{ id: 1 }, { id: 2 }], - }, - alice: { - firstname: 'Alice', - title: 'admin', - roles: [{ id: 1 }], - }, - kai: { - firstname: 'Kai', - title: 'admin', - roles: [{ id: 3 }], - }, - foo: { - firstname: 'Foo', - title: 'Bar', - roles: [{ id: 4 }], - }, - }, - roles: { - 1: { - permissions: [ - { - action: 'read', - subject: 'article', - properties: { fields: ['**'] }, - conditions: ['plugin::test.isBob'], - }, - { - action: 'read', - subject: 'user', - properties: { fields: ['title'] }, - conditions: ['plugin::test.isAdmin'], - }, - ], - }, - 2: { - permissions: [ - { - action: 'post', - subject: 'article', - properties: { fields: ['*'] }, - conditions: ['plugin::test.isBob'], - }, - ], - }, - 3: { - permissions: [ - { - action: 'read', - subject: 'user', - properties: { fields: ['title'] }, - conditions: ['plugin::test.isContainedIn'], - }, - ], - }, - 4: { - permissions: [ - { - action: 'read', - subject: 'user', - properties: { fields: [] }, - }, - ], - }, - }, - conditions: [ - { - plugin: 'test', - name: 'isBob', - category: 'default', - handler: async user => new Promise(resolve => resolve(user.firstname === 'Bob')), - }, - { - plugin: 'test', - name: 'isAdmin', - category: 'default', - handler: user => user.title === 'admin', - }, - { - plugin: 'test', - name: 'isCreatedBy', - category: 'default', - handler: user => ({ createdBy: user.firstname }), - }, - { - plugin: 'test', - name: 'isContainedIn', - category: 'default', - handler: () => ({ firstname: { $in: ['Alice', 'Foo'] } }), - }, - ], - }; - - const getUser = name => localTestData.users[name]; - - beforeEach(async () => { - global.strapi = { - isLoaded: false, - admin: { - services: { - permission: { - actionProvider: { - get() { - return { applyToProperties: undefined }; - }, - }, - findUserPermissions: jest.fn(({ roles }) => - _.reduce( - localTestData.roles, - (acc, { permissions: value }, key) => { - return roles.map(_.property('id')).includes(_.toNumber(key)) - ? [...acc, ...value] - : acc; - }, - [] - ) - ), - }, - }, - }, - }; - - conditionProvider = createConditionProvider(); - await conditionProvider.registerMany(localTestData.conditions); - - engine = createPermissionsEngine(conditionProvider); - - jest.spyOn(engine, 'evaluate'); - jest.spyOn(engine, 'createRegisterFunction'); - jest.spyOn(engine, 'generateAbilityCreatorFor'); - }); - - afterEach(() => { - jest.clearAllMocks(); - }); - - describe('GenerateUserAbility', () => { - test('Successfully creates an ability for Bob', async () => { - const user = getUser('bob'); - - const ability = await engine.generateUserAbility(user); - - const expected = [ - { - action: 'read', - fields: ['**'], - subject: 'article', - }, - { - action: 'post', - fields: ['*'], - subject: 'article', - }, - ]; - - expect(engine.generateAbilityCreatorFor).toHaveBeenCalledWith(user); - expect(_.orderBy(ability.rules, ['subject'], ['asc'])).toMatchObject(expected); - - expect(ability.can('post', 'article')).toBeTruthy(); - expect(ability.can('post', 'article', 'user')).toBeTruthy(); - expect(ability.can('post', 'article', 'user.nested')).toBeFalsy(); - - expect(ability.can('read', 'article')).toBeTruthy(); - expect(ability.can('read', 'article', 'title')).toBeTruthy(); - expect(ability.can('read', 'article', 'title.nested')).toBeTruthy(); - - expect(ability.can('read', 'user')).toBeFalsy(); - expect(ability.can('read', 'user', 'firstname')).toBeFalsy(); - expect(ability.can('read', 'user', 'title')).toBeFalsy(); - expect(ability.can('read', 'user', 'title.nested')).toBeFalsy(); - }); - - test('Successfully creates an ability for Alice', async () => { - const user = getUser('alice'); - - const ability = await engine.generateUserAbility(user); - - const expected = [ - { - action: 'read', - fields: ['title'], - subject: 'user', - }, - ]; - - expect(engine.generateAbilityCreatorFor).toHaveBeenCalledWith(user); - expect(_.orderBy(ability.rules, ['action'], ['asc'])).toMatchObject(expected); - - expect(ability.can('post', 'article')).toBeFalsy(); - expect(ability.can('post', 'article', 'user')).toBeFalsy(); - expect(ability.can('post', 'article', 'user.nested')).toBeFalsy(); - - expect(ability.can('read', 'article')).toBeFalsy(); - expect(ability.can('read', 'article', 'title')).toBeFalsy(); - expect(ability.can('read', 'article', 'title.nested')).toBeFalsy(); - - expect(ability.can('read', 'user')).toBeTruthy(); - expect(ability.can('read', 'user', 'firstname')).toBeFalsy(); - expect(ability.can('read', 'user', 'title')).toBeTruthy(); - expect(ability.can('read', 'user', 'title.nested')).toBeFalsy(); - }); - - test('Ignore permission on empty fields array', async () => { - const user = getUser('foo'); - - const ability = await engine.generateUserAbility(user); - - expect(engine.generateAbilityCreatorFor).toHaveBeenCalledWith(user); - expect(ability.rules).toHaveLength(0); - expect(ability.can('read', 'user')).toBeFalsy(); - }); - - describe('Use objects as subject', () => { - let ability; - - beforeAll(async () => { - const user = getUser('kai'); - ability = await engine.generateUserAbility(user); - }); - - test('Fails to validate the object condition', () => { - const args = ['read', subject('user', { firstname: 'Bar' }), 'title']; - - expect(ability.can(...args)).toBeFalsy(); - }); - - test('Fails to read a restricted field', () => { - const args = ['read', subject('user', { firstname: 'Foo' }), 'bar']; - - expect(ability.can(...args)).toBeFalsy(); - }); - - test('Successfully validate the permission', () => { - const args = ['read', subject('user', { firstname: 'Foo' }), 'title']; - - expect(ability.can(...args)).toBeTruthy(); - }); - }); - }); - - describe('Generate Ability Creator For', () => { - test('Successfully generates an ability creator for Alice', async () => { - const user = getUser('alice'); - - const abilityCreator = engine.generateAbilityCreatorFor(user); - const ability = await abilityCreator([]); - - expect(abilityCreator).not.toBeUndefined(); - expect(typeof abilityCreator).toBe('function'); - expect(ability.rules).toStrictEqual([]); - }); - }); - - describe('Evaluate', () => { - test('It should register the permission (no conditions)', async () => { - const permission = { action: 'read', subject: 'article', properties: { fields: ['title'] } }; - const user = getUser('alice'); - const registerFn = jest.fn(); - - await engine.evaluate({ permission, user, registerFn }); - - expect(registerFn).toHaveBeenCalledWith({ - ..._.pick(permission, ['action', 'subject']), - fields: permission.properties.fields, - }); - }); - - test('It should register the permission without a condition (non required true result)', async () => { - const permission = { - action: 'read', - subject: 'article', - properties: { fields: ['title'] }, - conditions: ['plugin::test.isAdmin'], - }; - const user = getUser('alice'); - const registerFn = jest.fn(); - - await engine.evaluate({ permission, user, registerFn }); - - const expected = { - ..._.omit(permission, ['conditions', 'properties']), - fields: permission.properties.fields, - }; - - expect(registerFn).toHaveBeenCalledWith(expected); - }); - - test('It should not register the permission (conditions / false result)', async () => { - const permission = { - action: 'read', - subject: 'article', - properties: { fields: ['title'] }, - conditions: ['plugin::test.isBob'], - }; - const user = getUser('alice'); - const registerFn = jest.fn(); - - await engine.evaluate({ permission, user, registerFn }); - - expect(registerFn).not.toHaveBeenCalled(); - }); - - test('It should register the permission (non required object result)', async () => { - const permission = { - action: 'read', - subject: 'article', - properties: { fields: ['title'] }, - conditions: ['plugin::test.isCreatedBy'], - }; - - global.strapi.admin.services.permission.actionProvider.get = () => ({ - applyToProperties: ['fields'], - }); - - const user = getUser('alice'); - const registerFn = jest.fn(); - - await engine.evaluate({ permission, user, registerFn }); - - const expected = { - ..._.omit(permission, ['conditions', 'properties']), - fields: permission.properties.fields, - condition: { - $and: [ - { - $or: [{ createdBy: user.firstname }], - }, - ], - }, - }; - - expect(registerFn).toHaveBeenCalledWith(expected); - }); - }); - - test('It should register the condition even if the subject is Nil', async () => { - const permission = { - action: 'read', - subject: null, - properties: {}, - conditions: ['plugin::test.isCreatedBy'], - }; - - const user = getUser('alice'); - const can = jest.fn(); - const registerFn = engine.createRegisterFunction(can, {}, user); - - await engine.evaluate({ permission, user, registerFn }); - - expect(can).toHaveBeenCalledWith('read', 'all', undefined, { - $and: [{ $or: [{ createdBy: user.firstname }] }], - }); - }); - - describe('Create Register Function', () => { - let can; - let registerFn; - - beforeEach(() => { - can = jest.fn(); - registerFn = engine.createRegisterFunction(can, {}, {}); - }); - - test('It should calls the can function without any condition', async () => { - await registerFn({ action: 'read', subject: 'article', fields: '*', condition: true }); - - expect(can).toHaveBeenCalledTimes(1); - expect(can).toHaveBeenCalledWith('read', 'article', '*', undefined); - }); - - test('It should calls the can function with a condition', async () => { - await registerFn({ - action: 'read', - subject: 'article', - fields: '*', - condition: { createdBy: 1 }, - }); - - expect(can).toHaveBeenCalledTimes(1); - expect(can).toHaveBeenCalledWith('read', 'article', '*', { createdBy: 1 }); - }); - - test(`It should use 'all' as a subject if it's Nil`, async () => { - await registerFn({ - action: 'read', - subject: null, - fields: null, - condition: { createdBy: 1 }, - }); - - expect(can).toHaveBeenCalledTimes(1); - expect(can).toHaveBeenCalledWith('read', 'all', null, { createdBy: 1 }); - }); - }); - - describe('Check Many', () => { - let ability; - const permissions = [ - { action: 'read', subject: 'user', field: 'title' }, - { action: 'post', subject: 'article' }, - ]; - - beforeEach(() => { - ability = { can: jest.fn(() => true) }; - }); - - afterEach(() => { - jest.clearAllMocks(); - }); - - test('Using curried version of checkMany', () => { - const checkMany = engine.checkMany(ability); - - const res = checkMany(permissions); - - expect(res).toHaveLength(permissions.length); - expect(ability.can).toHaveBeenCalledTimes(2); - }); - - test('Using raw version of checkMany', () => { - const res = engine.checkMany(ability, permissions); - - expect(res).toHaveLength(permissions.length); - expect(ability.can).toHaveBeenCalledTimes(2); - }); - }); -}); From faf531ef7bc326f03013c6c5b5edfa84a87590e2 Mon Sep 17 00:00:00 2001 From: Convly Date: Mon, 1 Aug 2022 10:37:52 +0200 Subject: [PATCH 3/6] Fix front tests --- .../EditView/tests/__snapshots__/index.test.js.snap | 8 ++++---- .../pages/ApiTokens/EditView/tests/index.test.js | 1 + 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/core/admin/admin/src/pages/SettingsPage/pages/ApiTokens/EditView/tests/__snapshots__/index.test.js.snap b/packages/core/admin/admin/src/pages/SettingsPage/pages/ApiTokens/EditView/tests/__snapshots__/index.test.js.snap index 398dbc09fc..d511838a4b 100644 --- a/packages/core/admin/admin/src/pages/SettingsPage/pages/ApiTokens/EditView/tests/__snapshots__/index.test.js.snap +++ b/packages/core/admin/admin/src/pages/SettingsPage/pages/ApiTokens/EditView/tests/__snapshots__/index.test.js.snap @@ -987,7 +987,7 @@ exports[`ADMIN | Pages | API TOKENS | EditView renders and matches the snapshot border: 1px solid #4945ff; } -.c71:hover:not([aria-disabled='true']) .sc-eaPPdL { +.c71:hover:not([aria-disabled='true']) .sc-inrDdN { color: #271fe0; } @@ -1011,7 +1011,7 @@ exports[`ADMIN | Pages | API TOKENS | EditView renders and matches the snapshot border: 1px solid #4945ff; } -.c88:hover:not([aria-disabled='true']) .sc-eaPPdL { +.c88:hover:not([aria-disabled='true']) .sc-inrDdN { color: #271fe0; } @@ -2845,7 +2845,7 @@ exports[`ADMIN | Pages | API TOKENS | EditView renders and matches the snapshot border: 1px solid #4945ff; } -.c82:hover:not([aria-disabled='true']) .sc-eaPPdL { +.c82:hover:not([aria-disabled='true']) .sc-inrDdN { color: #271fe0; } @@ -2869,7 +2869,7 @@ exports[`ADMIN | Pages | API TOKENS | EditView renders and matches the snapshot border: 1px solid #4945ff; } -.c99:hover:not([aria-disabled='true']) .sc-eaPPdL { +.c99:hover:not([aria-disabled='true']) .sc-inrDdN { color: #271fe0; } diff --git a/packages/core/admin/admin/src/pages/SettingsPage/pages/ApiTokens/EditView/tests/index.test.js b/packages/core/admin/admin/src/pages/SettingsPage/pages/ApiTokens/EditView/tests/index.test.js index 17908cf59e..4e9226862d 100644 --- a/packages/core/admin/admin/src/pages/SettingsPage/pages/ApiTokens/EditView/tests/index.test.js +++ b/packages/core/admin/admin/src/pages/SettingsPage/pages/ApiTokens/EditView/tests/index.test.js @@ -14,6 +14,7 @@ jest.mock('@strapi/helper-plugin', () => ({ ...jest.requireActual('@strapi/helper-plugin'), useNotification: jest.fn(), useFocusWhenNavigate: jest.fn(), + useTracking: jest.fn(() => ({ trackUsage: jest.fn() })), useRBAC: jest.fn(() => ({ allowedActions: { canCreate: true, canDelete: true, canRead: true, canUpdate: true }, })), From 7d40f78384aa6aed968d5de66059b500fe5815de Mon Sep 17 00:00:00 2001 From: Convly Date: Tue, 2 Aug 2022 16:51:06 +0200 Subject: [PATCH 4/6] Revert action/subject to action only --- .../server/services/permission.js | 16 +++------------- .../server/services/users-permissions.js | 5 +---- .../server/strategies/users-permissions.js | 6 +----- 3 files changed, 5 insertions(+), 22 deletions(-) diff --git a/packages/plugins/users-permissions/server/services/permission.js b/packages/plugins/users-permissions/server/services/permission.js index b3f70e9805..339a3ac67c 100644 --- a/packages/plugins/users-permissions/server/services/permission.js +++ b/packages/plugins/users-permissions/server/services/permission.js @@ -30,26 +30,16 @@ module.exports = ({ strapi }) => ({ }, /** - * Transform a Users-Permissions' permission into a content API one - * - * @example - * const upPermission = { action: 'api::foo.foo.find' }; - * - * const permission = toContentAPIPermission(upPermission); - * // ^? { action: 'find', subject: 'api::foo.foo' } + * Transform a Users-Permissions' action into a content API one * * @param {object} permission * @param {string} permission.action * - * @return {{ action: string, subject: string }} + * @return {{ action: string }} */ toContentAPIPermission(permission) { const { action } = permission; - const actionIndex = action.lastIndexOf('.'); - return { - action: action.slice(actionIndex + 1), - subject: action.slice(0, actionIndex), - }; + return { action }; }, }); diff --git a/packages/plugins/users-permissions/server/services/users-permissions.js b/packages/plugins/users-permissions/server/services/users-permissions.js index d6b0fd42a4..98099e16ed 100644 --- a/packages/plugins/users-permissions/server/services/users-permissions.js +++ b/packages/plugins/users-permissions/server/services/users-permissions.js @@ -172,10 +172,7 @@ module.exports = ({ strapi }) => ({ // Register actions into the content API action provider // TODO: do this in the content API bootstrap phase instead - allActions - // 'api::foo.foo.find' => { action: 'find', subject: 'api.foo.foo' } => 'find'; - .map(action => getService('permission').toContentAPIPermission({ action }).action) - .forEach(action => strapi.contentAPI.permissions.providers.action.register(action)); + allActions.forEach(action => strapi.contentAPI.permissions.providers.action.register(action)); await Promise.all( toDelete.map(action => { diff --git a/packages/plugins/users-permissions/server/strategies/users-permissions.js b/packages/plugins/users-permissions/server/strategies/users-permissions.js index 873aef0737..70ed4b0985 100644 --- a/packages/plugins/users-permissions/server/strategies/users-permissions.js +++ b/packages/plugins/users-permissions/server/strategies/users-permissions.js @@ -99,11 +99,7 @@ const verify = async (auth, config) => { // Make sure we're dealing with an array castArray, // Transform the scope array into an action array - map(scope => ({ action: scope })), - // Map the users-permissions permissions into content API permissions - map(getService('permission').toContentAPIPermission), - // Check that every required scope is allowed by the ability - every(({ action, subject }) => ability.can(action, subject)) + every(scope => ability.can(scope)) )(config.scope); if (!isAllowed) { From 53f642f2ed63e6a122371bd46a51725c7573872e Mon Sep 17 00:00:00 2001 From: Convly Date: Tue, 2 Aug 2022 17:42:38 +0200 Subject: [PATCH 5/6] Fix tests --- packages/core/permissions/README.md | 2 +- .../permissions/lib/__tests__/permissions.engine.test.js | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/core/permissions/README.md b/packages/core/permissions/README.md index e9aa6a6525..b5d0da9e9d 100644 --- a/packages/core/permissions/README.md +++ b/packages/core/permissions/README.md @@ -78,7 +78,7 @@ const engine = permissions.engine return false; } }) - .on('post-format::validate.permission', ({ permission }) => { + .on('after-format::validate.permission', ({ permission }) => { if (permission.action === 'update') { return false; } diff --git a/packages/core/permissions/lib/__tests__/permissions.engine.test.js b/packages/core/permissions/lib/__tests__/permissions.engine.test.js index 61d3357e40..bf09a44d82 100644 --- a/packages/core/permissions/lib/__tests__/permissions.engine.test.js +++ b/packages/core/permissions/lib/__tests__/permissions.engine.test.js @@ -377,7 +377,7 @@ describe('Permissions Engine', () => { }); }); - describe('post-format::validate.permission', () => { + describe('after-format::validate.permission', () => { it('can prevent action register', async () => { const permissions = [ { action: 'read', subject: 'article' }, @@ -390,7 +390,7 @@ describe('Permissions Engine', () => { permissions, engineHooks: [ { - name: 'post-format::validate.permission', + name: 'after-format::validate.permission', fn: generateInvalidateActionHook('read'), }, ], @@ -450,7 +450,7 @@ describe('Permissions Engine', () => { fn: generateInvalidateActionHook('view'), }, { - name: 'post-format::validate.permission', + name: 'after-format::validate.permission', fn: generateInvalidateActionHook('update'), }, ], From 787a90de69e5b722b1bb4314d08c1b307a0e925b Mon Sep 17 00:00:00 2001 From: Convly Date: Tue, 2 Aug 2022 17:43:40 +0200 Subject: [PATCH 6/6] Fix typings --- packages/core/permissions/index.d.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/permissions/index.d.ts b/packages/core/permissions/index.d.ts index d024d17d77..821ccca5a3 100644 --- a/packages/core/permissions/index.d.ts +++ b/packages/core/permissions/index.d.ts @@ -24,7 +24,7 @@ interface ConditionProvider extends Provider {} interface PermissionEngineHooks { 'before-format::validate.permission': ReturnType; 'format.permission': ReturnType; - 'post-format::validate.permission': ReturnType; + 'after-format::validate.permission': ReturnType; 'before-evaluate.permission': ReturnType; 'before-register.permission': ReturnType; }