From be59aa97f24cec63fb55bb1d4e760dd083c3766f Mon Sep 17 00:00:00 2001 From: Long Hao Date: Tue, 21 Apr 2026 05:21:17 +0000 Subject: [PATCH] fix(auth): restore AAD local emulator fallback and normalize openIdIssuer URL (#947) Fixes two regressions in the custom-auth AAD flow introduced by #905 in 2.0.3: 1. `/.auth/login/aad` now returns a 400 "AAD_CLIENT_ID not found in env" in local dev, even though the pre-2.0.3 behaviour was to serve the local auth emulator. Restore the fallback: when the config references env vars but those env vars are unset, delegate to the SWA local auth emulator instead of hard-failing. Missing config fields still 400 as before. 2. An `openIdIssuer` of `https://login.microsoftonline.com//oauth2/v2.0` (accepted by the deployed SWA runtime) caused ERR_TOO_MANY_REDIRECTS during CLI OIDC discovery. Normalize the URL to the canonical `/v2.0` form in `OpenIdHelper` so one `staticwebapp.config.json` works both locally and deployed. Adds 17 unit tests covering both regressions. All 502 existing tests still pass. Refs: #928, #941, #947, PR #905 --- src/core/utils/openidHelper.spec.ts | 77 +++++++++++ src/core/utils/openidHelper.ts | 30 ++++- .../routes/auth-login-provider-custom.spec.ts | 127 ++++++++++++++++++ .../auth/routes/auth-login-provider-custom.ts | 42 ++++++ 4 files changed, 275 insertions(+), 1 deletion(-) create mode 100644 src/core/utils/openidHelper.spec.ts create mode 100644 src/msha/auth/routes/auth-login-provider-custom.spec.ts diff --git a/src/core/utils/openidHelper.spec.ts b/src/core/utils/openidHelper.spec.ts new file mode 100644 index 000000000..2cf828dd2 --- /dev/null +++ b/src/core/utils/openidHelper.spec.ts @@ -0,0 +1,77 @@ +import { describe, it, expect } from "vitest"; +import { normalizeOpenIdIssuer } from "./openidHelper.js"; + +describe("normalizeOpenIdIssuer", () => { + describe("Microsoft identity platform (login.microsoftonline.com)", () => { + it("strips legacy /oauth2 segment for v2.0 issuer so discovery resolves (fixes ERR_TOO_MANY_REDIRECTS)", () => { + // Deployed SWA runtime accepts `...//v2.0` but the local CLI's + // `openid-client.discovery()` requires the canonical issuer (no /oauth2). + // Users must be able to use the same `staticwebapp.config.json` in both + // environments, so the CLI normalizes the legacy form before discovery. + const input = "https://login.microsoftonline.com/00000000-0000-0000-0000-000000000000/oauth2/v2.0"; + const expected = "https://login.microsoftonline.com/00000000-0000-0000-0000-000000000000/v2.0"; + expect(normalizeOpenIdIssuer(input)).toBe(expected); + }); + + it("preserves the canonical v2.0 issuer unchanged", () => { + const input = "https://login.microsoftonline.com/00000000-0000-0000-0000-000000000000/v2.0"; + expect(normalizeOpenIdIssuer(input)).toBe(input); + }); + + it("preserves the issuer regardless of a trailing slash", () => { + const input = "https://login.microsoftonline.com/00000000-0000-0000-0000-000000000000/v2.0/"; + expect(normalizeOpenIdIssuer(input)).toBe(input); + }); + + it("handles common-tenant (multi-tenant) endpoint with /oauth2/ prefix", () => { + const input = "https://login.microsoftonline.com/common/oauth2/v2.0"; + expect(normalizeOpenIdIssuer(input)).toBe("https://login.microsoftonline.com/common/v2.0"); + }); + + it("handles organizations-tenant endpoint with /oauth2/ prefix", () => { + const input = "https://login.microsoftonline.com/organizations/oauth2/v2.0"; + expect(normalizeOpenIdIssuer(input)).toBe("https://login.microsoftonline.com/organizations/v2.0"); + }); + }); + + describe("Entra External ID (ciamlogin.com)", () => { + it("preserves ciamlogin.com issuer unchanged (already canonical)", () => { + const input = "https://contoso.ciamlogin.com/contoso.onmicrosoft.com/v2.0"; + expect(normalizeOpenIdIssuer(input)).toBe(input); + }); + + it("strips legacy /oauth2 segment from ciamlogin.com issuer when present", () => { + const input = "https://contoso.ciamlogin.com/contoso.onmicrosoft.com/oauth2/v2.0"; + expect(normalizeOpenIdIssuer(input)).toBe("https://contoso.ciamlogin.com/contoso.onmicrosoft.com/v2.0"); + }); + }); + + describe("Entra custom URL domains", () => { + it("preserves custom-domain issuer unchanged", () => { + const input = "https://login.contoso.com/00000000-0000-0000-0000-000000000000/v2.0"; + expect(normalizeOpenIdIssuer(input)).toBe(input); + }); + + it("strips legacy /oauth2 segment from custom-domain issuer when present", () => { + const input = "https://login.contoso.com/00000000-0000-0000-0000-000000000000/oauth2/v2.0"; + expect(normalizeOpenIdIssuer(input)).toBe("https://login.contoso.com/00000000-0000-0000-0000-000000000000/v2.0"); + }); + }); + + describe("edge cases", () => { + it("returns an empty string unchanged (upstream validates separately)", () => { + expect(normalizeOpenIdIssuer("")).toBe(""); + }); + + it("returns a non-matching URL unchanged", () => { + const input = "https://example.com/some/other/issuer"; + expect(normalizeOpenIdIssuer(input)).toBe(input); + }); + + it("does not strip /oauth2 when it is not followed by /v2.0", () => { + // Defensive: we only target the known Microsoft legacy alias form. + const input = "https://example.com/tenant/oauth2/other"; + expect(normalizeOpenIdIssuer(input)).toBe(input); + }); + }); +}); diff --git a/src/core/utils/openidHelper.ts b/src/core/utils/openidHelper.ts index 2f09b7e78..54db08e85 100644 --- a/src/core/utils/openidHelper.ts +++ b/src/core/utils/openidHelper.ts @@ -1,5 +1,33 @@ import * as client from "openid-client"; +/** + * Normalize an `openIdIssuer` URL so that `openid-client`'s discovery + * (`/.well-known/openid-configuration`) resolves in both local CLI + * and deployed SWA environments. + * + * Background: the Microsoft identity platform v2.0 canonical issuer is + * `https://login.microsoftonline.com//v2.0`. A legacy/alias form, + * `https://login.microsoftonline.com//oauth2/v2.0`, is accepted by + * the deployed SWA runtime but causes the CLI's OIDC discovery to fail + * (ERR_TOO_MANY_REDIRECTS or 404). We strip the trailing `/oauth2` segment + * so that users can keep a single `staticwebapp.config.json` that works + * both locally and when deployed. + * + * The same normalization applies to Entra External ID (`*.ciamlogin.com`) + * and Entra custom URL domains — any `...//oauth2/v2.0` suffix is + * rewritten to `...//v2.0`. + * + * See: https://github.com/Azure/static-web-apps-cli/issues/947 + */ +export function normalizeOpenIdIssuer(issuer: string): string { + if (!issuer) { + return issuer; + } + // Rewrite `/oauth2/v2.0` (with optional trailing slash) to `/v2.0` while + // preserving any trailing slash the user provided. + return issuer.replace(/\/oauth2\/v2\.0(\/?)$/, "/v2.0$1"); +} + export class OpenIdHelper { private issuerUrl: URL; private clientId: string; @@ -11,7 +39,7 @@ export class OpenIdHelper { if (!clientId || clientId.trim() === "") { throw new Error("Client ID is required"); } - this.issuerUrl = new URL(issuerUrl); + this.issuerUrl = new URL(normalizeOpenIdIssuer(issuerUrl)); this.clientId = clientId; } diff --git a/src/msha/auth/routes/auth-login-provider-custom.spec.ts b/src/msha/auth/routes/auth-login-provider-custom.spec.ts new file mode 100644 index 000000000..cba208da1 --- /dev/null +++ b/src/msha/auth/routes/auth-login-provider-custom.spec.ts @@ -0,0 +1,127 @@ +// Tests that `/.auth/login/aad` in local dev — when the real AAD env vars are +// NOT set — falls back to the SWA local auth emulator instead of returning +// `AAD_CLIENT_ID not found in env for 'aad' provider`. +// +// Regression coverage for https://github.com/Azure/static-web-apps-cli/issues/947 +// which was broken in 2.0.3 by PR #905. + +vi.mock("../../../core/constants", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + SWA_CLI_APP_PROTOCOL: "http", + }; +}); + +// Mock the auth-login-provider module so we can assert emulator delegation +// without depending on the real HTML fixture file. +vi.mock("./auth-login-provider.js", () => { + return { + default: vi.fn(async (context: Context, _request: unknown) => { + context.res = { + status: 200, + headers: { "Content-Type": "text/html" }, + body: "", + }; + }), + }; +}); + +import { IncomingMessage } from "node:http"; +import authLoginProviderEmulator from "./auth-login-provider.js"; +import httpTrigger from "./auth-login-provider-custom.js"; + +describe("auth-login-provider-custom — AAD local emulator fallback (issue #947)", () => { + let context: Context; + let req: IncomingMessage; + + const baseCustomAuth = { + identityProviders: { + azureActiveDirectory: { + registration: { + openIdIssuer: "https://login.microsoftonline.com/tenant-id/v2.0", + clientIdSettingName: "AAD_CLIENT_ID", + clientSecretSettingName: "AAD_CLIENT_SECRET", + }, + }, + }, + }; + + beforeEach(() => { + context = { bindingData: { provider: "aad" } } as unknown as Context; + req = { + url: "/.auth/login/aad", + headers: { host: "localhost:4280" }, + } as IncomingMessage; + delete process.env.AAD_CLIENT_ID; + delete process.env.AAD_CLIENT_SECRET; + vi.mocked(authLoginProviderEmulator).mockClear(); + }); + + it("falls back to the local auth emulator when AAD clientId env var is unset", async () => { + // Only secret is set — clientId is missing. + process.env.AAD_CLIENT_SECRET = "test-secret"; + + await httpTrigger(context, req, baseCustomAuth); + + expect(authLoginProviderEmulator).toHaveBeenCalledOnce(); + // The custom handler should NOT have returned the 400 error from 2.0.3+. + expect(context.res.status).not.toBe(400); + }); + + it("falls back to the local auth emulator when AAD clientSecret env var is unset", async () => { + // Only clientId is set — secret is missing. + process.env.AAD_CLIENT_ID = "test-client-id"; + + await httpTrigger(context, req, baseCustomAuth); + + expect(authLoginProviderEmulator).toHaveBeenCalledOnce(); + expect(context.res.status).not.toBe(400); + }); + + it("falls back when both AAD env vars are unset (common local-dev case)", async () => { + await httpTrigger(context, req, baseCustomAuth); + + expect(authLoginProviderEmulator).toHaveBeenCalledOnce(); + // Before the fix, this would have been: 400 with body + // "AAD_CLIENT_ID not found in env for 'aad' provider". + expect(context.res.status).not.toBe(400); + }); + + it("does NOT fall back when both AAD env vars are set (real-auth path)", async () => { + process.env.AAD_CLIENT_ID = "test-client-id"; + process.env.AAD_CLIENT_SECRET = "test-secret"; + + // We don't care about discovery here — either a 302 to the issuer or a + // discovery error is fine; we only assert we did NOT delegate to the + // emulator. + try { + await httpTrigger(context, req, baseCustomAuth); + } catch { + // discovery may fail against the fake tenant — that's OK for this assertion. + } + + expect(authLoginProviderEmulator).not.toHaveBeenCalled(); + }); + + it("does NOT fall back when the config itself is missing a required field (hard error)", async () => { + // Missing `clientIdSettingName` entirely — this is a user config bug, not + // a local-dev signal, so the handler should surface a 400 as before. + const brokenAuth = { + identityProviders: { + azureActiveDirectory: { + registration: { + openIdIssuer: "https://login.microsoftonline.com/tenant-id/v2.0", + clientSecretSettingName: "AAD_CLIENT_SECRET", + }, + }, + }, + }; + process.env.AAD_CLIENT_SECRET = "test-secret"; + + await httpTrigger(context, req, brokenAuth as unknown as SWAConfigFileAuth); + + expect(authLoginProviderEmulator).not.toHaveBeenCalled(); + expect(context.res.status).toBe(400); + }); +}); diff --git a/src/msha/auth/routes/auth-login-provider-custom.ts b/src/msha/auth/routes/auth-login-provider-custom.ts index 5dfccd8bc..1f0144e4c 100644 --- a/src/msha/auth/routes/auth-login-provider-custom.ts +++ b/src/msha/auth/routes/auth-login-provider-custom.ts @@ -5,6 +5,8 @@ import { CUSTOM_AUTH_REQUIRED_FIELDS, ENTRAID_FULL_NAME, SWA_CLI_APP_PROTOCOL } import { DEFAULT_CONFIG } from "../../../config.js"; import { encryptAndSign, extractPostLoginRedirectUri, hashStateGuid, newNonceWithExpiration } from "../../../core/utils/auth.js"; import { OpenIdHelper } from "../../../core/utils/openidHelper.js"; +import { logger } from "../../../core/utils/logger.js"; +import authLoginProviderEmulator from "./auth-login-provider.js"; export const normalizeAuthProvider = function (providerName?: string) { if (providerName === ENTRAID_FULL_NAME) { @@ -13,6 +15,37 @@ export const normalizeAuthProvider = function (providerName?: string) { return providerName?.toLowerCase() || ""; }; +/** + * For the `aad` custom auth provider, when the user's `staticwebapp.config.json` + * references AAD env vars (clientIdSettingName / clientSecretSettingName) but + * those env vars are NOT set, we fall back to the SWA local auth emulator + * instead of hard-failing with a 400. + * + * This restores the pre-2.0.3 behaviour where `/.auth/login/aad` worked in + * local dev without requiring developers to provision a real tenant just to + * run the CLI. See https://github.com/Azure/static-web-apps-cli/issues/947. + * + * Returns `true` iff this is AAD, the config names env vars, and at least one + * of them is unset — i.e. the user is clearly in local-dev mode. + */ +export const shouldFallbackToAadEmulator = function (providerName: string, customAuth?: SWAConfigFileAuth): boolean { + if (providerName !== "aad") { + return false; + } + const registration = customAuth?.identityProviders?.[ENTRAID_FULL_NAME]?.registration; + const clientIdSettingName = registration?.clientIdSettingName; + const clientSecretSettingName = registration?.clientSecretSettingName; + // The config must reference env vars; if either name is missing entirely + // that's a config-level error and should surface as 400 (handled below by + // `checkCustomAuthConfigFields`). + if (!clientIdSettingName || !clientSecretSettingName) { + return false; + } + const clientIdValue = process.env[clientIdSettingName]; + const clientSecretValue = process.env[clientSecretSettingName]; + return !clientIdValue || !clientSecretValue; +}; + export const checkCustomAuthConfigFields = function (context: Context, providerName: string, customAuth?: SWAConfigFileAuth) { const generateResponse = function (msg: string) { return { @@ -60,6 +93,15 @@ const httpTrigger = async function (context: Context, request: IncomingMessage, await Promise.resolve(); const providerName: string = normalizeAuthProvider(context.bindingData?.provider); + + // Restore pre-2.0.3 behaviour for local dev: if the AAD env vars referenced + // by the user's config aren't set, delegate to the local auth emulator + // instead of hard-failing. See #947. + if (shouldFallbackToAadEmulator(providerName, customAuth)) { + logger.silly(`AAD env vars not set — falling back to the SWA local auth emulator for '/.auth/login/aad'`); + return authLoginProviderEmulator(context, request); + } + const authFields = checkCustomAuthConfigFields(context, providerName, customAuth); if (!authFields) { return;