Let members connect GitHub with OAuth through the GitHub App

crates/engram-coordinator/src/oauth_redirect.rs

@@ -640,7 +640,11 @@ impl OAuthManager {
             .redirect(reqwest::redirect::Policy::none())
             .build()
             .map_err(|e| driver_err("http_client", e.to_string()))?;
-        let request = http.post(token_url);
+        // RFC 6749 answers in JSON, but some providers (GitHub) answer the
+        // token request in form encoding unless the client asks for JSON.
+        let request = http
+            .post(token_url)
+            .header(reqwest::header::ACCEPT, "application/json");
         let request = match body {
             TokenRequestBody::Form => request.form(form),
             TokenRequestBody::Json => {
@@ -688,7 +692,9 @@ impl OAuthManager {
                         format!("no access token under {grant_path:?}")
                     }
                 });
-            let code = if err == "invalid_grant" {
+            // GitHub answers a dead refresh token with HTTP 200 and its own
+            // `bad_refresh_token` code, not the RFC's `invalid_grant`.
+            let code = if err == "invalid_grant" || err == "bad_refresh_token" {
                 "invalid_grant"
             } else {
                 "exchange_failed"

crates/engram-coordinator/tests/oauth_redirect.rs

@@ -7,6 +7,7 @@ use std::collections::BTreeMap;
 use std::sync::Arc;
 
 use axum::extract::State;
+use axum::http::HeaderMap;
 use axum::routing::{get, post};
 use axum::{Json, Router};
 use engram_coordinator::oauth::OAuthManager;
@@ -28,20 +29,29 @@ use serde_json::{json, Value};
 struct Provider {
     token_response: Arc<Mutex<Value>>,
     token_requests: Arc<Mutex<Vec<BTreeMap<String, String>>>>,
+    /// The `Accept` header of each token request.
+    token_accepts: Arc<Mutex<Vec<String>>>,
     viewer_response: Arc<Mutex<Value>>,
 }
 
 async fn serve(provider: Provider) -> String {
     let router = Router::new()
         .route(
             "/oauth/token",
-            post(|State(p): State<Provider>, body: String| async move {
-                let form: BTreeMap<String, String> = url::form_urlencoded::parse(body.as_bytes())
-                    .map(|(k, v)| (k.into_owned(), v.into_owned()))
-                    .collect();
-                p.token_requests.lock().push(form);
-                Json(p.token_response.lock().clone())
-            }),
+            post(
+                |State(p): State<Provider>, headers: HeaderMap, body: String| async move {
+                    let accept = headers.get("accept").and_then(|v| v.to_str().ok());
+                    p.token_accepts
+                        .lock()
+                        .push(accept.unwrap_or_default().to_owned());
+                    let form: BTreeMap<String, String> =
+                        url::form_urlencoded::parse(body.as_bytes())
+                            .map(|(k, v)| (k.into_owned(), v.into_owned()))
+                            .collect();
+                    p.token_requests.lock().push(form);
+                    Json(p.token_response.lock().clone())
+                },
+            ),
         )
         .route(
             "/graphql",
@@ -258,6 +268,11 @@ async fn complete_publishes_bundle_with_rotation_and_probe_metadata() {
     assert_eq!(reqs[0]["client_id"], "client-123");
     assert_eq!(reqs[0]["client_secret"], "secret-456");
     assert_eq!(reqs[0]["redirect_uri"], "https://cb.example.com/x");
+    // GitHub answers in form encoding unless the request asks for JSON.
+    assert_eq!(
+        *fx.provider.token_accepts.lock(),
+        vec!["application/json".to_owned()]
+    );
 
     // Probe metadata extracted; expiry mirrored outside the ciphertext.
     assert_eq!(row.metadata.account_id, "app-user-1");
@@ -746,6 +761,33 @@ async fn invalid_grant_breaks_the_credential_and_reconnect_repairs_it() {
     assert_eq!(resolved.secret, "at-3");
 }
 
+#[tokio::test]
+async fn github_bad_refresh_token_breaks_the_credential() {
+    let fx = fixture().await;
+    let sp = spec(&fx.host, false);
+    connected(&fx, &sp).await;
+
+    // GitHub's shape: HTTP 200, its own error code.
+    *fx.provider.token_response.lock() = json!({
+        "error": "bad_refresh_token",
+        "error_description": "The refresh token passed is incorrect or expired.",
+    });
+    fx.clock.advance(std::time::Duration::from_secs(19 * 3600));
+    let sweep = fx
+        .manager
+        .run_connector_refresh_once()
+        .await
+        .expect("sweep");
+    assert_eq!(sweep.broke, 1, "{sweep:?}");
+    let row = fx
+        .meta
+        .get_oauth_credential(&linear_key())
+        .await
+        .expect("get")
+        .expect("row");
+    assert_eq!(row.broken_reason.as_deref(), Some("invalid_grant"));
+}
+
 #[tokio::test]
 async fn non_refreshable_expiring_token_breaks_at_expiry_not_before() {
     let fx = fixture().await;

crates/engram-git-github/src/lib.rs

@@ -560,7 +560,9 @@ async fn ensure_ok(
 /// ID + RSA private-key PEM. An admin supplies both via the integrations UI (the
 /// PEM sealed in the org-secret store); the coordinator resolves the two fields
 /// (org secrets `github_app.app_id` / `github_app.private_key_pem`, name =
-/// `<kind>.<field>`) and calls `build` to construct the engine lazily. This is
+/// `<kind>.<field>`) and calls `build` to construct the engine lazily. The
+/// optional `client_id` / `client_secret` are the App's OAuth client, which
+/// the GitHub connector's personal OAuth flow reads. This is
 /// the data-driven frame around minting — the mint logic stays in [`GitHubApp`].
 pub fn github_app_descriptor() -> MintKindDescriptor {
     MintKindDescriptor {
@@ -580,6 +582,21 @@ pub fn github_app_descriptor() -> MintKindDescriptor {
                 field_kind: MintFieldKind::SealedSecret,
                 required: true,
             },
+            // The App's own OAuth client. Optional: with both set, members
+            // can connect their personal GitHub account through the App
+            // instead of pasting a token. Minting does not use them.
+            MintFieldSchema {
+                name: "client_id",
+                label: "Client ID (optional, for personal connect)",
+                field_kind: MintFieldKind::Config,
+                required: false,
+            },
+            MintFieldSchema {
+                name: "client_secret",
+                label: "Client secret (optional, for personal connect)",
+                field_kind: MintFieldKind::SealedSecret,
+                required: false,
+            },
         ],
         build: |fields: &ResolvedFields| {
             let app_id = fields.get("app_id").ok_or_else(|| {

crates/engram-git-github/tests/github.rs

@@ -121,7 +121,10 @@ fn github_app_descriptor_metadata() {
     // The field names are the org-secret suffixes the coordinator resolves; the
     // Plane-A form (C4) renders from this metadata, so pin the contract.
     let names: Vec<&str> = d.fields.iter().map(|f| f.name).collect();
-    assert_eq!(names, vec!["app_id", "private_key_pem"]);
+    assert_eq!(
+        names,
+        vec!["app_id", "private_key_pem", "client_id", "client_secret"]
+    );
     let pem = d
         .fields
         .iter()
@@ -131,6 +134,18 @@ fn github_app_descriptor_metadata() {
     assert!(pem.required);
     let app_id = d.fields.iter().find(|f| f.name == "app_id").unwrap();
     assert_eq!(app_id.field_kind, MintFieldKind::Config);
+    // The App's OAuth client serves personal connect only; minting works
+    // without it.
+    let client_secret = d.fields.iter().find(|f| f.name == "client_secret").unwrap();
+    assert_eq!(client_secret.field_kind, MintFieldKind::SealedSecret);
+    assert!(!client_secret.required);
+    assert!(
+        !d.fields
+            .iter()
+            .find(|f| f.name == "client_id")
+            .unwrap()
+            .required
+    );
 }
 
 #[test]

orchestrator/src/__tests__/connectors.test.ts

@@ -767,7 +767,17 @@ describe("on-disk registry", () => {
     expect(userScopes).toContain("channels:write");
     expect(userScopes).toContain("chat:write");
     expect(reg.get("sentry")!.userCredential?.token?.hint).toContain("User Auth Tokens");
-    expect(reg.get("github")!.userCredential?.oauth).toBeUndefined();
+    // GitHub's personal OAuth is the App's own user flow: it exchanges the
+    // code on github.com and names the account by its login.
+    expect(reg.get("github")!.userCredential?.oauth).toBe(true);
+    expect(reg.get("github")!.userCredential?.oauthFacet).toMatchObject({
+      authorizeUrl: "https://github.com/login/oauth/authorize",
+      tokenUrl: "https://github.com/login/oauth/access_token",
+      scopes: [],
+      clientIdRef: "github_app.client_id",
+      clientSecretRef: "github_app.client_secret",
+      metadata: { probe: { path: "/user", map: { accountId: "id", displayName: "login" } } },
+    });
     expect(reg.get("github")!.userCredential?.token?.identity).toEqual({
       path: "/user",
       map: { displayName: "login" },
@@ -1495,16 +1505,60 @@ describe("user-scoped credentials (ADR 0115)", () => {
       expect(() => parseConnector(noInject, "t")).toThrow(/"inject" is required on a mint connector/);
     });
 
-    test("rejects oauth mode on a mint connector", () => {
-      const withOauth = {
-        ...acmeMintRaw,
-        userCredential: {
-          oauth: true,
-          token: { hint: "x" },
-          inject: { header: "Authorization", template: "Bearer {}" },
+    // A mint connector has no org OAuth facet, so its personal flow is a
+    // complete authorization-code flow of its own (GitHub's App user flow).
+    const mintFlow = {
+      authorizeUrl: "https://acmemint.test/login/oauth/authorize",
+      acquisitionHosts: ["acmemint.test"],
+      tokenUrl: "https://acmemint.test/login/oauth/access_token",
+      clientIdRef: "acme_app.client_id",
+      clientSecretRef: "acme_app.client_secret",
+    };
+    const withMintOauth = (oauth: unknown) => ({
+      ...acmeMintRaw,
+      userCredential: {
+        oauth,
+        token: { hint: "x" },
+        inject: { header: "Authorization", template: "Bearer {}" },
+      },
+    });
+
+    test("accepts a mint connector's own personal OAuth flow", () => {
+      const c = parseConnector(withMintOauth(mintFlow), "acmemint");
+      expect(c.userCredential?.oauth).toBe(true);
+      // The token URL may sit on an acquisition host, and no scopes is fine.
+      expect(c.userCredential?.oauthFacet).toMatchObject({
+        grant: "authorization_code",
+        tokenUrl: "https://acmemint.test/login/oauth/access_token",
+        scopes: [],
+        clientIdRef: "acme_app.client_id",
+      });
+    });
+
+    test("rejects a mint connector's oauth that is not a flow of its own", () => {
+      expect(() => parseConnector(withMintOauth(true), "t")).toThrow(/must declare its own flow/);
+      expect(() =>
+        parseConnector(withMintOauth({ ...mintFlow, grant: "client_credentials" }), "t"),
+      ).toThrow(/"authorization_code" for a personal flow/);
+      expect(() =>
+        parseConnector(
+          withMintOauth({ ...mintFlow, tokenUrl: "https://elsewhere.test/token" }),
+          "t",
+        ),
+      ).toThrow(/"oauth.tokenUrl" host "elsewhere.test"/);
+    });
+
+    test("an org OAuth facet keeps its token URL on the connector's hosts", () => {
+      // The relaxation is for a personal flow only.
+      const org = {
+        ...acmeRaw,
+        credential: {
+          source: "inject",
+          injects: [{ header: "Authorization", template: "Bearer {}" }],
         },
+        oauth: { ...mintFlow, scopes: ["read"], tokenUrl: "https://acmemint.test/token" },
       };
-      expect(() => parseConnector(withOauth, "t")).toThrow(/token" mode only/);
+      expect(() => parseConnector(org, "t")).toThrow(/"oauth.tokenUrl" host/);
     });
 
     test("rejects oauth mode without the top-level oauth facet", () => {

orchestrator/src/__tests__/me-oauth.test.ts

@@ -27,10 +27,16 @@ const acmeConnector = {
   userCredential: { token: { hint: "Create a PAT." } },
 };
 
+/** The org secret names the fake coordinator holds. */
+let orgSecretNames: string[] = [];
+
 function appWith(oauth: OAuthCredentialClient) {
   return makeMeRoute({
     secrets,
     oauth,
+    orgSecrets: {
+      listSecrets: async () => ({ secrets: orgSecretNames.map((name) => ({ name })) }),
+    },
     getSession: async () => ({ user: { id: "user-42", role: "user" } }),
     connectors: { list: async () => [{ provider: "acme", config: acmeConnector }] },
     harnessCatalog: {
@@ -127,7 +133,10 @@ describe("/me OAuth credentials", () => {
 });
 
 describe("/me connector credentials (ADR 0115)", () => {
-  beforeEach(() => invalidateRegistry());
+  beforeEach(() => {
+    invalidateRegistry();
+    orgSecretNames = [];
+  });
 
   function fakeOauth() {
     const calls: {
@@ -222,6 +231,23 @@ describe("/me connector credentials (ADR 0115)", () => {
     );
   });
 
+  test("offers OAuth connect only when the app's client id and secret are set", async () => {
+    const { oauth } = fakeOauth();
+    const githubModes = async () => {
+      const listed = await appWith(oauth).request("/api/v1/me/credentials");
+      const body = (await listed.json()) as { credentials: Array<Record<string, unknown>> };
+      return body.credentials.find(
+        (entry) => entry.kind === "connector" && entry.provider === "github",
+      )?.modes;
+    };
+    // Without the GitHub App's OAuth client, a Connect button would only fail.
+    expect(await githubModes()).toEqual({ oauth: false, token: true });
+    orgSecretNames = ["github_app.client_id"];
+    expect(await githubModes()).toEqual({ oauth: false, token: true });
+    orgSecretNames = ["github_app.client_id", "github_app.client_secret"];
+    expect(await githubModes()).toEqual({ oauth: true, token: true });
+  });
+
   test("PUT 404s for a provider without token mode and 400s an empty value", async () => {
     const { oauth, calls } = fakeOauth();
     const app = appWith(oauth);

orchestrator/src/__tests__/slack-integration.test.ts

@@ -654,6 +654,39 @@ describe("integration oauth route", () => {
     });
   });
 
+  test("a mint connector's own personal flow: GitHub authorize and callback", async () => {
+    const { client, calls } = fakeRedirectClient({
+      flowSubject: { kind: OauthSubjectKind.USER_CONNECTOR, id: "u2" },
+    });
+    const app = makeIntegrationOauthRoute({
+      connectors: emptySource,
+      oauthCredential: client as never,
+      connectionIdFor,
+      getSession: memberSession,
+    });
+    const begun = await app.request("/api/v1/me/connector-credentials/github/oauth/authorize");
+    expect(begun.status).toBe(302);
+    const spec = calls.begin[0]!.spec!;
+    expect(calls.begin[0]!.subject).toMatchObject({ kind: OauthSubjectKind.USER_CONNECTOR, id: "u2" });
+    expect(spec.authorizeUrl).toBe("https://github.com/login/oauth/authorize");
+    expect(spec.tokenUrl).toBe("https://github.com/login/oauth/access_token");
+    expect(spec.scopes).toEqual([]);
+    expect(spec.clientIdRef).toBe("github_app.client_id");
+    expect(spec.metadata?.probe).toMatchObject({
+      host: "api.github.com",
+      path: "/user",
+      map: { account_id: "id", display_name: "login" },
+    });
+
+    const done = await app.request(
+      "/api/v1/integrations/github/oauth/callback?code=abc&state=10600000-0000-4000-8000-0000000000aa",
+    );
+    expect(done.status).toBe(302);
+    expect(done.headers.get("location")).toBe("/settings/credentials?connected=github");
+    // The completion uses the same spec the flow began with.
+    expect(calls.complete[0]!.spec?.tokenUrl).toBe("https://github.com/login/oauth/access_token");
+  });
+
   test("callback: a user-subject flow is fenced to its owner", async () => {
     const { client, calls } = fakeRedirectClient({
       flowSubject: { kind: OauthSubjectKind.USER_CONNECTOR, id: "someone-else" },

orchestrator/src/connectors/github.json

@@ -17,6 +17,24 @@
     }
   },
   "userCredential": {
+    "oauth": {
+      "authorizeUrl": "https://github.com/login/oauth/authorize",
+      "acquisitionHosts": [
+        "github.com"
+      ],
+      "tokenUrl": "https://github.com/login/oauth/access_token",
+      "clientIdRef": "github_app.client_id",
+      "clientSecretRef": "github_app.client_secret",
+      "metadata": {
+        "probe": {
+          "path": "/user",
+          "map": {
+            "accountId": "id",
+            "displayName": "login"
+          }
+        }
+      }
+    },
     "token": {
       "hint": "Create a fine-grained personal access token under GitHub Settings \u2192 Developer settings, with the permissions you want sessions to hold.",
       "identity": {