Feat/add alias to api tokens (#1931)

* refactor: remove unused API definition routes

* feat: embed proxy endpoints

* feat: check token metadata for alias if none is found

* fix: rename param

* feat: add test for retrieving token by alias

* fix: update schema

* fix: refactor to alias

* fix: refactor to null

* fix: update snapshot

* fix: update openapi snapshot

* fix: add check to getUserForToken

* refactor: add more token alias tests

* refactor: use timingSafeEqual for token comparisons

Co-authored-by: olav <mail@olav.io>

src/lib/db/api-token-store.ts

@@ -45,7 +45,7 @@ const tokenRowReducer = (acc, tokenRow) => {
             environment: token.environment ? token.environment : ALL,
             expiresAt: token.expires_at,
             createdAt: token.created_at,
-            metadata: token.metadata,
+            alias: token.alias,
         };
     }
     const currentToken = acc[tokenRow.secret];
@@ -66,7 +66,7 @@ const toRow = (newToken: IApiTokenCreate) => ({
     environment:
         newToken.environment === ALL ? undefined : newToken.environment,
     expires_at: newToken.expiresAt,
-    metadata: newToken.metadata || {},
+    alias: newToken.alias || null,
 });
 
 const toTokens = (rows: any[]): IApiToken[] => {
@@ -126,7 +126,7 @@ export class ApiTokenStore implements IApiTokenStore {
                 'type',
                 'expires_at',
                 'created_at',
-                'metadata',
+                'alias',
                 'seen_at',
                 'environment',
                 'token_project_link.project',
@@ -153,7 +153,7 @@ export class ApiTokenStore implements IApiTokenStore {
             await Promise.all(updateProjectTasks);
             return {
                 ...newToken,
-                metadata: newToken.metadata || {},
+                alias: newToken.alias || null,
                 project: newToken.projects?.join(',') || '*',
                 createdAt: row.created_at,
             };

src/lib/openapi/spec/__snapshots__/api-token-schema.test.ts.snap

@@ -16,20 +16,3 @@ Object {
   "schema": "#/components/schemas/apiTokenSchema",
 }
 `;
-
-exports[`apiTokenSchema metadata - does not allow custom metadata parameters 1`] = `
-Object {
-  "errors": Array [
-    Object {
-      "instancePath": "/metadata",
-      "keyword": "additionalProperties",
-      "message": "must NOT have additional properties",
-      "params": Object {
-        "additionalProperty": "arbitraryParameter",
-      },
-      "schemaPath": "#/properties/metadata/additionalProperties",
-    },
-  ],
-  "schema": "#/components/schemas/apiTokenSchema",
-}
-`;

src/lib/openapi/spec/api-token-schema.test.ts

@@ -22,44 +22,6 @@ test('apiTokenSchema', () => {
     ).toBeUndefined();
 });
 
-test('apiTokenSchema metadata - should allow empty object', () => {
-    const data: ApiTokenSchema = { ...defaultData, metadata: {} };
-
-    expect(
-        validateSchema('#/components/schemas/apiTokenSchema', data),
-    ).toBeUndefined();
-});
-
-test('apiTokenSchema metadata - allows for corsOrigins and/or alias', () => {
-    expect(
-        validateSchema('#/components/schemas/apiTokenSchema', {
-            ...defaultData,
-            metadata: { corsOrigins: ['*'] },
-        }),
-    ).toBeUndefined();
-    expect(
-        validateSchema('#/components/schemas/apiTokenSchema', {
-            ...defaultData,
-            metadata: { alias: 'secret' },
-        }),
-    ).toBeUndefined();
-    expect(
-        validateSchema('#/components/schemas/apiTokenSchema', {
-            ...defaultData,
-            metadata: { corsOrigins: ['*'], alias: 'abc' },
-        }),
-    ).toBeUndefined();
-});
-
-test('apiTokenSchema metadata - does not allow custom metadata parameters', () => {
-    expect(
-        validateSchema('#/components/schemas/apiTokenSchema', {
-            ...defaultData,
-            metadata: { arbitraryParameter: true },
-        }),
-    ).toMatchSnapshot();
-});
-
 test('apiTokenSchema empty', () => {
     expect(
         validateSchema('#/components/schemas/apiTokenSchema', {}),

src/lib/openapi/spec/api-token-schema.ts

@@ -44,20 +44,8 @@ export const apiTokenSchema = {
             format: 'date-time',
             nullable: true,
         },
-        metadata: {
+        alias: {
-            type: 'object',
+            type: 'string',
-            additionalProperties: false,
-            properties: {
-                corsOrigins: {
-                    type: 'array',
-                    items: {
-                        type: 'string',
-                    },
-                },
-                alias: {
-                    type: 'string',
-                },
-            },
             nullable: true,
         },
     },

src/lib/schema/api-token-schema.test.ts

@@ -43,27 +43,11 @@ test('should not have projects set if project is present', async () => {
     expect(token.projects).not.toBeDefined();
 });
 
-test('should set metadata', async () => {
+test('should allow for embedded proxy (frontend) key', async () => {
-    let token = await createApiToken.validateAsync({
-        username: 'test',
-        type: 'admin',
-        project: 'default',
-        metadata: {
-            corsOrigins: ['*'],
-            alias: 'secret',
-        },
-    });
-    expect(token.projects).toBeUndefined();
-});
-
-test('should allow for frontend key (embedded proxy)', async () => {
     let token = await createApiToken.validateAsync({
         username: 'test',
         type: 'frontend',
         project: 'default',
-        metadata: {
-            corsOrigins: ['*'],
-        },
     });
     expect(token.error).toBeUndefined();
 });
@@ -73,9 +57,6 @@ test('should set environment to default for frontend key', async () => {
         username: 'test',
         type: 'frontend',
         project: 'default',
-        metadata: {
-            corsOrigins: ['*'],
-        },
     });
     expect(token.environment).toEqual('default');
 });

src/lib/services/api-token-service.ts

@@ -19,6 +19,7 @@ import { FOREIGN_KEY_VIOLATION } from '../error/db-error';
 import BadDataError from '../error/bad-data-error';
 import { minutesToMilliseconds } from 'date-fns';
 import { IEnvironmentStore } from 'lib/types/stores/environment-store';
+import { constantTimeCompare } from '../util/constantTimeCompare';
 
 const resolveTokenPermissions = (tokenType: string) => {
     if (tokenType === ApiTokenType.ADMIN) {
@@ -106,13 +107,19 @@ export class ApiTokenService {
             return undefined;
         }
 
-        let token = this.activeTokens.find((t) => t.secret === secret);
+        let token = this.activeTokens.find(
+            (activeToken) =>
+                Boolean(activeToken.secret) &&
+                constantTimeCompare(activeToken.secret, secret),
+        );
 
-        // If the token is not found, try to find it in the legacy format with the metadata alias
+        // If the token is not found, try to find it in the legacy format with alias.
-        // This is to ensure that previous proxies we set up for our customers continue working
+        // This allows us to support the old format of tokens migrating to the embedded proxy.
-        if (!token && secret) {
+        if (!token) {
             token = this.activeTokens.find(
-                (t) => t.metadata.alias && t.metadata.alias === secret,
+                (activeToken) =>
+                    Boolean(activeToken.alias) &&
+                    constantTimeCompare(activeToken.alias, secret),
             );
         }
 
@@ -126,6 +133,7 @@ export class ApiTokenService {
                 secret: token.secret,
             });
         }
+
         return undefined;
     }
 

src/lib/types/models/api-token.ts

@@ -22,21 +22,19 @@ export interface ILegacyApiTokenCreate {
 export interface IApiTokenCreate {
     secret: string;
     username: string;
-    metadata?: Metadata;
+    alias?: string;
     type: ApiTokenType;
     environment: string;
     projects: string[];
     expiresAt?: Date;
 }
 
-type Metadata = { [key: string]: unknown };
-
 export interface IApiToken extends IApiTokenCreate {
     createdAt: Date;
     seenAt?: Date;
     environment: string;
     project: string;
-    metadata: Metadata;
+    alias: string | null;
 }
 
 export const isAllProjects = (projects: string[]): boolean => {

src/lib/util/constantTimeCompare.test.ts

@@ -0,0 +1,55 @@
+import { constantTimeCompare } from './constantTimeCompare';
+
+test('constantTimeCompare', () => {
+    expect(constantTimeCompare('', '')).toEqual(false);
+    expect(constantTimeCompare(' ', '')).toEqual(false);
+    expect(constantTimeCompare('a', '')).toEqual(false);
+    expect(constantTimeCompare('', 'b')).toEqual(false);
+    expect(constantTimeCompare('a', 'b')).toEqual(false);
+
+    expect(constantTimeCompare(' ', ' ')).toEqual(true);
+    expect(constantTimeCompare('a', 'a')).toEqual(true);
+    expect(constantTimeCompare('b', 'b')).toEqual(true);
+
+    expect(
+        constantTimeCompare(
+            '*:*.63df5492f027129de9c9368bc80fb8200677e97fb107455497dc42d6',
+            '*:production.431c724bd84fbe8484bc6437d8e189f0ee288ebee6332bd030a539f5',
+        ),
+    ).toEqual(false);
+
+    expect(
+        constantTimeCompare(
+            '*:production.559520cc3c1a3b071260f77d80c4650a08699a1d918ea4e7b18c487e',
+            'default:development.148bbfa96e91ca41d6580232b331bbbdbc40fcc3626a815055ba79b5',
+        ),
+    ).toEqual(false);
+
+    expect(
+        constantTimeCompare(
+            '*:production.559520cc3c1a3b071260f77d80c4650a08699a1d918ea4e7b18c487e',
+            '*:production.431c724bd84fbe8484bc6437d8e189f0ee288ebee6332bd030a539f5',
+        ),
+    ).toEqual(false);
+
+    expect(
+        constantTimeCompare(
+            '*:production.431c724bd84fbe8484bc6437d8e189f0ee288ebee6332bd030a539f5',
+            '*:production.559520cc3c1a3b071260f77d80c4650a08699a1d918ea4e7b18c487e',
+        ),
+    ).toEqual(false);
+
+    expect(
+        constantTimeCompare(
+            '*:*.63df5492f027129de9c9368bc80fb8200677e97fb107455497dc42d6',
+            '*:*.63df5492f027129de9c9368bc80fb8200677e97fb107455497dc42d6',
+        ),
+    ).toEqual(true);
+
+    expect(
+        constantTimeCompare(
+            '*:production.431c724bd84fbe8484bc6437d8e189f0ee288ebee6332bd030a539f5',
+            '*:production.431c724bd84fbe8484bc6437d8e189f0ee288ebee6332bd030a539f5',
+        ),
+    ).toEqual(true);
+});

src/lib/util/constantTimeCompare.ts

@@ -0,0 +1,12 @@
+import crypto from 'crypto';
+
+export const constantTimeCompare = (a: string, b: string): boolean => {
+    if (!a || !b || a.length !== b.length) {
+        return false;
+    }
+
+    return crypto.timingSafeEqual(
+        Buffer.from(a, 'utf8'),
+        Buffer.from(b, 'utf8'),
+    );
+};

src/migrations/20220817130250-alter-api-tokens.js

@@ -0,0 +1,17 @@
+'use strict';
+
+exports.up = function (db, cb) {
+    db.runSql(
+        `ALTER TABLE api_tokens DROP COLUMN metadata;
+         ALTER TABLE api_tokens ADD COLUMN alias text;`,
+        cb,
+    );
+};
+
+exports.down = function (db, cb) {
+    db.runSql(
+        `ALTER TABLE api_tokens ADD COLUMN metadata JSONB NOT NULL DEFAULT '{}'::jsonb; 
+        ALTER TABLE api_tokens DROP COLUMN alias;`,
+        cb,
+    );
+};

src/test/e2e/api/openapi/__snapshots__/openapi.e2e.test.ts.snap

@@ -199,6 +199,10 @@ Object {
       "apiTokenSchema": Object {
         "additionalProperties": false,
         "properties": Object {
+          "alias": Object {
+            "nullable": true,
+            "type": "string",
+          },
           "createdAt": Object {
             "format": "date-time",
             "nullable": true,
@@ -212,22 +216,6 @@ Object {
             "nullable": true,
             "type": "string",
           },
-          "metadata": Object {
-            "additionalProperties": false,
-            "nullable": true,
-            "properties": Object {
-              "alias": Object {
-                "type": "string",
-              },
-              "corsOrigins": Object {
-                "items": Object {
-                  "type": "string",
-                },
-                "type": "array",
-              },
-            },
-            "type": "object",
-          },
           "project": Object {
             "type": "string",
           },

src/test/e2e/api/proxy/proxy.e2e.test.ts

@@ -106,6 +106,89 @@ test('should not allow requests with a client token', async () => {
         .expect(403);
 });
 
+test('should allow requests with a token secret alias', async () => {
+    const featureA = randomId();
+    const featureB = randomId();
+    const envA = randomId();
+    const envB = randomId();
+    await db.stores.environmentStore.create({ name: envA, type: 'test' });
+    await db.stores.environmentStore.create({ name: envB, type: 'test' });
+    await db.stores.projectStore.addEnvironmentToProject('default', envA);
+    await db.stores.projectStore.addEnvironmentToProject('default', envB);
+    await createFeatureToggle({
+        name: featureA,
+        enabled: true,
+        environment: envA,
+        strategies: [{ name: 'default', constraints: [], parameters: {} }],
+    });
+    await createFeatureToggle({
+        name: featureB,
+        enabled: true,
+        environment: envB,
+        strategies: [{ name: 'default', constraints: [], parameters: {} }],
+    });
+    const tokenA = await createApiToken(ApiTokenType.FRONTEND, {
+        alias: randomId(),
+        environment: envA,
+    });
+    const tokenB = await createApiToken(ApiTokenType.FRONTEND, {
+        alias: randomId(),
+        environment: envB,
+    });
+    await app.request
+        .get('/api/frontend')
+        .expect('Content-Type', /json/)
+        .expect(401);
+    await app.request
+        .get('/api/frontend')
+        .set('Authorization', '')
+        .expect('Content-Type', /json/)
+        .expect(401);
+    await app.request
+        .get('/api/frontend')
+        .set('Authorization', 'null')
+        .expect('Content-Type', /json/)
+        .expect(401);
+    await app.request
+        .get('/api/frontend')
+        .set('Authorization', randomId())
+        .expect('Content-Type', /json/)
+        .expect(401);
+    await app.request
+        .get('/api/frontend')
+        .set('Authorization', tokenA.secret.slice(0, -1))
+        .expect('Content-Type', /json/)
+        .expect(401);
+    await app.request
+        .get('/api/frontend')
+        .set('Authorization', tokenA.secret)
+        .expect('Content-Type', /json/)
+        .expect(200)
+        .expect((res) => expect(res.body.toggles).toHaveLength(1))
+        .expect((res) => expect(res.body.toggles[0].name).toEqual(featureA));
+    await app.request
+        .get('/api/frontend')
+        .set('Authorization', tokenB.secret)
+        .expect('Content-Type', /json/)
+        .expect(200)
+        .expect((res) => expect(res.body.toggles).toHaveLength(1))
+        .expect((res) => expect(res.body.toggles[0].name).toEqual(featureB));
+    await app.request
+        .get('/api/frontend')
+        .set('Authorization', tokenA.alias)
+        .expect('Content-Type', /json/)
+        .expect(200)
+        .expect((res) => expect(res.body.toggles).toHaveLength(1))
+        .expect((res) => expect(res.body.toggles[0].name).toEqual(featureA));
+    await app.request
+        .get('/api/frontend')
+        .set('Authorization', tokenB.alias)
+        .expect('Content-Type', /json/)
+        .expect(200)
+        .expect((res) => expect(res.body.toggles).toHaveLength(1))
+        .expect((res) => expect(res.body.toggles[0].name).toEqual(featureB));
+});
+
 test('should allow requests with an admin token', async () => {
     const adminToken = await createApiToken(ApiTokenType.ADMIN, {
         projects: ['*'],

src/test/fixtures/fake-api-token-store.ts

@@ -53,8 +53,8 @@ export default class FakeApiTokenStore
         const apiToken = {
             createdAt: new Date(),
             project: newToken.projects?.join(',') || '*',
+            alias: null,
             ...newToken,
-            metadata: {},
         };
         this.tokens.push(apiToken);
         this.emit('insert');