From b8b5c0123ea560bc06c08f52d61cdcf6749161c1 Mon Sep 17 00:00:00 2001 From: Greg Joseph Date: Tue, 14 Jul 2026 10:54:30 -0700 Subject: [PATCH 1/3] Fix misleading AADSTS9002326 sign-in guidance and surface silent SPA-repair failures The scaffolded React SPA could fail sign-in with AADSTS9002326 even when the SPA redirect URI was already registered, and the app's own error copy asserted this was definitively a server-side missing-redirect-URI problem - misdirecting users away from the most common cause: the running build signs in as a different app/tenant than the one edited in the portal. - create-app: capture the reused-app addSpaRedirectUris({bestEffort}) result and emit a non-blocking advisory (new spa-redirect-advisory.ts) naming the client id, object id, and manual az rest PATCH/GET when the local SPA redirect URI cannot be confirmed. Previously the failure was silently swallowed. - hydrate-config: append an offline "verify this .env targets the right app" advisory to tool output (never to .env); warn when tenant id is blank. - auth-error-guidance (+ App.tsx twin + run-local note): reframe as "almost always a config mismatch", lead remediation with confirming VITE_CLIENT_ID / VITE_TENANT_ID match the app in the portal Overview and rebuilding after .env changes; keep SPA-redirect add steps and az rest GET verification. Fixes #57 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- samples/react-spa-functions/src/App.tsx | 53 +++++++------ src/auth-error-guidance.test.ts | 18 ++--- src/auth-error-guidance.ts | 59 ++++++++------ src/spa-redirect-advisory.test.ts | 77 ++++++++++++++++++ src/spa-redirect-advisory.ts | 101 ++++++++++++++++++++++++ src/tools/create-app.test.ts | 10 ++- src/tools/create-app.ts | 23 +++++- src/tools/hydrate-config.test.ts | 35 ++++++++ src/tools/hydrate-config.ts | 70 +++++++++++++++- src/tools/run-local.ts | 11 ++- 10 files changed, 391 insertions(+), 66 deletions(-) create mode 100644 src/spa-redirect-advisory.test.ts create mode 100644 src/spa-redirect-advisory.ts diff --git a/samples/react-spa-functions/src/App.tsx b/samples/react-spa-functions/src/App.tsx index 32d6016..c208e21 100644 --- a/samples/react-spa-functions/src/App.tsx +++ b/samples/react-spa-functions/src/App.tsx @@ -33,10 +33,11 @@ const pca = new PublicClientApplication({ }); // Turn a Microsoft Entra (AAD) sign-in error into clear, actionable guidance for -// the SERVER-SIDE app-registration / redirect-URI failures (AADSTS9002326 -// cross-origin SPA token redemption, AADSTS50011 redirect URI mismatch) that -// otherwise surface as an opaque 400. This is a byte-for-byte copy of the -// canonical, unit-tested helper in the SPE Builder MCP +// the app-registration / redirect-URI failures (AADSTS9002326 cross-origin SPA +// token redemption, AADSTS50011 redirect URI mismatch) that otherwise surface as +// an opaque 400 — almost always a config mismatch on the owning Entra app (often +// a stale .env pointing at the wrong app), not a client bug. This is a +// byte-for-byte copy of the canonical, unit-tested helper in the SPE Builder MCP // (src/auth-error-guidance.ts); auth-error-guidance.test.ts asserts this sample // stays in sync. Returns null for unrelated errors. function interpretAuthError(errorText: string, origin: string): string | null { @@ -52,33 +53,39 @@ function interpretAuthError(errorText: string, origin: string): string | null { const azBody = '"{\\"spa\\":{\\"redirectUris\\":[\\"' + origin + '\\"]}}"'; return [ - "Sign-in failed because of a SERVER-SIDE Microsoft Entra app-registration issue.", - "", - "This is NOT a bug in this app and NOT a stale or not-reloaded dev server: the", - "owning Entra app registration is missing a Single-Page Application (SPA) redirect", - "URI for this app's origin, so re-running the same client build keeps failing.", + "Sign-in failed: Entra rejected this build's sign-in for its current origin.", "", cause, "", - "Fix: add this app's origin as a Single-page application (SPA) redirect URI on the", - "owning Entra app registration:", + "This is almost always a configuration mismatch on the owning Entra app-registration,", + "not a bug in this app's code. Check these two things, in order:", + "", + "1) Is this build signing in as the app you think it is? This SPA authenticates as the", + " VITE_CLIENT_ID / VITE_TENANT_ID baked into its .env at build time. If those were", + " hydrated from stale or mismatched state, you may have registered the redirect URI on", + " a DIFFERENT app than the one signing in. Confirm they match the app you are viewing", + " in the portal (Entra ID > App registrations > your app > Overview: Application", + " (client) ID and Directory (tenant) ID). If you change .env, rebuild — Vite bakes", + " these values in at build time, so a running dev server will not pick up an edit.", + "", + "2) Does THAT app list this exact origin as a Single-page application (SPA) redirect URI?", + " Look the app up by client id (display names are not unique) and check its spa block:", + " az rest --method GET --uri \"https://graph.microsoft.com/v1.0/applications?$filter=appId eq ''&$select=appId,spa\"", + " If spa.redirectUris does not include the origin below, add it:", "", " " + origin, "", - "How to apply it:", - " - Newly provisioned apps: re-run provisioning / deploy — it now adds this SPA", - " redirect URI automatically.", - " - An app created before that fix (or a deployed origin not yet added): add it", - " manually —", - " Portal: Entra ID > App registrations > (this app) > Authentication >", - " Add a platform > Single-page application > Redirect URI:", - " " + origin, - " or with Azure CLI (replace with the app registration object id):", - " az rest --method PATCH --uri \"https://graph.microsoft.com/v1.0/applications/\" --headers \"Content-Type=application/json\" --body " + + " Portal: Entra ID > App registrations > (this app) > Authentication >", + " Add a platform > Single-page application > Redirect URI:", + " " + origin, + " or with Azure CLI (replace with the app registration object id):", + " az rest --method PATCH --uri \"https://graph.microsoft.com/v1.0/applications/\" --headers \"Content-Type=application/json\" --body " + azBody, "", - "Entra app-registration changes are server-side: re-provision / redeploy to apply", - "them. They are NOT picked up by client hot-reload.", + "Newly provisioned apps get this SPA redirect URI automatically, so you can also just", + "re-run provisioning / deploy to (re)apply it. App-registration changes take effect on", + "the next sign-in but are not applied by client hot-reload, and a changed .env needs a", + "rebuild.", ].join("\n"); } diff --git a/src/auth-error-guidance.test.ts b/src/auth-error-guidance.test.ts index b918fca..ff6ecc7 100644 --- a/src/auth-error-guidance.test.ts +++ b/src/auth-error-guidance.test.ts @@ -5,7 +5,7 @@ * Unit tests for the React SPA sign-in guidance. * * Focus: the `interpretAuthError` helper that turns an opaque Microsoft Entra - * sign-in failure into clear, actionable guidance for the SERVER-SIDE + * sign-in failure into clear, actionable guidance for the Entra * app-registration / redirect-URI errors (AADSTS9002326 cross-origin SPA token * redemption, AADSTS50011 redirect URI mismatch), plus a drift guard asserting * the runnable React sample (`samples/react-spa-functions`) embeds the same @@ -32,15 +32,14 @@ describe("interpretAuthError — AADSTS9002326 (cross-origin SPA token redemptio expect(msg).not.toBeNull(); }); - it("identifies it as a server-side Entra app-registration issue", () => { - expect(msg).toContain("SERVER-SIDE"); + it("identifies it as an Entra app-registration configuration issue", () => { expect(msg).toContain("app-registration"); }); - it("makes clear it is not a client bug or a stale dev server", () => { - expect(msg).toContain("NOT a bug in this app"); - expect(msg).toContain("NOT a stale"); - expect(msg).toContain("hot-reload"); + it("leads with confirming the build signs in as the right app (stale .env / rebuild)", () => { + expect(msg).toContain("VITE_CLIENT_ID"); + expect(msg).toContain("VITE_TENANT_ID"); + expect(msg).toContain("rebuild"); }); it("states the concrete fix: register the origin as a SPA redirect URI", () => { @@ -76,8 +75,7 @@ describe("interpretAuthError — AADSTS50011 (redirect URI mismatch)", () => { expect(msg).not.toBeNull(); }); - it("identifies it as a server-side app-registration / redirect-URI issue", () => { - expect(msg).toContain("SERVER-SIDE"); + it("identifies it as an app-registration / redirect-URI issue", () => { expect(msg).toContain("app-registration"); expect(msg).toContain("redirect URI"); }); @@ -117,7 +115,7 @@ describe("the react-spa-functions sample embeds the interpreter and wires it int it("interprets the same AAD error codes the unit tests cover", () => { expect(app).toContain("AADSTS9002326"); expect(app).toContain("AADSTS50011"); - expect(app).toContain("SERVER-SIDE"); + expect(app).toContain("VITE_CLIENT_ID"); }); it("uses window.location.origin so the message shows the running origin", () => { diff --git a/src/auth-error-guidance.ts b/src/auth-error-guidance.ts index ac85c53..b39dd08 100644 --- a/src/auth-error-guidance.ts +++ b/src/auth-error-guidance.ts @@ -2,17 +2,20 @@ // Licensed under the MIT license. /** - * Actionable guidance for the SERVER-SIDE Microsoft Entra app-registration / - * redirect-URI sign-in failures that otherwise surface in the - * scaffolded React SPA as an opaque 400: + * Actionable guidance for the Microsoft Entra app-registration / redirect-URI + * sign-in failures that otherwise surface in the scaffolded React SPA as an + * opaque 400: * - AADSTS9002326 — cross-origin token redemption refused because this app's * origin is not registered as a Single-Page Application (SPA) redirect URI. * - AADSTS50011 — redirect URI mismatch (the current origin is not listed on * the owning Entra app registration). * - * The point is to tell the user this is a SERVER-SIDE Entra app-registration - * issue (not a client bug, not a stale/not-reloaded dev server) and exactly how - * to fix it, showing the precise origin that must be registered. + * These are almost always a configuration mismatch on the owning Entra app + * registration — not a client code bug. The guidance leads with the most common + * real-world cause we have seen: the running build's baked-in VITE_CLIENT_ID / + * VITE_TENANT_ID point at a DIFFERENT app than the one the user added the + * redirect URI to (a stale or mismatched .env), so it first has the user confirm + * the app identity, then shows how to register the exact origin on THAT app. * * This is the canonical, unit-tested copy. A byte-for-byte copy of this function * is embedded in the runnable React sample (`samples/react-spa-functions/src/ @@ -34,32 +37,38 @@ export function interpretAuthError(errorText: string, origin: string): string | const azBody = '"{\\"spa\\":{\\"redirectUris\\":[\\"' + origin + '\\"]}}"'; return [ - "Sign-in failed because of a SERVER-SIDE Microsoft Entra app-registration issue.", - "", - "This is NOT a bug in this app and NOT a stale or not-reloaded dev server: the", - "owning Entra app registration is missing a Single-Page Application (SPA) redirect", - "URI for this app's origin, so re-running the same client build keeps failing.", + "Sign-in failed: Entra rejected this build's sign-in for its current origin.", "", cause, "", - "Fix: add this app's origin as a Single-page application (SPA) redirect URI on the", - "owning Entra app registration:", + "This is almost always a configuration mismatch on the owning Entra app-registration,", + "not a bug in this app's code. Check these two things, in order:", + "", + "1) Is this build signing in as the app you think it is? This SPA authenticates as the", + " VITE_CLIENT_ID / VITE_TENANT_ID baked into its .env at build time. If those were", + " hydrated from stale or mismatched state, you may have registered the redirect URI on", + " a DIFFERENT app than the one signing in. Confirm they match the app you are viewing", + " in the portal (Entra ID > App registrations > your app > Overview: Application", + " (client) ID and Directory (tenant) ID). If you change .env, rebuild — Vite bakes", + " these values in at build time, so a running dev server will not pick up an edit.", + "", + "2) Does THAT app list this exact origin as a Single-page application (SPA) redirect URI?", + " Look the app up by client id (display names are not unique) and check its spa block:", + " az rest --method GET --uri \"https://graph.microsoft.com/v1.0/applications?$filter=appId eq ''&$select=appId,spa\"", + " If spa.redirectUris does not include the origin below, add it:", "", " " + origin, "", - "How to apply it:", - " - Newly provisioned apps: re-run provisioning / deploy — it now adds this SPA", - " redirect URI automatically.", - " - An app created before that fix (or a deployed origin not yet added): add it", - " manually —", - " Portal: Entra ID > App registrations > (this app) > Authentication >", - " Add a platform > Single-page application > Redirect URI:", - " " + origin, - " or with Azure CLI (replace with the app registration object id):", - " az rest --method PATCH --uri \"https://graph.microsoft.com/v1.0/applications/\" --headers \"Content-Type=application/json\" --body " + + " Portal: Entra ID > App registrations > (this app) > Authentication >", + " Add a platform > Single-page application > Redirect URI:", + " " + origin, + " or with Azure CLI (replace with the app registration object id):", + " az rest --method PATCH --uri \"https://graph.microsoft.com/v1.0/applications/\" --headers \"Content-Type=application/json\" --body " + azBody, "", - "Entra app-registration changes are server-side: re-provision / redeploy to apply", - "them. They are NOT picked up by client hot-reload.", + "Newly provisioned apps get this SPA redirect URI automatically, so you can also just", + "re-run provisioning / deploy to (re)apply it. App-registration changes take effect on", + "the next sign-in but are not applied by client hot-reload, and a changed .env needs a", + "rebuild.", ].join("\n"); } diff --git a/src/spa-redirect-advisory.test.ts b/src/spa-redirect-advisory.test.ts new file mode 100644 index 0000000..e4ce07f --- /dev/null +++ b/src/spa-redirect-advisory.test.ts @@ -0,0 +1,77 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +/** + * Unit tests for the reuse-path SPA redirect advisory (spa-redirect-advisory.ts). + * + * The advisory turns a swallowed best-effort `addSpaRedirectUris` failure into a + * visible, copy-pasteable fix so a reused owning app that could not be + * self-repaired does not silently break the scaffolded SPA sign-in with + * AADSTS9002326. + */ + +import { describe, it, expect } from "vitest"; +import { LOCAL_SPA_REDIRECT_URI } from "./constants.js"; +import { + isSpaRedirectConfirmed, + spaRedirectUnconfirmedWarning, +} from "./spa-redirect-advisory.js"; + +describe("isSpaRedirectConfirmed", () => { + it("is NOT confirmed when the helper swallowed a best-effort failure (undefined)", () => { + expect(isSpaRedirectConfirmed(undefined)).toBe(false); + }); + + it("is confirmed when the returned list includes the local SPA redirect URI", () => { + expect( + isSpaRedirectConfirmed({ added: [], redirectUris: [LOCAL_SPA_REDIRECT_URI] }), + ).toBe(true); + }); + + it("is confirmed when the URI is present with a trailing slash / different case", () => { + expect( + isSpaRedirectConfirmed({ + added: [], + redirectUris: ["HTTP://LOCALHOST:5173/"], + }), + ).toBe(true); + }); + + it("is NOT confirmed when the list does not contain the local origin", () => { + expect( + isSpaRedirectConfirmed({ + added: ["https://app.example.net"], + redirectUris: ["https://app.example.net"], + }), + ).toBe(false); + }); +}); + +describe("spaRedirectUnconfirmedWarning", () => { + const warning = spaRedirectUnconfirmedWarning("client-abc", "obj-xyz"); + + it("flags an action is needed and names the failing error code", () => { + expect(warning).toContain("Action needed"); + expect(warning).toContain("AADSTS9002326"); + }); + + it("names the exact app (client id + object id) so the right app is edited", () => { + expect(warning).toContain("client-abc"); + expect(warning).toContain("obj-xyz"); + }); + + it("gives a copy-pasteable az rest PATCH that adds the local SPA redirect URI", () => { + expect(warning).toContain("az rest --method PATCH"); + expect(warning).toContain("applications/obj-xyz"); + expect(warning).toContain(`[\\"${LOCAL_SPA_REDIRECT_URI}\\"]`); + }); + + it("gives an az rest GET to verify the change landed", () => { + expect(warning).toContain("az rest --method GET"); + expect(warning).toContain("$select=appId,spa"); + }); + + it("frames it as a non-blocking heads-up, not a failure", () => { + expect(warning).toContain("heads-up"); + }); +}); diff --git a/src/spa-redirect-advisory.ts b/src/spa-redirect-advisory.ts new file mode 100644 index 0000000..e9e782f --- /dev/null +++ b/src/spa-redirect-advisory.ts @@ -0,0 +1,101 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +/** + * NON-BLOCKING advisory for the owning-app REUSE path when the local + * Single-page application (SPA) redirect URI could not be confirmed on the app. + * + * project_app_create self-repairs a REUSED owning app by adding the local Vite + * dev origin (http://localhost:5173) to its `spa.redirectUris`. That add is + * best-effort: if the signed-in identity lacks `Application.ReadWrite` on the + * registration, `addSpaRedirectUris` logs and swallows the failure (returns + * undefined). Left silent, the scaffolded React SPA then fails sign-in with + * `AADSTS9002326` and the user has no hint why — this was the exact trap behind + * a reported "I added the redirect URI and it still fails" repro. + * + * This helper turns that swallowed failure into a VISIBLE, copy-pasteable fix + * WITHOUT blocking the tool (app reuse still succeeds). It only produces + * human-readable text — it never opens a browser, blocks provisioning, throws, + * or changes control flow. Kept in its own tested module so the copy lives in + * one place (mirrors guest-advisory.ts / onboarding-messages.ts). + */ + +import { LOCAL_SPA_REDIRECT_URI } from "./constants.js"; + +/** + * Case- and trailing-slash-insensitive membership test for a redirect URI in a + * list — matches the dedupe semantics of graph-client `mergeRedirectUris`, so a + * URI already registered as e.g. `http://localhost:5173/` still counts as + * present. + */ +function listHasRedirectUri(redirectUris: string[] | undefined, uri: string): boolean { + if (!redirectUris) return false; + const normalize = (u: string) => u.replace(/\/+$/, "").toLowerCase(); + const target = normalize(uri); + return redirectUris.some((u) => normalize(u) === target); +} + +/** + * Whether the reuse-path SPA self-repair is CONFIRMED for a given + * `addSpaRedirectUris` result. + * + * `undefined` means a best-effort failure was swallowed → NOT confirmed. A + * defined result's `redirectUris` always includes the origin on success, so the + * membership check is a defensive backstop (never a false "confirmed"). + * + * @param result The value returned by `addSpaRedirectUris` (or undefined). + * @returns true only when the local SPA redirect URI is known to be registered. + */ +export function isSpaRedirectConfirmed( + result: { added: string[]; redirectUris: string[] } | undefined, +): boolean { + return !!result && listHasRedirectUri(result.redirectUris, LOCAL_SPA_REDIRECT_URI); +} + +/** + * Build the NON-BLOCKING warning appended to `project_app_create` output when + * the local SPA redirect URI could not be confirmed on a REUSED owning app. + * + * Names the app's client id + object id and gives the exact manual `az rest` + * PATCH (and a GET to verify), so the user can self-repair even without + * re-running the tool. Purely informational — it does NOT block, open a browser, + * or throw; owning-app reuse still succeeds. + * + * @param clientId The reused app's Application (client) ID (public). + * @param objectId The reused app's Entra object ID (for the Graph PATCH/GET). + * @returns A Markdown section to append to the tool's user-visible output. + */ +export function spaRedirectUnconfirmedWarning(clientId: string, objectId: string): string { + const azBody = '"{\\"spa\\":{\\"redirectUris\\":[\\"' + LOCAL_SPA_REDIRECT_URI + '\\"]}}"'; + return ( + "\n\n### ⚠️ Action needed: confirm the local SPA redirect URI\n\n" + + "I could not confirm (or add) the Single-page application (SPA) redirect URI `" + + LOCAL_SPA_REDIRECT_URI + + "` on this reused owning app — most likely the signed-in identity lacks " + + "`Application.ReadWrite` on the registration, so the automatic self-repair was skipped. " + + "Until that origin is registered on **this exact app**, the scaffolded React SPA sign-in " + + "fails with `AADSTS9002326`.\n\n" + + "Fix it on the app with client ID `" + + clientId + + "` (object ID `" + + objectId + + "`):\n\n" + + "```bash\n" + + "az rest --method PATCH \\\n" + + ' --uri "https://graph.microsoft.com/v1.0/applications/' + + objectId + + '" \\\n' + + ' --headers "Content-Type=application/json" \\\n' + + " --body " + + azBody + + "\n```\n\n" + + "Then verify it landed (the origin should appear under `spa.redirectUris`):\n\n" + + "```bash\n" + + "az rest --method GET \\\n" + + ' --uri "https://graph.microsoft.com/v1.0/applications/' + + objectId + + '?$select=appId,spa"\n' + + "```\n\n" + + "> This is a **heads-up**, not a failure — the owning app is otherwise ready to use." + ); +} diff --git a/src/tools/create-app.test.ts b/src/tools/create-app.test.ts index 40c85ac..f24b693 100644 --- a/src/tools/create-app.test.ts +++ b/src/tools/create-app.test.ts @@ -112,9 +112,11 @@ describe("project_app_create — reuse/attach SPA self-repair", () => { expect(r.isError).toBeUndefined(); expect(addSpaRedirectUrisMock).toHaveBeenCalledTimes(1); + // Confirmed on the app → NO action-needed warning is surfaced. + expect(r.content[0].text).not.toContain("Action needed"); }); - it("swallows a best-effort PATCH failure — the tool still succeeds", async () => { + it("surfaces a visible, copy-pasteable warning when the best-effort PATCH failed — the tool still succeeds", async () => { findApplicationByNameMock.mockResolvedValue(EXISTING_APP); // best-effort failure surfaces as undefined from the helper (it logs + swallows). addSpaRedirectUrisMock.mockResolvedValue(undefined); @@ -123,6 +125,12 @@ describe("project_app_create — reuse/attach SPA self-repair", () => { expect(r.isError).toBeUndefined(); expect(r.content[0].text).toContain("Owning App Found"); + // The swallowed failure is now VISIBLE: a non-blocking, copy-pasteable fix + // naming the exact app (object id) and the manual az rest PATCH. + expect(r.content[0].text).toContain("Action needed"); + expect(r.content[0].text).toContain(EXISTING_APP.objectId); + expect(r.content[0].text).toContain("az rest --method PATCH"); + expect(r.content[0].text).toContain("AADSTS9002326"); }); it("does NOT self-repair on the fresh-create path (createApplication already sets spa)", async () => { diff --git a/src/tools/create-app.ts b/src/tools/create-app.ts index 8d6585d..c27cf89 100644 --- a/src/tools/create-app.ts +++ b/src/tools/create-app.ts @@ -37,6 +37,10 @@ import { LOCAL_SPA_REDIRECT_URI } from "../constants.js"; import { setAuthConfig } from "../auth.js"; import { guestSignInAdvisory } from "../guest-advisory.js"; import { adminConsentSection } from "../onboarding-messages.js"; +import { + isSpaRedirectConfirmed, + spaRedirectUnconfirmedWarning, +} from "../spa-redirect-advisory.js"; import { clientSafeMessage } from "../errors.js"; import { elicitChoice, elicitText } from "../elicitation.js"; import { isContextConfirmedThisSession, stampContextConfirmed } from "../session.js"; @@ -184,6 +188,10 @@ export const createAppTool: McpTool = { ? await findApplicationByAppId(persisted.appId as string, getToken) : await findApplicationByName(displayName, getToken); let reused = false; + // NON-BLOCKING warning appended to output when the reuse-path SPA + // self-repair could not be confirmed (best-effort PATCH swallowed). Empty + // on the fresh-create path and when the origin is confirmed on the app. + let spaRedirectWarning = ""; if (app) { reused = true; // Attach/reuse path: adding permissions is best-effort and non-blocking. @@ -197,9 +205,16 @@ export const createAppTool: McpTool = { // present) and best-effort (a missing Application.ReadWrite grant must // not fail app reuse — mirrors addSpePermissions above). Only on the // reuse path: createApplication already sets `spa` at create time. - await addSpaRedirectUris(app.objectId, [LOCAL_SPA_REDIRECT_URI], getToken, { + const spaResult = await addSpaRedirectUris(app.objectId, [LOCAL_SPA_REDIRECT_URI], getToken, { bestEffort: true, }); + // If the best-effort add was swallowed (undefined) the app was NOT + // repaired, so surface a visible, copy-pasteable fix instead of letting + // the SPA fail sign-in later with an unexplained AADSTS9002326. The tool + // still succeeds — this is a heads-up, not a failure. + if (!isSpaRedirectConfirmed(spaResult)) { + spaRedirectWarning = spaRedirectUnconfirmedWarning(app.appId, app.objectId); + } } else { app = await createApplication(displayName, getToken); // Create path: permissions are required, so errors propagate. @@ -260,7 +275,11 @@ export const createAppTool: McpTool = { // NON-BLOCKING heads-up appended for a B2B guest identity (guests often // lack permission to create Entra apps / own container types). Empty for // a member; never blocks the operation. (PR #3 review.) - guestSignInAdvisory(identity.username); + guestSignInAdvisory(identity.username) + + // NON-BLOCKING heads-up appended when the reuse-path SPA self-repair + // could not be confirmed (best-effort PATCH swallowed). Empty on + // create and when the local origin is confirmed on the reused app. + spaRedirectWarning; return { content: [{ type: "text" as const, text: output }] }; } catch (error) { diff --git a/src/tools/hydrate-config.test.ts b/src/tools/hydrate-config.test.ts index 7c5f73f..e3a2ae3 100644 --- a/src/tools/hydrate-config.test.ts +++ b/src/tools/hydrate-config.test.ts @@ -179,3 +179,38 @@ describe("project_hydrate_config formats validation", () => { expect(existsSync(join(dir, "azure.yaml"))).toBe(true); }); }); + +describe("project_hydrate_config sign-in verification advisory", () => { + it("appends a copy-pasteable verify-the-right-app advisory when writing .env", async () => { + const result = await hydrateConfigTool.handler({ targetDir: dir, formats: ["env"] }); + + expect(result.isError).toBeFalsy(); + const text = result.content[0].text; + // Leads the user to confirm .env matches the app they see + verify the SPA URI. + expect(text).toContain("verify this `.env` targets the right app"); + expect(text).toContain("AADSTS9002326"); + expect(text).toContain("VITE_CLIENT_ID"); + expect(text).toContain("az rest --method GET"); + expect(text).toContain("$select=appId,spa"); + // Looks the app up by the emitted client id (display names are not unique). + expect(text).toContain("appId eq 'app-1'"); + }); + + it("warns (non-blocking) when the tenant id is empty — MSAL authority would be invalid", async () => { + stateStore.tenantId = ""; + + const result = await hydrateConfigTool.handler({ targetDir: dir, formats: ["env"] }); + + expect(result.isError).toBeFalsy(); + const text = result.content[0].text; + expect(text).toContain("Config warnings"); + expect(text).toContain("`TENANT_ID` / `VITE_TENANT_ID` is empty"); + }); + + it("does NOT append the .env sign-in advisory when only azure.yaml is written", async () => { + const result = await hydrateConfigTool.handler({ targetDir: dir, formats: ["azureyaml"] }); + + expect(result.isError).toBeFalsy(); + expect(result.content[0].text).not.toContain("verify this `.env` targets the right app"); + }); +}); diff --git a/src/tools/hydrate-config.ts b/src/tools/hydrate-config.ts index 17e6ef1..fb7a178 100644 --- a/src/tools/hydrate-config.ts +++ b/src/tools/hydrate-config.ts @@ -14,6 +14,7 @@ import { existsSync, mkdirSync, readFileSync, writeFileSync } from "node:fs"; import { join, resolve } from "node:path"; import { buildAzureYaml, findArchitecture } from "../reference-architectures.js"; import { readState } from "../state.js"; +import { LOCAL_SPA_REDIRECT_URI } from "../constants.js"; import type { McpTool } from "../types.js"; interface HydrateArgs { @@ -58,6 +59,66 @@ function appSettings(s: ReturnType): string { ) + "\n"; } +/** + * Offline, NON-BLOCKING sign-in warnings for the hydrated config. + * + * This tool is deterministic/offline (and its unit tests do not mock Graph), so + * it cannot read the live app registration back. It only flags what is checkable + * from state: a blank tenant id makes the SPA's MSAL authority + * (`https://login.microsoftonline.com/`) invalid, so sign-in fails + * before any redirect-URI check. Returns human-readable lines; never throws. + */ +function signInConfigWarnings(s: ReturnType): string[] { + const warnings: string[] = []; + if (!s.tenantId || s.tenantId.trim() === "") { + warnings.push( + "`TENANT_ID` / `VITE_TENANT_ID` is empty — MSAL builds its sign-in authority from the " + + "tenant id, so the SPA cannot sign in until a tenant is provisioned (`project_provision`).", + ); + } + return warnings; +} + +/** + * Persistent, copy-pasteable advisory appended to hydrate OUTPUT (never to + * `.env`) telling the user to confirm the emitted `VITE_CLIENT_ID` / + * `VITE_TENANT_ID` actually match the app registration they are looking at, and + * to verify that exact app lists the local SPA redirect URI. + * + * A stale/mismatched `.env` is the most common cause of `AADSTS9002326` + * ("I added the redirect URI and sign-in still fails" — it was added to a + * DIFFERENT app than the one baked into `.env`). Purely informational: it does + * not block, open a browser, or throw. + */ +function signInVerificationAdvisory(s: ReturnType): string { + const clientId = s.appId ?? ""; + const lookup = s.appObjectId + ? "applications/" + s.appObjectId + "?$select=appId,spa" + : "applications?$filter=appId eq '" + clientId + "'&$select=appId,spa"; + return ( + "\n\n> **Before you sign in — verify this `.env` targets the right app.** A stale or " + + "mismatched `VITE_CLIENT_ID` / `VITE_TENANT_ID` is the most common cause of the " + + "`AADSTS9002326` sign-in error: the SPA authenticates as the app baked into `.env`, which " + + "may be a *different* registration than the one you added the redirect URI to.\n>\n" + + "> 1. Confirm these match the app in the portal (Entra ID > App registrations > Overview): " + + "Application (client) ID `" + + clientId + + "`" + + (s.tenantId ? " and Directory (tenant) ID `" + s.tenantId + "`" : "") + + ".\n>\n" + + "> 2. Confirm that exact app lists `" + + LOCAL_SPA_REDIRECT_URI + + "` as a Single-page application (SPA) redirect URI:\n>\n" + + "> ```bash\n" + + '> az rest --method GET --uri "https://graph.microsoft.com/v1.0/' + + lookup + + '"\n' + + "> ```\n>\n" + + "> If it is missing, re-run `project_app_create` / `project_provision` (which self-repairs " + + "it) or add it in the portal. Vite bakes `.env` in at build time, so rebuild after any change." + ); +} + /** * Build the `azure.yaml` (azd) descriptor that matches the architecture the user * actually scaffolded. The previous implementation emitted a constant @@ -237,6 +298,7 @@ export const hydrateConfigTool: McpTool = { } } + const warnings = signInConfigWarnings(state); const output = "## Config Hydrated\n\n" + `Wrote ${written.length} file(s) to \`${dir}\`:\n\n` + @@ -245,7 +307,13 @@ export const hydrateConfigTool: McpTool = { `| TENANT_ID | \`${state.tenantId ?? ""}\` |\n` + `| CLIENT_ID | \`${state.appId}\` |\n` + `| CONTAINER_TYPE_ID | \`${state.containerTypeId}\` |\n` + - `| CONTAINER_ID | \`${state.containerId ?? "(pending)"}\` |\n`; + `| CONTAINER_ID | \`${state.containerId ?? "(pending)"}\` |\n` + + (warnings.length + ? "\n### ⚠️ Config warnings\n\n" + warnings.map((w) => `- ${w}`).join("\n") + "\n" + : "") + + // The sign-in verification advisory is about the SPA's `.env`, so only + // append it when `.env` was actually written. Offline/non-blocking. + (written.includes(".env") ? signInVerificationAdvisory(state) : ""); return { content: [{ type: "text" as const, text: output }] }; } catch (error) { diff --git a/src/tools/run-local.ts b/src/tools/run-local.ts index fdd9bf7..a0b9252 100644 --- a/src/tools/run-local.ts +++ b/src/tools/run-local.ts @@ -283,10 +283,13 @@ export const runLocalTool: McpTool = { `→ ${project.url}\n\n` + "> The dev server is running in the background. Open the URL above.\n\n" + "> Sign-in note: if sign-in fails with `AADSTS9002326` (cross-origin SPA token " + - "redemption) or `AADSTS50011` (redirect URI mismatch), that is a **server-side " + - "Entra app-registration** change — the owning app must list this origin as a " + - "Single-page application (SPA) redirect URI. Re-provision/redeploy (`project_deploy`) " + - "to apply it; app-registration changes are **not** picked up by client hot-reload."; + "redemption) or `AADSTS50011` (redirect URI mismatch), it is almost always an Entra " + + "app-registration config issue. First confirm this build's `.env` (`VITE_CLIENT_ID` / " + + "`VITE_TENANT_ID`) points at the SAME app registration you added the redirect URI to — " + + "a stale `.env` means you may have edited a different app (rebuild after changing `.env`). " + + "Then make sure that app lists this origin as a Single-page application (SPA) redirect " + + "URI. Re-provision/redeploy (`project_deploy`) self-repairs it; app-registration changes " + + "are **not** picked up by client hot-reload."; return { content: [{ type: "text" as const, text: output }] }; } catch (error) { const msg = error instanceof Error ? error.message : "Unknown error"; From b056de8045e4b831790f97a03c9aef431d944a05 Mon Sep 17 00:00:00 2001 From: Greg Joseph Date: Tue, 14 Jul 2026 11:01:40 -0700 Subject: [PATCH 2/3] Add issue-reporting templates, sign-in troubleshooting, and reporting skill Companion to the AADSTS9002326 fixes: make it easy for users to self-diagnose sign-in failures and, when needed, file triage-ready reports. - .github/ISSUE_TEMPLATE: guided Bug report and Sign-in / AADSTS error forms (the latter collects .env vs. portal identity, spa.redirectUris via az rest, and the exact AADSTS code), plus config.yml with support/security/troubleshoot contact links. - .github/skills/report-spe-mcp-issue: an agent skill that walks a user through gathering those diagnostics, redacting secrets, and drafting a gh issue create. - docs/TROUBLESHOOTING.md: new "React SPA sign-in fails with AADSTS9002326" section documenting the config-mismatch root cause and az rest verify/repair. - README, SUPPORT, CHANGELOG: link the templates + skill and record the change. Relates to #57 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/ISSUE_TEMPLATE/bug_report.yml | 93 +++++++++++++ .github/ISSUE_TEMPLATE/config.yml | 11 ++ .github/ISSUE_TEMPLATE/signin_issue.yml | 136 +++++++++++++++++++ .github/skills/report-spe-mcp-issue/SKILL.md | 100 ++++++++++++++ CHANGELOG.md | 27 ++++ README.md | 2 +- SUPPORT.md | 8 ++ docs/TROUBLESHOOTING.md | 58 ++++++++ 8 files changed, 434 insertions(+), 1 deletion(-) create mode 100644 .github/ISSUE_TEMPLATE/bug_report.yml create mode 100644 .github/ISSUE_TEMPLATE/config.yml create mode 100644 .github/ISSUE_TEMPLATE/signin_issue.yml create mode 100644 .github/skills/report-spe-mcp-issue/SKILL.md diff --git a/.github/ISSUE_TEMPLATE/bug_report.yml b/.github/ISSUE_TEMPLATE/bug_report.yml new file mode 100644 index 0000000..4248228 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/bug_report.yml @@ -0,0 +1,93 @@ +name: 🐛 Bug report +description: Report a defect in the SPE MCP server or its scaffolded samples. +title: "[Bug]: " +body: + - type: markdown + attributes: + value: | + Thanks for taking the time to file a bug! Please **search + [existing issues](https://github.com/microsoft/SharePoint-Embedded-MCP-Server/issues)** + first to avoid duplicates. + + > 🔑 **Sign-in / `AADSTS…` error?** Use the dedicated + > **"Sign-in / AADSTS error"** template instead — it collects the exact + > diagnostics needed to root-cause auth failures. + + > 🔒 **Never paste secrets** (client secrets, refresh/access tokens, `az` + > output containing credentials). Redact tenant/subscription GUIDs if you + > consider them sensitive. + - type: textarea + id: what-happened + attributes: + label: What happened? + description: A clear description of the bug, including the exact error text or the client-facing correlation ID. + placeholder: | + e.g. `container_create` returned "The tool failed. See server logs for details. (correlationId: a1b2c3d4)" + validations: + required: true + - type: textarea + id: repro + attributes: + label: Steps to reproduce + description: The natural-language prompt(s) and/or tool calls that trigger the problem. + placeholder: | + 1. Prompt the agent: "Create a trial SPE app and scaffold a React sample" + 2. Run `project_run_local` + 3. … + validations: + required: true + - type: textarea + id: expected + attributes: + label: Expected behavior + validations: + required: true + - type: dropdown + id: mcp-client + attributes: + label: MCP client + options: + - VS Code (Copilot) + - Cursor + - Claude Desktop + - Azure AI Foundry + - MCP Inspector + - Other (describe below) + validations: + required: true + - type: input + id: server-version + attributes: + label: Server version + description: "`@microsoft/spe-mcp` version (e.g. 0.2.0-alpha.1), or the commit SHA if built from source." + validations: + required: true + - type: input + id: tool + attributes: + label: Tool(s) involved + description: Which MCP tool name(s), if known (e.g. `project_app_create`, `container_create`). + - type: input + id: env + attributes: + label: OS and Node.js version + placeholder: "e.g. Windows 11 / macOS 14; Node 22.11.0" + validations: + required: true + - type: textarea + id: logs + attributes: + label: Relevant logs + description: Paste the server stderr lines for the correlation ID (`grep spe-mcp.log`). Redact any secrets. + render: text + - type: checkboxes + id: checks + attributes: + label: Pre-flight + options: + - label: I searched existing issues and this is not a duplicate. + required: true + - label: This is **not** a security vulnerability (those go through [SECURITY.md](https://github.com/microsoft/SharePoint-Embedded-MCP-Server/blob/main/SECURITY.md)). + required: true + - label: I removed all secrets/tokens from the details above. + required: true diff --git a/.github/ISSUE_TEMPLATE/config.yml b/.github/ISSUE_TEMPLATE/config.yml new file mode 100644 index 0000000..7ce8f30 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/config.yml @@ -0,0 +1,11 @@ +blank_issues_enabled: true +contact_links: + - name: 📘 Support & documentation + url: https://github.com/microsoft/SharePoint-Embedded-MCP-Server/blob/main/SUPPORT.md + about: How to get help and where to find docs. Please search existing issues first. + - name: 🔑 Sign-in / AADSTS troubleshooting + url: https://github.com/microsoft/SharePoint-Embedded-MCP-Server/blob/main/docs/TROUBLESHOOTING.md#react-spa-sign-in-fails-with-aadsts9002326-redirect-uri-already-registered + about: Fixes for common sign-in errors (including AADSTS9002326) — often resolves the problem without filing. + - name: 🔒 Report a security vulnerability + url: https://github.com/microsoft/SharePoint-Embedded-MCP-Server/blob/main/SECURITY.md + about: Do NOT open a public issue for security problems — follow the private process described here. diff --git a/.github/ISSUE_TEMPLATE/signin_issue.yml b/.github/ISSUE_TEMPLATE/signin_issue.yml new file mode 100644 index 0000000..563607f --- /dev/null +++ b/.github/ISSUE_TEMPLATE/signin_issue.yml @@ -0,0 +1,136 @@ +name: 🔑 Sign-in / AADSTS error +description: Sign-in fails in a scaffolded sample (e.g. AADSTS9002326, AADSTS50011, AADSTS650051). +title: "[Sign-in]: AADSTS" +body: + - type: markdown + attributes: + value: | + This template collects the diagnostics needed to root-cause sign-in + failures. Most `AADSTS9002326` reports turn out to be a **configuration + mismatch** — the running build signs in as a *different* app/tenant than + the one edited in the portal — not a missing redirect URI. See the + [sign-in troubleshooting guide](https://github.com/microsoft/SharePoint-Embedded-MCP-Server/blob/main/docs/TROUBLESHOOTING.md#react-spa-sign-in-fails-with-aadsts9002326-redirect-uri-already-registered) + before filing; it resolves most cases in a couple of minutes. + + > 🔒 **Do not paste tokens or secrets.** The values below (client id, + > tenant id, redirect URIs) are **not** secrets, but if you consider your + > tenant/client GUIDs sensitive you may mask the middle characters. + - type: input + id: aadsts-code + attributes: + label: Exact AADSTS code + placeholder: "e.g. AADSTS9002326" + validations: + required: true + - type: textarea + id: error-text + attributes: + label: Full error text + description: The complete error shown in the app and/or browser console. + render: text + validations: + required: true + - type: input + id: request-url + attributes: + label: Token request URL + description: From browser DevTools → Network. Include the path/tenant segment; you may trim query params. + placeholder: "https://login.microsoftonline.com//oauth2/v2.0/token?..." + - type: markdown + attributes: + value: | + ### The key comparison + + Sign-in uses the `VITE_CLIENT_ID` / `VITE_TENANT_ID` **baked into the running + build** (Vite inlines env vars at build time). Confirm they match the app you + edited in the portal's **Overview** blade. If you changed `.env`, you must + **rebuild** for it to take effect. + - type: input + id: env-client-id + attributes: + label: "`VITE_CLIENT_ID` in the sample's `.env`" + description: From the scaffolded sample's `.env` (or `.env.local`). + validations: + required: true + - type: input + id: env-tenant-id + attributes: + label: "`VITE_TENANT_ID` in the sample's `.env`" + validations: + required: true + - type: input + id: portal-client-id + attributes: + label: Application (client) ID shown in the portal Overview + description: Entra portal → App registrations → your app → Overview. Should equal `VITE_CLIENT_ID`. + validations: + required: true + - type: input + id: portal-tenant-id + attributes: + label: Directory (tenant) ID shown in the portal Overview + description: Should equal `VITE_TENANT_ID`. + validations: + required: true + - type: dropdown + id: rebuilt + attributes: + label: Did you rebuild the sample after the most recent `.env` change? + options: + - "Yes — rebuilt after editing .env" + - "No / not sure" + - "Did not edit .env" + validations: + required: true + - type: textarea + id: spa-redirects + attributes: + label: The signing-in app's SPA redirect URIs + description: | + Run this against the **client id from `.env`** and paste the output: + ```bash + az rest --method GET \ + --url "https://graph.microsoft.com/v1.0/applications?\$filter=appId eq ''&\$select=appId,spa" \ + -o json + ``` + render: json + validations: + required: true + - type: dropdown + id: guest + attributes: + label: Is the signed-in user a guest/external (B2B) user in this tenant? + options: + - "No — member of the tenant" + - "Yes — guest/external user" + - "Not sure" + - type: dropdown + id: mcp-client + attributes: + label: MCP client used to scaffold/run + options: + - VS Code (Copilot) + - Cursor + - Claude Desktop + - Azure AI Foundry + - Other + validations: + required: true + - type: input + id: server-version + attributes: + label: Server version + description: "`@microsoft/spe-mcp` version or commit SHA." + validations: + required: true + - type: checkboxes + id: checks + attributes: + label: Pre-flight + options: + - label: I confirmed `VITE_CLIENT_ID` / `VITE_TENANT_ID` against the portal Overview and rebuilt after any `.env` change. + required: true + - label: I searched existing issues and this is not a duplicate. + required: true + - label: I did **not** paste any tokens or secrets. + required: true diff --git a/.github/skills/report-spe-mcp-issue/SKILL.md b/.github/skills/report-spe-mcp-issue/SKILL.md new file mode 100644 index 0000000..b7f93c7 --- /dev/null +++ b/.github/skills/report-spe-mcp-issue/SKILL.md @@ -0,0 +1,100 @@ +--- +name: Report an SPE MCP issue +description: >- + Help a user file a high-quality bug report for the SharePoint Embedded MCP + server (microsoft/SharePoint-Embedded-MCP-Server). Use when someone hits an + error with the server or a scaffolded sample — especially sign-in / Entra + "AADSTS…" failures (e.g. AADSTS9002326) — and wants to report it, or asks + "how do I file an issue / bug". Gathers the exact diagnostics the repo's issue + templates expect, redacts secrets, and drafts a ready-to-submit GitHub issue. +--- + +# Report an SPE MCP issue + +Turn a vague "it's broken" into a triage-ready issue for +`microsoft/SharePoint-Embedded-MCP-Server`, using the repository's issue-form +templates. Two issue classes are supported: **sign-in / AADSTS errors** and +**general bugs**. + +## 0. Try to resolve it first (sign-in errors) + +Many sign-in reports are self-fixable. Before drafting an issue for an +`AADSTS…` error, walk the user through +[`docs/TROUBLESHOOTING.md` → "React SPA sign-in fails with AADSTS9002326"](../../../docs/TROUBLESHOOTING.md#react-spa-sign-in-fails-with-aadsts9002326-redirect-uri-already-registered). +The single most common cause is a **config mismatch**: the running build signs in +with the `VITE_CLIENT_ID` / `VITE_TENANT_ID` baked into its `.env` at build time, +which points at a *different* app/tenant than the one the user edited in the +portal. Confirm those two values match the app's **Overview** blade and that the +sample was **rebuilt** after any `.env` change. If that fixes it, no issue is +needed. + +## 1. Classify the issue + +- Error text contains `AADSTS`, "sign in", "redirect URI", "token", or the + failing token request is to `login.microsoftonline.com` → **sign-in issue** + (template `signin_issue.yml`). +- Anything else (a tool call failed, scaffold/build/deploy problem, unexpected + output) → **general bug** (template `bug_report.yml`). + +## 2. Gather diagnostics + +Ask for only what's missing; don't re-request what the user already provided. + +**Sign-in issue — collect:** + +| Field | How to get it | +|-------|---------------| +| Exact `AADSTS` code + full error text | From the app UI / browser console | +| Token request URL | Browser DevTools → Network → the failing `…/oauth2/v2.0/token` call | +| `VITE_CLIENT_ID`, `VITE_TENANT_ID` | The scaffolded sample's `.env` (or `.env.local`) | +| Portal **client id** + **tenant id** | Entra portal → App registrations → the app → **Overview** | +| Rebuilt after `.env` change? | Ask directly (Vite inlines env at build time) | +| SPA redirect URIs on the `.env` app | `az rest --method GET --url "https://graph.microsoft.com/v1.0/applications?\$filter=appId eq ''&\$select=appId,spa" -o json` | +| Guest/member user? | Ask whether they signed in as a guest/external (B2B) user | +| MCP client + server version | e.g. VS Code Copilot; `@microsoft/spe-mcp` version or commit | + +**General bug — collect:** what happened (incl. any `correlationId`), steps to +reproduce (the prompt/tool calls), expected behavior, MCP client, server version, +OS + Node version, the tool name(s) involved, and server stderr lines for the +correlation id (`grep spe-mcp.log`). + +## 3. Redact secrets — always + +Before drafting anything, strip: client secrets, refresh/access tokens, `az` +output containing credentials, cookies, and authorization headers. Client IDs, +tenant IDs, and redirect URIs are **not** secrets and are needed for triage +(offer to mask the middle of GUIDs if the user considers them sensitive). + +## 4. Draft and submit + +Prefer opening the prefilled web form so the template's required checkboxes are +honored: + +```bash +gh issue create --repo microsoft/SharePoint-Embedded-MCP-Server \ + --web --template signin_issue.yml # or bug_report.yml +``` + +For a fully non-interactive draft, write the body to a file and run: + +```bash +gh issue create --repo microsoft/SharePoint-Embedded-MCP-Server \ + --title "[Sign-in]: AADSTS9002326 — " \ + --body-file issue.md +``` + +Structure the body to mirror the chosen template's sections (Summary, the +`.env` vs. portal comparison for sign-in, repro steps, versions). Show the user +the drafted title + body and get confirmation before creating. If `gh` isn't +available or auth is restricted, output the same content and point the user to +[**New issue**](https://github.com/microsoft/SharePoint-Embedded-MCP-Server/issues/new/choose). + +## 5. Search for duplicates first + +```bash +gh issue list --repo microsoft/SharePoint-Embedded-MCP-Server \ + --search "AADSTS9002326 in:title,body" --state all +``` + +If a matching issue exists, add the new diagnostics as a comment instead of +opening a duplicate. diff --git a/CHANGELOG.md b/CHANGELOG.md index ec93124..91fc68e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,33 @@ project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). tenants, or a published build alongside a local build) without clobbering shared state. Applies uniformly to `start`, `auth`, and `logout`. The default path is unchanged and byte-identical to prior releases. +- **Guided issue reporting.** GitHub issue-form templates (a general **Bug + report** and a dedicated **Sign-in / AADSTS error** form that collects `.env` + vs. portal identity, `spa.redirectUris`, and the exact AADSTS code) plus a + **Report an SPE MCP issue** agent skill (`.github/skills/report-spe-mcp-issue`) + that gathers those diagnostics and drafts the issue. New + "React SPA sign-in fails with `AADSTS9002326`" troubleshooting section. + +### Fixed + +- **Misleading `AADSTS9002326` sign-in guidance.** The scaffolded React SPA + could fail sign-in with `AADSTS9002326` even when the SPA redirect URI was + already registered, and the app's own error copy asserted this was definitively + a server-side missing-redirect-URI problem. The guidance + (`auth-error-guidance.ts` and its `App.tsx` twin, plus the `project_run_local` + note) is reframed as "almost always a configuration mismatch" and now leads + with confirming the running build's `VITE_CLIENT_ID` / `VITE_TENANT_ID` match + the app in the portal Overview (and rebuilding after `.env` changes) before the + redirect-URI add steps. +- **Silent SPA self-repair failure surfaced.** When `project_app_create` reuses + an existing owning app, a best-effort `addSpaRedirectUris` failure (e.g. + missing `Application.ReadWrite`) was swallowed, leaving the app unrepaired with + no signal. The tool now emits a non-blocking advisory naming the client id / + object id and the exact `az rest` PATCH/GET to self-repair. +- **`project_hydrate_config` verification advisory.** The tool now appends an + offline "verify this `.env` targets the right app" advisory to its output + (never to `.env`) and warns when the tenant id is blank (invalid MSAL + authority), so a stale/mismatched app identity is caught before sign-in. ### Security diff --git a/README.md b/README.md index eb3bc87..3bb5210 100644 --- a/README.md +++ b/README.md @@ -10,7 +10,7 @@ A [Model Context Protocol (MCP)](https://modelcontextprotocol.io/) server for Sh - **Get started on Microsoft Learn:** [SharePoint Embedded MCP server](https://learn.microsoft.com/sharepoint/dev/embedded/getting-started/spe-mcp-server) - **SharePoint Embedded product docs:** -- **In this repo:** [Available Tools](#available-tools) · [Configuration](#configuration) · [Security controls](docs/SECURITY-CONTROLS.md) · [Troubleshooting](docs/TROUBLESHOOTING.md) +- **In this repo:** [Available Tools](#available-tools) · [Configuration](#configuration) · [Security controls](docs/SECURITY-CONTROLS.md) · [Troubleshooting](docs/TROUBLESHOOTING.md) · [Report a bug](https://github.com/microsoft/SharePoint-Embedded-MCP-Server/issues/new/choose) ## Available Tools diff --git a/SUPPORT.md b/SUPPORT.md index b06dfa1..5279a32 100644 --- a/SUPPORT.md +++ b/SUPPORT.md @@ -7,6 +7,14 @@ existing [issues](https://github.com/microsoft/SharePoint-Embedded-MCP-Server/is before filing new issues to avoid duplicates. For new issues, file your bug or feature request as a new Issue. +When you open a [new issue](https://github.com/microsoft/SharePoint-Embedded-MCP-Server/issues/new/choose) +you can pick a guided form — including a **Sign-in / AADSTS error** template that collects +the exact diagnostics needed to root-cause auth failures (see +[Troubleshooting](docs/TROUBLESHOOTING.md#react-spa-sign-in-fails-with-aadsts9002326-redirect-uri-already-registered) +first — it resolves most sign-in reports). If you use an AI agent with this MCP server, the +[**Report an SPE MCP issue** skill](.github/skills/report-spe-mcp-issue/SKILL.md) can gather +those diagnostics and draft the issue for you. + For help and questions about using this project, file a GitHub Issue in this repository. Please do **not** report security vulnerabilities through public GitHub issues — follow the process described in [SECURITY.md](SECURITY.md) instead. diff --git a/docs/TROUBLESHOOTING.md b/docs/TROUBLESHOOTING.md index 9a01d83..a841332 100644 --- a/docs/TROUBLESHOOTING.md +++ b/docs/TROUBLESHOOTING.md @@ -22,6 +22,64 @@ az login --scope https://management.core.windows.net//.default --tenant '&\$select=appId,spa" \ + -o json + ``` + + If `spa.redirectUris` does not contain `http://localhost:5173`, add it (note + the `spa` platform — a Web or public-client redirect will **not** satisfy the + SPA code-redemption check): + + ```bash + az rest --method PATCH \ + --url "https://graph.microsoft.com/v1.0/applications/" \ + --headers "Content-Type=application/json" \ + --body '{"spa":{"redirectUris":["http://localhost:5173"]}}' + ``` + +3. **Reused-app self-repair may have been skipped.** `project_app_create` + self-repairs a *reused* owning app by adding the SPA redirect URI, but that + step needs `Application.ReadWrite` on the signed-in identity. If it could not + be confirmed, the tool now prints a **non-blocking advisory** naming the exact + client id / object id and the `az rest` commands above — run them (or re-run + provisioning with sufficient permissions). + +4. **Guest / B2B users.** If you signed in as a guest/external user, redemption + can be refused cross-tenant. Sign in with a member account of the tenant that + owns the app, or provision in that tenant. + +If none of the above resolves it, open a +[Sign-in / AADSTS error issue](https://github.com/microsoft/SharePoint-Embedded-MCP-Server/issues/new?template=signin_issue.yml) +with the diagnostics that template collects. + ## Container-type registration delays After `container_type_register` or `project_provision`, Graph and SharePoint registration state can take time to propagate. If `container_create`, `container_type_get`, or app access fails immediately after registration, retry after a short delay. From 5fec1f4ee7734d143d8e4a4f71f4c9fa85808b7e Mon Sep 17 00:00:00 2001 From: Greg Joseph Date: Tue, 14 Jul 2026 11:34:27 -0700 Subject: [PATCH 3/3] Make issue reporting generic; sign-in/AADSTS is one category, not the headline The issue-reporting skill and templates led with sign-in / AADSTS as the primary focus. Reframe the whole apparatus as general SPE MCP issue reporting, with sign-in/AADSTS demoted to one specialized category (its dedicated form and diagnostics are kept). - SKILL.md: generic-first. "Try to resolve first" and "Classify" now span MCP tool calls, scaffolding/build, deploy, install/packaging, and docs, with sign-in/AADSTS as a special case. Diagnostics list the general fields first and the sign-in extras second. Examples default to the Bug report form. - config.yml: replace the "Sign-in / AADSTS troubleshooting" contact link with a general "Troubleshooting guide" link (sign-in is one of the topics it covers). - bug_report.yml: broaden the description and add an "Area" dropdown (tool call / scaffolding / build / deploy / install / docs / other) so it reads as the catch-all. - SUPPORT.md / CHANGELOG.md: describe the general Bug report form first and the sign-in form second. Relates to #57. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/ISSUE_TEMPLATE/bug_report.yml | 17 +++- .github/ISSUE_TEMPLATE/config.yml | 6 +- .github/skills/report-spe-mcp-issue/SKILL.md | 99 ++++++++++++-------- CHANGELOG.md | 13 +-- SUPPORT.md | 13 +-- 5 files changed, 92 insertions(+), 56 deletions(-) diff --git a/.github/ISSUE_TEMPLATE/bug_report.yml b/.github/ISSUE_TEMPLATE/bug_report.yml index 4248228..e19f60d 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.yml +++ b/.github/ISSUE_TEMPLATE/bug_report.yml @@ -1,5 +1,5 @@ name: 🐛 Bug report -description: Report a defect in the SPE MCP server or its scaffolded samples. +description: Report a defect in the SPE MCP server or a scaffolded sample — tool failures, scaffolding/build/deploy, install/packaging, or unexpected output. title: "[Bug]: " body: - type: markdown @@ -16,6 +16,21 @@ body: > 🔒 **Never paste secrets** (client secrets, refresh/access tokens, `az` > output containing credentials). Redact tenant/subscription GUIDs if you > consider them sensitive. + - type: dropdown + id: area + attributes: + label: Area + description: Which part of the workflow is affected? + options: + - MCP tool call (server) + - Scaffolding / generated sample + - Local run / build (npm run dev, Vite) + - Deployment (Static Web Apps / Azure) + - Install / packaging (npx, npm i -g) + - Documentation + - Other + validations: + required: true - type: textarea id: what-happened attributes: diff --git a/.github/ISSUE_TEMPLATE/config.yml b/.github/ISSUE_TEMPLATE/config.yml index 7ce8f30..7047e6e 100644 --- a/.github/ISSUE_TEMPLATE/config.yml +++ b/.github/ISSUE_TEMPLATE/config.yml @@ -3,9 +3,9 @@ contact_links: - name: 📘 Support & documentation url: https://github.com/microsoft/SharePoint-Embedded-MCP-Server/blob/main/SUPPORT.md about: How to get help and where to find docs. Please search existing issues first. - - name: 🔑 Sign-in / AADSTS troubleshooting - url: https://github.com/microsoft/SharePoint-Embedded-MCP-Server/blob/main/docs/TROUBLESHOOTING.md#react-spa-sign-in-fails-with-aadsts9002326-redirect-uri-already-registered - about: Fixes for common sign-in errors (including AADSTS9002326) — often resolves the problem without filing. + - name: 📖 Troubleshooting guide + url: https://github.com/microsoft/SharePoint-Embedded-MCP-Server/blob/main/docs/TROUBLESHOOTING.md + about: Fixes for common problems — container-type registration delays, deploy/config, and sign-in (AADSTS) errors. Often resolves the issue without filing. - name: 🔒 Report a security vulnerability url: https://github.com/microsoft/SharePoint-Embedded-MCP-Server/blob/main/SECURITY.md about: Do NOT open a public issue for security problems — follow the private process described here. diff --git a/.github/skills/report-spe-mcp-issue/SKILL.md b/.github/skills/report-spe-mcp-issue/SKILL.md index b7f93c7..13b81b1 100644 --- a/.github/skills/report-spe-mcp-issue/SKILL.md +++ b/.github/skills/report-spe-mcp-issue/SKILL.md @@ -1,46 +1,69 @@ --- name: Report an SPE MCP issue description: >- - Help a user file a high-quality bug report for the SharePoint Embedded MCP - server (microsoft/SharePoint-Embedded-MCP-Server). Use when someone hits an - error with the server or a scaffolded sample — especially sign-in / Entra - "AADSTS…" failures (e.g. AADSTS9002326) — and wants to report it, or asks - "how do I file an issue / bug". Gathers the exact diagnostics the repo's issue - templates expect, redacts secrets, and drafts a ready-to-submit GitHub issue. + Help a user file a high-quality, triage-ready issue for the SharePoint + Embedded MCP server (microsoft/SharePoint-Embedded-MCP-Server). Use whenever + someone hits a problem with the server or a scaffolded sample — a failing MCP + tool call, a scaffolding / build / deploy error, an install or packaging + problem, wrong or confusing output, a docs gap, or a sign-in / Entra "AADSTS…" + failure — and wants to report it, or asks "how do I file an issue / bug". + Picks the right issue-form template, gathers the diagnostics it expects, + redacts secrets, and drafts a ready-to-submit GitHub issue. --- # Report an SPE MCP issue Turn a vague "it's broken" into a triage-ready issue for `microsoft/SharePoint-Embedded-MCP-Server`, using the repository's issue-form -templates. Two issue classes are supported: **sign-in / AADSTS errors** and -**general bugs**. +templates. This works for **any** kind of problem — a failing MCP tool call, a +scaffolding / build / deploy error, an install or packaging issue, unexpected +output, or a docs gap. Most reports use the general **Bug report** form; +**sign-in / `AADSTS…` errors** have a dedicated form because they need a few +extra auth-specific diagnostics. + +## 0. Try to resolve it first + +Many problems are self-fixable. Before drafting an issue, check +[`docs/TROUBLESHOOTING.md`](../../../docs/TROUBLESHOOTING.md) and the relevant +`README` / `docs` section for whatever is failing — container-type registration +delays, deploy/config, and sign-in all have known fixes there. + +The most common self-fix is for **sign-in / `AADSTS…`** errors, which are almost +always a **config mismatch**: the running build signs in with the +`VITE_CLIENT_ID` / `VITE_TENANT_ID` baked into its `.env` at build time, which +points at a *different* app/tenant than the one the user edited in the portal. +Confirm those two values match the app's **Overview** blade and that the sample +was **rebuilt** after any `.env` change (see +["React SPA sign-in fails with AADSTS9002326"](../../../docs/TROUBLESHOOTING.md#react-spa-sign-in-fails-with-aadsts9002326-redirect-uri-already-registered)). +If a known fix resolves it, no issue is needed. -## 0. Try to resolve it first (sign-in errors) +## 1. Classify the issue -Many sign-in reports are self-fixable. Before drafting an issue for an -`AADSTS…` error, walk the user through -[`docs/TROUBLESHOOTING.md` → "React SPA sign-in fails with AADSTS9002326"](../../../docs/TROUBLESHOOTING.md#react-spa-sign-in-fails-with-aadsts9002326-redirect-uri-already-registered). -The single most common cause is a **config mismatch**: the running build signs in -with the `VITE_CLIENT_ID` / `VITE_TENANT_ID` baked into its `.env` at build time, -which points at a *different* app/tenant than the one the user edited in the -portal. Confirm those two values match the app's **Overview** blade and that the -sample was **rebuilt** after any `.env` change. If that fixes it, no issue is -needed. +Pick the template that fits. Everything except sign-in uses the general **Bug +report** form (`bug_report.yml`): -## 1. Classify the issue +- A failing **MCP tool call** (error text or a client-facing `correlationId`). +- A **scaffolding / build / local-run** problem in a generated sample. +- A **deployment** problem (e.g. Static Web Apps / Azure). +- An **install / packaging** problem (`npx` / `npm i -g @microsoft/spe-mcp`). +- **Unexpected or confusing output**, or a **documentation** gap. -- Error text contains `AADSTS`, "sign in", "redirect URI", "token", or the - failing token request is to `login.microsoftonline.com` → **sign-in issue** - (template `signin_issue.yml`). -- Anything else (a tool call failed, scaffold/build/deploy problem, unexpected - output) → **general bug** (template `bug_report.yml`). +Use the dedicated **Sign-in / AADSTS** form (`signin_issue.yml`) only when the +symptom is an auth failure — the error text contains `AADSTS`, "sign in", +"redirect URI", or "token", or the failing request is to +`login.microsoftonline.com`. ## 2. Gather diagnostics Ask for only what's missing; don't re-request what the user already provided. -**Sign-in issue — collect:** +**For any issue — collect:** what happened (include any client-facing +`correlationId`), steps to reproduce (the prompt / tool calls), expected +behavior, MCP client (e.g. VS Code Copilot), `@microsoft/spe-mcp` version or +commit, OS + Node version, the tool name(s) involved, and the server stderr +lines for the correlation id (`grep spe-mcp.log`). + +**If it's a sign-in / AADSTS issue — also collect:** | Field | How to get it | |-------|---------------| @@ -51,12 +74,6 @@ Ask for only what's missing; don't re-request what the user already provided. | Rebuilt after `.env` change? | Ask directly (Vite inlines env at build time) | | SPA redirect URIs on the `.env` app | `az rest --method GET --url "https://graph.microsoft.com/v1.0/applications?\$filter=appId eq ''&\$select=appId,spa" -o json` | | Guest/member user? | Ask whether they signed in as a guest/external (B2B) user | -| MCP client + server version | e.g. VS Code Copilot; `@microsoft/spe-mcp` version or commit | - -**General bug — collect:** what happened (incl. any `correlationId`), steps to -reproduce (the prompt/tool calls), expected behavior, MCP client, server version, -OS + Node version, the tool name(s) involved, and server stderr lines for the -correlation id (`grep spe-mcp.log`). ## 3. Redact secrets — always @@ -72,29 +89,31 @@ honored: ```bash gh issue create --repo microsoft/SharePoint-Embedded-MCP-Server \ - --web --template signin_issue.yml # or bug_report.yml + --web --template bug_report.yml # or signin_issue.yml for auth errors ``` For a fully non-interactive draft, write the body to a file and run: ```bash gh issue create --repo microsoft/SharePoint-Embedded-MCP-Server \ - --title "[Sign-in]: AADSTS9002326 — " \ + --title "[Bug]: " \ --body-file issue.md ``` -Structure the body to mirror the chosen template's sections (Summary, the -`.env` vs. portal comparison for sign-in, repro steps, versions). Show the user -the drafted title + body and get confirmation before creating. If `gh` isn't -available or auth is restricted, output the same content and point the user to +Structure the body to mirror the chosen template's sections (what happened, +repro steps, expected behavior, versions; for a sign-in issue, add the `.env` +vs. portal comparison). Show the user the drafted title + body and get +confirmation before creating. If `gh` isn't available or auth is restricted, +output the same content and point the user to [**New issue**](https://github.com/microsoft/SharePoint-Embedded-MCP-Server/issues/new/choose). ## 5. Search for duplicates first ```bash gh issue list --repo microsoft/SharePoint-Embedded-MCP-Server \ - --search "AADSTS9002326 in:title,body" --state all + --search " in:title,body" --state all ``` -If a matching issue exists, add the new diagnostics as a comment instead of -opening a duplicate. +Search on a distinctive fragment — the tool name, a `correlationId`, or the +`AADSTS` code for sign-in issues. If a matching issue exists, add the new +diagnostics as a comment instead of opening a duplicate. diff --git a/CHANGELOG.md b/CHANGELOG.md index 91fc68e..9d5576d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,12 +15,13 @@ project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). tenants, or a published build alongside a local build) without clobbering shared state. Applies uniformly to `start`, `auth`, and `logout`. The default path is unchanged and byte-identical to prior releases. -- **Guided issue reporting.** GitHub issue-form templates (a general **Bug - report** and a dedicated **Sign-in / AADSTS error** form that collects `.env` - vs. portal identity, `spa.redirectUris`, and the exact AADSTS code) plus a - **Report an SPE MCP issue** agent skill (`.github/skills/report-spe-mcp-issue`) - that gathers those diagnostics and drafts the issue. New - "React SPA sign-in fails with `AADSTS9002326`" troubleshooting section. +- **Guided issue reporting.** A general **Bug report** GitHub issue form (for + tool failures, scaffolding/build/deploy, install/packaging, or unexpected + output) and a dedicated **Sign-in / AADSTS error** form for auth failures, + plus a **Report an SPE MCP issue** agent skill + (`.github/skills/report-spe-mcp-issue`) that picks the right form, gathers the + diagnostics, redacts secrets, and drafts the issue. New "React SPA sign-in + fails with `AADSTS9002326`" troubleshooting section. ### Fixed diff --git a/SUPPORT.md b/SUPPORT.md index 5279a32..1e41f25 100644 --- a/SUPPORT.md +++ b/SUPPORT.md @@ -8,12 +8,13 @@ before filing new issues to avoid duplicates. For new issues, file your bug or f request as a new Issue. When you open a [new issue](https://github.com/microsoft/SharePoint-Embedded-MCP-Server/issues/new/choose) -you can pick a guided form — including a **Sign-in / AADSTS error** template that collects -the exact diagnostics needed to root-cause auth failures (see -[Troubleshooting](docs/TROUBLESHOOTING.md#react-spa-sign-in-fails-with-aadsts9002326-redirect-uri-already-registered) -first — it resolves most sign-in reports). If you use an AI agent with this MCP server, the -[**Report an SPE MCP issue** skill](.github/skills/report-spe-mcp-issue/SKILL.md) can gather -those diagnostics and draft the issue for you. +you can pick a guided form: a general **Bug report** for tool failures, +scaffolding/build/deploy problems, install/packaging issues, or unexpected output, +plus a dedicated **Sign-in / AADSTS error** form for auth failures. Check +[Troubleshooting](docs/TROUBLESHOOTING.md) first — it resolves many reports without +filing. If you use an AI agent with this MCP server, the +[**Report an SPE MCP issue** skill](.github/skills/report-spe-mcp-issue/SKILL.md) can +pick the right form, gather the diagnostics, and draft the issue for you. For help and questions about using this project, file a GitHub Issue in this repository. Please do **not** report security vulnerabilities through public GitHub issues — follow the