Merge pull request #15935 from strapi/data-transfer/disable-transfer-when-missing-salt

packages/core/admin/server/middlewares/data-transfer.js

@@ -0,0 +1,26 @@
+'use strict';
+
+const { getService } = require('../utils');
+
+module.exports = () => async (ctx, next) => {
+  const transferUtils = getService('transfer').utils;
+
+  const { hasValidTokenSalt, isDataTransferEnabled, isDisabledFromEnv } = transferUtils;
+
+  if (isDataTransferEnabled()) {
+    return next();
+  }
+
+  if (!hasValidTokenSalt()) {
+    return ctx.notImplemented(
+      'The server configuration for data transfer is invalid. Please contact your server administrator.'
+    );
+  }
+
+  if (isDisabledFromEnv()) {
+    return ctx.notFound();
+  }
+
+  // This should never happen as long as we're handling individual scenarios above
+  throw new Error('Unexpected error while trying to access a data transfer route');
+};

packages/core/admin/server/middlewares/index.js

@@ -4,4 +4,5 @@ const rateLimit = require('./rateLimit');
 
 module.exports = {
   rateLimit,
+  'data-transfer': require('./data-transfer'),
 };

packages/core/admin/server/routes/transfer.js

@@ -9,15 +9,7 @@ module.exports = [
     path: '/transfer/runner/connect',
     handler: 'transfer.runner-connect',
     config: {
-      middlewares: [
-        (ctx, next) => {
-          if (process.env.STRAPI_DISABLE_REMOTE_DATA_TRANSFER === 'true') {
-            return ctx.notFound();
-          }
-
-          return next();
-        },
-      ],
+      middlewares: ['admin::data-transfer'],
       // TODO: Allow not passing any scope <> Add a way to prevent assigning one by default
       auth: { strategies: [dataTransferAuthStrategy], scope: ['push'] },
     },
@@ -28,6 +20,7 @@ module.exports = [
     path: '/transfer/tokens',
     handler: 'transfer.token-create',
     config: {
+      middlewares: ['admin::data-transfer'],
       policies: [
         'admin::isAuthenticatedAdmin',
         { name: 'admin::hasPermissions', config: { actions: ['admin::transfer.tokens.create'] } },
@@ -39,6 +32,7 @@ module.exports = [
     path: '/transfer/tokens',
     handler: 'transfer.token-list',
     config: {
+      middlewares: ['admin::data-transfer'],
       policies: [
         'admin::isAuthenticatedAdmin',
         { name: 'admin::hasPermissions', config: { actions: ['admin::transfer.tokens.read'] } },
@@ -50,6 +44,7 @@ module.exports = [
     path: '/transfer/tokens/:id',
     handler: 'transfer.token-revoke',
     config: {
+      middlewares: ['admin::data-transfer'],
       policies: [
         'admin::isAuthenticatedAdmin',
         { name: 'admin::hasPermissions', config: { actions: ['admin::transfer.tokens.delete'] } },
@@ -61,6 +56,7 @@ module.exports = [
     path: '/transfer/tokens/:id',
     handler: 'transfer.token-getById',
     config: {
+      middlewares: ['admin::data-transfer'],
       policies: [
         'admin::isAuthenticatedAdmin',
         { name: 'admin::hasPermissions', config: { actions: ['admin::transfer.tokens.read'] } },
@@ -72,6 +68,7 @@ module.exports = [
     path: '/transfer/tokens/:id',
     handler: 'transfer.token-update',
     config: {
+      middlewares: ['admin::data-transfer'],
       policies: [
         'admin::isAuthenticatedAdmin',
         { name: 'admin::hasPermissions', config: { actions: ['admin::transfer.tokens.update'] } },
@@ -83,6 +80,7 @@ module.exports = [
     path: '/transfer/tokens/:id/regenerate',
     handler: 'transfer.token-regenerate',
     config: {
+      middlewares: ['admin::data-transfer'],
       policies: [
         'admin::isAuthenticatedAdmin',
         {

packages/core/admin/server/services/transfer/index.js

@@ -3,4 +3,5 @@
 module.exports = {
   permission: require('./permission'),
   token: require('./token'),
+  utils: require('./utils'),
 };

packages/core/admin/server/services/transfer/token.js

@@ -8,6 +8,7 @@ const {
 } = require('@strapi/utils');
 
 const constants = require('../constants');
+const { getService } = require('../../utils');
 
 const TRANSFER_TOKEN_UID = 'admin::transfer-token';
 const TRANSFER_TOKEN_PERMISSION_UID = 'admin::transfer-token-permission';
@@ -327,6 +328,12 @@ const getExpirationFields = (lifespan) => {
  * @returns {string}
  */
 const hash = (accessKey) => {
+  const { hasValidTokenSalt } = getService('transfer').utils;
+
+  if (!hasValidTokenSalt()) {
+    throw new TypeError('Required token salt is not defined');
+  }
+
   return crypto
     .createHmac('sha512', strapi.config.get('admin.transfer.token.salt'))
     .update(accessKey)
@@ -337,9 +344,17 @@ const hash = (accessKey) => {
  * @returns {void}
  */
 const checkSaltIsDefined = () => {
-  if (!strapi.config.get('admin.transfer.token.salt')) {
-    throw new Error(
-      `Missing transfer.token.salt. Please set transfer.token.salt in config/admin.js (ex: you can generate one using Node with \`crypto.randomBytes(16).toString('base64')\`).
+  const { hasValidTokenSalt, isDisabledFromEnv } = getService('transfer').utils;
+
+  // Ignore the check if the data-transfer feature is manually disabled
+  if (isDisabledFromEnv()) {
+    return;
+  }
+
+  if (!hasValidTokenSalt()) {
+    process.emitWarning(
+      `Missing transfer.token.salt: Data transfer features have been disabled.
+Please set transfer.token.salt in config/admin.js (ex: you can generate one using Node with \`crypto.randomBytes(16).toString('base64')\`)
 For security reasons, prefer storing the secret in an environment variable and read it in config/admin.js. See https://docs.strapi.io/developer-docs/latest/setup-deployment-guides/configurations/optional/environment.html#configuration-using-environment-variables.`
     );
   }

packages/core/admin/server/services/transfer/utils.js

@@ -0,0 +1,38 @@
+'use strict';
+
+const { env } = require('@strapi/utils');
+
+const { getService } = require('../../utils');
+
+/**
+ * Returns whether the data transfer features have been disabled from the env configuration
+ *
+ * @returns {boolean}
+ */
+const isDisabledFromEnv = () => {
+  return env.bool('STRAPI_DISABLE_REMOTE_DATA_TRANSFER', false);
+};
+
+/**
+ * A valid transfer token salt must be a non-empty string defined in the Strapi config
+ *
+ * @returns {boolean}
+ */
+const hasValidTokenSalt = () => {
+  const salt = strapi.config.get('admin.transfer.token.salt', null);
+
+  return typeof salt === 'string' && salt.length > 0;
+};
+
+/**
+ * Checks whether data transfer features are enabled
+ *
+ * @returns {boolean}
+ */
+const isDataTransferEnabled = () => {
+  const { utils } = getService('transfer');
+
+  return !utils.isDisabledFromEnv() && utils.hasValidTokenSalt();
+};
+
+module.exports = { isDataTransferEnabled, isDisabledFromEnv, hasValidTokenSalt };