feat(connectors): set up Microsoft accounts in the connector form (#15966)

.agents/feature-map/components/connectors.md

@@ -90,7 +90,7 @@ frontend changes a new connector requires.
 | GET | `/connector/{google-drive,gmail}/authorize/{credential_id}`, `/callback` | Google-specific OAuth flows in `connector.py`. |
 | GET | `/connector/oauth/authorize/{source}`, `/callback/{source}`, `/details/{source}` | Generic OAuth flow for every `OAuthConnector` implementation (`server/documents/standard_oauth.py`). |
 | POST/GET | `/admin/credential/{credential_id}/capability-check`, `/capability-report`, `/admin/credential/capability-reports`, `/admin/credential/{credential_id}/binding-check` | Run and read capability checks, and check credential-bound config fields (`server/documents/credential_capabilities.py`). A run applies the credential use rule (§4.9) to the config it uses. A `connector_specific_config` override needs credential visibility; only a run on the stored config of an existing pair is open to a pairing manager who cannot see the credential. |
-| POST/GET | `/admin/connector-checks/runs`, `/admin/connector-checks/runs/{run_id}`, `/admin/connector-checks/runs/{run_id}/cancel` | Draft capability-check runs on an unsaved connector form (`server/documents/capability_check_runs.py`), on a saved credential or the user's draft. A new account's `credential_json` is saved as a draft credential (`Credential.is_draft`), which only its owner sees; the snapshot's `credential_id` names it, and later requests send the values again only when they changed. The add-connector form sends a typed account's values only with a check run or Create, and its binding check names the draft by id once a run saved the current values. The `CLEANUP_STALE_DRAFT_CREDENTIALS` beat task deletes drafts unchanged for 7 days. Cancel stops a run its starter owns; the form calls it when its credential changes. A run starts only when the credential use rule (§4.9) allows the form's config. |
+| POST/GET | `/admin/connector-checks/runs`, `/admin/connector-checks/runs/{run_id}`, `/admin/connector-checks/runs/{run_id}/cancel` | Draft capability-check runs on an unsaved connector form (`server/documents/capability_check_runs.py`), on a saved credential or the user's draft. A new account's `credential_json` is saved as a draft credential (`Credential.is_draft`), which only its owner sees; the snapshot's `credential_id` names it, and later requests send the values again only when they changed. The add-connector form sends a typed account's values only with a check run or Create, and its binding check names the draft by id once a run saved the current values. A Microsoft `.pfx` certificate travels in `credential_json` as base64 text, the form the private-key upload stores; `resolve_form_credential` checks it as PKCS#12 (`private_key_types.validate_pkcs12_credential_value`). The `CLEANUP_STALE_DRAFT_CREDENTIALS` beat task deletes drafts unchanged for 7 days. Cancel stops a run its starter owns; the form calls it when its credential changes. A run starts only when the credential use rule (§4.9) allows the form's config. |
 | POST | `/admin/connector-checks/plan` | The checks a draft run would hold for an unsaved form, each in its state before anything runs (pending, waiting, not applicable). Needs no credential and starts no run. |
 | GET | `/connector`, `/connector/{connector_id}`, `/indexed-sources` | Read paths, including the anonymous-ish `/connector-status` used by chat surfaces. |
 

backend/onyx/server/documents/draft_credentials.py

@@ -21,6 +21,7 @@ from onyx.db.models import Credential, User
 from onyx.error_handling.error_codes import OnyxErrorCode
 from onyx.error_handling.exceptions import OnyxError
 from onyx.server.documents.models import NAMES_A_CREDENTIAL
+from onyx.server.documents.private_key_types import validate_pkcs12_credential_value
 from onyx.utils.encryption import reject_masked_credentials
 
 
@@ -80,6 +81,9 @@ def resolve_form_credential(
 
     try:
         if credential_json is not None:
+            # A certificate comes as base64 text, not an upload: check it as
+            # the upload is checked.
+            validate_pkcs12_credential_value(source, credential_json)
             if credential is None:
                 # A new draft, or a lost draft made again.
                 return create_draft_credential(

backend/onyx/server/documents/private_key_types.py

@@ -1,9 +1,11 @@
 import base64
+import binascii
 from enum import Enum
-from typing import Protocol
+from typing import Any, Protocol
 
 from fastapi import UploadFile
 
+from onyx.configs.constants import DocumentSource
 from onyx.error_handling.error_codes import OnyxErrorCode
 from onyx.error_handling.exceptions import OnyxError
 from onyx.server.documents.document_utils import validate_pkcs12_content
@@ -45,3 +47,38 @@ FILE_TYPE_TO_FILE_PROCESSOR: dict[
     PrivateKeyFileTypes.SHAREPOINT_PFX_FILE: process_pkcs12_private_key_file,
     PrivateKeyFileTypes.ONEDRIVE_PFX_FILE: process_pkcs12_private_key_file,
 }
+
+
+# The credential key that holds each source's .pfx certificate, as base64.
+PKCS12_CREDENTIAL_KEYS: dict[DocumentSource, str] = {
+    DocumentSource.SHAREPOINT: "sp_private_key",
+    DocumentSource.ONEDRIVE: "onedrive_private_key",
+    DocumentSource.TEAMS: "teams_private_key",
+    DocumentSource.OUTLOOK: "outlook_private_key",
+}
+
+
+def validate_pkcs12_credential_value(
+    source: DocumentSource, credential_json: dict[str, Any]
+) -> None:
+    """A new account typed into a connector form carries its .pfx
+    certificate as base64 text, the form the private-key upload stores it in.
+    Checks it as that upload checks the file.
+
+    Raises:
+        ValueError: the value is not base64, or not a PKCS#12 (.pfx) file.
+    """
+    key = PKCS12_CREDENTIAL_KEYS.get(source)
+    if key is None:
+        return
+    value = credential_json.get(key)
+    if value is None or value == "":
+        return
+    if not isinstance(value, str):
+        raise ValueError(f"{key} must be the .pfx file's base64 content.")
+    try:
+        private_key_bytes = base64.b64decode(value, validate=True)
+    except (binascii.Error, ValueError) as e:
+        raise ValueError(f"{key} is not valid base64.") from e
+    if not validate_pkcs12_content(private_key_bytes):
+        raise ValueError(f"{key} is not a valid PKCS#12 (.pfx) file.")

backend/tests/unit/onyx/server/documents/test_pkcs12_credential_value.py

@@ -0,0 +1,59 @@
+"""A typed account's .pfx certificate, sent as base64 in credential_json, is
+checked as the private-key upload checks the file."""
+
+import base64
+
+import pytest
+from cryptography.hazmat.primitives.asymmetric import ec
+from cryptography.hazmat.primitives.serialization import BestAvailableEncryption, pkcs12
+
+from onyx.configs.constants import DocumentSource
+from onyx.server.documents.private_key_types import validate_pkcs12_credential_value
+
+
+def _pfx_base64() -> str:
+    key = ec.generate_private_key(ec.SECP256R1())
+    pfx = pkcs12.serialize_key_and_certificates(
+        b"onyx", key, None, None, BestAvailableEncryption(b"secret")
+    )
+    return base64.b64encode(pfx).decode("ascii")
+
+
+@pytest.mark.parametrize(
+    ("source", "key"),
+    [
+        (DocumentSource.SHAREPOINT, "sp_private_key"),
+        (DocumentSource.ONEDRIVE, "onedrive_private_key"),
+        (DocumentSource.TEAMS, "teams_private_key"),
+        (DocumentSource.OUTLOOK, "outlook_private_key"),
+    ],
+)
+def test_accepts_a_pfx_certificate(source: DocumentSource, key: str) -> None:
+    validate_pkcs12_credential_value(source, {key: _pfx_base64()})
+
+
+def test_accepts_an_account_without_a_certificate() -> None:
+    validate_pkcs12_credential_value(
+        DocumentSource.SHAREPOINT, {"sp_client_secret": "secret"}
+    )
+
+
+def test_rejects_text_that_is_not_base64() -> None:
+    with pytest.raises(ValueError, match="not valid base64"):
+        validate_pkcs12_credential_value(
+            DocumentSource.SHAREPOINT, {"sp_private_key": "not base64!"}
+        )
+
+
+def test_rejects_a_file_that_is_not_pkcs12() -> None:
+    not_pfx = base64.b64encode(b"just some text, not a certificate").decode()
+    with pytest.raises(ValueError, match="not a valid PKCS#12"):
+        validate_pkcs12_credential_value(
+            DocumentSource.SHAREPOINT, {"sp_private_key": not_pfx}
+        )
+
+
+def test_ignores_sources_without_a_certificate() -> None:
+    validate_pkcs12_credential_value(
+        DocumentSource.CONFLUENCE, {"sp_private_key": "not base64!"}
+    )

web/src/lib/credentials/components/CredentialFieldsRenderer.tsx

@@ -1,6 +1,6 @@
 "use client";
 
-import { useMemo, useState } from "react";
+import { useEffect, useMemo, useRef, useState } from "react";
 import {
   InputSingleUpload,
   InputPasswordTypeIn,
@@ -22,14 +22,12 @@ import type { IconFunctionComponent, RichStr } from "@opal/types";
 import { markdown } from "@opal/utils";
 import { getIn, useField, useFormikContext } from "formik";
 import { useTranslations } from "next-intl";
-import {
-  FILE_TYPE_DEFINITIONS,
-  type TypedFile,
-} from "@/lib/connectors/fileTypes";
+import { FILE_TYPE_DEFINITIONS, TypedFile } from "@/lib/connectors/fileTypes";
 import { createTypedFile } from "@/lib/connectors/utils";
 import { FormikField } from "@/refresh-components/form/FormikField";
 import { useCredentialFieldCopy } from "@/lib/credentials/hooks";
 import {
+  fileToBase64,
   getCredentialFileType,
   getCredentialSpec,
   methodFields,
@@ -41,12 +39,20 @@ import type {
 } from "@/lib/credentials/types";
 import type { ValidSources } from "@/lib/connectors/types/source";
 
+/**
+ * How a file field holds its file: a `TypedFile`, uploaded on its own when
+ * the account is saved, or the file's base64 text, sent with the account's
+ * other values (a new account typed into a connector form).
+ */
+export type CredentialFileValues = "typedFile" | "base64";
+
 interface CredentialFieldProps {
   source: ValidSources;
   fieldKey: string;
   field: CredentialSpecField;
   /** The Formik path the field lives under, e.g. `new_account`. */
   namePrefix?: string;
+  fileValues?: CredentialFileValues;
 }
 
 /**
@@ -73,6 +79,7 @@ function CredentialField({
   fieldKey,
   field,
   namePrefix,
+  fileValues = "typedFile",
 }: CredentialFieldProps) {
   const t = useTranslations("admin");
   const copy = useCredentialFieldCopy(source)(fieldKey);
@@ -88,6 +95,7 @@ function CredentialField({
         fieldKey={fieldKey}
         label={label}
         description={copy.description}
+        values={fileValues}
       />
     );
   }
@@ -186,42 +194,75 @@ interface CredentialFileFieldProps {
    * backticks shows as code.
    */
   description?: string | RichStr;
+  /** How the field holds the file. @default "typedFile" */
+  values?: CredentialFileValues;
 }
 
 /**
- * A key file, such as a .pfx certificate. It stays binary: the field holds a
- * `TypedFile`, checked against its type's size and extensions when picked.
+ * A key file, such as a .pfx certificate, checked against its type's size
+ * and extensions when picked. The field holds it as a `TypedFile`, or as its
+ * base64 text with `values="base64"`; either way the bytes stay intact.
  */
 export function CredentialFileField({
   name,
   fieldKey,
   label,
   description,
+  values = "typedFile",
 }: CredentialFileFieldProps) {
   const t = useTranslations("common.field");
-  const [field, , helpers] = useField<TypedFile | null>(name);
+  const [field, , helpers] = useField<TypedFile | string | null>(name);
   const [fileError, setFileError] = useState<string | null>(null);
+  // As base64 text the field cannot name its file, so the picked file is
+  // kept to show; a cleared or reset field drops it.
+  const [pickedFile, setPickedFile] = useState<File | null>(null);
+  const hasValue: boolean = field.value !== null && field.value !== "";
+  useEffect(() => {
+    if (!hasValue) setPickedFile(null);
+  }, [hasValue]);
+  const shownFile: File | null =
+    field.value instanceof TypedFile
+      ? field.value.file
+      : hasValue
+        ? pickedFile
+        : null;
   const fileType = getCredentialFileType(fieldKey);
   const definition = fileType ? FILE_TYPE_DEFINITIONS[fileType] : null;
 
+  // Each pick is numbered; a read that a later pick or a clear overtook is
+  // dropped, so it cannot bring back an older file.
+  const latestPickRef = useRef<number>(0);
+
   async function takeFile(file: File | null) {
+    const pick = ++latestPickRef.current;
+    const isLatest = () => pick === latestPickRef.current;
     setFileError(null);
-    if (!file) {
-      void helpers.setValue(null);
-      return;
-    }
+    // Until the new file is checked and read, the field holds no file, so
+    // nothing can send the previous one under the new name.
+    setPickedFile(null);
+    void helpers.setValue(null);
+    if (!file) return;
     if (!fileType) {
       setFileError(t("fileValidation.noTypeDefinition", { name: fieldKey }));
       return;
     }
     const typedFile = createTypedFile(file, fieldKey, fileType);
     const { isValid, errors } = await typedFile.validate();
+    if (!isLatest()) return;
     if (!isValid) {
       setFileError(errors.join(", ") || t("fileValidation.unknownError"));
-      void helpers.setValue(null);
       return;
     }
-    void helpers.setValue(typedFile);
+    let value: TypedFile | string;
+    try {
+      value = values === "base64" ? await fileToBase64(file) : typedFile;
+    } catch {
+      if (isLatest()) setFileError(t("fileValidation.selectionError"));
+      return;
+    }
+    if (!isLatest()) return;
+    setPickedFile(file);
+    void helpers.setValue(value);
     void helpers.setTouched(true, false);
   }
 
@@ -239,7 +280,7 @@ export function CredentialFileField({
         name={name}
         data-testid={fieldKey}
         accept={definition?.validation?.allowedExtensions?.join(",")}
-        value={field.value?.file ?? null}
+        value={shownFile}
         onFileChange={(file) => void takeFile(file)}
         variant={fileError ? "error" : "primary"}
       />
@@ -260,6 +301,8 @@ interface CredentialFieldsRendererProps {
   namePrefix?: string;
   /** Field keys this renderer leaves out, e.g. ones drawn elsewhere. */
   exclude?: ReadonlySet<string>;
+  /** How file fields hold their file. @default "typedFile" */
+  fileValues?: CredentialFileValues;
 }
 
 export function CredentialFieldsRenderer({
@@ -269,6 +312,7 @@ export function CredentialFieldsRenderer({
   setAuthMethod,
   namePrefix,
   exclude,
+  fileValues,
 }: CredentialFieldsRendererProps) {
   const t = useTranslations("admin");
   const { values, setValues, setFieldValue } =
@@ -334,6 +378,7 @@ export function CredentialFieldsRenderer({
                     fieldKey={key}
                     field={field}
                     namePrefix={namePrefix}
+                    fileValues={fileValues}
                   />
                 ))}
             </Section>
@@ -354,6 +399,7 @@ export function CredentialFieldsRenderer({
             fieldKey={key}
             field={field}
             namePrefix={namePrefix}
+            fileValues={fileValues}
           />
         ))}
     </>

web/src/lib/credentials/components/CredentialFileField.test.tsx

@@ -0,0 +1,122 @@
+import { render, screen, setupUser, waitFor } from "@tests/setup/test-utils";
+import { Formik, type FormikProps } from "formik";
+import { CredentialFileField } from "@/lib/credentials/components/CredentialFieldsRenderer";
+
+jest.mock("@/lib/credentials/utils", () => {
+  const actual = jest.requireActual("@/lib/credentials/utils");
+  return { ...actual, fileToBase64: jest.fn(actual.fileToBase64) };
+});
+
+const utils = jest.requireMock("@/lib/credentials/utils");
+
+interface CertificateForm {
+  sp_private_key: string | null;
+}
+
+function renderField(onForm: (form: FormikProps<CertificateForm>) => void) {
+  return render(
+    <Formik<CertificateForm>
+      initialValues={{ sp_private_key: null }}
+      onSubmit={() => {}}
+    >
+      {(formik) => {
+        onForm(formik);
+        return (
+          <CredentialFileField
+            name="sp_private_key"
+            fieldKey="sp_private_key"
+            label="Private key"
+            values="base64"
+          />
+        );
+      }}
+    </Formik>
+  );
+}
+
+function picker(container: HTMLElement): HTMLInputElement {
+  const input = container.querySelector<HTMLInputElement>('input[type="file"]');
+  if (!input) throw new Error("no file input");
+  return input;
+}
+
+describe("CredentialFileField as base64", () => {
+  it("holds the certificate's bytes as base64 text, and shows its name", async () => {
+    const user = setupUser();
+    let form: FormikProps<CertificateForm> | null = null;
+    const { container } = render(
+      <Formik<CertificateForm>
+        initialValues={{ sp_private_key: null }}
+        onSubmit={() => {}}
+      >
+        {(formik) => {
+          form = formik;
+          return (
+            <CredentialFileField
+              name="sp_private_key"
+              fieldKey="sp_private_key"
+              label="Private key"
+              values="base64"
+            />
+          );
+        }}
+      </Formik>
+    );
+    const picker =
+      container.querySelector<HTMLInputElement>('input[type="file"]');
+    if (!picker) throw new Error("no file input");
+
+    // Bytes that are not valid text, as a .pfx's are.
+    await user.upload(
+      picker,
+      new File([new Uint8Array([0x30, 0x82, 0xff, 0x00, 0x01])], "app.pfx")
+    );
+
+    await waitFor(() => expect(form?.values.sp_private_key).toBe("MIL/AAE="));
+    expect(screen.getByText("app.pfx")).toBeInTheDocument();
+  });
+
+  it("keeps the latest pick when an earlier read lands after it", async () => {
+    const user = setupUser();
+    const form: { current: FormikProps<CertificateForm> | null } = {
+      current: null,
+    };
+    const { container } = renderField((formik) => (form.current = formik));
+    let finishFirst: (value: string) => void = () => {};
+    utils.fileToBase64
+      .mockImplementationOnce(
+        () => new Promise<string>((resolve) => (finishFirst = resolve))
+      )
+      .mockImplementationOnce(() => Promise.resolve("U0VDT05E"));
+
+    await user.upload(picker(container), new File(["first"], "first.pfx"));
+    await user.upload(picker(container), new File(["second"], "second.pfx"));
+    await waitFor(() =>
+      expect(form.current?.values.sp_private_key).toBe("U0VDT05E")
+    );
+    finishFirst("RklSU1Q=");
+
+    await waitFor(() =>
+      expect(screen.getByText("second.pfx")).toBeInTheDocument()
+    );
+    expect(form.current?.values.sp_private_key).toBe("U0VDT05E");
+    expect(screen.queryByText("first.pfx")).not.toBeInTheDocument();
+  });
+
+  it("shows an error, and holds no file, when a read fails", async () => {
+    const user = setupUser();
+    const form: { current: FormikProps<CertificateForm> | null } = {
+      current: null,
+    };
+    const { container } = renderField((formik) => (form.current = formik));
+    utils.fileToBase64.mockImplementationOnce(() =>
+      Promise.reject(new Error("unreadable"))
+    );
+
+    await user.upload(picker(container), new File(["x"], "broken.pfx"));
+
+    expect(await screen.findByText("File selection error")).toBeInTheDocument();
+    expect(form.current?.values.sp_private_key).toBeNull();
+    expect(screen.queryByText("broken.pfx")).not.toBeInTheDocument();
+  });
+});

web/src/lib/credentials/utils.ts

@@ -587,3 +587,18 @@ export function bindingCheckCredentialId(
     ? request.credential_id
     : null;
 }
+
+/** A file's bytes as base64 text, as an account stores a .pfx certificate. */
+export function fileToBase64(file: File): Promise<string> {
+  return new Promise((resolve, reject) => {
+    const reader = new FileReader();
+    // A data URL is "data:<type>;base64,<content>"; the content follows the
+    // first comma.
+    reader.onload = () => {
+      const result = typeof reader.result === "string" ? reader.result : "";
+      resolve(result.slice(result.indexOf(",") + 1));
+    };
+    reader.onerror = () => reject(reader.error);
+    reader.readAsDataURL(file);
+  });
+}

web/src/views/admin/connectors/AddConnectorPage/newAccount.test.ts

@@ -53,3 +53,49 @@ test("a new account gets the realm, and the connector config does not", () => {
     )
   ).toEqual({ content_type: "articles" });
 });
+
+test("a Microsoft account's certificate is typed in, as base64 text", () => {
+  const source = ValidSources.Sharepoint;
+  const spec = typedAccountSpec(source);
+  if (spec === null) throw new Error("SharePoint takes a typed account");
+  const formValues: Record<string, unknown> = {
+    [NEW_ACCOUNT_FIELD]: {
+      ...initialNewAccountValues(spec),
+      authentication_method: "certificate",
+      sp_client_id: "client",
+      sp_directory_id: "tenant",
+      sp_certificate_password: "secret",
+      sp_private_key: "MIIKHgIBAzCCCdQ=",
+    },
+  };
+
+  const draft = typedDraft(
+    source,
+    newAccountSchema(spec, MESSAGES),
+    formValues
+  );
+
+  expect(draft?.credential_json).toMatchObject({
+    authentication_method: "certificate",
+    sp_private_key: "MIIKHgIBAzCCCdQ=",
+    sp_certificate_password: "secret",
+  });
+});
+
+test("a Microsoft certificate account is not ready without its file", () => {
+  const source = ValidSources.Sharepoint;
+  const spec = typedAccountSpec(source);
+  if (spec === null) throw new Error("SharePoint takes a typed account");
+
+  const draft = typedDraft(source, newAccountSchema(spec, MESSAGES), {
+    [NEW_ACCOUNT_FIELD]: {
+      ...initialNewAccountValues(spec),
+      authentication_method: "certificate",
+      sp_client_id: "client",
+      sp_directory_id: "tenant",
+      sp_certificate_password: "secret",
+    },
+  });
+
+  expect(draft).toBeNull();
+});

web/src/views/admin/connectors/AddConnectorPage/newAccount.ts

@@ -41,18 +41,13 @@ const OWN_ACCOUNT_SETUP: ReadonlySet<ValidSources> = new Set([
 
 /**
  * The source's credential spec when a new account is typed into the form;
- * `null` when it is not: Google Drive and Gmail set accounts up their own
- * way, and a file field (a key file) cannot be sent as typed values yet, so
- * those still save an account through the account form.
+ * `null` when it is not, as Google Drive and Gmail set accounts up their own
+ * way. A file field (a .pfx certificate) is typed in too, as the file's
+ * base64 text, which is how the account stores it.
  */
 export function typedAccountSpec(source: ValidSources): CredentialSpec | null {
   if (OWN_ACCOUNT_SETUP.has(source)) return null;
-  const spec = getCredentialSpec(source);
-  if (!spec) return null;
-  const hasFile = Object.values(spec.fields).some(
-    (field) => field.kind === "file"
-  );
-  return hasFile ? null : spec;
+  return getCredentialSpec(source) ?? null;
 }
 
 /**

web/src/views/admin/connectors/AddConnectorPage/sections/AuthenticationAccountSection.tsx

@@ -252,6 +252,8 @@ export default function AuthenticationAccountSection({
             source={connector}
             spec={typedSpec}
             namePrefix={NEW_ACCOUNT_FIELD}
+            // Sent with the account's other values, as the draft holds it.
+            fileValues="base64"
             // Asked for above this section, first.
             exclude={realmKeys}
             authMethod={

web/tests/e2e/admin/connector/onedrive_connector_setup.spec.ts

@@ -9,22 +9,20 @@ test.describe("OneDrive connector setup", () => {
     const setup = new OneDriveConnectorSetupPage(page);
     await setup.mockRoutes();
     await setup.goto();
-    await setup.createClientSecretCredential();
+    await setup.fillClientSecretCredential();
     await setup.runChecks();
     await setup.selectSpecificScope("owner@example.com");
     await setup.selectGeneralScope();
     await setup.submitConnector("General OneDrive");
     await setup.expectCreated();
 
-    expect(setup.credentialRequests).toEqual([
-      {
-        authenticationMethod: "client_secret",
-        clientId: "client-id",
-        directoryId: "directory-id",
-        hasClientSecret: true,
-        hasCertificatePassword: false,
-      },
-    ]);
+    expect(setup.credentialRequest).toEqual({
+      authenticationMethod: "client_secret",
+      clientId: "client-id",
+      directoryId: "directory-id",
+      hasClientSecret: true,
+      hasCertificatePassword: false,
+    });
     const expectedConfig: OneDriveConfigRequest = {
       connector_specific_config: {
         users: [],
@@ -37,23 +35,20 @@ test.describe("OneDrive connector setup", () => {
     const setup = new OneDriveConnectorSetupPage(page);
     await setup.mockRoutes();
     await setup.goto();
-    await setup.createCertificateCredential();
+    await setup.fillCertificateCredential();
     await setup.runChecks();
     await setup.selectSpecificScope("owner@example.com");
     await setup.submitConnector("Specific OneDrive");
     await setup.expectCreated();
 
-    expect(setup.credentialRequests).toEqual([
-      {
-        authenticationMethod: "certificate",
-        clientId: "client-id",
-        directoryId: "directory-id",
-        hasClientSecret: false,
-        hasCertificatePassword: true,
-        privateKeyField: "onedrive_private_key",
-        privateKeyType: "onedrive_pfx_file",
-      },
-    ]);
+    expect(setup.credentialRequest).toEqual({
+      authenticationMethod: "certificate",
+      clientId: "client-id",
+      directoryId: "directory-id",
+      hasClientSecret: false,
+      hasCertificatePassword: true,
+      privateKey: Buffer.from("test-pfx").toString("base64"),
+    });
     const expectedConfig: OneDriveConfigRequest = {
       connector_specific_config: {
         users: ["owner@example.com"],
@@ -67,7 +62,7 @@ test.describe("OneDrive connector setup", () => {
     const setup = new OneDriveConnectorSetupPage(page);
     await setup.mockRoutes();
     await setup.goto();
-    await setup.createClientSecretCredential();
+    await setup.fillClientSecretCredential();
     await setup.runChecks();
     await setup.selectSpecificScope();
     await setup.submitInvalidConnector(

web/tests/e2e/pages/OneDriveConnectorSetupPage.ts

@@ -1,4 +1,4 @@
-import { expect, type Page, type Request } from "@playwright/test";
+import { expect, type Page } from "@playwright/test";
 import {
   mockPassingConnectorChecks,
   runConnectorChecks,
@@ -16,22 +16,21 @@ export interface OneDriveCredentialRequest {
   directoryId: string;
   hasClientSecret: boolean;
   hasCertificatePassword: boolean;
-  privateKeyField?: string;
-  privateKeyType?: string;
+  /** The certificate, as the base64 text the account stores. */
+  privateKey?: string;
 }
 
 function isRecord(value: unknown): value is Record<string, unknown> {
   return typeof value === "object" && value !== null;
 }
 
-function parseCredentialJson(value: unknown): OneDriveCredentialRequest {
-  if (!isRecord(value)) {
-    throw new Error("Credential request must be an object");
-  }
+/** The draft account a check run sends, or `null` for a saved account. */
+function parseDraftCredential(
+  value: unknown
+): OneDriveCredentialRequest | null {
+  if (!isRecord(value) || !isRecord(value.credential_json)) return null;
   const credentialJson = value.credential_json;
-  if (!isRecord(credentialJson)) {
-    throw new Error("Credential JSON must be an object");
-  }
+  const privateKey = credentialJson.onedrive_private_key;
   return {
     authenticationMethod: String(credentialJson.authentication_method ?? ""),
     clientId: String(credentialJson.onedrive_client_id ?? ""),
@@ -42,39 +41,13 @@ function parseCredentialJson(value: unknown): OneDriveCredentialRequest {
     hasCertificatePassword:
       typeof credentialJson.onedrive_certificate_password === "string" &&
       credentialJson.onedrive_certificate_password.length > 0,
-  };
-}
-
-function getMultipartField(body: string, fieldName: string): string {
-  const fieldMarker = `name="${fieldName}"`;
-  const fieldStart = body.indexOf(fieldMarker);
-  if (fieldStart < 0) {
-    return "";
-  }
-  const valueStart = body.indexOf("\r\n\r\n", fieldStart);
-  if (valueStart < 0) {
-    return "";
-  }
-  const valueEnd = body.indexOf("\r\n--", valueStart);
-  return body.slice(valueStart + 4, valueEnd < 0 ? undefined : valueEnd);
-}
-
-function parsePrivateKeyCredential(
-  request: Request
-): OneDriveCredentialRequest {
-  const body = request.postData() ?? "";
-  const credential = parseCredentialJson({
-    credential_json: JSON.parse(getMultipartField(body, "credential_json")),
-  });
-  return {
-    ...credential,
-    privateKeyField: getMultipartField(body, "field_key"),
-    privateKeyType: getMultipartField(body, "type_definition_key"),
+    ...(typeof privateKey === "string" ? { privateKey } : {}),
   };
 }
 
 export class OneDriveConnectorSetupPage {
-  readonly credentialRequests: OneDriveCredentialRequest[] = [];
+  /** The draft account of the latest check run. */
+  credentialRequest: OneDriveCredentialRequest | null = null;
   readonly connectorRequests: OneDriveConfigRequest[] = [];
 
   constructor(private readonly page: Page) {}
@@ -93,16 +66,6 @@ export class OneDriveConnectorSetupPage {
         },
       })
     );
-    await this.page.route("**/api/manage/credential/private-key", (route) => {
-      this.credentialRequests.push(parsePrivateKeyCredential(route.request()));
-      return route.fulfill({ json: this.credentialResponse(42) });
-    });
-    await this.page.route("**/api/manage/credential", (route) => {
-      this.credentialRequests.push(
-        parseCredentialJson(route.request().postDataJSON())
-      );
-      return route.fulfill({ json: this.credentialResponse(41) });
-    });
     // Create makes the connector and pairs it in one request.
     await this.page.route(
       "**/api/manage/admin/connector-with-credential",
@@ -139,6 +102,16 @@ export class OneDriveConnectorSetupPage {
       }
     );
     await mockPassingConnectorChecks(this.page);
+    // A typed account travels with the check run, which saves it as a draft.
+    // Registered last, so it sees the run first and hands it to the mock.
+    await this.page.route(
+      "**/api/manage/admin/connector-checks/runs",
+      (route) => {
+        const draft = parseDraftCredential(route.request().postDataJSON());
+        if (draft) this.credentialRequest = draft;
+        return route.fallback();
+      }
+    );
   }
 
   async goto() {
@@ -148,14 +121,13 @@ export class OneDriveConnectorSetupPage {
     ).toBeVisible();
   }
 
-  async createClientSecretCredential() {
+  async fillClientSecretCredential() {
     await this.openCredentialForm();
     await this.fillCredentialIdentity();
     await this.page.getByTestId("onedrive_client_secret").fill("secret-value");
-    await this.submitCredential();
   }
 
-  async createCertificateCredential() {
+  async fillCertificateCredential() {
     await this.openCredentialForm();
     await this.page.getByRole("tab", { name: "Certificate" }).click();
     await this.fillCredentialIdentity();
@@ -167,7 +139,6 @@ export class OneDriveConnectorSetupPage {
       mimeType: "application/x-pkcs12",
       buffer: Buffer.from("test-pfx"),
     });
-    await this.submitCredential();
   }
 
   /** Run the connector checks and wait for the configuration to unlock. */
@@ -216,11 +187,6 @@ export class OneDriveConnectorSetupPage {
     await expect(this.page).toHaveURL(/\/admin\/indexing-status/);
   }
 
-  /** The creation form, once its card is expanded. */
-  private get credentialForm() {
-    return this.page.getByTestId("credential-form");
-  }
-
   /**
    * Opens the new-account card. It starts open when there is no saved
    * account, so it is clicked only while closed; retried in case its starting
@@ -245,43 +211,4 @@ export class OneDriveConnectorSetupPage {
     await this.page.getByTestId("onedrive_client_id").fill("client-id");
     await this.page.getByTestId("onedrive_directory_id").fill("directory-id");
   }
-
-  private async submitCredential() {
-    const responsePromise = this.page.waitForResponse(
-      (response) =>
-        response.request().method() === "POST" &&
-        [
-          "/api/manage/credential",
-          "/api/manage/credential/private-key",
-        ].includes(new URL(response.url()).pathname)
-    );
-    await this.credentialForm
-      .getByRole("button", { name: "Create", exact: true })
-      .click();
-    const response = await responsePromise;
-    expect(response.ok()).toBeTruthy();
-    // The card folds itself away on success. The fold is hidden from the
-    // accessibility tree, so the form's own button stops resolving.
-    await expect(
-      this.page.getByRole("button", { name: "Create", exact: true })
-    ).toBeHidden();
-  }
-
-  private credentialResponse(id: number) {
-    const timestamp = new Date(0).toISOString();
-    return {
-      id,
-      credential: {
-        id,
-        credential_json: {},
-        admin_public: true,
-        source: "onedrive",
-        name: "OneDrive test credential",
-        user_id: null,
-        user_email: null,
-        time_created: timestamp,
-        time_updated: timestamp,
-      },
-    };
-  }
 }