Skip to content

Commit 1a521c3

Browse files
Ugonnaak1Ugonnaak1
authored
Enable passing of redirect uri for Nativebroker (#8005)
Current PCA requests don't allow the customer to pass in their own redirecturi. This is an issue for signed MacOS apps because the customer is forced to use the unsigned redirect URI which will hit a redirecturi validation error in the Apple Broker ``` errorMessage: 'Description: Error Domain=MSALErrorDomain Code=-50000 "(null)" UserInfo={MSALErrorDescriptionKey=MSAL redirectUri validation error: redirect uri has incorrect scheme - it must be in the form of msauth.<app_bundle_id>://auth\n' + 'ADAL redirectUri validation error: Source application does not match redirect uri host. Invalid source app., MSALInternalErrorCodeKey=-42000, MSALBrokerVersionKey=5.2508.0}, Domain: MSALErrorDomain.Error was thrown in sourceArea: Broker (Error Code: -42000, Tag: 508638916)', ``` Solution: Make providing a redirecturi optional. If the customer doesn't provide one, we fallback to the default for each platform in broker scenarios For non-broker scenarios, if redirecturi is provided, we warn them it won't be used and use the default Also added a function to convert error tags to their string version to make error lookup quicker.
1 parent d1cfe74 commit 1a521c3

File tree

13 files changed

+170
-23
lines changed

13 files changed

+170
-23
lines changed

‎change/@azure-msal-node-118ac835-7803-4e30-81cd-91c4afe6bf9d.json‎

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

‎change/@azure-msal-node-extensions-bce3798f-5840-4955-a647-70f9009359fc.json‎

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

‎extensions/msal-node-extensions/src/broker/NativeBrokerPlugin.ts‎

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ import {
3838
LogLevel as MsalRuntimeLogLevel,
3939
} from "@azure/msal-node-runtime";
4040
import { ErrorCodes } from "../utils/Constants.js";
41+
import { StringUtils } from "../utils/StringUtils.js";
4142
import { NativeAuthError } from "../error/NativeAuthError.js";
4243
import { version, name } from "../packageMetadata.js";
4344

@@ -193,10 +194,16 @@ export class NativeBrokerPlugin implements INativeBrokerPlugin {
193194
request.correlationId
194195
);
195196
const platformRequest = request;
197+
if (!platformRequest.redirectUri) {
198+
platformRequest.redirectUri =
199+
this.chooseRedirectUriByPlatform(platformRequest);
200+
this.logger.info(
201+
"NativeBrokerPlugin - No Redirect URI provided, using default",
202+
platformRequest.redirectUri
203+
);
204+
}
196205
const authParams = this.generateRequestParameters(platformRequest);
197206
const account = await this.getAccount(platformRequest);
198-
platformRequest.redirectUri =
199-
this.chooseRedirectUriByPlatform(platformRequest);
200207

201208
return new Promise(
202209
(resolve: (value: AuthenticationResult) => void, reject) => {
@@ -251,9 +258,15 @@ export class NativeBrokerPlugin implements INativeBrokerPlugin {
251258
request.correlationId
252259
);
253260
const platformRequest = request;
261+
if (!platformRequest.redirectUri) {
262+
platformRequest.redirectUri =
263+
this.chooseRedirectUriByPlatform(platformRequest);
264+
this.logger.info(
265+
"NativeBrokerPlugin - No Redirect URI provided, using default",
266+
platformRequest.redirectUri
267+
);
268+
}
254269
const authParams = this.generateRequestParameters(platformRequest);
255-
platformRequest.redirectUri =
256-
this.chooseRedirectUriByPlatform(platformRequest);
257270
const account = await this.getAccount(platformRequest);
258271
const windowHandle = providedWindowHandle || Buffer.from([0]);
259272

@@ -466,9 +479,7 @@ export class NativeBrokerPlugin implements INativeBrokerPlugin {
466479
request.authority
467480
);
468481

469-
authParams.SetRedirectUri(
470-
this.chooseRedirectUriByPlatform(request)
471-
);
482+
authParams.SetRedirectUri(request.redirectUri);
472483
authParams.SetRequestedScopes(request.scopes.join(" "));
473484

474485
if (request.claims) {
@@ -645,9 +656,10 @@ export class NativeBrokerPlugin implements INativeBrokerPlugin {
645656
) {
646657
const { errorCode, errorStatus, errorContext, errorTag } =
647658
error as MsalRuntimeError;
659+
const tagString = StringUtils.tagToString(errorTag);
648660
const enhancedErrorContext = errorContext
649-
? `${errorContext} (Error Code: ${errorCode}, Tag: ${errorTag})`
650-
: `(Error Code: ${errorCode}, Tag: ${errorTag})`;
661+
? `${errorContext} (Error Code: ${errorCode}, Tag: ${tagString})`
662+
: `(Error Code: ${errorCode}, Tag: ${tagString})`;
651663
switch (errorStatus) {
652664
case ErrorStatus.InteractionRequired:
653665
case ErrorStatus.AccountUnusable:

‎extensions/msal-node-extensions/src/error/NativeAuthError.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,11 @@
44
*/
55

66
import { AuthError } from "@azure/msal-common/node";
7+
import { StringUtils } from "../utils/StringUtils.js";
78

89
export class NativeAuthError extends AuthError {
910
public statusCode: number;
10-
public tag: number;
11+
public tag: string;
1112

1213
constructor(
1314
errorStatus: string,
@@ -18,7 +19,7 @@ export class NativeAuthError extends AuthError {
1819
super(errorStatus, errorContext);
1920
this.name = "NativeAuthError";
2021
this.statusCode = errorCode;
21-
this.tag = errorTag;
22+
this.tag = StringUtils.tagToString(errorTag);
2223
Object.setPrototypeOf(this, NativeAuthError.prototype);
2324
}
2425
}

‎extensions/msal-node-extensions/src/index.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,4 +14,5 @@ export { CrossPlatformLockOptions } from "./lock/CrossPlatformLockOptions.js";
1414
export { PersistenceCreator } from "./persistence/PersistenceCreator.js";
1515
export { IPersistenceConfiguration } from "./persistence/IPersistenceConfiguration.js";
1616
export { Environment } from "./utils/Environment.js";
17+
export { StringUtils } from "./utils/StringUtils.js";
1718
export { NativeBrokerPlugin } from "./broker/NativeBrokerPlugin.js";

‎extensions/msal-node-extensions/src/utils/StringUtils.ts‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
/*
2+
* Copyright (c) Microsoft Corporation. All rights reserved.
3+
* Licensed under the MIT License.
4+
*/
5+
6+
export class StringUtils {
7+
/**
8+
* Converts a numeric tag to a string representation
9+
* @param tag - The numeric tag to convert
10+
* @returns The string representation of the tag
11+
*/
12+
static tagToString(tag: number): string {
13+
if (tag === 0) {
14+
return "UNTAG";
15+
}
16+
17+
const tagSymbolSpace =
18+
"abcdefghijklmnopqrstuvwxyz0123456789****************************";
19+
let tagBuffer = "*****";
20+
21+
const chars = [
22+
tagSymbolSpace[(tag >> 24) & 0x3f],
23+
tagSymbolSpace[(tag >> 18) & 0x3f],
24+
tagSymbolSpace[(tag >> 12) & 0x3f],
25+
tagSymbolSpace[(tag >> 6) & 0x3f],
26+
tagSymbolSpace[(tag >> 0) & 0x3f],
27+
];
28+
29+
tagBuffer = chars.join("");
30+
31+
return tagBuffer;
32+
}
33+
}

‎extensions/msal-node-extensions/test/broker/NativeBrokerPlugin.spec.ts‎

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ import {
5555
} from "@azure/msal-common";
5656
import { randomUUID } from "crypto";
5757
import { NativeAuthError } from "../../src/error/NativeAuthError";
58+
import { StringUtils } from "../../src/utils/StringUtils.js";
5859
import {
5960
testMsalRuntimeAccount,
6061
testAccountInfo,
@@ -87,8 +88,16 @@ function createMockAuthResult(
8788
if (process.platform === "win32") {
8889
describe("NativeBrokerPlugin", () => {
8990
const enhancedErrorContext = msalRuntimeExampleError.errorContext
90-
? `${msalRuntimeExampleError.errorContext} (Error Code: ${msalRuntimeExampleError.errorCode}, Tag: ${msalRuntimeExampleError.errorTag})`
91-
: `(Error Code: ${msalRuntimeExampleError.errorCode}, Tag: ${msalRuntimeExampleError.errorTag})`;
91+
? `${msalRuntimeExampleError.errorContext} (Error Code: ${
92+
msalRuntimeExampleError.errorCode
93+
}, Tag: ${StringUtils.tagToString(
94+
msalRuntimeExampleError.errorTag
95+
)})`
96+
: `(Error Code: ${
97+
msalRuntimeExampleError.errorCode
98+
}, Tag: ${StringUtils.tagToString(
99+
msalRuntimeExampleError.errorTag
100+
)})`;
92101
const testNativeAuthError = new NativeAuthError(
93102
ErrorStatus[msalRuntimeExampleError.errorStatus],
94103
enhancedErrorContext,
@@ -672,7 +681,7 @@ if (process.platform === "win32") {
672681
scopes: testAuthenticationResult.scopes,
673682
correlationId: testCorrelationId,
674683
authority: testAuthenticationResult.authority,
675-
redirectUri: TEST_REDIRECTURI,
684+
redirectUri: "",
676685
};
677686

678687
const chooseRedirectUriMock = jest.spyOn(
@@ -1509,7 +1518,7 @@ if (process.platform === "win32") {
15091518
scopes: testAuthenticationResult.scopes,
15101519
correlationId: testCorrelationId,
15111520
authority: testAuthenticationResult.authority,
1512-
redirectUri: TEST_REDIRECTURI,
1521+
redirectUri: "",
15131522
};
15141523

15151524
const chooseRedirectUriMock = jest.spyOn(
@@ -2287,7 +2296,7 @@ if (process.platform === "win32") {
22872296
scopes: testAuthenticationResult.scopes,
22882297
correlationId: testCorrelationId,
22892298
authority: testAuthenticationResult.authority,
2290-
redirectUri: TEST_REDIRECTURI,
2299+
redirectUri: "",
22912300
};
22922301

22932302
const chooseRedirectUriMock = jest.spyOn(
@@ -2332,7 +2341,7 @@ if (process.platform === "win32") {
23322341
scopes: testAuthenticationResult.scopes,
23332342
correlationId: testCorrelationId,
23342343
authority: testAuthenticationResult.authority,
2335-
redirectUri: TEST_REDIRECTURI,
2344+
redirectUri: "",
23362345
};
23372346

23382347
const chooseRedirectUriMock = jest.spyOn(
@@ -2385,7 +2394,7 @@ if (process.platform === "win32") {
23852394
scopes: testAuthenticationResult.scopes,
23862395
correlationId: testCorrelationId,
23872396
authority: testAuthenticationResult.authority,
2388-
redirectUri: TEST_REDIRECTURI,
2397+
redirectUri: "",
23892398
};
23902399

23912400
const chooseRedirectUriMock = jest.spyOn(
@@ -2430,7 +2439,7 @@ if (process.platform === "win32") {
24302439
scopes: testAuthenticationResult.scopes,
24312440
correlationId: testCorrelationId,
24322441
authority: testAuthenticationResult.authority,
2433-
redirectUri: TEST_REDIRECTURI,
2442+
redirectUri: "",
24342443
};
24352444

24362445
const chooseRedirectUriMock = jest.spyOn(

‎extensions/msal-node-extensions/test/utils/StringUtils.test.ts‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
/*
2+
* Copyright (c) Microsoft Corporation. All rights reserved.
3+
* Licensed under the MIT License.
4+
*/
5+
6+
import { StringUtils } from "../../src/utils/StringUtils.js";
7+
8+
describe("StringUtils", () => {
9+
describe("tagToString tests", () => {
10+
it("Returns 'UNTAG' for tag value 0", () => {
11+
expect(StringUtils.tagToString(0)).toBe("UNTAG");
12+
});
13+
14+
it("Converts numeric tag to 5-character string", () => {
15+
const tag = 0x01234567;
16+
const result = StringUtils.tagToString(tag);
17+
18+
expect(result).toHaveLength(5);
19+
expect(typeof result).toBe("string");
20+
});
21+
22+
it("Produce same results for same input", () => {
23+
const tag = 0x12345678;
24+
const result1 = StringUtils.tagToString(tag);
25+
const result2 = StringUtils.tagToString(tag);
26+
27+
expect(result1).toBe(result2);
28+
});
29+
});
30+
});

‎lib/msal-node/apiReview/msal-node.api.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -325,7 +325,7 @@ export { InteractionRequiredAuthErrorCodes }
325325
export { InteractionRequiredAuthErrorMessage }
326326

327327
// @public
328-
export type InteractiveRequest = Partial<Omit<CommonAuthorizationUrlRequest, "scopes" | "redirectUri" | "requestedClaimsHash" | "storeInCache">> & {
328+
export type InteractiveRequest = Partial<Omit<CommonAuthorizationUrlRequest, "scopes" | "requestedClaimsHash" | "storeInCache">> & {
329329
openBrowser: (url: string) => Promise<void>;
330330
scopes?: Array<string>;
331331
successTemplate?: string;

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

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ export class PublicClientApplication
163163
...remainingProperties,
164164
clientId: this.config.auth.clientId,
165165
scopes: request.scopes || OIDC_DEFAULT_SCOPES,
166-
redirectUri: `${Constants.HTTP_PROTOCOL}${Constants.LOCALHOST}`,
166+
redirectUri: request.redirectUri || "",
167167
authority: request.authority || this.config.auth.authority,
168168
correlationId: correlationId,
169169
extraParameters: {
@@ -179,6 +179,9 @@ export class PublicClientApplication
179179
);
180180
}
181181

182+
if (request.redirectUri) {
183+
throw NodeAuthError.createRedirectUriNotSupportedError();
184+
}
182185
const { verifier, challenge } =
183186
await this.cryptoProvider.generatePkceCodes();
184187

@@ -258,7 +261,7 @@ export class PublicClientApplication
258261
...request,
259262
clientId: this.config.auth.clientId,
260263
scopes: request.scopes || OIDC_DEFAULT_SCOPES,
261-
redirectUri: `${Constants.HTTP_PROTOCOL}${Constants.LOCALHOST}`,
264+
redirectUri: request.redirectUri || "",
262265
authority: request.authority || this.config.auth.authority,
263266
correlationId: correlationId,
264267
extraParameters: {
@@ -271,6 +274,10 @@ export class PublicClientApplication
271274
return this.nativeBrokerPlugin.acquireTokenSilent(brokerRequest);
272275
}
273276

277+
if (request.redirectUri) {
278+
throw NodeAuthError.createRedirectUriNotSupportedError();
279+
}
280+
274281
return super.acquireTokenSilent(request);
275282
}
276283

0 commit comments

Comments
 (0)