Fix Cookie Login (#259)

* Fixed a bug in the new 1.0.0 cookie that doesn't allow legacy imports * Cleaned up error messages and configs * Additional files found * Found 2 more * Fixed the yaml files that were being reverted * Fixed race condition on unit tests * Made a cleaner fix for the race condition
FamilySearch · Nov 6, 2024 · 2470b10 · 2470b10
1 parent c676cb1
commit 2470b10
Show file tree

Hide file tree

Showing 25 changed files with 40 additions and 50 deletions.
diff --git a/agent/createtest/createtest.yaml b/agent/createtest/createtest.yaml
@@ -18,8 +18,7 @@ config:
     headers:
       TestTime: '${epoch("ms")}'
       Accept: application/json
-      FS-User-Agent-Chain: PPAAS-Agent-Performance Test
-      User-Agent: FS-QA-SystemTest PPAAS Agent Performance Test
+      User-Agent: PPAAS Agent Performance Test
   general:
     bucket_size: 1m
     log_provider_stats: 1m

diff --git a/agent/createtest/shorter/createtest.yaml b/agent/createtest/shorter/createtest.yaml
@@ -14,8 +14,7 @@ config:
     headers:
       TestTime: '${epoch("ms")}'
       Accept: application/json
-      FS-User-Agent-Chain: PPAAS-Agent-Performance Test
-      User-Agent: FS-QA-SystemTest PPAAS Agent Performance Test
+      User-Agent: PPAAS Agent Performance Test
   general:
     bucket_size: 1m
     log_provider_stats: 1m

diff --git a/agent/src/tests.ts b/agent/src/tests.ts
@@ -42,8 +42,7 @@ config:
     headers:
       TestTime: '\${epoch("ms")}'
       Accept: application/json
-      FS-User-Agent-Chain: PPAAS-Agent-Performance Test
-      User-Agent: FS-QA-SystemTest PPAAS Agent Performance Test
+      User-Agent: PPAAS Agent Performance Test
   general:
     bucket_size: 1m
     log_provider_stats: 1m

diff --git a/common/test/basic.yaml b/common/test/basic.yaml
@@ -14,8 +14,7 @@ config:
     headers:
       TestTime: '${epoch("ms")}'
       Accept: application/json
-      FS-User-Agent-Chain: PPAAS-Agent-Performance Test
-      User-Agent: FS-QA-SystemTest PPAAS Agent Performance Test
+      User-Agent: PPAAS Agent Performance Test
   general:
     bucket_size: 1m
     log_provider_stats: 1m

diff --git a/common/test/basicheadersall.yaml b/common/test/basicheadersall.yaml
@@ -12,8 +12,7 @@ config:
     headers:
       TestTime: '${epoch("ms")}'
       Accept: application/json
-      FS-User-Agent-Chain: PPAAS-Agent-Performance Test
-      User-Agent: FS-QA-SystemTest PPAAS Agent Performance Test
+      User-Agent: PPAAS Agent Performance Test
   general:
     bucket_size: 1m
     log_provider_stats: 1m

diff --git a/common/test/basicnopeakload.yaml b/common/test/basicnopeakload.yaml
@@ -12,8 +12,7 @@ config:
     headers:
       TestTime: '${epoch("ms")}'
       Accept: application/json
-      FS-User-Agent-Chain: PPAAS-Agent-Performance Test
-      User-Agent: FS-QA-SystemTest PPAAS Agent Performance Test
+      User-Agent: PPAAS Agent Performance Test
   general:
     bucket_size: 1m
     log_provider_stats: 1m

diff --git a/common/test/basicwithenv.yaml b/common/test/basicwithenv.yaml
@@ -19,8 +19,7 @@ config:
     headers:
       TestTime: '${epoch("ms")}'
       Accept: application/json
-      FS-User-Agent-Chain: PPAAS-Agent-Performance Test
-      User-Agent: FS-QA-SystemTest PPAAS Agent Performance Test
+      User-Agent: PPAAS Agent Performance Test
       test: ${test}
   general:
     bucket_size: 1m

diff --git a/common/test/basicwithfiles.yaml b/common/test/basicwithfiles.yaml
@@ -18,8 +18,7 @@ config:
     headers:
       TestTime: '${epoch("ms")}'
       Accept: application/json
-      FS-User-Agent-Chain: PPAAS-Agent-Performance Test
-      User-Agent: FS-QA-SystemTest PPAAS Agent Performance Test
+      User-Agent: PPAAS Agent Performance Test
   general:
     bucket_size: 1m
     log_provider_stats: 1m

diff --git a/common/test/basicwithvars.yaml b/common/test/basicwithvars.yaml
@@ -18,8 +18,7 @@ config:
     headers:
       TestTime: '${epoch("ms")}'
       Accept: application/json
-      FS-User-Agent-Chain: PPAAS-Agent-Performance Test
-      User-Agent: FS-QA-SystemTest PPAAS Agent Performance Test
+      User-Agent: PPAAS Agent Performance Test
   general:
     bucket_size: 1m
     log_provider_stats: 1m

diff --git a/controller/components/YamlViewer/story.tsx b/controller/components/YamlViewer/story.tsx
@@ -24,8 +24,7 @@ client:
   headers:
     TestTime: '\${epoch("ms")}'
     Accept: application/json
-    FS-User-Agent-Chain: PPAAS-Agent-Performance Test
-    User-Agent: FS-QA-SystemTest PPAAS Agent Performance Test
+    User-Agent: PPAAS Agent Performance Test
 general:
   bucket_size: 1m
   log_provider_stats: 1m
@@ -54,8 +53,7 @@ client:
   headers:
     TestTime: '\${epoch("ms")}'
     Accept: application/json
-    FS-User-Agent-Chain: PPAAS-Agent-Performance Test
-    User-Agent: FS-QA-SystemTest PPAAS Agent Performance Test
+    User-Agent: PPAAS Agent Performance Test
 general:
   bucket_size: 1m
   log_provider_stats: 1m

diff --git a/controller/components/YamlWriterForm/writeyaml.ts b/controller/components/YamlWriterForm/writeyaml.ts
@@ -43,7 +43,7 @@ export const createYamlJson = ({ urls, patterns, vars, providers, loggers }: Omi
   // Default config
   myYaml.config = {};
   myYaml.config.client = {};
-  myYaml.config.client.headers = {"User-Agent": "FS-QA-SystemTest"};
+  myYaml.config.client.headers = {"User-Agent": "PewPew Performance Load Test"};
   // eslint-disable-next-line camelcase
   myYaml.config.general = {bucket_size: "1m", log_provider_stats: "1m"};
 

diff --git a/controller/pages/api/util/authserver.ts b/controller/pages/api/util/authserver.ts
@@ -34,9 +34,9 @@ import {
 } from "openid-client";
 import { GetServerSidePropsContext, NextApiRequest, NextApiResponse } from "next";
 import { LogLevel, log, logger } from "@fs/ppaas-common";
+import { parse as cookieParse, serialize as cookieSerialize } from "cookie";
 import { formatPageHref, getHostUrl } from "./clientutil";
 import { IncomingMessage } from "http";
-import cookie from "cookie";
 import { createErrorResponse } from "./util";
 import { getClientSecretOpenId } from "./secrets";
 import nextCookie from "next-cookies";
@@ -265,7 +265,7 @@ function getTokenFromQueryOrHeader (req: NextApiRequest, headerName: string = AU
     log("header token: " + JSON.stringify(token), LogLevel.DEBUG);
   }
   if (!token && req.headers.cookie && !Array.isArray(req.headers.cookie)) {
-    const cookies = cookie.parse(req.headers.cookie);
+    const cookies = cookieParse(req.headers.cookie);
     token = cookies[headerName];
     log("cookie token: " + JSON.stringify(token), LogLevel.DEBUG);
   }
@@ -374,7 +374,7 @@ export async function authApi (req: NextApiRequest, res: NextApiResponse, requir
     // If we don't have permissions or the permissions are not greater than requiredPermissions
     if (authPermissions.authPermission < requiredPermissions) {
       log("User was not authorized for api", LogLevel.WARN, { token, method: req.method, url: req.url });
-      res.status(403).json({ message: "User is not authorized for this api. If you think this is an error, please contact the PerformanceQA team." });
+      res.status(403).json({ message: "User is not authorized for this api. If you think this is an error, please contact the Performance team." });
       return undefined;
     }
 
@@ -428,15 +428,20 @@ export function setCookies (
   log(`Set cookie to ${token} on ${domain}`, LogLevel.DEBUG);
   // server side
   const oneDay: number = 60 * 60 * 24;
-  const cookies: string[] = [cookie.serialize(AUTH_COOKIE_NAME, token, { domain, path, maxAge: oneDay * COOKIE_DURATION_DAYS })];
-  if (refreshToken) {
-    cookies.push(cookie.serialize(REFRESH_COOKIE_NAME, refreshToken, { domain, path, maxAge: oneDay * REFRESH_COOKIE_DURATION_DAYS }));
-  }
-  if (hintToken) {
-    cookies.push(cookie.serialize(HINT_COOKIE_NAME, hintToken, { domain, path, maxAge: oneDay * COOKIE_DURATION_DAYS }));
+  try {
+    const cookies: string[] = [cookieSerialize(AUTH_COOKIE_NAME, token, { domain, path, maxAge: oneDay * COOKIE_DURATION_DAYS })];
+    if (refreshToken) {
+      cookies.push(cookieSerialize(REFRESH_COOKIE_NAME, refreshToken, { domain, path, maxAge: oneDay * REFRESH_COOKIE_DURATION_DAYS }));
+    }
+    if (hintToken) {
+      cookies.push(cookieSerialize(HINT_COOKIE_NAME, hintToken, { domain, path, maxAge: oneDay * COOKIE_DURATION_DAYS }));
+    }
+    // Set the cookie and then redirect
+    ctx.res.setHeader("Set-Cookie", cookies);
+  } catch (error: unknown) {
+    log("Error setting cookies in header", LogLevel.WARN, error, { token: token !== undefined, refreshToken: refreshToken !== undefined, hintToken: hintToken !== undefined });
+    throw error;
   }
-  // Set the cookie and then redirect
-  ctx.res.setHeader("Set-Cookie", cookies);
 }
 
 function getTokenFromCookieOrHeader (ctx: GetServerSidePropsContext, cookieName: string = AUTH_COOKIE_NAME, headerName?: string): string | undefined {

diff --git a/controller/pages/api/util/secrets.ts b/controller/pages/api/util/secrets.ts
@@ -80,7 +80,7 @@ export async function createSecret (secretKeyName: string, value: string | Buffe
   try {
     const input: CreateSecretCommandInput = {
       Name: secretKeyName,
-      Description: "Testing Secrets Manager for PerformanceQA/Pewpew",
+      Description: "Testing Secrets Manager for Performance/Pewpew",
       SecretString: typeof value === "string" ? value : undefined,
       SecretBinary: typeof value !== "string" ? new Uint8Array(value.buffer) : undefined
     };

diff --git a/controller/pages/api/util/testmanager.ts b/controller/pages/api/util/testmanager.ts
@@ -446,7 +446,7 @@ export async function validateYamlfile (
   if (bypassParser) {
     if (authPermissions.authPermission < AuthPermission.Admin) {
       log("Unauthorized User attempted to bypass the config parser.", LogLevel.WARN, { yamlFile, userId: authPermissions.userId });
-      return { json: { message: "User is not authorized to bypass the config parser. If you think this is an error, please contact the PerformanceQA team." }, status: 403 };
+      return { json: { message: "User is not authorized to bypass the config parser. If you think this is an error, please contact the Performance team." }, status: 403 };
     }
   } else {
     // bypassPaser is false or undefined, run the parser
@@ -1167,7 +1167,7 @@ export abstract class TestManager {
           } else if (PEWPEW_BINARY_EXECUTABLE_NAMES.includes(file.originalFilename)) {
             if (authPermission < AuthPermission.Admin) {
               log("Unauthorized User attempted to use custom pewpew binary.", LogLevel.WARN, { yamlFile });
-              return { json: { message: "User is not authorized to use custom pewpew binaries. If you think this is an error, please contact the PerformanceQA team." }, status: 403 };
+              return { json: { message: "User is not authorized to use custom pewpew binaries. If you think this is an error, please contact the Performance team." }, status: 403 };
             }
             log("Authorized user uploaded custom binary.", LogLevel.INFO, { yamlFile });
             additionalFileNames.push(file.originalFilename);
@@ -1511,7 +1511,7 @@ export abstract class TestManager {
         if (bypassParser) {
           if (authPermission < AuthPermission.Admin) {
             log("Unauthorized User attempted to bypass the config parser.", LogLevel.WARN, { yamlFile });
-            return { json: { message: "User is not authorized to bypass the config parser. If you think this is an error, please contact the PerformanceQA team." }, status: 403 };
+            return { json: { message: "User is not authorized to bypass the config parser. If you think this is an error, please contact the Performance team." }, status: 403 };
           }
         } else {
           // Read in the actual variables so we can inject them and make sure it's valid.

diff --git a/controller/test/basic.yaml b/controller/test/basic.yaml
@@ -15,7 +15,7 @@ config:
       TestTime: '${epoch("ms")}'
       Accept: application/json
       FS-User-Agent-Chain: PPAAS-Agent-Performance Test
-      User-Agent: FS-QA-SystemTest PPAAS Agent Performance Test
+      User-Agent: PPAAS Agent Performance Test
   general:
     bucket_size: 1m
     log_provider_stats: 1m

diff --git a/controller/test/basicheadersall.yaml b/controller/test/basicheadersall.yaml
@@ -12,8 +12,7 @@ config:
     headers:
       TestTime: '${epoch("ms")}'
       Accept: application/json
-      FS-User-Agent-Chain: PPAAS-Agent-Performance Test
-      User-Agent: FS-QA-SystemTest PPAAS Agent Performance Test
+      User-Agent: PPAAS Agent Performance Test
   general:
     bucket_size: 1m
     log_provider_stats: 1m

diff --git a/controller/test/basicnopeakload.yaml b/controller/test/basicnopeakload.yaml
@@ -12,8 +12,7 @@ config:
     headers:
       TestTime: '${epoch("ms")}'
       Accept: application/json
-      FS-User-Agent-Chain: PPAAS-Agent-Performance Test
-      User-Agent: FS-QA-SystemTest PPAAS Agent Performance Test
+      User-Agent: PPAAS Agent Performance Test
   general:
     bucket_size: 1m
     log_provider_stats: 1m

diff --git a/controller/test/basicwithenv.yaml b/controller/test/basicwithenv.yaml
@@ -21,7 +21,7 @@ config:
       TestTime: '${epoch("ms")}'
       Accept: application/json
       FS-User-Agent-Chain: PPAAS-Agent-Performance Test
-      User-Agent: FS-QA-SystemTest PPAAS Agent Performance Test
+      User-Agent: PPAAS Agent Performance Test
   general:
     bucket_size: 1m
     log_provider_stats: 1m

diff --git a/controller/test/basicwithfiles.yaml b/controller/test/basicwithfiles.yaml
@@ -18,8 +18,7 @@ config:
     headers:
       TestTime: '${epoch("ms")}'
       Accept: application/json
-      FS-User-Agent-Chain: PPAAS-Agent-Performance Test
-      User-Agent: FS-QA-SystemTest PPAAS Agent Performance Test
+      User-Agent: PPAAS Agent Performance Test
   general:
     bucket_size: 1m
     log_provider_stats: 1m

diff --git a/controller/test/ppaasencryptenvfile.spec.ts b/controller/test/ppaasencryptenvfile.spec.ts
@@ -517,7 +517,7 @@ describe("PpaasEncryptEnvironmentFile", () => {
 
     it("Upload a test file force should upload unchanged files", (done: Mocha.Done) => {
       const lastModified: number = Date.now();
-      testPpaasEncryptEnvironmentFileUpload.setLastModifiedLocal(lastModified);
+      testPpaasEncryptEnvironmentFileUpload.setLastModifiedLocal(lastModified - 1);
       testPpaasEncryptEnvironmentFileUpload.upload(true).then(() => {
         log("testPpaasEncryptEnvironmentFileDownload.upload(true) succeeded", LogLevel.DEBUG);
         // If it's newer, but forced we should upload it and set the time to last modified

diff --git a/controller/test/ppaasencrypts3file.spec.ts b/controller/test/ppaasencrypts3file.spec.ts
@@ -258,7 +258,7 @@ describe("PpaasEncryptS3File", () => {
     it("Upload a test file force should upload unchanged files", (done: Mocha.Done) => {
       mockUploadObject();
       const lastModified: number = Date.now();
-      testPpaasEncryptS3FileUpload.setLastModifiedLocal(lastModified);
+      testPpaasEncryptS3FileUpload.setLastModifiedLocal(lastModified - 1);
       testPpaasEncryptS3FileUpload.upload(true).then(() => {
         s3FileKey = testPpaasEncryptS3FileUpload.key;
         log("testPpaasEncryptS3FileDownload.upload(true) succeeded", LogLevel.DEBUG);

diff --git a/controller/test/settings.yaml b/controller/test/settings.yaml
@@ -14,8 +14,7 @@ config:
     headers:
       TestTime: '${epoch("ms")}'
       Accept: application/json
-      FS-User-Agent-Chain: PPAAS-Agent-Performance Test
-      User-Agent: FS-QA-SystemTest PPAAS Agent Performance Test
+      User-Agent: PPAAS Agent Performance Test
   general:
     bucket_size: 1m
     log_provider_stats: 1m

diff --git a/controller/test/testyaml.zip b/controller/test/testyaml.zip
diff --git a/controller/test/testyamlenv.zip b/controller/test/testyamlenv.zip
diff --git a/controller/test/testyamls.zip b/controller/test/testyamls.zip