fix: api and transfer token lifespan select lists work with all durations

e2e/app-template/template/src/api/config/controllers/config.js

@@ -1,3 +1,5 @@
+const { createTestTransferToken } = require('../../../create-transfer-token');
+
 module.exports = {
   rateLimitEnable(ctx) {
     const { value } = ctx.request.body;
@@ -6,6 +8,11 @@ module.exports = {
 
     configService.rateLimitEnable(value);
 
+    ctx.send(200);
+  },
+  async resetTransferToken(ctx) {
+    await createTestTransferToken(strapi);
+
     ctx.send(200);
   },
 };

e2e/app-template/template/src/api/config/routes/config.js

@@ -8,5 +8,13 @@ module.exports = {
         auth: false,
       },
     },
+    {
+      method: 'POST',
+      path: '/config/resettransfertoken',
+      handler: 'config.resetTransferToken',
+      config: {
+        auth: false,
+      },
+    },
   ],
 };

e2e/app-template/template/src/create-transfer-token.js

@@ -0,0 +1,27 @@
+const { CUSTOM_TRANSFER_TOKEN_ACCESS_KEY } = require('./constants');
+
+/**
+ * Make sure the test transfer token exists in the database
+ * @param {Strapi.Strapi} strapi
+ * @returns {Promise<void>}
+ */
+const createTestTransferToken = async (strapi) => {
+  const { token: transferTokenService } = strapi.admin.services.transfer;
+
+  const accessKeyHash = transferTokenService.hash(CUSTOM_TRANSFER_TOKEN_ACCESS_KEY);
+  const exists = await transferTokenService.exists({ accessKey: accessKeyHash });
+
+  if (!exists) {
+    await transferTokenService.create({
+      name: 'TestToken',
+      description: 'Transfer token used to seed the e2e database',
+      lifespan: null,
+      permissions: ['push'],
+      accessKey: CUSTOM_TRANSFER_TOKEN_ACCESS_KEY,
+    });
+  }
+};
+
+module.exports = {
+  createTestTransferToken,
+};

e2e/app-template/template/src/index.js

@@ -1,4 +1,4 @@
-const { CUSTOM_TRANSFER_TOKEN_ACCESS_KEY } = require('./constants');
+const { createTestTransferToken } = require('./create-transfer-token');
 
 module.exports = {
   /**
@@ -23,25 +23,3 @@ module.exports = {
     await createTestTransferToken(strapi);
   },
 };
-
-/**
- * Make sure the test transfer token exists in the database
- * @param {Strapi.Strapi} strapi
- * @returns {Promise<void>}
- */
-const createTestTransferToken = async (strapi) => {
-  const { token: transferTokenService } = strapi.admin.services.transfer;
-
-  const accessKeyHash = transferTokenService.hash(CUSTOM_TRANSFER_TOKEN_ACCESS_KEY);
-  const exists = await transferTokenService.exists({ accessKey: accessKeyHash });
-
-  if (!exists) {
-    await transferTokenService.create({
-      name: 'TestToken',
-      description: 'Transfer token used to seed the e2e database',
-      lifespan: null,
-      permissions: ['push'],
-      accessKey: CUSTOM_TRANSFER_TOKEN_ACCESS_KEY,
-    });
-  }
-};

e2e/constants.js

@@ -4,6 +4,8 @@ const ALLOWED_CONTENT_TYPES = [
   'admin::user',
   'admin::role',
   'admin::permission',
+  'admin::api-token',
+  'admin::transfer-token',
   'api::article.article',
   'api::author.author',
   'api::homepage.homepage',

e2e/scripts/dts-import.js

@@ -48,6 +48,19 @@ export const resetDatabaseAndImportDataFromPath = async (filePath) => {
 
   engine.diagnostics.onDiagnostic(console.log);
 
+  try {
+    // reset the transfer token to allow the transfer if it's been wiped (that is, not included in previous import data)
+    const res = await fetch(
+      `http://127.0.0.1:${process.env.PORT ?? 1337}/api/config/resettransfertoken`,
+      {
+        method: 'POST',
+      }
+    );
+  } catch (err) {
+    console.error('Token reset failed.' + JSON.stringify(err, null, 2));
+    process.exit(1);
+  }
+
   try {
     await engine.transfer();
   } catch {

e2e/tests/admin/transfer/tokens.spec.ts

@@ -0,0 +1,65 @@
+import { test, expect } from '@playwright/test';
+import { login } from '../../../utils/login';
+import { resetDatabaseAndImportDataFromPath } from '../../../scripts/dts-import';
+import { navToHeader, delay } from '../../../utils/shared';
+
+const createTransferToken = async (page, tokenName, duration, type) => {
+  await navToHeader(
+    page,
+    ['Settings', 'Transfer Tokens', 'Create new Transfer Token'],
+    'Create Transfer Token'
+  );
+
+  await page.getByLabel('Name*').click();
+  await page.getByLabel('Name*').fill(tokenName);
+
+  await page.getByLabel('Token duration').click();
+  await page.getByRole('option', { name: duration }).click();
+
+  await page.getByLabel('Token type').click();
+  await page.getByRole('option', { name: type }).click();
+
+  await page.getByRole('button', { name: 'Save' }).click();
+
+  await expect(page.getByText(/copy this token/)).toBeVisible();
+  await expect(page.getByText('Expiration date:')).toBeVisible();
+};
+
+test.describe('Transfer Tokens', () => {
+  test.beforeEach(async ({ page }) => {
+    await resetDatabaseAndImportDataFromPath('./e2e/data/with-admin.tar');
+    await page.goto('/admin');
+    await login({ page });
+  });
+
+  // Test token creation
+  const testCases = [
+    ['30-day push token', '30 days', 'Push'],
+    ['30-day pull token', '30 days', 'Pull'],
+    ['30-day full-access token', '30 days', 'Full access'],
+    // if push+pull work generally that's good enough for e2e
+    ['7-day token', '7 days', 'Full access'],
+    ['90-day token', '90 days', 'Full access'],
+    ['unlimited token', 'Unlimited', 'Full access'],
+  ];
+  for (const [name, duration, type] of testCases) {
+    test(`A user should be able to create a ${name}`, async ({ page }) => {
+      await createTransferToken(page, name, duration, type);
+    });
+  }
+
+  test('Created tokens list page should be correct', async ({ page }) => {
+    await createTransferToken(page, 'my test token', 'unlimited', 'Full access');
+
+    // if we don't wait until createdAt is at least 1s, we see "NaN" for the timestamp
+    // TODO: fix the bug and remove this
+    await page.waitForTimeout(1100);
+
+    await navToHeader(page, ['Settings', 'Transfer Tokens'], 'Transfer Tokens');
+
+    const row = page.getByRole('gridcell', { name: 'my test token', exact: true });
+    await expect(row).toBeVisible();
+    await expect(page.getByText(/\d+ (second|minute)s? ago/)).toBeVisible();
+    // TODO: expand on this test, it could check edit and delete icons
+  });
+});

e2e/utils/shared.ts

@@ -1,7 +1,25 @@
-import { test } from '@playwright/test';
+import { test, Page, expect } from '@playwright/test';
 
 /**
  * Execute a test suite only if the condition is true
  */
 export const describeOnCondition = (shouldDescribe: boolean) =>
   shouldDescribe ? test.describe : test.describe.skip;
+
+/**
+ * Navigate to a page and confirm the header, awaiting each step
+ */
+export const navToHeader = async (page: Page, navItems: string[], headerText: string) => {
+  for (const navItem of navItems) {
+    // This does not use getByRole because sometimes "Settings" is "Settings 1" if there's a badge notification
+    // BUT if we don't match exact it conflicts with "Advanceed Settings"
+    // As a workaround, we implement our own startsWith with page.locator
+    const item = page.locator(`role=link[name^="${navItem}"]`);
+    await expect(item).toBeVisible();
+    await item.click();
+  }
+
+  const header = page.getByRole('heading', { name: headerText, exact: true });
+  await expect(header).toBeVisible();
+  return header;
+};

packages/core/admin/admin/src/pages/Settings/pages/ApiTokens/EditView/EditViewPage.tsx

@@ -201,8 +201,8 @@ export const EditView = () => {
       if (isCreating) {
         const res = await createToken({
           ...body,
-          // in case a token has a lifespan of "unlimited" the API only accepts zero as a number
-          lifespan: body.lifespan === '0' ? parseInt(body.lifespan) : null,
+          // lifespan must be "null" for unlimited (0 would mean instantly expired and isn't accepted)
+          lifespan: body?.lifespan || null,
           permissions: body.type === 'custom' ? state.selectedActions : null,
         });
 
@@ -338,7 +338,7 @@ export const EditView = () => {
             name: apiToken?.name || '',
             description: apiToken?.description || '',
             type: apiToken?.type,
-            lifespan: apiToken?.lifespan ? apiToken.lifespan.toString() : apiToken?.lifespan,
+            lifespan: apiToken?.lifespan,
           }}
           enableReinitialize
           onSubmit={(body, actions) => handleSubmit(body, actions)}

packages/core/admin/admin/src/pages/Settings/pages/TransferTokens/EditView.tsx

@@ -150,6 +150,8 @@ const EditView = () => {
         if (isCreating) {
           const res = await createToken({
             ...body,
+            // lifespan must be "null" for unlimited (0 would mean instantly expired and isn't accepted)
+            lifespan: body?.lifespan || null,
             permissions,
           });
 
@@ -251,7 +253,7 @@ const EditView = () => {
           {
             name: transferToken?.name || '',
             description: transferToken?.description || '',
-            lifespan: transferToken?.lifespan ?? null,
+            lifespan: transferToken?.lifespan || null,
             /**
              * We need to cast the permissions to satisfy the type for `permissions`
              * in the request body incase we don't have a transferToken and instead

packages/core/admin/server/src/services/api-token.ts

@@ -1,5 +1,5 @@
 import crypto from 'crypto';
-import { omit, difference, isNil, isEmpty, map, isArray, uniq } from 'lodash/fp';
+import { omit, difference, isNil, isEmpty, map, isArray, uniq, isNumber } from 'lodash/fp';
 import { errors } from '@strapi/utils';
 import type { Update, ApiToken, ApiTokenBody } from '../../../shared/contracts/api-token';
 import constants from './constants';
@@ -61,14 +61,25 @@ const assertCustomTokenPermissionsValidity = (
 };
 
 /**
- * Assert that a token's lifespan is valid
+ * Check if a token's lifespan is valid
  */
-const assertValidLifespan = (lifespan: ApiTokenBody['lifespan']) => {
+const isValidLifespan = (lifespan: unknown) => {
   if (isNil(lifespan)) {
-    return;
+    return true;
   }
 
-  if (!Object.values(constants.API_TOKEN_LIFESPANS).includes(lifespan as number)) {
+  if (!isNumber(lifespan) || !Object.values(constants.API_TOKEN_LIFESPANS).includes(lifespan)) {
+    return false;
+  }
+
+  return true;
+};
+
+/**
+ * Assert that a token's lifespan is valid
+ */
+const assertValidLifespan = (lifespan: unknown) => {
+  if (!isValidLifespan(lifespan)) {
     throw new ValidationError(
       `lifespan must be one of the following values: 
       ${Object.values(constants.API_TOKEN_LIFESPANS).join(', ')}`
@@ -138,14 +149,14 @@ const hash = (accessKey: string) => {
 
 const getExpirationFields = (lifespan: ApiTokenBody['lifespan']) => {
   // it must be nil or a finite number >= 0
-  const isValidNumber = Number.isFinite(lifespan) && (lifespan as number) > 0;
+  const isValidNumber = isNumber(lifespan) && Number.isFinite(lifespan) && lifespan > 0;
   if (!isValidNumber && !isNil(lifespan)) {
     throw new ValidationError('lifespan must be a positive number or null');
   }
 
   return {
     lifespan: lifespan || null,
-    expiresAt: lifespan ? Date.now() + (lifespan as number) : null,
+    expiresAt: lifespan ? Date.now() + lifespan : null,
   };
 };
 

packages/core/admin/server/src/services/transfer/token.ts

@@ -1,6 +1,6 @@
 import crypto from 'crypto';
 import assert from 'assert';
-import { map, isArray, omit, uniq, isNil, difference, isEmpty } from 'lodash/fp';
+import { map, isArray, omit, uniq, isNil, difference, isEmpty, isNumber } from 'lodash/fp';
 import { errors } from '@strapi/utils';
 import '@strapi/types';
 import constants from '../constants';
@@ -79,7 +79,7 @@ const create = async (attributes: TokenCreatePayload): Promise<TransferToken> =>
   delete attributes.accessKey;
 
   assertTokenPermissionsValidity(attributes);
-  assertValidLifespan(attributes);
+  assertValidLifespan(attributes.lifespan);
 
   const result = (await strapi.db.transaction(async () => {
     const transferToken = await strapi.query(TRANSFER_TOKEN_UID).create({
@@ -131,7 +131,7 @@ const update = async (
   }
 
   assertTokenPermissionsValidity(attributes);
-  assertValidLifespan(attributes);
+  assertValidLifespan(attributes.lifespan);
 
   return strapi.db.transaction(async () => {
     const updatedToken = await strapi.query(TRANSFER_TOKEN_UID).update({
@@ -281,11 +281,9 @@ const regenerate = async (id: string | number): Promise<TransferToken> => {
   };
 };
 
-const getExpirationFields = (
-  lifespan: number | null
-): { lifespan: null | number; expiresAt: null | number } => {
+const getExpirationFields = (lifespan: TransferToken['lifespan']) => {
   // it must be nil or a finite number >= 0
-  const isValidNumber = Number.isFinite(lifespan) && lifespan !== null && lifespan > 0;
+  const isValidNumber = isNumber(lifespan) && Number.isFinite(lifespan) && lifespan > 0;
   if (!isValidNumber && !isNil(lifespan)) {
     throw new ValidationError('lifespan must be a positive number or null');
   }
@@ -359,14 +357,28 @@ const assertTokenPermissionsValidity = (attributes: TokenUpdatePayload) => {
 };
 
 /**
- * Assert that a token's lifespan is valid
+ * Check if a token's lifespan is valid
  */
-const assertValidLifespan = ({ lifespan }: { lifespan?: TransferToken['lifespan'] }) => {
+const isValidLifespan = (lifespan: unknown) => {
   if (isNil(lifespan)) {
-    return;
+    return true;
   }
 
-  if (!Object.values(constants.TRANSFER_TOKEN_LIFESPANS).includes(lifespan)) {
+  if (
+    !isNumber(lifespan) ||
+    !Object.values(constants.TRANSFER_TOKEN_LIFESPANS).includes(lifespan)
+  ) {
+    return false;
+  }
+
+  return true;
+};
+
+/**
+ * Assert that a token's lifespan is valid
+ */
+const assertValidLifespan = (lifespan: unknown) => {
+  if (!isValidLifespan(lifespan)) {
     throw new ValidationError(
       `lifespan must be one of the following values: 
       ${Object.values(constants.TRANSFER_TOKEN_LIFESPANS).join(', ')}`

packages/core/admin/shared/contracts/api-token.ts

@@ -8,7 +8,7 @@ export type ApiToken = {
   expiresAt: string;
   id: Entity.ID;
   lastUsedAt: string | null;
-  lifespan: string | number;
+  lifespan: string | number | null;
   name: string;
   permissions: string[];
   type: 'custom' | 'full-access' | 'read-only';

packages/core/admin/shared/contracts/transfer.ts

@@ -1,7 +1,7 @@
 import { errors } from '@strapi/utils';
 
 export interface TransferTokenPermission {
-  id: number | string;
+  id: number | `${number}`;
   action: 'push' | 'pull' | 'push-pull';
   token: TransferToken | number;
 }
@@ -12,7 +12,7 @@ export interface DatabaseTransferToken {
   description: string;
   accessKey: string;
   lastUsedAt?: number;
-  lifespan: number | null;
+  lifespan: string | number | null;
   expiresAt: number;
   permissions: TransferTokenPermission[];
 }