diff --git a/extensions/connector-namespaces/README.md b/extensions/connector-namespaces/README.md index 43e7bc990..66c534890 100644 --- a/extensions/connector-namespaces/README.md +++ b/extensions/connector-namespaces/README.md @@ -16,8 +16,8 @@ and connected-server management into the Copilot side panel. - **My MCPs** - see which servers are connected and ready to add to Copilot. - **Namespace playground** - open any connected server in the Connector Namespace playground with **Sandbox**. -- **Persistent setup** - retain the selected namespace and restore Azure sign-in - securely across app restarts. +- **Persistent namespace selection** - retain the selected namespace while Azure + tokens remain in memory and sign-in is requested again after a restart. ## Install @@ -36,10 +36,6 @@ and select **Install in GitHub Copilot app**. - Permission to view the namespace and create its connections and hosted MCP server configurations. - A browser for Microsoft Entra sign-in and connector consent. -- An operating-system secure credential store. Windows and macOS provide one by - default. Linux and WSL require a Secret Service-compatible keyring, such as - GNOME Keyring, with `libsecret` available. Unencrypted token storage is - intentionally disabled. Connector Namespace is currently an Azure preview service and availability can vary by region. @@ -64,13 +60,13 @@ playground. Use **Change namespace** to switch subscriptions or namespaces. Azure sign-in and connector sign-in are separate: - **Azure sign-in** lets the canvas discover and manage Connector Namespace - resources. Access and refresh tokens are stored in the operating system's - encrypted credential store. To select that encrypted cache entry after an app - restart, the extension separately saves a non-secret authentication record - containing the authority, client ID, account ID, tenant ID, and username under - `~/.copilot/extensions/connector-namespaces/artifacts/azure-auth-record.json`, - with user-only permissions where supported. Raw tokens are never written to - extension files. + resources. Access and refresh tokens remain in the extension process and are + never written to extension files. Reloading the extension or restarting the + app requires Azure sign-in again. The selected namespace coordinates are + retained in + `~/.copilot/extensions/connector-namespaces/artifacts/gateway-config.json` so + the canvas can explain that the namespace is still linked and return directly + to its connectors after sign-in. - **Connector sign-in** grants an individual MCP server access to its backing service. The resulting connection is managed by Connector Namespace. diff --git a/extensions/connector-namespaces/auth.mjs b/extensions/connector-namespaces/auth.mjs index 0b72597b0..c0eb5fd6a 100644 --- a/extensions/connector-namespaces/auth.mjs +++ b/extensions/connector-namespaces/auth.mjs @@ -2,16 +2,9 @@ import { randomUUID } from "node:crypto"; import { promises as fs } from "node:fs"; import { homedir } from "node:os"; import { join } from "node:path"; -import { - deserializeAuthenticationRecord, - InteractiveBrowserCredential, - serializeAuthenticationRecord, - useIdentityPlugin, -} from "@azure/identity"; -import { cachePersistencePlugin } from "@azure/identity-cache-persistence"; +import { InteractiveBrowserCredential } from "@azure/identity"; export const ARM_SCOPE = "https://management.azure.com/.default"; -export const TOKEN_CACHE_NAME = "github-copilot-connector-namespaces"; const TOKEN_EXPIRY_SKEW_MS = 5 * 60 * 1000; const SIGN_IN_SESSION_TTL_MS = 10 * 60 * 1000; @@ -24,50 +17,20 @@ const AUTH_STORAGE_DIR = join( const AUTH_RECORD_FILE = join(AUTH_STORAGE_DIR, "azure-auth-record.json"); const LEGACY_AUTH_CACHE = join(AUTH_STORAGE_DIR, "auth-cache.json"); -let legacyAuthCacheRemoved = false; +let legacyAuthArtifactsRemoved = false; -useIdentityPlugin(cachePersistencePlugin); - -export async function loadAuthenticationRecord({ - readFile = fs.readFile, - deserialize = deserializeAuthenticationRecord, - authRecordFile = AUTH_RECORD_FILE, -} = {}) { - let serialized; - try { - serialized = await readFile(authRecordFile, "utf-8"); - } catch (error) { - if (error?.code === "ENOENT") return undefined; - throw error; - } - try { - return deserialize(serialized); - } catch { - return undefined; - } -} - -async function saveAuthenticationRecord(record) { - await fs.mkdir(AUTH_STORAGE_DIR, { recursive: true, mode: 0o700 }); - await fs.chmod(AUTH_STORAGE_DIR, 0o700); - await fs.writeFile( - AUTH_RECORD_FILE, - serializeAuthenticationRecord(record), - { encoding: "utf-8", mode: 0o600 }, - ); - await fs.chmod(AUTH_RECORD_FILE, 0o600); -} - -async function removeLegacyAuthCache() { - if (legacyAuthCacheRemoved) return; - try { - await fs.unlink(LEGACY_AUTH_CACHE); - } catch (error) { - if (error?.code !== "ENOENT") { - throw new Error(`Could not remove the legacy connector credential cache at ${LEGACY_AUTH_CACHE}: ${error.message}`); +async function removeLegacyAuthArtifacts() { + if (legacyAuthArtifactsRemoved) return; + for (const path of [AUTH_RECORD_FILE, LEGACY_AUTH_CACHE]) { + try { + await fs.unlink(path); + } catch (error) { + if (error?.code !== "ENOENT") { + throw new Error(`Could not remove the legacy connector authentication file at ${path}: ${error.message}`); + } } } - legacyAuthCacheRemoved = true; + legacyAuthArtifactsRemoved = true; } export class ConnectorAuthenticationRequiredError extends Error { @@ -103,20 +66,14 @@ export class InteractiveAuthBroker { createCredential = credentialFactory, createSessionId = randomUUID, cleanupLegacyCredentials = async () => {}, - loadAuthRecord = async () => undefined, - saveAuthRecord = async () => {}, now = Date.now, scope = ARM_SCOPE, - tokenCacheName = TOKEN_CACHE_NAME, } = {}) { this.createCredential = createCredential; this.createSessionId = createSessionId; this.cleanupLegacyCredentials = cleanupLegacyCredentials; - this.loadAuthRecord = loadAuthRecord; - this.saveAuthRecord = saveAuthRecord; this.now = now; this.scope = scope; - this.tokenCacheName = tokenCacheName; this.credential = null; this.accessToken = null; this.cleanupInFlight = null; @@ -124,15 +81,10 @@ export class InteractiveAuthBroker { this.sessions = new Map(); } - createInteractiveCredential(authenticationRecord) { + createInteractiveCredential() { return this.createCredential({ redirectUri: "http://localhost", disableAutomaticAuthentication: true, - tokenCachePersistenceOptions: { - enabled: true, - name: this.tokenCacheName, - }, - ...(authenticationRecord ? { authenticationRecord } : {}), }); } @@ -188,7 +140,6 @@ export class InteractiveAuthBroker { if (!authenticationRecord) { throw new Error("Azure identity did not return an authentication record."); } - await this.saveAuthRecord(authenticationRecord); const accessToken = await credential.getToken(this.scope, { abortSignal: abortController.signal }); if (!accessToken?.token || !Number.isFinite(accessToken.expiresOnTimestamp)) { throw new Error("Azure identity returned an incomplete ARM access token."); @@ -235,13 +186,8 @@ export class InteractiveAuthBroker { let credential = this.credential; if (!credential) { try { - const authenticationRecord = await this.loadAuthRecord(); - if (hasUsableToken(this.accessToken, this.now())) return this.accessToken.token; - credential = this.credential; - if (!credential) { - credential = this.createInteractiveCredential(authenticationRecord); - this.credential = credential; - } + credential = this.createInteractiveCredential(); + this.credential = credential; } catch (error) { if (isAuthenticationRequiredError(error)) { throw new ConnectorAuthenticationRequiredError( @@ -281,9 +227,7 @@ export class InteractiveAuthBroker { } export const interactiveAuth = new InteractiveAuthBroker({ - cleanupLegacyCredentials: removeLegacyAuthCache, - loadAuthRecord: loadAuthenticationRecord, - saveAuthRecord: saveAuthenticationRecord, + cleanupLegacyCredentials: removeLegacyAuthArtifacts, }); export const startSignIn = () => interactiveAuth.startSignIn(); diff --git a/extensions/connector-namespaces/auth.test.mjs b/extensions/connector-namespaces/auth.test.mjs index 43356644d..332786465 100644 --- a/extensions/connector-namespaces/auth.test.mjs +++ b/extensions/connector-namespaces/auth.test.mjs @@ -4,8 +4,6 @@ import assert from "node:assert/strict"; import { ConnectorAuthenticationRequiredError, InteractiveAuthBroker, - loadAuthenticationRecord, - TOKEN_CACHE_NAME, } from "./auth.mjs"; function accessToken(token = "token", expiresOnTimestamp = 2_000_000_000_000) { @@ -35,7 +33,6 @@ test("ARM token requests require an explicit browser sign-in", async () => { }, }; }, - loadAuthRecord: async () => undefined, }); await assert.rejects( @@ -43,32 +40,13 @@ test("ARM token requests require an explicit browser sign-in", async () => { (error) => error instanceof ConnectorAuthenticationRequiredError && error.code === "authentication_required", ); - assert.deepEqual(credentialOptions.tokenCachePersistenceOptions, { - enabled: true, - name: TOKEN_CACHE_NAME, + assert.deepEqual(credentialOptions, { + redirectUri: "http://localhost", + disableAutomaticAuthentication: true, }); }); -test("a malformed authentication record falls back to browser sign-in", async () => { - assert.equal( - await loadAuthenticationRecord({ - readFile: async () => "{malformed-json", - }), - undefined, - ); -}); - -test("authentication record read failures remain operational errors", async () => { - const readError = Object.assign(new Error("authentication record is unreadable"), { code: "EACCES" }); - await assert.rejects( - loadAuthenticationRecord({ - readFile: async () => { throw readError; }, - }), - (error) => error === readError, - ); -}); - -test("interactive sign-in reports pending then done and caches the ARM token", async () => { +test("interactive sign-in reports pending then done and keeps the ARM token in memory", async () => { let credentialOptions; let authenticateOptions; const credential = { @@ -89,7 +67,6 @@ test("interactive sign-in reports pending then done and caches the ARM token", a return credential; }, createSessionId: () => "signin-session", - saveAuthRecord: async (record) => assert.deepEqual(record, authenticationRecord()), now: () => 1_000, }); @@ -110,10 +87,6 @@ test("interactive sign-in reports pending then done and caches the ARM token", a assert.deepEqual(credentialOptions, { redirectUri: "http://localhost", disableAutomaticAuthentication: true, - tokenCachePersistenceOptions: { - enabled: true, - name: TOKEN_CACHE_NAME, - }, }); assert.equal(authenticateOptions.abortSignal.aborted, false); assert.deepEqual(broker.getSignInStatus(started.sessionId), { ok: true, status: "done" }); @@ -124,22 +97,24 @@ test("interactive sign-in reports pending then done and caches the ARM token", a assert.equal(await broker.getToken(), "token"); }); -test("a new broker restores the persisted credential without reopening the browser", async () => { - const cache = { record: null, token: null }; +test("a new broker requires browser sign-in after the extension reloads", async () => { let authenticateCalls = 0; + let createCredentialCalls = 0; const createCredential = (options) => { - assert.deepEqual(options.tokenCachePersistenceOptions, { - enabled: true, - name: TOKEN_CACHE_NAME, + createCredentialCalls++; + assert.deepEqual(options, { + redirectUri: "http://localhost", + disableAutomaticAuthentication: true, }); + let signedIn = false; return { async authenticate() { authenticateCalls++; - cache.token = accessToken("persisted-token"); + signedIn = true; return authenticationRecord(); }, async getToken() { - if (cache.token && options.authenticationRecord) return cache.token; + if (signedIn) return accessToken("memory-token"); const error = new Error("No cached account found."); error.name = "AuthenticationRequiredError"; throw error; @@ -149,41 +124,36 @@ test("a new broker restores the persisted credential without reopening the brows const signedInBroker = new InteractiveAuthBroker({ createCredential, createSessionId: () => "persist-session", - saveAuthRecord: async (record) => { cache.record = record; }, now: () => 1_000, }); const started = signedInBroker.startSignIn(); await signedInBroker.sessions.get(started.sessionId).promise; assert.equal(authenticateCalls, 1); + assert.equal(await signedInBroker.getToken(), "memory-token"); const restartedBroker = new InteractiveAuthBroker({ createCredential, - loadAuthRecord: async () => cache.record, now: () => 1_000, }); - assert.equal(await restartedBroker.getToken(), "persisted-token"); + await assert.rejects(restartedBroker.getToken(), ConnectorAuthenticationRequiredError); assert.equal(authenticateCalls, 1); + assert.equal(createCredentialCalls, 2); }); -test("concurrent first-time token requests share credential initialization and acquisition", async () => { - let releaseAuthenticationRecord; - const authenticationRecordReady = new Promise((resolve) => { - releaseAuthenticationRecord = resolve; +test("concurrent first-time token requests share credential acquisition", async () => { + let releaseToken; + const tokenReady = new Promise((resolve) => { + releaseToken = resolve; }); - let loadAuthRecordCalls = 0; let createCredentialCalls = 0; let tokenCalls = 0; const broker = new InteractiveAuthBroker({ - async loadAuthRecord() { - loadAuthRecordCalls++; - await authenticationRecordReady; - return authenticationRecord(); - }, createCredential: () => { createCredentialCalls++; return { async getToken() { tokenCalls++; + await tokenReady; return accessToken("shared-token"); }, }; @@ -193,15 +163,12 @@ test("concurrent first-time token requests share credential initialization and a const firstRequest = broker.getToken(); const secondRequest = broker.getToken(); await new Promise((resolve) => setImmediate(resolve)); - const loadsBeforeRelease = loadAuthRecordCalls; - releaseAuthenticationRecord(); + releaseToken(); assert.deepEqual( await Promise.all([firstRequest, secondRequest]), ["shared-token", "shared-token"], ); - assert.equal(loadsBeforeRelease, 1); - assert.equal(loadAuthRecordCalls, 1); assert.equal(createCredentialCalls, 1); assert.equal(tokenCalls, 1); }); @@ -224,7 +191,6 @@ test("cancelling sign-in aborts the credential request", async () => { const broker = new InteractiveAuthBroker({ createCredential: () => credential, createSessionId: () => "cancel-session", - loadAuthRecord: async () => undefined, now: () => 1_000, }); @@ -289,7 +255,6 @@ test("legacy credential cleanup retries after a transient failure", async () => return accessToken(); }, }), - loadAuthRecord: async () => authenticationRecord(), }); await assert.rejects(broker.getToken(), /legacy cache is locked/); @@ -312,7 +277,6 @@ test("token acquisition preserves operational errors and retries the credential" }, }; }, - loadAuthRecord: async () => authenticationRecord(), }); await assert.rejects(broker.getToken(), (error) => error === outage); @@ -328,7 +292,6 @@ test("incomplete tokens remain operational errors instead of prompting sign-in", return { token: "incomplete" }; }, }), - loadAuthRecord: async () => authenticationRecord(), }); await assert.rejects( diff --git a/extensions/connector-namespaces/package.json b/extensions/connector-namespaces/package.json index 24e6f05b4..26f54bbcb 100644 --- a/extensions/connector-namespaces/package.json +++ b/extensions/connector-namespaces/package.json @@ -15,7 +15,6 @@ "license": "MIT", "dependencies": { "@azure/identity": "4.13.1", - "@azure/identity-cache-persistence": "1.3.0", "@github/copilot-sdk": "1.0.6" } } diff --git a/extensions/connector-namespaces/renderer.mjs b/extensions/connector-namespaces/renderer.mjs index a6cdf4136..b2a74c6a1 100644 --- a/extensions/connector-namespaces/renderer.mjs +++ b/extensions/connector-namespaces/renderer.mjs @@ -304,14 +304,23 @@ button:focus-visible, a:focus-visible, [tabindex]:focus-visible { outline: 2px s // Setup / Namespace Picker // --------------------------------------------------------------------------- -export function renderSetupHtml(subscriptions = [], notice = "", capabilityToken = "") { +export function renderSetupHtml(subscriptions = [], notice = "", capabilityToken = "", { linkedNamespace = "" } = {}) { + const hasLinkedNamespace = typeof linkedNamespace === "string" && linkedNamespace.length > 0; + const pageTitle = hasLinkedNamespace ? "Sign in to MCP Connectors" : "Select Connector Namespace"; + const heading = hasLinkedNamespace ? "Sign in to see your connectors" : "Select a Connector Namespace"; + const subheading = hasLinkedNamespace + ? `Connector namespace ${esc(linkedNamespace)} is already linked.` + : "Choose which connector namespace to browse. This choice is saved for future sessions."; + const defaultSigninMessage = hasLinkedNamespace + ? "Sign in to Azure to view and manage its connectors." + : "Sign in to Azure to load your subscriptions and connector namespaces."; const subOptions = subscriptions.map((s) => `` ).join(""); return ` -Select Connector Namespace${baseStyles()} +${pageTitle}${baseStyles()}
-

${brandMark(30, "setup")}Select a Connector Namespace

-
Choose which connector namespace to browse. This choice is saved for future sessions.
+

${brandMark(30, "setup")}${heading}

+
${subheading}
${notice ? `
${esc(notice)}
` : ""} -
+
@@ -386,6 +395,8 @@ ${notice ? `
${esc(notice)}
` : ""}
`; } diff --git a/extensions/connector-namespaces/renderer.test.mjs b/extensions/connector-namespaces/renderer.test.mjs index 3a3899cb9..fbb57e524 100644 --- a/extensions/connector-namespaces/renderer.test.mjs +++ b/extensions/connector-namespaces/renderer.test.mjs @@ -67,6 +67,27 @@ test("setup prompts, polls, cancels, and reloads subscriptions after browser sig assert.match(html, /\/api\/subscriptions/); }); +test("a linked namespace gets a focused sign-in state and returns to its catalog", () => { + const html = renderSetupHtml([], "", "token", { linkedNamespace: "saved-gateway" }); + assert.match(html, /Sign in to see your connectors/); + assert.match(html, /Connector namespace saved-gateway<\/code> is already linked\./); + assert.match(html, /Sign in to Azure to view and manage its connectors\./); + assert.match(html, /const hasLinkedNamespace = true/); + assert.match(html, /window\.location\.replace\("\/"\)/); + assert.match(html, /