Skip to content

Commit be59aa9

Browse files
committed
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/<tenant>/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
1 parent 3ecb7b4 commit be59aa9

4 files changed

Lines changed: 275 additions & 1 deletion

File tree

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
import { describe, it, expect } from "vitest";
2+
import { normalizeOpenIdIssuer } from "./openidHelper.js";
3+
4+
describe("normalizeOpenIdIssuer", () => {
5+
describe("Microsoft identity platform (login.microsoftonline.com)", () => {
6+
it("strips legacy /oauth2 segment for v2.0 issuer so discovery resolves (fixes ERR_TOO_MANY_REDIRECTS)", () => {
7+
// Deployed SWA runtime accepts `.../<tenant>/v2.0` but the local CLI's
8+
// `openid-client.discovery()` requires the canonical issuer (no /oauth2).
9+
// Users must be able to use the same `staticwebapp.config.json` in both
10+
// environments, so the CLI normalizes the legacy form before discovery.
11+
const input = "https://login.microsoftonline.com/00000000-0000-0000-0000-000000000000/oauth2/v2.0";
12+
const expected = "https://login.microsoftonline.com/00000000-0000-0000-0000-000000000000/v2.0";
13+
expect(normalizeOpenIdIssuer(input)).toBe(expected);
14+
});
15+
16+
it("preserves the canonical v2.0 issuer unchanged", () => {
17+
const input = "https://login.microsoftonline.com/00000000-0000-0000-0000-000000000000/v2.0";
18+
expect(normalizeOpenIdIssuer(input)).toBe(input);
19+
});
20+
21+
it("preserves the issuer regardless of a trailing slash", () => {
22+
const input = "https://login.microsoftonline.com/00000000-0000-0000-0000-000000000000/v2.0/";
23+
expect(normalizeOpenIdIssuer(input)).toBe(input);
24+
});
25+
26+
it("handles common-tenant (multi-tenant) endpoint with /oauth2/ prefix", () => {
27+
const input = "https://login.microsoftonline.com/common/oauth2/v2.0";
28+
expect(normalizeOpenIdIssuer(input)).toBe("https://login.microsoftonline.com/common/v2.0");
29+
});
30+
31+
it("handles organizations-tenant endpoint with /oauth2/ prefix", () => {
32+
const input = "https://login.microsoftonline.com/organizations/oauth2/v2.0";
33+
expect(normalizeOpenIdIssuer(input)).toBe("https://login.microsoftonline.com/organizations/v2.0");
34+
});
35+
});
36+
37+
describe("Entra External ID (ciamlogin.com)", () => {
38+
it("preserves ciamlogin.com issuer unchanged (already canonical)", () => {
39+
const input = "https://contoso.ciamlogin.com/contoso.onmicrosoft.com/v2.0";
40+
expect(normalizeOpenIdIssuer(input)).toBe(input);
41+
});
42+
43+
it("strips legacy /oauth2 segment from ciamlogin.com issuer when present", () => {
44+
const input = "https://contoso.ciamlogin.com/contoso.onmicrosoft.com/oauth2/v2.0";
45+
expect(normalizeOpenIdIssuer(input)).toBe("https://contoso.ciamlogin.com/contoso.onmicrosoft.com/v2.0");
46+
});
47+
});
48+
49+
describe("Entra custom URL domains", () => {
50+
it("preserves custom-domain issuer unchanged", () => {
51+
const input = "https://login.contoso.com/00000000-0000-0000-0000-000000000000/v2.0";
52+
expect(normalizeOpenIdIssuer(input)).toBe(input);
53+
});
54+
55+
it("strips legacy /oauth2 segment from custom-domain issuer when present", () => {
56+
const input = "https://login.contoso.com/00000000-0000-0000-0000-000000000000/oauth2/v2.0";
57+
expect(normalizeOpenIdIssuer(input)).toBe("https://login.contoso.com/00000000-0000-0000-0000-000000000000/v2.0");
58+
});
59+
});
60+
61+
describe("edge cases", () => {
62+
it("returns an empty string unchanged (upstream validates separately)", () => {
63+
expect(normalizeOpenIdIssuer("")).toBe("");
64+
});
65+
66+
it("returns a non-matching URL unchanged", () => {
67+
const input = "https://example.com/some/other/issuer";
68+
expect(normalizeOpenIdIssuer(input)).toBe(input);
69+
});
70+
71+
it("does not strip /oauth2 when it is not followed by /v2.0", () => {
72+
// Defensive: we only target the known Microsoft legacy alias form.
73+
const input = "https://example.com/tenant/oauth2/other";
74+
expect(normalizeOpenIdIssuer(input)).toBe(input);
75+
});
76+
});
77+
});

src/core/utils/openidHelper.ts

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,33 @@
11
import * as client from "openid-client";
22

3+
/**
4+
* Normalize an `openIdIssuer` URL so that `openid-client`'s discovery
5+
* (`<issuer>/.well-known/openid-configuration`) resolves in both local CLI
6+
* and deployed SWA environments.
7+
*
8+
* Background: the Microsoft identity platform v2.0 canonical issuer is
9+
* `https://login.microsoftonline.com/<tenant>/v2.0`. A legacy/alias form,
10+
* `https://login.microsoftonline.com/<tenant>/oauth2/v2.0`, is accepted by
11+
* the deployed SWA runtime but causes the CLI's OIDC discovery to fail
12+
* (ERR_TOO_MANY_REDIRECTS or 404). We strip the trailing `/oauth2` segment
13+
* so that users can keep a single `staticwebapp.config.json` that works
14+
* both locally and when deployed.
15+
*
16+
* The same normalization applies to Entra External ID (`*.ciamlogin.com`)
17+
* and Entra custom URL domains — any `.../<tenant>/oauth2/v2.0` suffix is
18+
* rewritten to `.../<tenant>/v2.0`.
19+
*
20+
* See: https://github.com/Azure/static-web-apps-cli/issues/947
21+
*/
22+
export function normalizeOpenIdIssuer(issuer: string): string {
23+
if (!issuer) {
24+
return issuer;
25+
}
26+
// Rewrite `/oauth2/v2.0` (with optional trailing slash) to `/v2.0` while
27+
// preserving any trailing slash the user provided.
28+
return issuer.replace(/\/oauth2\/v2\.0(\/?)$/, "/v2.0$1");
29+
}
30+
331
export class OpenIdHelper {
432
private issuerUrl: URL;
533
private clientId: string;
@@ -11,7 +39,7 @@ export class OpenIdHelper {
1139
if (!clientId || clientId.trim() === "") {
1240
throw new Error("Client ID is required");
1341
}
14-
this.issuerUrl = new URL(issuerUrl);
42+
this.issuerUrl = new URL(normalizeOpenIdIssuer(issuerUrl));
1543
this.clientId = clientId;
1644
}
1745

Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
// Tests that `/.auth/login/aad` in local dev — when the real AAD env vars are
2+
// NOT set — falls back to the SWA local auth emulator instead of returning
3+
// `AAD_CLIENT_ID not found in env for 'aad' provider`.
4+
//
5+
// Regression coverage for https://github.com/Azure/static-web-apps-cli/issues/947
6+
// which was broken in 2.0.3 by PR #905.
7+
8+
vi.mock("../../../core/constants", async (importOriginal) => {
9+
const actual = await importOriginal<typeof import("../../../core/constants.js")>();
10+
return {
11+
...actual,
12+
SWA_CLI_APP_PROTOCOL: "http",
13+
};
14+
});
15+
16+
// Mock the auth-login-provider module so we can assert emulator delegation
17+
// without depending on the real HTML fixture file.
18+
vi.mock("./auth-login-provider.js", () => {
19+
return {
20+
default: vi.fn(async (context: Context, _request: unknown) => {
21+
context.res = {
22+
status: 200,
23+
headers: { "Content-Type": "text/html" },
24+
body: "<emulator></emulator>",
25+
};
26+
}),
27+
};
28+
});
29+
30+
import { IncomingMessage } from "node:http";
31+
import authLoginProviderEmulator from "./auth-login-provider.js";
32+
import httpTrigger from "./auth-login-provider-custom.js";
33+
34+
describe("auth-login-provider-custom — AAD local emulator fallback (issue #947)", () => {
35+
let context: Context;
36+
let req: IncomingMessage;
37+
38+
const baseCustomAuth = {
39+
identityProviders: {
40+
azureActiveDirectory: {
41+
registration: {
42+
openIdIssuer: "https://login.microsoftonline.com/tenant-id/v2.0",
43+
clientIdSettingName: "AAD_CLIENT_ID",
44+
clientSecretSettingName: "AAD_CLIENT_SECRET",
45+
},
46+
},
47+
},
48+
};
49+
50+
beforeEach(() => {
51+
context = { bindingData: { provider: "aad" } } as unknown as Context;
52+
req = {
53+
url: "/.auth/login/aad",
54+
headers: { host: "localhost:4280" },
55+
} as IncomingMessage;
56+
delete process.env.AAD_CLIENT_ID;
57+
delete process.env.AAD_CLIENT_SECRET;
58+
vi.mocked(authLoginProviderEmulator).mockClear();
59+
});
60+
61+
it("falls back to the local auth emulator when AAD clientId env var is unset", async () => {
62+
// Only secret is set — clientId is missing.
63+
process.env.AAD_CLIENT_SECRET = "test-secret";
64+
65+
await httpTrigger(context, req, baseCustomAuth);
66+
67+
expect(authLoginProviderEmulator).toHaveBeenCalledOnce();
68+
// The custom handler should NOT have returned the 400 error from 2.0.3+.
69+
expect(context.res.status).not.toBe(400);
70+
});
71+
72+
it("falls back to the local auth emulator when AAD clientSecret env var is unset", async () => {
73+
// Only clientId is set — secret is missing.
74+
process.env.AAD_CLIENT_ID = "test-client-id";
75+
76+
await httpTrigger(context, req, baseCustomAuth);
77+
78+
expect(authLoginProviderEmulator).toHaveBeenCalledOnce();
79+
expect(context.res.status).not.toBe(400);
80+
});
81+
82+
it("falls back when both AAD env vars are unset (common local-dev case)", async () => {
83+
await httpTrigger(context, req, baseCustomAuth);
84+
85+
expect(authLoginProviderEmulator).toHaveBeenCalledOnce();
86+
// Before the fix, this would have been: 400 with body
87+
// "AAD_CLIENT_ID not found in env for 'aad' provider".
88+
expect(context.res.status).not.toBe(400);
89+
});
90+
91+
it("does NOT fall back when both AAD env vars are set (real-auth path)", async () => {
92+
process.env.AAD_CLIENT_ID = "test-client-id";
93+
process.env.AAD_CLIENT_SECRET = "test-secret";
94+
95+
// We don't care about discovery here — either a 302 to the issuer or a
96+
// discovery error is fine; we only assert we did NOT delegate to the
97+
// emulator.
98+
try {
99+
await httpTrigger(context, req, baseCustomAuth);
100+
} catch {
101+
// discovery may fail against the fake tenant — that's OK for this assertion.
102+
}
103+
104+
expect(authLoginProviderEmulator).not.toHaveBeenCalled();
105+
});
106+
107+
it("does NOT fall back when the config itself is missing a required field (hard error)", async () => {
108+
// Missing `clientIdSettingName` entirely — this is a user config bug, not
109+
// a local-dev signal, so the handler should surface a 400 as before.
110+
const brokenAuth = {
111+
identityProviders: {
112+
azureActiveDirectory: {
113+
registration: {
114+
openIdIssuer: "https://login.microsoftonline.com/tenant-id/v2.0",
115+
clientSecretSettingName: "AAD_CLIENT_SECRET",
116+
},
117+
},
118+
},
119+
};
120+
process.env.AAD_CLIENT_SECRET = "test-secret";
121+
122+
await httpTrigger(context, req, brokenAuth as unknown as SWAConfigFileAuth);
123+
124+
expect(authLoginProviderEmulator).not.toHaveBeenCalled();
125+
expect(context.res.status).toBe(400);
126+
});
127+
});

src/msha/auth/routes/auth-login-provider-custom.ts

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ import { CUSTOM_AUTH_REQUIRED_FIELDS, ENTRAID_FULL_NAME, SWA_CLI_APP_PROTOCOL }
55
import { DEFAULT_CONFIG } from "../../../config.js";
66
import { encryptAndSign, extractPostLoginRedirectUri, hashStateGuid, newNonceWithExpiration } from "../../../core/utils/auth.js";
77
import { OpenIdHelper } from "../../../core/utils/openidHelper.js";
8+
import { logger } from "../../../core/utils/logger.js";
9+
import authLoginProviderEmulator from "./auth-login-provider.js";
810

911
export const normalizeAuthProvider = function (providerName?: string) {
1012
if (providerName === ENTRAID_FULL_NAME) {
@@ -13,6 +15,37 @@ export const normalizeAuthProvider = function (providerName?: string) {
1315
return providerName?.toLowerCase() || "";
1416
};
1517

18+
/**
19+
* For the `aad` custom auth provider, when the user's `staticwebapp.config.json`
20+
* references AAD env vars (clientIdSettingName / clientSecretSettingName) but
21+
* those env vars are NOT set, we fall back to the SWA local auth emulator
22+
* instead of hard-failing with a 400.
23+
*
24+
* This restores the pre-2.0.3 behaviour where `/.auth/login/aad` worked in
25+
* local dev without requiring developers to provision a real tenant just to
26+
* run the CLI. See https://github.com/Azure/static-web-apps-cli/issues/947.
27+
*
28+
* Returns `true` iff this is AAD, the config names env vars, and at least one
29+
* of them is unset — i.e. the user is clearly in local-dev mode.
30+
*/
31+
export const shouldFallbackToAadEmulator = function (providerName: string, customAuth?: SWAConfigFileAuth): boolean {
32+
if (providerName !== "aad") {
33+
return false;
34+
}
35+
const registration = customAuth?.identityProviders?.[ENTRAID_FULL_NAME]?.registration;
36+
const clientIdSettingName = registration?.clientIdSettingName;
37+
const clientSecretSettingName = registration?.clientSecretSettingName;
38+
// The config must reference env vars; if either name is missing entirely
39+
// that's a config-level error and should surface as 400 (handled below by
40+
// `checkCustomAuthConfigFields`).
41+
if (!clientIdSettingName || !clientSecretSettingName) {
42+
return false;
43+
}
44+
const clientIdValue = process.env[clientIdSettingName];
45+
const clientSecretValue = process.env[clientSecretSettingName];
46+
return !clientIdValue || !clientSecretValue;
47+
};
48+
1649
export const checkCustomAuthConfigFields = function (context: Context, providerName: string, customAuth?: SWAConfigFileAuth) {
1750
const generateResponse = function (msg: string) {
1851
return {
@@ -60,6 +93,15 @@ const httpTrigger = async function (context: Context, request: IncomingMessage,
6093
await Promise.resolve();
6194

6295
const providerName: string = normalizeAuthProvider(context.bindingData?.provider);
96+
97+
// Restore pre-2.0.3 behaviour for local dev: if the AAD env vars referenced
98+
// by the user's config aren't set, delegate to the local auth emulator
99+
// instead of hard-failing. See #947.
100+
if (shouldFallbackToAadEmulator(providerName, customAuth)) {
101+
logger.silly(`AAD env vars not set — falling back to the SWA local auth emulator for '/.auth/login/aad'`);
102+
return authLoginProviderEmulator(context, request);
103+
}
104+
63105
const authFields = checkCustomAuthConfigFields(context, providerName, customAuth);
64106
if (!authFields) {
65107
return;

0 commit comments

Comments
 (0)