Use null when token is not found (#658)

integration-tests/testkit/flow.ts

@@ -8,6 +8,7 @@ import type {
   CreateProjectInput,
   UpdateProjectNameInput,
   CreateTokenInput,
+  DeleteTokensInput,
   OrganizationMemberAccessInput,
   SchemaCheckInput,
   PublishPersistedOperationInput,
@@ -297,6 +298,9 @@ export function createToken(input: CreateTokenInput, authToken: string) {
         createToken(input: $input) {
           ok {
             secret
+            createdToken {
+              id
+            }
           }
         }
       }
@@ -308,6 +312,22 @@ export function createToken(input: CreateTokenInput, authToken: string) {
   });
 }
 
+export function deleteTokens(input: DeleteTokensInput, authToken: string) {
+  return execute({
+    document: gql(/* GraphQL */ `
+      mutation deleteTokens($input: DeleteTokensInput!) {
+        deleteTokens(input: $input) {
+          deletedTokens
+        }
+      }
+    `),
+    authToken,
+    variables: {
+      input,
+    },
+  });
+}
+
 export function readTokenInfo(token: string) {
   return execute({
     document: gql(/* GraphQL */ `

integration-tests/tests/api/tokens.spec.ts

@@ -0,0 +1,91 @@
+import { ProjectType } from '@app/gql/graphql';
+import { createOrganization, createProject, createToken, readTokenInfo, deleteTokens } from '../../testkit/flow';
+import { authenticate } from '../../testkit/auth';
+
+test('deleting a token should clear the cache', async () => {
+  const { access_token: owner_access_token } = await authenticate('main');
+  const orgResult = await createOrganization(
+    {
+      name: 'foo',
+    },
+    owner_access_token
+  );
+
+  const org = orgResult.body.data!.createOrganization.ok!.createdOrganizationPayload.organization;
+
+  const projectResult = await createProject(
+    {
+      organization: org.cleanId,
+      type: ProjectType.Single,
+      name: 'foo',
+    },
+    owner_access_token
+  );
+
+  const project = projectResult.body.data!.createProject.ok!.createdProject;
+  const target = projectResult.body.data!.createProject.ok!.createdTargets[0];
+
+  // member should not have access to target:registry:write
+  const tokenResult = await createToken(
+    {
+      name: 'test',
+      organization: org.cleanId,
+      project: project.cleanId,
+      target: target.cleanId,
+      organizationScopes: [],
+      projectScopes: [],
+      targetScopes: [],
+    },
+    owner_access_token
+  );
+
+  expect(tokenResult.body.errors).not.toBeDefined();
+  const secret = tokenResult.body.data?.createToken.ok?.secret;
+  const createdToken = tokenResult.body.data?.createToken.ok?.createdToken;
+
+  expect(secret).toBeDefined();
+
+  let tokenInfoResult = await readTokenInfo(secret!);
+  expect(tokenInfoResult.body.errors).not.toBeDefined();
+
+  if (tokenInfoResult.body.data?.tokenInfo.__typename === 'TokenNotFoundError' || !createdToken) {
+    throw new Error('Token not found');
+  }
+
+  const tokenInfo = tokenInfoResult.body.data?.tokenInfo;
+  // organization
+  expect(tokenInfo?.hasOrganizationRead).toBe(true);
+  expect(tokenInfo?.hasOrganizationDelete).toBe(false);
+  expect(tokenInfo?.hasOrganizationIntegrations).toBe(false);
+  expect(tokenInfo?.hasOrganizationMembers).toBe(false);
+  expect(tokenInfo?.hasOrganizationSettings).toBe(false);
+  // project
+  expect(tokenInfo?.hasProjectRead).toBe(true);
+  expect(tokenInfo?.hasProjectDelete).toBe(false);
+  expect(tokenInfo?.hasProjectAlerts).toBe(false);
+  expect(tokenInfo?.hasProjectOperationsStoreRead).toBe(false);
+  expect(tokenInfo?.hasProjectOperationsStoreWrite).toBe(false);
+  expect(tokenInfo?.hasProjectSettings).toBe(false);
+  // target
+  expect(tokenInfo?.hasTargetRead).toBe(true);
+  expect(tokenInfo?.hasTargetDelete).toBe(false);
+  expect(tokenInfo?.hasTargetSettings).toBe(false);
+  expect(tokenInfo?.hasTargetRegistryRead).toBe(false);
+  expect(tokenInfo?.hasTargetRegistryWrite).toBe(false);
+  expect(tokenInfo?.hasTargetTokensRead).toBe(false);
+  expect(tokenInfo?.hasTargetTokensWrite).toBe(false);
+
+  // test invalidation
+  await deleteTokens(
+    {
+      organization: org.cleanId,
+      project: project.cleanId,
+      target: target.cleanId,
+      tokens: [createdToken.id],
+    },
+    owner_access_token
+  );
+
+  tokenInfoResult = await readTokenInfo(secret!);
+  expect(tokenInfoResult.body.errors).toHaveLength(1);
+});

packages/libraries/client/src/version.ts

@@ -1 +1 @@
-export const version = '0.20.0';
+export const version = '0.21.1';

packages/services/api/src/modules/token/providers/token-storage.ts

@@ -135,11 +135,19 @@ export class TokenStorage {
     this.logger.debug('Fetching token (token=%s)', maskToken(token));
 
     try {
-      return await this.tokensService.query('getToken', { token });
-    } catch (e: any) {
-      this.logger.error(e);
+      const tokenInfo = await this.tokensService.query('getToken', { token });
 
-      throw new HiveError('Invalid token provided!');
+      if (!tokenInfo) {
+        throw new Error('Token not found');
+      }
+
+      return tokenInfo;
+    } catch (error: any) {
+      this.logger.error(error);
+
+      throw new HiveError('Invalid token provided', {
+        originalError: error,
+      });
     }
   }
 }

packages/services/storage/src/tokens.ts

@@ -24,7 +24,7 @@ export async function createTokenStorage(connection: string, maximumPoolSize: nu
       return result.rows;
     },
     async getToken({ token }: { token: string }) {
-      return pool.one<Slonik<tokens>>(
+      return pool.maybeOne<Slonik<tokens>>(
         sql`
           SELECT * 
           FROM public.tokens 

packages/services/tokens/src/api.ts

@@ -56,10 +56,17 @@ export type Context = {
   logger: FastifyLoggerInstance;
   errorHandler: ReturnType<typeof createErrorHandler>;
   getStorage: ReturnType<typeof useCache>['getStorage'];
-  tokenReadFailuresCache: LruType<{
-    error: string;
-    checkAt: number;
-  }>;
+  tokenReadFailuresCache: LruType<
+    | {
+        type: 'error';
+        error: string;
+        checkAt: number;
+      }
+    | {
+        type: 'not-found';
+        checkAt: number;
+      }
+  >;
   errorCachingInterval: number;
 };
 
@@ -186,9 +193,13 @@ export const tokensApiRouter = trpc
       const failedRead = ctx.tokenReadFailuresCache.get(hash);
 
       if (failedRead) {
-        // let's re-throw the same error
+        // let's re-throw the same error (or return null)
         if (failedRead.checkAt >= Date.now()) {
-          throw new Error(failedRead.error);
+          if (failedRead.type === 'error') {
+            throw new Error(failedRead.error);
+          } else {
+            return null;
+          }
         }
         // or look for it again if last time we checked was 10 minutes ago
       }
@@ -197,15 +208,26 @@ export const tokensApiRouter = trpc
         const storage = await ctx.getStorage();
         const result = await storage.readToken(hash);
 
-        // removes the token from the failures cache
+        // removes the token from the failures cache (in case the value expired)
         ctx.tokenReadFailuresCache.delete(hash);
 
+        if (!result) {
+          // set token read as not found
+          // so we don't try to read it again for next X minutes
+          ctx.tokenReadFailuresCache.set(hash, {
+            type: 'not-found',
+            checkAt: Date.now() + ctx.errorCachingInterval,
+          });
+        }
+
         return result;
       } catch (error) {
         ctx.errorHandler(`Failed to get a token "${alias}"`, error as Error, ctx.logger);
 
         // set token read as failure
+        // so we don't try to read it again for next X minutes
         ctx.tokenReadFailuresCache.set(hash, {
+          type: 'error',
           error: (error as Error).message,
           checkAt: Date.now() + ctx.errorCachingInterval,
         });

packages/services/tokens/src/cache.ts

@@ -166,6 +166,11 @@ export function useCache(
         }
 
         const item = await readToken(hashed_token);
+
+        if (!item) {
+          return null;
+        }
+
         // Read the tokens of the target and cache them
         await readAndFill(item.target).catch(() => {});
         cacheMisses.inc(1);
@@ -184,6 +189,11 @@ export function useCache(
       }),
       deleteToken: tracker.wrap(async hashed_token => {
         const item = await cachedStorage.readToken(hashed_token);
+
+        if (!item) {
+          return;
+        }
+
         invalidate(item.target);
 
         return storage.deleteToken(hashed_token);

packages/services/tokens/src/index.ts

@@ -38,10 +38,17 @@ export async function main() {
 
   try {
     const { start, stop, readiness, getStorage } = useCache(createStorage(env.postgres), server.log);
-    const tokenReadFailuresCache = LRU<{
-      error: string;
-      checkAt: number;
-    }>(50);
+    const tokenReadFailuresCache = LRU<
+      | {
+          type: 'error';
+          error: string;
+          checkAt: number;
+        }
+      | {
+          type: 'not-found';
+          checkAt: number;
+        }
+    >(200);
     // Cache failures for 10 minutes
     const errorCachingInterval = ms('10m');
 

packages/services/tokens/src/storage.ts

@@ -16,7 +16,7 @@ export interface StorageItem {
 export interface Storage {
   destroy(): Promise<void>;
   readTarget(targetId: string, res?: FastifyReply): Promise<StorageItem[]>;
-  readToken(token: string, res?: FastifyReply): Promise<StorageItem>;
+  readToken(token: string, res?: FastifyReply): Promise<StorageItem | null>;
   writeToken(item: Omit<StorageItem, 'date' | 'lastUsedAt'>): Promise<StorageItem>;
   deleteToken(token: string): Promise<void>;
   touchTokens(tokens: Array<{ token: string; date: Date }>): Promise<void>;
@@ -52,6 +52,10 @@ export async function createStorage(config: Parameters<typeof createConnectionSt
     async readToken(hashed_token) {
       const result = await db.getToken({ token: hashed_token });
 
+      if (!result) {
+        return null;
+      }
+
       return transformToken(result);
     },
     async writeToken(item) {