Preserve OpenAPI security requirement semantics (#586)

plugins/importer-openapi/src/index.ts

@@ -20,6 +20,11 @@ type ImportResources = {
   folders: AtLeast<Folder, "name" | "id" | "model" | "workspaceId">[];
   httpRequests: AtLeast<HttpRequest, "name" | "id" | "model" | "workspaceId">[];
 };
+type ImportedAuthentication = Pick<HttpRequest, "authentication" | "authenticationType"> & {
+  headers: HttpRequestHeader[];
+  urlParameters: HttpUrlParameter[];
+};
+type AuthenticationVariableRegistry = Map<string, { name: string; value: string }>;
 
 const HTTP_METHODS = ["delete", "get", "head", "options", "patch", "post", "put", "query", "trace"];
 const BODY_CONTENT_TYPE_PREFERENCE = [
@@ -62,6 +67,7 @@ export async function convertOpenApi(contents: string): Promise<ImportPluginResp
     folders: [],
     httpRequests: [],
   };
+  const authenticationVariables: AuthenticationVariableRegistry = new Map();
   const baseUrl = importBaseUrl(spec);
   // A local spec has no document URL against which OpenAPI's implicit "/"
   // server can resolve. Keep the shared variable even when its initial value
@@ -123,6 +129,7 @@ export async function convertOpenApi(contents: string): Promise<ImportPluginResp
         spec,
         workspaceId: workspace.id,
         folderId,
+        authenticationVariables,
       });
       routeLabels.set(request.id, `${method.toUpperCase()} ${rawPath}`);
       resources.httpRequests.push(request);
@@ -131,6 +138,24 @@ export async function convertOpenApi(contents: string): Promise<ImportPluginResp
 
   if (resources.httpRequests.length === 0) return undefined;
 
+  if (authenticationVariables.size > 0) {
+    let environment = resources.environments[0];
+    if (environment == null) {
+      environment = {
+        model: "environment",
+        id: importState.generateId("environment"),
+        workspaceId: workspace.id,
+        name: "Global Variables",
+        variables: [],
+        parentModel: "workspace",
+        parentId: null,
+        sortPriority: importState.nextSortPriority(),
+      };
+      resources.environments.push(environment);
+    }
+    environment.variables.push(...authenticationVariables.values());
+  }
+
   disambiguateNames(resources.httpRequests, routeLabels);
 
   return {
@@ -200,6 +225,7 @@ function importOperation({
   spec,
   workspaceId,
   folderId,
+  authenticationVariables,
 }: {
   importState: ImportState;
   method: string;
@@ -211,6 +237,7 @@ function importOperation({
   spec: UnknownRecord;
   workspaceId: string;
   folderId: string | null;
+  authenticationVariables: AuthenticationVariableRegistry;
 }): ImportResources["httpRequests"][0] {
   importState.beginOperation();
   const parameters = mergeParameters({
@@ -219,13 +246,27 @@ function importOperation({
     operationParameters: toArray(operation.parameters),
   });
   const body = importBody({ importState, operation, parameters, spec });
-  const urlParameters = importUrlParameters({ importState, parameters });
+  const authentication = importAuthentication({
+    authenticationVariables,
+    importState,
+    operation,
+    spec,
+  });
+  const urlParameters = [
+    ...importUrlParameters({ importState, parameters }),
+    ...authentication.urlParameters,
+  ];
   const headers = mergeHeaders(
+    authentication.headers,
     importHeaderParameters({ importState, parameters }),
     body.headers,
     importAcceptHeader({ importState, operation, spec }),
   );
-  const authentication = importAuthentication({ importState, operation, spec });
+  const {
+    headers: _authenticationHeaders,
+    urlParameters: _authenticationParameters,
+    ...auth
+  } = authentication;
 
   // Built after everything else, so it can report the refs they left unresolved
   const description = importOperationDescription({
@@ -249,7 +290,7 @@ function importOperation({
     body: body.body,
     bodyType: body.bodyType,
     sortPriority: importState.nextSortPriority(),
-    ...authentication,
+    ...auth,
   };
 }
 
@@ -842,52 +883,142 @@ function inferSchemaType(schema: UnknownRecord): string {
 }
 
 function importAuthentication({
+  authenticationVariables,
   importState,
   operation,
   spec,
 }: {
+  authenticationVariables: AuthenticationVariableRegistry;
   importState: ImportState;
   operation: UnknownRecord;
   spec: UnknownRecord;
-}): Pick<HttpRequest, "authentication" | "authenticationType"> {
+}): ImportedAuthentication {
   const security = operation.security ?? spec.security;
+  if (Array.isArray(operation.security) && operation.security.length === 0) {
+    return { ...emptyAuthentication(), authenticationType: "none" };
+  }
   if (!Array.isArray(security) || security.length === 0) {
-    return { authenticationType: null, authentication: {} };
+    return emptyAuthentication();
+  }
+
+  // Security Requirement Objects are alternatives. If any alternative is
+  // empty, authentication is optional regardless of where it appears.
+  if (
+    security.some((requirement) => isRecord(requirement) && Object.keys(requirement).length === 0)
+  ) {
+    return { ...emptyAuthentication(), authenticationType: "none" };
   }
 
   const schemes = {
     ...toRecord(toRecord(spec.components).securitySchemes),
     ...toRecord(spec.securityDefinitions),
   };
-  for (const requirement of security) {
+  for (const rawRequirement of security) {
-    for (const [schemeName, rawScopes] of Object.entries(toRecord(requirement))) {
+    if (!isRecord(rawRequirement)) continue;
-      const scheme = toRecord(importState.resolve(schemes[schemeName]));
+
-      const type = stringAt(scheme, "type");
+    const imported = importSecurityRequirement({
-      if (type === "oauth2") {
+      authenticationVariables,
-        const oauth2 = importOAuth2(scheme, rawScopes);
+      importState,
-        if (oauth2 != null) return oauth2;
+      requirement: rawRequirement,
-        continue;
+      schemes,
-      }
+    });
-      if (type === "apiKey") {
+    if (imported != null) return imported;
-        return { authenticationType: "apikey", authentication: importApiKey(scheme, schemeName) };
-      }
-      // Swagger 2.0 spells basic auth as its own type rather than an HTTP scheme
-      if (type === "basic" || (type === "http" && schemeIs(scheme, "basic"))) {
-        return {
-          authenticationType: "basic",
-          authentication: { username: "", password: "" },
-        };
-      }
-      if (type === "http" && schemeIs(scheme, "bearer")) {
-        return {
-          authenticationType: "bearer",
-          authentication: { token: "", prefix: "Bearer" },
-        };
-      }
-    }
   }
 
-  return { authenticationType: null, authentication: {} };
+  return emptyAuthentication();
+}
+
+function importSecurityRequirement({
+  authenticationVariables,
+  importState,
+  requirement,
+  schemes,
+}: {
+  authenticationVariables: AuthenticationVariableRegistry;
+  importState: ImportState;
+  requirement: UnknownRecord;
+  schemes: UnknownRecord;
+}): ImportedAuthentication | null {
+  const entries = Object.entries(requirement);
+  const headers: HttpRequestHeader[] = [];
+  const urlParameters: HttpUrlParameter[] = [];
+  let primaryAuthentication: Pick<HttpRequest, "authentication" | "authenticationType"> | null =
+    null;
+
+  for (const [schemeName, rawScopes] of entries) {
+    const scheme = toRecord(importState.resolve(schemes[schemeName]));
+    const type = stringAt(scheme, "type");
+    if (type === "apiKey") {
+      const variable = registerAuthenticationVariable(authenticationVariables, schemeName, "key");
+      if (entries.length === 1) {
+        primaryAuthentication = {
+          authenticationType: "apikey",
+          authentication: importApiKey(scheme, schemeName, variable),
+        };
+      } else {
+        materializeApiKey(scheme, schemeName, variable, headers, urlParameters);
+      }
+      continue;
+    }
+
+    let candidate: Pick<HttpRequest, "authentication" | "authenticationType"> | null = null;
+    if (type === "oauth2") {
+      candidate = importOAuth2(scheme, rawScopes);
+    } else if (type === "openIdConnect") {
+      const token = registerAuthenticationVariable(authenticationVariables, schemeName, "token");
+      candidate = {
+        authenticationType: "bearer",
+        authentication: { token: templateVariable(token), prefix: "Bearer" },
+      };
+    } else if (type === "basic" || (type === "http" && schemeIs(scheme, "basic"))) {
+      const username = registerAuthenticationVariable(
+        authenticationVariables,
+        schemeName,
+        "username",
+      );
+      const password = registerAuthenticationVariable(
+        authenticationVariables,
+        schemeName,
+        "password",
+      );
+      candidate = {
+        authenticationType: "basic",
+        authentication: {
+          username: templateVariable(username),
+          password: templateVariable(password),
+        },
+      };
+    } else if (type === "http" && schemeIs(scheme, "bearer")) {
+      const token = registerAuthenticationVariable(authenticationVariables, schemeName, "token");
+      candidate = {
+        authenticationType: "bearer",
+        authentication: { token: templateVariable(token), prefix: "Bearer" },
+      };
+    }
+
+    // A requirement is an AND. Yaak can combine one auth plugin with explicit
+    // API-key parameters, but cannot represent two auth plugins on one request.
+    if (candidate == null || primaryAuthentication != null) return null;
+    primaryAuthentication = candidate;
+  }
+
+  return {
+    ...(primaryAuthentication ?? {
+      authenticationType: entries.length > 1 ? "none" : null,
+      authentication: {},
+    }),
+    headers,
+    urlParameters,
+  };
+}
+
+function emptyAuthentication(): ImportedAuthentication {
+  return {
+    authenticationType: null,
+    authentication: {},
+    headers: [],
+    urlParameters: [],
+  };
 }
 
 function schemeIs(scheme: UnknownRecord, name: string): boolean {
@@ -899,14 +1030,67 @@ function schemeIs(scheme: UnknownRecord, name: string): boolean {
  * cookie key becomes the Cookie header it would have ended up in, pre-filled
  * with its name. Sending it as a header named after the cookie would just fail.
  */
-function importApiKey(scheme: UnknownRecord, schemeName: string): Record<string, string> {
+function importApiKey(
+  scheme: UnknownRecord,
+  schemeName: string,
+  variableName: string,
+): Record<string, string> {
   const key = stringAt(scheme, "name") ?? schemeName;
   const location = stringAt(scheme, "in");
+  const value = templateVariable(variableName);
 
   if (location === "cookie") {
-    return { location: "header", key: "Cookie", value: `${key}=` };
+    return { location: "header", key: "Cookie", value: `${key}=${value}` };
   }
-  return { location: location === "query" ? "query" : "header", key, value: "" };
+  return { location: location === "query" ? "query" : "header", key, value };
+}
+
+function materializeApiKey(
+  scheme: UnknownRecord,
+  schemeName: string,
+  variableName: string,
+  headers: HttpRequestHeader[],
+  urlParameters: HttpUrlParameter[],
+): void {
+  const key = stringAt(scheme, "name") ?? schemeName;
+  const location = stringAt(scheme, "in");
+  const value = templateVariable(variableName);
+  if (location === "query") {
+    urlParameters.push({ enabled: true, name: key, value });
+  } else if (location === "cookie") {
+    headers.push({ enabled: true, name: "Cookie", value: `${key}=${value}` });
+  } else {
+    headers.push({ enabled: true, name: key, value });
+  }
+}
+
+function registerAuthenticationVariable(
+  variables: AuthenticationVariableRegistry,
+  schemeName: string,
+  field: string,
+): string {
+  const identity = JSON.stringify([schemeName, field]);
+  const existing = variables.get(identity);
+  if (existing != null) return existing.name;
+
+  const schemePart = schemeName
+    .replaceAll(/([a-z0-9])([A-Z])/g, "$1_$2")
+    .replaceAll(/[^a-zA-Z0-9]+/g, "_")
+    .replaceAll(/^_+|_+$/g, "")
+    .toLowerCase();
+  const baseName = `auth_${schemePart || "security"}_${field}`;
+  let name = baseName;
+  let suffix = 2;
+  const names = new Set([...variables.values()].map((variable) => variable.name));
+  while (names.has(name)) {
+    name = `${baseName}_${suffix++}`;
+  }
+  variables.set(identity, { name, value: "" });
+  return name;
+}
+
+function templateVariable(name: string): string {
+  return `\${[${name}]}`;
 }
 
 /**

plugins/importer-openapi/tests/__snapshots__/index.test.ts.snap

@@ -2617,6 +2617,10 @@ exports[`importer-openapi > Snapshots real-world fixture nasa-apod.yaml 1`] = `
             "name": "baseUrl",
             "value": "https://api.nasa.gov/planetary",
           },
+          {
+            "name": "auth_api_key_key",
+            "value": "",
+          },
         ],
         "workspaceId": "GENERATE_ID::WORKSPACE_0",
       },
@@ -2640,7 +2644,7 @@ Here's a link: https://example.com",
         "authentication": {
           "key": "api_key",
           "location": "query",
-          "value": "",
+          "value": "\${[auth_api_key_key]}",
         },
         "authenticationType": "apikey",
         "body": {},

plugins/importer-openapi/tests/index.test.ts

@@ -150,7 +150,10 @@ describe("importer-openapi", () => {
     expect(imported?.resources.environments).toEqual([
       expect.objectContaining({
         name: "Global Variables",
-        variables: [{ name: "baseUrl", value: "https://api.example.com/v1" }],
+        variables: [
+          { name: "baseUrl", value: "https://api.example.com/v1" },
+          { name: "auth_token_auth_token", value: "" },
+        ],
       }),
     ]);
     expect(imported?.resources.httpRequests).toEqual([
@@ -159,7 +162,7 @@ describe("importer-openapi", () => {
         method: "POST",
         url: "${[baseUrl]}/accounts/:accountId/members",
         authenticationType: "bearer",
-        authentication: { token: "", prefix: "Bearer" },
+        authentication: { token: "${[auth_token_auth_token]}", prefix: "Bearer" },
         bodyType: "application/json",
         body: {
           text: JSON.stringify(
@@ -573,16 +576,166 @@ describe("importer-openapi", () => {
     expect(imported?.resources.httpRequests[0]).toEqual(
       expect.objectContaining({
         authenticationType: "basic",
-        authentication: { username: "", password: "" },
+        authentication: {
+          username: "${[auth_basic_auth_username]}",
+          password: "${[auth_basic_auth_password]}",
+        },
       }),
     );
     // The auth plugin has no cookie location, so it becomes the Cookie header
     expect(imported?.resources.httpRequests[1]).toEqual(
       expect.objectContaining({
         authenticationType: "apikey",
-        authentication: { location: "header", key: "Cookie", value: "session=" },
+        authentication: {
+          location: "header",
+          key: "Cookie",
+          value: "session=${[auth_cookie_key_key]}",
+        },
       }),
     );
+    expect(imported?.resources.environments).toEqual([
+      expect.objectContaining({
+        name: "Global Variables",
+        variables: [
+          { name: "baseUrl", value: "https://example.com/" },
+          { name: "auth_basic_auth_username", value: "" },
+          { name: "auth_basic_auth_password", value: "" },
+          { name: "auth_cookie_key_key", value: "" },
+        ],
+      }),
+    ]);
+  });
+
+  test("Preserves anonymous security alternatives and explicit auth overrides", async () => {
+    const imported = await convertOpenApi(
+      JSON.stringify({
+        openapi: "3.1.0",
+        info: { title: "Optional Auth", version: "1.0.0" },
+        security: [{ bearerAuth: [] }],
+        paths: {
+          "/optional-auth-first": {
+            get: { security: [{ bearerAuth: [] }, {}], responses: {} },
+          },
+          "/optional-anonymous-first": {
+            get: { security: [{}, { bearerAuth: [] }], responses: {} },
+          },
+          "/public": { get: { security: [], responses: {} } },
+        },
+        components: {
+          securitySchemes: {
+            bearerAuth: { type: "http", scheme: "bearer" },
+          },
+        },
+      }),
+    );
+
+    expect(imported?.resources.httpRequests).toEqual([
+      expect.objectContaining({ authenticationType: "none", authentication: {} }),
+      expect.objectContaining({ authenticationType: "none", authentication: {} }),
+      expect.objectContaining({ authenticationType: "none", authentication: {} }),
+    ]);
+  });
+
+  test("Imports AND security requirements without dropping API keys", async () => {
+    const imported = await convertOpenApi(
+      JSON.stringify({
+        openapi: "3.1.0",
+        info: { title: "Combined Auth", version: "1.0.0" },
+        paths: {
+          "/combined": {
+            get: {
+              security: [{ bearerAuth: [], tenantKey: [], queryKey: [] }],
+              parameters: [
+                {
+                  in: "header",
+                  name: "X-Tenant-Key",
+                  example: "operation-value-must-not-replace-auth",
+                },
+              ],
+              responses: {},
+            },
+          },
+        },
+        components: {
+          securitySchemes: {
+            bearerAuth: { type: "http", scheme: "bearer" },
+            tenantKey: { type: "apiKey", in: "header", name: "X-Tenant-Key" },
+            queryKey: { type: "apiKey", in: "query", name: "api_key" },
+          },
+        },
+      }),
+    );
+
+    expect(imported?.resources.httpRequests).toEqual([
+      expect.objectContaining({
+        authenticationType: "bearer",
+        authentication: { token: "${[auth_bearer_auth_token]}", prefix: "Bearer" },
+        headers: [{ enabled: true, name: "X-Tenant-Key", value: "${[auth_tenant_key_key]}" }],
+        urlParameters: [{ enabled: true, name: "api_key", value: "${[auth_query_key_key]}" }],
+      }),
+    ]);
+  });
+
+  test("Keeps distinct credentials for security scheme names that normalize alike", async () => {
+    const imported = await convertOpenApi(
+      JSON.stringify({
+        openapi: "3.1.0",
+        info: { title: "Auth Variable Names", version: "1.0.0" },
+        paths: {
+          "/hyphen": { get: { security: [{ "api-key": [] }], responses: {} } },
+          "/underscore": { get: { security: [{ api_key: [] }], responses: {} } },
+          "/hyphen-again": { get: { security: [{ "api-key": [] }], responses: {} } },
+        },
+        components: {
+          securitySchemes: {
+            "api-key": { type: "apiKey", in: "header", name: "X-Hyphen-Key" },
+            api_key: { type: "apiKey", in: "header", name: "X-Underscore-Key" },
+          },
+        },
+      }),
+    );
+
+    expect(imported?.resources.environments[0]?.variables).toEqual([
+      { name: "baseUrl", value: "" },
+      { name: "auth_api_key_key", value: "" },
+      { name: "auth_api_key_key_2", value: "" },
+    ]);
+    expect(imported?.resources.httpRequests).toEqual([
+      expect.objectContaining({
+        authentication: expect.objectContaining({ value: "${[auth_api_key_key]}" }),
+      }),
+      expect.objectContaining({
+        authentication: expect.objectContaining({ value: "${[auth_api_key_key_2]}" }),
+      }),
+      expect.objectContaining({
+        authentication: expect.objectContaining({ value: "${[auth_api_key_key]}" }),
+      }),
+    ]);
+  });
+
+  test("Imports OpenID Connect as bearer authentication", async () => {
+    const imported = await convertOpenApi(
+      JSON.stringify({
+        openapi: "3.1.0",
+        info: { title: "OpenID Connect", version: "1.0.0" },
+        paths: { "/me": { get: { security: [{ oidc: [] }], responses: {} } } },
+        components: {
+          securitySchemes: {
+            oidc: {
+              type: "openIdConnect",
+              openIdConnectUrl: "https://accounts.example.com/.well-known/openid-configuration",
+            },
+          },
+        },
+      }),
+    );
+
+    expect(imported?.resources.httpRequests).toEqual([
+      expect.objectContaining({
+        authenticationType: "bearer",
+        authentication: { token: "${[auth_oidc_token]}", prefix: "Bearer" },
+      }),
+    ]);
   });
 
   test("Reports references that point outside the document", async () => {