Skip to content

Commit aafeda0

Browse files
authored
Reset Redirecturi during broker fallback (#8049)
If a user attempts a broker flow but the broker isn’t available on their machine, the flow falls back to the browser. However, since a redirect URI was provided for the broker flow, an error is thrown. This PR adds a check to determine whether the broker is enabled in the config and resets the redirect URI to an empty string instead of throwing an error.
1 parent f65f2dc commit aafeda0

File tree

3 files changed

+101
-2
lines changed

3 files changed

+101
-2
lines changed
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"type": "patch",
3+
"comment": "broker redirect uri changes",
4+
"packageName": "@azure/msal-node",
5+
"email": "[email protected]",
6+
"dependentChangeType": "patch"
7+
}

lib/msal-node/src/client/PublicClientApplication.ts

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -180,8 +180,14 @@ export class PublicClientApplication
180180
}
181181

182182
if (request.redirectUri) {
183-
throw NodeAuthError.createRedirectUriNotSupportedError();
183+
// If its not a broker fallback scenario, we throw a error
184+
if (!this.config.broker.nativeBrokerPlugin) {
185+
throw NodeAuthError.createRedirectUriNotSupportedError();
186+
}
187+
// If a redirect URI is provided for a broker flow but MSAL runtime startup failed, we fall back to the browser flow and will ignore the redirect URI provided for the broker flow
188+
request.redirectUri = "";
184189
}
190+
185191
const { verifier, challenge } =
186192
await this.cryptoProvider.generatePkceCodes();
187193

@@ -275,7 +281,11 @@ export class PublicClientApplication
275281
}
276282

277283
if (request.redirectUri) {
278-
throw NodeAuthError.createRedirectUriNotSupportedError();
284+
// If its not a broker fallback scenario, we throw a error
285+
if (!this.config.broker.nativeBrokerPlugin) {
286+
throw NodeAuthError.createRedirectUriNotSupportedError();
287+
}
288+
request.redirectUri = "";
279289
}
280290

281291
return super.acquireTokenSilent(request);

lib/msal-node/test/client/PublicClientApplication.spec.ts

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -629,6 +629,43 @@ describe("PublicClientApplication", () => {
629629
"RedirectUri is not supported in this scenario"
630630
);
631631
});
632+
633+
test("acquireTokenSilent resets redirectUri when broker fallback occurs", async () => {
634+
// Create a broker plugin with broker unavailable
635+
const mockBrokerPlugin = new MockNativeBrokerPlugin();
636+
mockBrokerPlugin.isBrokerAvailable = false;
637+
638+
const authApp = new PublicClientApplication({
639+
...appConfig,
640+
broker: {
641+
nativeBrokerPlugin: mockBrokerPlugin,
642+
},
643+
});
644+
645+
const silentFlowClient = getMsalCommonAutoMock().SilentFlowClient;
646+
jest.spyOn(msalCommon, "SilentFlowClient").mockImplementation(
647+
(config) => new silentFlowClient(config)
648+
);
649+
jest.spyOn(
650+
silentFlowClient.prototype,
651+
"acquireCachedToken"
652+
).mockResolvedValue([
653+
mockAuthenticationResult,
654+
CacheOutcome.NOT_APPLICABLE,
655+
]);
656+
657+
const request: SilentFlowRequest = {
658+
scopes: TEST_CONSTANTS.DEFAULT_GRAPH_SCOPE,
659+
account: testAccount,
660+
redirectUri: "http://localhost:3000/redirect",
661+
};
662+
663+
// This should not throw and should reset redirectUri to empty string and continue with the request
664+
const response = await authApp.acquireTokenSilent(request);
665+
666+
expect(response).toEqual(mockAuthenticationResult);
667+
expect(request.redirectUri).toBe("");
668+
});
632669
});
633670

634671
describe("acquireTokenInteractive tests", () => {
@@ -1050,6 +1087,51 @@ describe("PublicClientApplication", () => {
10501087
authApp.acquireTokenInteractive(request)
10511088
).rejects.toThrow("RedirectUri is not supported in this scenario");
10521089
});
1090+
1091+
test("acquireTokenInteractive resets redirectUri when broker fallback occurs", async () => {
1092+
// Create a broker plugin with broker unavailable to simulate fallback scenario
1093+
const mockBrokerPlugin = new MockNativeBrokerPlugin();
1094+
mockBrokerPlugin.isBrokerAvailable = false;
1095+
1096+
const authApp = new PublicClientApplication({
1097+
...appConfig,
1098+
broker: {
1099+
nativeBrokerPlugin: mockBrokerPlugin,
1100+
},
1101+
});
1102+
1103+
const request: InteractiveRequest = {
1104+
scopes: TEST_CONSTANTS.DEFAULT_GRAPH_SCOPE,
1105+
redirectUri: "http://localhost:3000/redirect",
1106+
openBrowser: jest.fn(),
1107+
};
1108+
1109+
jest.spyOn(
1110+
LoopbackClient.prototype,
1111+
"listenForAuthCode"
1112+
).mockResolvedValue({
1113+
code: TEST_CONSTANTS.AUTHORIZATION_CODE,
1114+
});
1115+
1116+
jest.spyOn(
1117+
LoopbackClient.prototype,
1118+
"getRedirectUri"
1119+
).mockReturnValue(TEST_CONSTANTS.REDIRECT_URI);
1120+
1121+
jest.spyOn(authApp, "acquireTokenByCode").mockResolvedValue(
1122+
mockAuthenticationResult
1123+
);
1124+
1125+
jest.spyOn(authApp, "getAuthCodeUrl").mockResolvedValue(
1126+
TEST_CONSTANTS.AUTH_CODE_URL
1127+
);
1128+
1129+
// This should not throw and should reset redirectUri to empty string
1130+
const response = await authApp.acquireTokenInteractive(request);
1131+
1132+
expect(response).toEqual(mockAuthenticationResult);
1133+
expect(request.redirectUri).toBe("");
1134+
});
10531135
});
10541136

10551137
describe("signOut tests", () => {

0 commit comments

Comments
 (0)