From 386579ff668edc7a81d66cbfe434b4a77f43b9d0 Mon Sep 17 00:00:00 2001 From: Convly Date: Mon, 6 Feb 2023 10:03:08 +0100 Subject: [PATCH] Update transfer services, fix api tests --- packages/core/admin/server/bootstrap.js | 2 - packages/core/admin/server/routes/transfer.js | 3 +- .../server/services/transfer/permission.js | 52 ++++------------ .../admin/server/services/transfer/token.js | 62 +++++++++++++------ .../admin/server/strategies/data-transfer.js | 2 +- .../providers/remote-destination/utils.ts | 1 - .../src/strapi/remote/constants.ts | 2 +- 7 files changed, 61 insertions(+), 63 deletions(-) diff --git a/packages/core/admin/server/bootstrap.js b/packages/core/admin/server/bootstrap.js index 4534f0dcc6..e04c989bb0 100644 --- a/packages/core/admin/server/bootstrap.js +++ b/packages/core/admin/server/bootstrap.js @@ -96,6 +96,4 @@ module.exports = async () => { apiTokenService.checkSaltIsDefined(); transferService.token.checkSaltIsDefined(); tokenService.checkSecretIsDefined(); - - transferService.permission.init(); }; diff --git a/packages/core/admin/server/routes/transfer.js b/packages/core/admin/server/routes/transfer.js index f82dac17c2..7b88c927ce 100644 --- a/packages/core/admin/server/routes/transfer.js +++ b/packages/core/admin/server/routes/transfer.js @@ -18,7 +18,8 @@ module.exports = [ return next(); }, ], - auth: { strategies: [dataTransferAuthStrategy] }, + // TODO: Allow not passing any scope <> Add a way to prevent assigning one by default + auth: { strategies: [dataTransferAuthStrategy], scope: ['push'] }, }, }, // Transfer Tokens diff --git a/packages/core/admin/server/services/transfer/permission.js b/packages/core/admin/server/services/transfer/permission.js index afaeecdbe0..d9d3646244 100644 --- a/packages/core/admin/server/services/transfer/permission.js +++ b/packages/core/admin/server/services/transfer/permission.js @@ -5,44 +5,18 @@ const { providerFactory } = require('@strapi/utils'); const DEFAULT_TRANSFER_ACTIONS = ['push']; -const createPermissionService = () => { - const state = { - engine: null, - - providers: { - action: null, - condition: null, - }, - }; - - const registerTransferActions = () => { - if (!state.providers.action) { - return; - } - - DEFAULT_TRANSFER_ACTIONS.forEach((action) => { - state.providers.action.register(action, { action }); - }); - }; - - return { - get engine() { - return state.engine; - }, - - get providers() { - return state.providers; - }, - - init() { - state.providers.action = providerFactory(); - state.providers.condition = providerFactory(); - - registerTransferActions(); - - state.engine = permissions.engine.new({ providers: state.providers }); - }, - }; +const providers = { + action: providerFactory(), + condition: providerFactory(), }; -module.exports = createPermissionService(); +DEFAULT_TRANSFER_ACTIONS.forEach((action) => { + providers.action.register(action, { action }); +}); + +const engine = permissions.engine.new({ providers }); + +module.exports = { + engine, + providers, +}; diff --git a/packages/core/admin/server/services/transfer/token.js b/packages/core/admin/server/services/transfer/token.js index 1522dc4dbf..60685d97bf 100644 --- a/packages/core/admin/server/services/transfer/token.js +++ b/packages/core/admin/server/services/transfer/token.js @@ -139,7 +139,7 @@ const update = async (id, attributes) => { assertTokenPermissionsValidity(attributes); assertValidLifespan(attributes); - const updatedToken = await strapi.db.transaction(async () => { + return strapi.db.transaction(async () => { const updatedToken = await strapi.query(TRANSFER_TOKEN_UID).update({ select: SELECT_FIELDS, where: { id }, @@ -148,24 +148,50 @@ const update = async (id, attributes) => { }, }); - await strapi.query(TRANSFER_TOKEN_PERMISSION_UID).delete({ - where: { token: id }, - }); + const currentPermissionsResult = await strapi.entityService.load( + TRANSFER_TOKEN_UID, + updatedToken, + 'permissions' + ); - return updatedToken; + const currentPermissions = map('action', currentPermissionsResult || []); + const newPermissions = uniq(attributes.permissions); + + const actionsToDelete = difference(currentPermissions, newPermissions); + const actionsToAdd = difference(newPermissions, currentPermissions); + + // TODO: improve efficiency here + // method using a loop -- works but very inefficient + await Promise.all( + actionsToDelete.map((action) => + strapi.query(TRANSFER_TOKEN_PERMISSION_UID).delete({ + where: { action, token: id }, + }) + ) + ); + + // TODO: improve efficiency here + // using a loop -- works but very inefficient + await Promise.all( + actionsToAdd.map((action) => + strapi.query(TRANSFER_TOKEN_PERMISSION_UID).create({ + data: { action, token: id }, + }) + ) + ); + + // retrieve permissions + const permissionsFromDb = await strapi.entityService.load( + TRANSFER_TOKEN_UID, + updatedToken, + 'permissions' + ); + + return { + ...updatedToken, + permissions: permissionsFromDb ? permissionsFromDb.map((p) => p.action) : undefined, + }; }); - - // retrieve permissions - const permissionsFromDb = await strapi.entityService.load( - 'admin::transfer-token', - updatedToken, - 'permissions' - ); - - return { - ...updatedToken, - permissions: permissionsFromDb ? permissionsFromDb.map((p) => p.action) : undefined, - }; }; /** @@ -341,7 +367,7 @@ const flattenTokenPermissions = (token) => { * @param {TransferToken} token */ const assertTokenPermissionsValidity = (attributes) => { - const { permission: permissionService } = strapi.admin.services.transfer; + const permissionService = strapi.admin.services.transfer.permission; const validPermissions = permissionService.providers.action.keys(); const invalidPermissions = difference(attributes.permissions, validPermissions); diff --git a/packages/core/admin/server/strategies/data-transfer.js b/packages/core/admin/server/strategies/data-transfer.js index 0429bf1753..2bc5f05955 100644 --- a/packages/core/admin/server/strategies/data-transfer.js +++ b/packages/core/admin/server/strategies/data-transfer.js @@ -59,7 +59,7 @@ const authenticate = async (ctx) => { }); // Generate an ability based on the token permissions - const ability = getService('transfer').permission.engine.generateAbility( + const ability = await getService('transfer').permission.engine.generateAbility( transferToken.permissions.map((action) => ({ action })) ); diff --git a/packages/core/data-transfer/src/strapi/providers/remote-destination/utils.ts b/packages/core/data-transfer/src/strapi/providers/remote-destination/utils.ts index 0a1931cb4e..2f55ca9227 100644 --- a/packages/core/data-transfer/src/strapi/providers/remote-destination/utils.ts +++ b/packages/core/data-transfer/src/strapi/providers/remote-destination/utils.ts @@ -1,4 +1,3 @@ -import { set } from 'lodash/fp'; import { v4 } from 'uuid'; import { RawData, WebSocket } from 'ws'; diff --git a/packages/core/data-transfer/src/strapi/remote/constants.ts b/packages/core/data-transfer/src/strapi/remote/constants.ts index 4b8400ece4..edf455fcf8 100644 --- a/packages/core/data-transfer/src/strapi/remote/constants.ts +++ b/packages/core/data-transfer/src/strapi/remote/constants.ts @@ -1,2 +1,2 @@ -export const TRANSFER_PATH = '/transfer'; +export const TRANSFER_PATH = '/transfer/runner/connect'; export const TRANSFER_METHODS = ['push', 'pull'];