From 8e4b664577672409573431df264307fb4864564d Mon Sep 17 00:00:00 2001 From: Jo Arroyo Date: Mon, 3 Jun 2024 09:50:33 -0700 Subject: [PATCH] Fix MSAL Angular MsalInterceptor bug matching to query string (#7137) This PR addresses a bug where the `MsalInterceptor` could match the protectedResource to the query string part of the URL instead of the host name and port part of the URL. It also refactors the code surrounding relative URLs. This addresses issue #7111 --- ...-f0ec2f9a-33cf-4b4e-ade9-0be40add58ed.json | 7 ++ .../src/msal.interceptor.config.ts | 5 -- lib/msal-angular/src/msal.interceptor.spec.ts | 46 ++++++++++- lib/msal-angular/src/msal.interceptor.ts | 78 +++++++++---------- 4 files changed, 89 insertions(+), 47 deletions(-) create mode 100644 change/@azure-msal-angular-f0ec2f9a-33cf-4b4e-ade9-0be40add58ed.json diff --git a/change/@azure-msal-angular-f0ec2f9a-33cf-4b4e-ade9-0be40add58ed.json b/change/@azure-msal-angular-f0ec2f9a-33cf-4b4e-ade9-0be40add58ed.json new file mode 100644 index 0000000000..369a8a594e --- /dev/null +++ b/change/@azure-msal-angular-f0ec2f9a-33cf-4b4e-ade9-0be40add58ed.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "Fix query string instead of HostNameAndPort bug in MsalInterceptor #7137", + "packageName": "@azure/msal-angular", + "email": "joarroyo@microsoft.com", + "dependentChangeType": "patch" +} diff --git a/lib/msal-angular/src/msal.interceptor.config.ts b/lib/msal-angular/src/msal.interceptor.config.ts index e899a81cb0..69d77a09b8 100644 --- a/lib/msal-angular/src/msal.interceptor.config.ts +++ b/lib/msal-angular/src/msal.interceptor.config.ts @@ -37,8 +37,3 @@ export type ProtectedResourceScopes = { httpMethod: string; scopes: Array | null; }; - -export type MatchingResources = { - absoluteResources: Array; - relativeResources: Array; -}; diff --git a/lib/msal-angular/src/msal.interceptor.spec.ts b/lib/msal-angular/src/msal.interceptor.spec.ts index 5b686e3c5e..4d61d74aff 100644 --- a/lib/msal-angular/src/msal.interceptor.spec.ts +++ b/lib/msal-angular/src/msal.interceptor.spec.ts @@ -57,6 +57,8 @@ function MSALInterceptorFactory(): MsalInterceptorConfiguration { string, Array | null >([ + ["https://MY_API_SITE_2", ["api://MY_API_SITE_2/as_user"]], + ["https://MY_API_SITE_1", ["api://MY_API_SITE_1/as_user"]], ["https://graph.microsoft.com/v1.0/me", ["user.read"]], ["relative/me", ["relative.scope"]], ["https://myapplication.com/user/*", ["customscope.read"]], @@ -906,9 +908,9 @@ describe("MsalInterceptor", () => { sampleAccountInfo, ]); - httpClient.get("http://site.com/relative/me").subscribe(); + httpClient.get("http://localhost:9876/relative/me").subscribe(); setTimeout(() => { - const request = httpMock.expectOne("http://site.com/relative/me"); + const request = httpMock.expectOne("http://localhost:9876/relative/me"); request.flush({ data: "test" }); expect(request.request.headers.get("Authorization")).toEqual( "Bearer access-token" @@ -1122,4 +1124,44 @@ describe("MsalInterceptor", () => { done(); }, 200); }); + + it("attaches authorization header with access token when endpoint match is in HostNameAndPort instead of query string", (done) => { + const spy = spyOn( + PublicClientApplication.prototype, + "acquireTokenSilent" + ).and.returnValue( + new Promise((resolve) => { + //@ts-ignore + resolve({ + accessToken: "access-token", + }); + }) + ); + + spyOn(PublicClientApplication.prototype, "getAllAccounts").and.returnValue([ + sampleAccountInfo, + ]); + + httpClient + .get( + "https://MY_API_SITE_1/api/sites?$filter=siteUrl eq 'https://MY_API_SITE_2'" + ) + .subscribe(); + + setTimeout(() => { + const request = httpMock.expectOne( + "https://MY_API_SITE_1/api/sites?$filter=siteUrl eq 'https://MY_API_SITE_2'" + ); + request.flush({ data: "test" }); + expect(request.request.headers.get("Authorization")).toEqual( + "Bearer access-token" + ); + expect(spy).toHaveBeenCalledWith({ + account: sampleAccountInfo, + scopes: ["api://MY_API_SITE_1/as_user"], + }); + httpMock.verify(); + done(); + }, 200); + }); }); diff --git a/lib/msal-angular/src/msal.interceptor.ts b/lib/msal-angular/src/msal.interceptor.ts index 28051c7384..de0b6928f1 100644 --- a/lib/msal-angular/src/msal.interceptor.ts +++ b/lib/msal-angular/src/msal.interceptor.ts @@ -18,7 +18,6 @@ import { InteractionStatus, InteractionType, StringUtils, - UrlString, } from "@azure/msal-browser"; import { Observable, EMPTY, of } from "rxjs"; import { switchMap, catchError, take, filter } from "rxjs/operators"; @@ -27,7 +26,6 @@ import { MsalInterceptorAuthRequest, MsalInterceptorConfiguration, ProtectedResourceScopes, - MatchingResources, } from "./msal.interceptor.config"; import { MsalBroadcastService } from "./msal.broadcast.service"; import { MSAL_INTERCEPTOR_CONFIG } from "./constants"; @@ -239,17 +237,10 @@ export class MsalInterceptor implements HttpInterceptor { normalizedEndpoint ); - // Check absolute urls of resources first before checking relative to prevent incorrect matching where multiple resources have similar relative urls - if (matchingProtectedResources.absoluteResources.length > 0) { + if (matchingProtectedResources.length > 0) { return this.matchScopesToEndpoint( this.msalInterceptorConfig.protectedResourceMap, - matchingProtectedResources.absoluteResources, - httpMethod - ); - } else if (matchingProtectedResources.relativeResources.length > 0) { - return this.matchScopesToEndpoint( - this.msalInterceptorConfig.protectedResourceMap, - matchingProtectedResources.relativeResources, + matchingProtectedResources, httpMethod ); } @@ -266,46 +257,53 @@ export class MsalInterceptor implements HttpInterceptor { private matchResourcesToEndpoint( protectedResourcesEndpoints: string[], endpoint: string - ): MatchingResources { - const matchingResources: MatchingResources = { - absoluteResources: [], - relativeResources: [], - }; + ): Array { + const matchingResources: Array = []; protectedResourcesEndpoints.forEach((key) => { - // Normalizes and adds resource to matchingResources.absoluteResources if key matches endpoint. StringUtils.matchPattern accounts for wildcards const normalizedKey = this.location.normalize(key); - if (StringUtils.matchPattern(normalizedKey, endpoint)) { - matchingResources.absoluteResources.push(key); - } - // Get url components for relative urls - const absoluteKey = this.getAbsoluteUrl(key); - const keyComponents = new UrlString(absoluteKey).getUrlComponents(); + // Get url components + const absoluteKey = this.getAbsoluteUrl(normalizedKey); + const keyComponents = new URL(absoluteKey); const absoluteEndpoint = this.getAbsoluteUrl(endpoint); - const endpointComponents = new UrlString( - absoluteEndpoint - ).getUrlComponents(); - - // Normalized key should include query strings if applicable - const relativeNormalizedKey = keyComponents.QueryString - ? `${keyComponents.AbsolutePath}?${keyComponents.QueryString}` - : this.location.normalize(keyComponents.AbsolutePath); - - // Add resource to matchingResources.relativeResources if same origin, relativeKey matches endpoint, and is not empty - if ( - keyComponents.HostNameAndPort === endpointComponents.HostNameAndPort && - StringUtils.matchPattern(relativeNormalizedKey, absoluteEndpoint) && - relativeNormalizedKey !== "" && - relativeNormalizedKey !== "/*" - ) { - matchingResources.relativeResources.push(key); + const endpointComponents = new URL(absoluteEndpoint); + + if (this.checkUrlComponents(keyComponents, endpointComponents)) { + matchingResources.push(key); } }); return matchingResources; } + /** + * Compares URL segments between key and endpoint + * @param key + * @param endpoint + * @returns + */ + private checkUrlComponents( + keyComponents: URL, + endpointComponents: URL + ): boolean { + // URL properties from https://developer.mozilla.org/en-US/docs/Web/API/URL + const urlProperties = ["protocol", "host", "pathname", "search", "hash"]; + + for (const property of urlProperties) { + if (keyComponents[property]) { + const decodedInput = decodeURIComponent(keyComponents[property]); + if ( + !StringUtils.matchPattern(decodedInput, endpointComponents[property]) + ) { + return false; + } + } + } + + return true; + } + /** * Transforms relative urls to absolute urls * @param url