diff --git a/change/@azure-msal-browser-e8d3cc67-0e49-4dfc-9d6b-fe1e3f0060b6.json b/change/@azure-msal-browser-e8d3cc67-0e49-4dfc-9d6b-fe1e3f0060b6.json new file mode 100644 index 0000000000..da37f7f936 --- /dev/null +++ b/change/@azure-msal-browser-e8d3cc67-0e49-4dfc-9d6b-fe1e3f0060b6.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "Add config option to not encode extra params", + "packageName": "@azure/msal-browser", + "email": "shylasummers@microsoft.com", + "dependentChangeType": "patch" +} diff --git a/change/@azure-msal-common-6bcb71b8-7cc2-4044-ad9b-3aa8ac0be8d9.json b/change/@azure-msal-common-6bcb71b8-7cc2-4044-ad9b-3aa8ac0be8d9.json new file mode 100644 index 0000000000..a851f15c3f --- /dev/null +++ b/change/@azure-msal-common-6bcb71b8-7cc2-4044-ad9b-3aa8ac0be8d9.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "Add config option to not encode extra params", + "packageName": "@azure/msal-common", + "email": "shylasummers@microsoft.com", + "dependentChangeType": "patch" +} diff --git a/change/@azure-msal-node-ce9f6cb7-c0a0-4379-a8f8-b975b6e9f8b8.json b/change/@azure-msal-node-ce9f6cb7-c0a0-4379-a8f8-b975b6e9f8b8.json new file mode 100644 index 0000000000..fe616d624e --- /dev/null +++ b/change/@azure-msal-node-ce9f6cb7-c0a0-4379-a8f8-b975b6e9f8b8.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "Add config option to not encode extra params", + "packageName": "@azure/msal-node", + "email": "shylasummers@microsoft.com", + "dependentChangeType": "patch" +} diff --git a/lib/msal-browser/apiReview/msal-browser.api.md b/lib/msal-browser/apiReview/msal-browser.api.md index d165adf709..74e3a3d4fb 100644 --- a/lib/msal-browser/apiReview/msal-browser.api.md +++ b/lib/msal-browser/apiReview/msal-browser.api.md @@ -446,6 +446,7 @@ export type BrowserAuthOptions = { supportsNestedAppAuth?: boolean; onRedirectNavigate?: (url: string) => boolean | void; instanceAware?: boolean; + encodeExtraQueryParams?: boolean; }; // Warning: (ae-missing-release-tag) "BrowserCacheLocation" is part of the package's API, but it is missing a release tag (@alpha, @beta, @public, or @internal) @@ -1785,7 +1786,7 @@ export type WrapperSKU = (typeof WrapperSKU)[keyof typeof WrapperSKU]; // src/cache/LocalStorage.ts:296:8 - (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen // src/cache/LocalStorage.ts:354:8 - (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen // src/cache/LocalStorage.ts:385:8 - (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen -// src/config/Configuration.ts:247:5 - (ae-forgotten-export) The symbol "InternalAuthOptions" needs to be exported by the entry point index.d.ts +// src/config/Configuration.ts:252:5 - (ae-forgotten-export) The symbol "InternalAuthOptions" needs to be exported by the entry point index.d.ts // src/event/EventHandler.ts:113:8 - (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen // src/event/EventHandler.ts:139:8 - (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen // src/index.ts:8:12 - (tsdoc-characters-after-block-tag) The token "@azure" looks like a TSDoc tag but contains an invalid character "/"; if it is not a tag, use a backslash to escape the "@" diff --git a/lib/msal-browser/src/config/Configuration.ts b/lib/msal-browser/src/config/Configuration.ts index 6c718ea907..91e19147c2 100644 --- a/lib/msal-browser/src/config/Configuration.ts +++ b/lib/msal-browser/src/config/Configuration.ts @@ -108,6 +108,11 @@ export type BrowserAuthOptions = { * Flag of whether the STS will send back additional parameters to specify where the tokens should be retrieved from. */ instanceAware?: boolean; + /** + * Flag of whether to encode query parameters + * @deprecated This flag is deprecated and will be removed in the next major version where all extra query params will be encoded by default. + */ + encodeExtraQueryParams?: boolean; }; /** @internal */ @@ -296,6 +301,7 @@ export function buildConfiguration( skipAuthorityMetadataCache: false, supportsNestedAppAuth: false, instanceAware: false, + encodeExtraQueryParams: false, }; // Default cache options for browser diff --git a/lib/msal-browser/src/protocol/Authorize.ts b/lib/msal-browser/src/protocol/Authorize.ts index 3dfeb35e95..775b7bc405 100644 --- a/lib/msal-browser/src/protocol/Authorize.ts +++ b/lib/msal-browser/src/protocol/Authorize.ts @@ -158,7 +158,12 @@ export async function getAuthCodeRequestUrl( request.extraQueryParameters || {} ); - return AuthorizeProtocol.getAuthorizeUrl(authority, parameters); + return AuthorizeProtocol.getAuthorizeUrl( + authority, + parameters, + config.auth.encodeExtraQueryParams, + request.extraQueryParameters + ); } /** @@ -195,7 +200,12 @@ export async function getEARForm( queryParams, request.extraQueryParameters || {} ); - const url = AuthorizeProtocol.getAuthorizeUrl(authority, queryParams); + const url = AuthorizeProtocol.getAuthorizeUrl( + authority, + queryParams, + config.auth.encodeExtraQueryParams, + request.extraQueryParameters + ); return createForm(frame, url, parameters); } diff --git a/lib/msal-common/apiReview/msal-common.api.md b/lib/msal-common/apiReview/msal-common.api.md index f4e3f10dca..957bd58cb9 100644 --- a/lib/msal-common/apiReview/msal-common.api.md +++ b/lib/msal-common/apiReview/msal-common.api.md @@ -628,6 +628,7 @@ export type AuthOptions = { azureCloudOptions?: AzureCloudOptions; skipAuthorityMetadataCache?: boolean; instanceAware?: boolean; + encodeExtraQueryParams?: boolean; }; // Warning: (ae-internal-missing-underscore) The name "Authority" should be prefixed with an underscore because the declaration is marked as @internal @@ -2312,7 +2313,7 @@ function getAuthorizationCodePayload(serverParams: AuthorizeResponse, cachedStat // Warning: (ae-missing-release-tag) "getAuthorizeUrl" is part of the package's API, but it is missing a release tag (@alpha, @beta, @public, or @internal) // // @public -function getAuthorizeUrl(authority: Authority, requestParameters: Map): string; +function getAuthorizeUrl(authority: Authority, requestParameters: Map, encodeParams?: boolean, extraQueryParameters?: StringDict | undefined): string; // Warning: (ae-missing-release-tag) "getClientAssertion" is part of the package's API, but it is missing a release tag (@alpha, @beta, @public, or @internal) // @@ -2949,7 +2950,7 @@ const logoutRequestEmpty = "logout_request_empty"; // Warning: (ae-missing-release-tag) "mapToQueryString" is part of the package's API, but it is missing a release tag (@alpha, @beta, @public, or @internal) // // @public -function mapToQueryString(parameters: Map): string; +function mapToQueryString(parameters: Map, encodeExtraParams?: boolean, extraQueryParameters?: StringDict): string; // Warning: (ae-missing-release-tag) "maxAgeTranspired" is part of the package's API, but it is missing a release tag (@alpha, @beta, @public, or @internal) // diff --git a/lib/msal-common/src/client/AuthorizationCodeClient.ts b/lib/msal-common/src/client/AuthorizationCodeClient.ts index ce00200387..0af2bdd5c1 100644 --- a/lib/msal-common/src/client/AuthorizationCodeClient.ts +++ b/lib/msal-common/src/client/AuthorizationCodeClient.ts @@ -507,6 +507,10 @@ export class AuthorizationCodeClient extends BaseClient { RequestParameterBuilder.addInstanceAware(parameters); } - return UrlUtils.mapToQueryString(parameters); + return UrlUtils.mapToQueryString( + parameters, + this.config.authOptions.encodeExtraQueryParams, + request.extraQueryParameters + ); } } diff --git a/lib/msal-common/src/config/ClientConfiguration.ts b/lib/msal-common/src/config/ClientConfiguration.ts index e3eecd1206..3d1864f6ad 100644 --- a/lib/msal-common/src/config/ClientConfiguration.ts +++ b/lib/msal-common/src/config/ClientConfiguration.ts @@ -83,6 +83,7 @@ export type CommonClientConfiguration = { * - skipAuthorityMetadataCache - A flag to choose whether to use or not use the local metadata cache during authority initialization. Defaults to false. * - instanceAware - A flag of whether the STS will send back additional parameters to specify where the tokens should be retrieved from. * - redirectUri - The redirect URI where authentication responses can be received by your application. It must exactly match one of the redirect URIs registered in the Azure portal. + * - encodeExtraQueryParams - A flag to choose whether to encode the extra query parameters or not. Defaults to false. * @internal */ export type AuthOptions = { @@ -93,6 +94,10 @@ export type AuthOptions = { azureCloudOptions?: AzureCloudOptions; skipAuthorityMetadataCache?: boolean; instanceAware?: boolean; + /** + * @deprecated This flag is deprecated and will be removed in the next major version where all extra query params will be encoded by default. + */ + encodeExtraQueryParams?: boolean; }; /** @@ -276,6 +281,7 @@ function buildAuthOptions(authOptions: AuthOptions): Required { azureCloudOptions: DEFAULT_AZURE_CLOUD_OPTIONS, skipAuthorityMetadataCache: false, instanceAware: false, + encodeExtraQueryParams: false, ...authOptions, }; } diff --git a/lib/msal-common/src/protocol/Authorize.ts b/lib/msal-common/src/protocol/Authorize.ts index 71fe91ed25..8d17420684 100644 --- a/lib/msal-common/src/protocol/Authorize.ts +++ b/lib/msal-common/src/protocol/Authorize.ts @@ -26,6 +26,7 @@ import { isInteractionRequiredError, } from "../error/InteractionRequiredAuthError.js"; import { ServerError } from "../error/ServerError.js"; +import { StringDict } from "../utils/MsalTypes.js"; /** * Returns map of parameters that are applicable to all calls to /authorize whether using PKCE or EAR @@ -264,10 +265,15 @@ export function getStandardAuthorizeRequestParameters( */ export function getAuthorizeUrl( authority: Authority, - requestParameters: Map + requestParameters: Map, + encodeParams?: boolean, + extraQueryParameters?: StringDict | undefined ): string { - const queryString = mapToQueryString(requestParameters); - + const queryString = mapToQueryString( + requestParameters, + encodeParams, + extraQueryParameters + ); return UrlString.appendQueryString( authority.authorizationEndpoint, queryString diff --git a/lib/msal-common/src/utils/UrlUtils.ts b/lib/msal-common/src/utils/UrlUtils.ts index 38a325d94e..7d972f596b 100644 --- a/lib/msal-common/src/utils/UrlUtils.ts +++ b/lib/msal-common/src/utils/UrlUtils.ts @@ -8,6 +8,7 @@ import { ClientAuthErrorCodes, createClientAuthError, } from "../error/ClientAuthError.js"; +import { StringDict } from "./MsalTypes.js"; /** * Parses hash string from given string. Returns empty string if no hash symbol is found. @@ -64,11 +65,23 @@ export function getDeserializedResponse( /** * Utility to create a URL from the params map */ -export function mapToQueryString(parameters: Map): string { +export function mapToQueryString( + parameters: Map, + encodeExtraParams: boolean = true, + extraQueryParameters?: StringDict +): string { const queryParameterArray: Array = new Array(); parameters.forEach((value, key) => { - queryParameterArray.push(`${key}=${encodeURIComponent(value)}`); + if ( + !encodeExtraParams && + extraQueryParameters && + key in extraQueryParameters + ) { + queryParameterArray.push(`${key}=${value}`); + } else { + queryParameterArray.push(`${key}=${encodeURIComponent(value)}`); + } }); return queryParameterArray.join("&"); diff --git a/lib/msal-common/test/request/RequestParameterBuilder.spec.ts b/lib/msal-common/test/request/RequestParameterBuilder.spec.ts index 14263fe3ba..244a4b3a02 100644 --- a/lib/msal-common/test/request/RequestParameterBuilder.spec.ts +++ b/lib/msal-common/test/request/RequestParameterBuilder.spec.ts @@ -230,6 +230,231 @@ describe("RequestParameterBuilder unit tests", () => { ).toBe(true); }); + it("Doesn't encode extra params by default", () => { + const parameters = new Map(); + RequestParameterBuilder.addResponseType( + parameters, + OAuthResponseType.CODE + ); + RequestParameterBuilder.addResponseMode( + parameters, + ResponseMode.FORM_POST + ); + RequestParameterBuilder.addScopes( + parameters, + TEST_CONFIG.DEFAULT_SCOPES + ); + RequestParameterBuilder.addClientId( + parameters, + TEST_CONFIG.MSAL_CLIENT_ID + ); + RequestParameterBuilder.addRedirectUri( + parameters, + TEST_URIS.TEST_REDIRECT_URI_LOCALHOST + ); + RequestParameterBuilder.addDomainHint( + parameters, + TEST_CONFIG.DOMAIN_HINT + ); + RequestParameterBuilder.addLoginHint( + parameters, + TEST_CONFIG.LOGIN_HINT + ); + RequestParameterBuilder.addClaims(parameters, TEST_CONFIG.CLAIMS, []); + RequestParameterBuilder.addCorrelationId( + parameters, + TEST_CONFIG.CORRELATION_ID + ); + RequestParameterBuilder.addPrompt( + parameters, + PromptValue.SELECT_ACCOUNT + ); + RequestParameterBuilder.addState(parameters, TEST_CONFIG.STATE); + RequestParameterBuilder.addNonce(parameters, TEST_CONFIG.NONCE); + RequestParameterBuilder.addCodeChallengeParams( + parameters, + TEST_CONFIG.TEST_CHALLENGE, + TEST_CONFIG.CODE_CHALLENGE_METHOD + ); + RequestParameterBuilder.addAuthorizationCode( + parameters, + TEST_TOKENS.AUTHORIZATION_CODE + ); + RequestParameterBuilder.addDeviceCode( + parameters, + DEVICE_CODE_RESPONSE.deviceCode + ); + RequestParameterBuilder.addCodeVerifier( + parameters, + TEST_CONFIG.TEST_VERIFIER + ); + RequestParameterBuilder.addGrantType( + parameters, + GrantType.DEVICE_CODE_GRANT + ); + RequestParameterBuilder.addSid(parameters, TEST_CONFIG.SID); + RequestParameterBuilder.addLogoutHint( + parameters, + TEST_CONFIG.LOGIN_HINT + ); + RequestParameterBuilder.addExtraQueryParameters(parameters, { + extra_params: "param1,param2", + }); + + const requestQueryString = UrlUtils.mapToQueryString( + parameters, + false, + { + extra_params: "param1,param2", + } + ); + expect( + requestQueryString.includes( + `${AADServerParamKeys.RESPONSE_TYPE}=${OAuthResponseType.CODE}` + ) + ).toBe(true); + expect( + requestQueryString.includes( + `${AADServerParamKeys.RESPONSE_MODE}=${encodeURIComponent( + ResponseMode.FORM_POST + )}` + ) + ).toBe(true); + expect( + requestQueryString.includes( + `${AADServerParamKeys.SCOPE}=${Constants.OPENID_SCOPE}%20${Constants.PROFILE_SCOPE}%20${Constants.OFFLINE_ACCESS_SCOPE}` + ) + ).toBe(true); + expect( + requestQueryString.includes( + `${AADServerParamKeys.CLIENT_ID}=${TEST_CONFIG.MSAL_CLIENT_ID}` + ) + ).toBe(true); + expect( + requestQueryString.includes( + `${AADServerParamKeys.REDIRECT_URI}=${encodeURIComponent( + TEST_URIS.TEST_REDIRECT_URI_LOCALHOST + )}` + ) + ).toBe(true); + expect( + requestQueryString.includes( + `${AADServerParamKeys.DOMAIN_HINT}=${encodeURIComponent( + TEST_CONFIG.DOMAIN_HINT + )}` + ) + ).toBe(true); + expect( + requestQueryString.includes( + `${AADServerParamKeys.LOGIN_HINT}=${encodeURIComponent( + TEST_CONFIG.LOGIN_HINT + )}` + ) + ).toBe(true); + expect( + requestQueryString.includes( + `${AADServerParamKeys.CLAIMS}=${encodeURIComponent( + TEST_CONFIG.CLAIMS + )}` + ) + ).toBe(true); + expect( + requestQueryString.includes( + `${AADServerParamKeys.CLIENT_REQUEST_ID}=${encodeURIComponent( + TEST_CONFIG.CORRELATION_ID + )}` + ) + ).toBe(true); + expect( + requestQueryString.includes( + `${AADServerParamKeys.PROMPT}=${PromptValue.SELECT_ACCOUNT}` + ) + ).toBe(true); + expect( + requestQueryString.includes( + `${AADServerParamKeys.STATE}=${encodeURIComponent( + TEST_CONFIG.STATE + )}` + ) + ).toBe(true); + expect( + requestQueryString.includes( + `${AADServerParamKeys.NONCE}=${encodeURIComponent( + TEST_CONFIG.NONCE + )}` + ) + ).toBe(true); + expect( + requestQueryString.includes( + `${AADServerParamKeys.CODE_CHALLENGE}=${encodeURIComponent( + TEST_CONFIG.TEST_CHALLENGE + )}` + ) + ).toBe(true); + expect( + requestQueryString.includes( + `${ + AADServerParamKeys.CODE_CHALLENGE_METHOD + }=${encodeURIComponent(TEST_CONFIG.CODE_CHALLENGE_METHOD)}` + ) + ).toBe(true); + expect( + requestQueryString.includes( + `${AADServerParamKeys.CODE}=${encodeURIComponent( + TEST_TOKENS.AUTHORIZATION_CODE + )}` + ) + ).toBe(true); + expect( + requestQueryString.includes( + `${AADServerParamKeys.DEVICE_CODE}=${encodeURIComponent( + DEVICE_CODE_RESPONSE.deviceCode + )}` + ) + ).toBe(true); + expect( + requestQueryString.includes( + `${AADServerParamKeys.CODE_VERIFIER}=${encodeURIComponent( + TEST_CONFIG.TEST_VERIFIER + )}` + ) + ).toBe(true); + expect( + requestQueryString.includes( + `${AADServerParamKeys.SID}=${encodeURIComponent( + TEST_CONFIG.SID + )}` + ) + ).toBe(true); + expect( + requestQueryString.includes( + `${AADServerParamKeys.LOGOUT_HINT}=${encodeURIComponent( + TEST_CONFIG.LOGIN_HINT + )}` + ) + ).toBe(true); + expect(requestQueryString.includes(`extra_params=param1,param2`)).toBe( + true + ); + }); + + it("Encodes extra params if encodeParams is true and extra params are passed in", () => { + const parameters = new Map(); + RequestParameterBuilder.addExtraQueryParameters(parameters, { + extra_params: "param1,param2", + }); + + const requestQueryString = UrlUtils.mapToQueryString(parameters, true, { + extra_params: "param1,param2", + }); + + expect( + requestQueryString.includes( + `extra_params=${encodeURIComponent("param1,param2")}` + ) + ).toBe(true); + }); + it("Adds token type and req_cnf correctly for proof-of-possession tokens", () => { const parameters = new Map(); RequestParameterBuilder.addPopToken( diff --git a/lib/msal-node/apiReview/msal-node.api.md b/lib/msal-node/apiReview/msal-node.api.md index a35aa7a913..3112bd94cd 100644 --- a/lib/msal-node/apiReview/msal-node.api.md +++ b/lib/msal-node/apiReview/msal-node.api.md @@ -452,6 +452,7 @@ export type NodeAuthOptions = { protocolMode?: ProtocolMode; azureCloudOptions?: AzureCloudOptions; skipAuthorityMetadataCache?: boolean; + encodeExtraQueryParams?: boolean; }; // @public diff --git a/lib/msal-node/src/config/Configuration.ts b/lib/msal-node/src/config/Configuration.ts index b7b7fa12ce..3c744d7e0a 100644 --- a/lib/msal-node/src/config/Configuration.ts +++ b/lib/msal-node/src/config/Configuration.ts @@ -31,6 +31,7 @@ import { NodeAuthError } from "../error/NodeAuthError.js"; * - clientCertificate - Certificate that the application uses when requesting a token. Only used in confidential client applications. Requires hex encoded X.509 SHA-1 or SHA-256 thumbprint of the certificate, and the PEM encoded private key (string should contain -----BEGIN PRIVATE KEY----- ... -----END PRIVATE KEY----- ) * - protocolMode - Enum that represents the protocol that msal follows. Used for configuring proper endpoints. * - skipAuthorityMetadataCache - A flag to choose whether to use or not use the local metadata cache during authority initialization. Defaults to false. + * - encodeExtraQueryParams - A flag to choose whether to encode extra query parameters in the request URL. Defaults to false. * @public */ export type NodeAuthOptions = { @@ -55,6 +56,10 @@ export type NodeAuthOptions = { protocolMode?: ProtocolMode; azureCloudOptions?: AzureCloudOptions; skipAuthorityMetadataCache?: boolean; + /** + * @deprecated This flag is deprecated and will be removed in the next major version where all extra query params will be encoded by default. + */ + encodeExtraQueryParams?: boolean; }; /** @@ -154,6 +159,7 @@ const DEFAULT_AUTH_OPTIONS: Required = { tenant: Constants.EMPTY_STRING, }, skipAuthorityMetadataCache: false, + encodeExtraQueryParams: false, }; const DEFAULT_CACHE_OPTIONS: CacheOptions = { diff --git a/lib/msal-node/src/protocol/Authorize.ts b/lib/msal-node/src/protocol/Authorize.ts index f790b16b75..5d69e70c59 100644 --- a/lib/msal-node/src/protocol/Authorize.ts +++ b/lib/msal-node/src/protocol/Authorize.ts @@ -65,5 +65,10 @@ export function getAuthCodeRequestUrl( request.extraQueryParameters || {} ); - return AuthorizeProtocol.getAuthorizeUrl(authority, parameters); + return AuthorizeProtocol.getAuthorizeUrl( + authority, + parameters, + config.auth.encodeExtraQueryParams, + request.extraQueryParameters + ); } diff --git a/package-lock.json b/package-lock.json index 57ce375a98..b4573897c3 100644 --- a/package-lock.json +++ b/package-lock.json @@ -44259,7 +44259,7 @@ } }, "node_modules/vite": { - "version": "4.5.12", + "version": "4.5.13", "resolved": "https://identitydivision.pkgs.visualstudio.com/fac9d424-53d2-45c0-91b5-ef6ba7a6bf26/_packaging/dd15892d-fc68-4d1c-93a5-090f3b303f31/npm/registry/vite/-/vite-4.5.12.tgz", "integrity": "sha1-SPSNvPeJcidl6RvDKpnLZsYo6tw=", "dev": true,