Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The MSAL Interceptor finds invalid resources from the endpoint if another match is found in "QueryString". #7111

Closed
2 tasks
SuperPanda911 opened this issue May 17, 2024 · 4 comments
Assignees
Labels
bug A problem that needs to be fixed for the feature to function as intended. msal-angular Related to @azure/msal-angular package public-client Issues regarding PublicClientApplications

Comments

@SuperPanda911
Copy link

Core Library

MSAL.js (@azure/msal-browser)

Core Library Version

3.0.15

Wrapper Library

MSAL Angular (@azure/msal-angular)

Wrapper Library Version

2.0.0

Public or Confidential Client?

Public

Description

Our Angular application uses multiple resources, each of which requires a separate token. We use "MsalInterceptorConfiguration" to specify these resources.
The order of the resources affects the reception of the error.

const resourceMap = new Map<string, Array<string>>();
resourceMap.set( 'https://MY_API_SITE_2', [ 'api://MY_API_SITE_2/as_user' ]);
resourceMap.set( 'https://MY_API_SITE_1', [ 'api://MY_API_SITE_1/as_user' ]);

Everything works fine until a request happens that contains two endpoints URLs in the request.
As an example, here is such a request:
https://MY_API_SITE_1/api/sites?$filter=siteUrl eq 'https://MY_API_SITE_2'

In the code, the request address is passed in full (req.url), and problems arise with this this.getScopesForEndpoint(req.url, req.method)

MSAL interceptor

intercept(req, next) {
        if (this.msalInterceptorConfig.interactionType !== InteractionType.Popup &&
            this.msalInterceptorConfig.interactionType !== InteractionType.Redirect) {
            throw new BrowserConfigurationAuthError("invalid_interaction_type", "Invalid interaction type provided to MSAL Interceptor. InteractionType.Popup, InteractionType.Redirect must be provided in the msalInterceptorConfiguration");
        }
        this.authService.getLogger().verbose("MSAL Interceptor activated");
        const scopes = this.getScopesForEndpoint(req.url, req.method);
        // If no scopes for endpoint, does not acquire token
        if (!scopes || scopes.length === 0) {
            this.authService
                .getLogger()
                .verbose("Interceptor - no scopes for endpoint");
            return next.handle(req);
        }
        // Sets account as active account or first account
        let account;
        if (!!this.authService.instance.getActiveAccount()) {
            this.authService
                .getLogger()
                .verbose("Interceptor - active account selected");
            account = this.authService.instance.getActiveAccount();
        }
        else {
            this.authService
                .getLogger()
                .verbose("Interceptor - no active account, fallback to first account");
            account = this.authService.instance.getAllAccounts()[0];
        }
        const authRequest = typeof this.msalInterceptorConfig.authRequest === "function"
            ? this.msalInterceptorConfig.authRequest(this.authService, req, {
                account: account,
            })
            : { ...this.msalInterceptorConfig.authRequest, account };
        this.authService
            .getLogger()
            .info(`Interceptor - ${scopes.length} scopes found for endpoint`);
        this.authService
            .getLogger()
            .infoPii(`Interceptor - [${scopes}] scopes found for ${req.url}`);
        return this.acquireToken(authRequest, scopes, account).pipe(switchMap((result) => {
            this.authService
                .getLogger()
                .verbose("Interceptor - setting authorization headers");
            const headers = req.headers.set("Authorization", `Bearer ${result.accessToken}`);
            const requestClone = req.clone({ headers });
            return next.handle(requestClone);
        }));
    }

Then in the "matchResourcesToEndpoint" method we additionally receive an unnecessary resource from protectedResourcesEndpoints because it's in the full URL endpoint

if (StringUtils.matchPattern(normalizedKey, endpoint)) {
  matchingResources.absoluteResources.push(key);
}

MSAL interceptor

    /**
     * Finds resource endpoints that match request endpoint
     * @param protectedResourcesEndpoints
     * @param endpoint
     * @returns
     */
    matchResourcesToEndpoint(protectedResourcesEndpoints, endpoint) {
        const matchingResources = {
            absoluteResources: [],
            relativeResources: [],
        };
        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();
            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);
            }
        });
        return matchingResources;
    }

Then I have a warning that more than one match was found allMatchedScopes and the matchScopesToEndpoint function returns me the first element of the array return allMatchedScopes[0].
And if the resource order doesn't match, then I don't get a token for the correct resource.

MSAL interceptor

/**
     * Finds scopes from first matching endpoint with HTTP method that matches request
     * @param protectedResourceMap Protected resource map
     * @param endpointArray Array of resources that match request endpoint
     * @param httpMethod Http method of the request
     * @returns
     */
    matchScopesToEndpoint(protectedResourceMap, endpointArray, httpMethod) {
        const allMatchedScopes = [];
        // Check each matched endpoint for matching HttpMethod and scopes
        endpointArray.forEach((matchedEndpoint) => {
            const scopesForEndpoint = [];
            const methodAndScopesArray = protectedResourceMap.get(matchedEndpoint);
            // Return if resource is unprotected
            if (methodAndScopesArray === null) {
                allMatchedScopes.push(null);
                return;
            }
            methodAndScopesArray.forEach((entry) => {
                // Entry is either array of scopes or ProtectedResourceScopes object
                if (typeof entry === "string") {
                    scopesForEndpoint.push(entry);
                }
                else {
                    // Ensure methods being compared are normalized
                    const normalizedRequestMethod = httpMethod.toLowerCase();
                    const normalizedResourceMethod = entry.httpMethod.toLowerCase();
                    // Method in protectedResourceMap matches request http method
                    if (normalizedResourceMethod === normalizedRequestMethod) {
                        // Validate if scopes comes null to unprotect the resource in a certain http method
                        if (entry.scopes === null) {
                            allMatchedScopes.push(null);
                        }
                        else {
                            entry.scopes.forEach((scope) => {
                                scopesForEndpoint.push(scope);
                            });
                        }
                    }
                }
            });
            // Only add to all scopes if scopes for endpoint and method is found
            if (scopesForEndpoint.length > 0) {
                allMatchedScopes.push(scopesForEndpoint);
            }
        });
        if (allMatchedScopes.length > 0) {
            if (allMatchedScopes.length > 1) {
                this.authService
                    .getLogger()
                    .warning("Interceptor - More than 1 matching scopes for endpoint found.");
            }
            // Returns scopes for first matching endpoint
            return allMatchedScopes[0];
        }
        return null;
    }

This may not be a very common problem, but it can occur when using multiple related resources and is very annoying.

Error Message

No response

MSAL Logs

No response

Network Trace (Preferrably Fiddler)

  • Sent
  • Pending

MSAL Configuration

function MSALInstanceFactory(config: MyMsalConfig): IPublicClientApplication {
  return new PublicClientApplication({
    auth: {
      clientId: config.clientId,
      authority: `${config.msLoginUrl}/${config.tenantId}`,
      redirectUri: config.redirectUri,
      postLogoutRedirectUri: config.redirectUri
    },
    cache: {
      cacheLocation: BrowserCacheLocation.LocalStorage,
      storeAuthStateInCookie: true,
    }
  });
}

function MSALInterceptorConfigFactory(): MsalInterceptorConfiguration {
  const resourceMap = new Map<string, Array<string>>();
  resourceMap.set( 'https://MY_API_SITE_2', [ 'api://MY_API_SITE_2/as_user' ]);
  resourceMap.set( 'https://MY_API_SITE_1', [ 'api://MY_API_SITE_1/as_user' ]);

  return {
    interactionType: InteractionType.Redirect,
    protectedResourceMap: resourceMap
  };
}

Relevant Code Snippets

/**
     * Finds resource endpoints that match request endpoint
     * @param protectedResourcesEndpoints
     * @param endpoint
     * @returns
     */
    matchResourcesToEndpoint(protectedResourcesEndpoints, endpoint) {
        const matchingResources = {
            absoluteResources: [],
            relativeResources: [],
        };
        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();
            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);
            }
        });
        return matchingResources;
    }

Reproduction Steps

  1. Use the default MSAL interceptor
  2. Configure the MSAL interceptor with the provider protectedResourceMap
  3. Do a request to first service https://MY_API_SITE_1 using a QueryString that contains a URL to the second resource https://MY_API_SITE_1/api/sites?$filter=siteUrl eq 'https://MY_API_SITE_2'
  4. Get a 401 error because the token contains the scope for service https://MY_API_SITE_2

Expected Behavior

When sending a request, the interceptor should not take into account the QueryString parameters, so that there are no incorrect matches when issuing tokens

Identity Provider

Entra ID (formerly Azure AD) / MSA

Browsers Affected (Select all that apply)

Chrome, Firefox, Edge, Safari, None (Server), Other

Regression

No response

Source

External (Customer)

@SuperPanda911 SuperPanda911 added bug-unconfirmed A reported bug that needs to be investigated and confirmed question Customer is asking for a clarification, use case or information. labels May 17, 2024
@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs: Attention 👋 Awaiting response from the MSAL.js team label May 17, 2024
@github-actions github-actions bot added msal-angular Related to @azure/msal-angular package msal-browser Related to msal-browser package public-client Issues regarding PublicClientApplications labels May 17, 2024
@jo-arroyo jo-arroyo self-assigned this May 22, 2024
@jo-arroyo jo-arroyo added bug A problem that needs to be fixed for the feature to function as intended. and removed question Customer is asking for a clarification, use case or information. msal-browser Related to msal-browser package bug-unconfirmed A reported bug that needs to be investigated and confirmed Needs: Attention 👋 Awaiting response from the MSAL.js team labels May 22, 2024
jo-arroyo added a commit that referenced this issue Jun 3, 2024
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
@jo-arroyo
Copy link
Collaborator

@SuperPanda911 Thanks for the catch. A fix for this bug has been merged and will be available in the next MSAL Angular release. Closing as completed.

@SuperPanda911
Copy link
Author

Thank you very much for the fix. I am waiting for release.

@geo242
Copy link

geo242 commented Jun 17, 2024

This change broke my app.

I had to change: protectedResourceMap.set('*/api/*', [...])

To this: protectedResourceMap.set('${window.location.origin}/api/*', [...])

It appears that wildcards for origin are no longer supported.

@MatthiasHuygelen
Copy link

This change broke my app.

I had to change: protectedResourceMap.set('*/api/*', [...])

To this: protectedResourceMap.set('${window.location.origin}/api/*', [...])

It appears that wildcards for origin are no longer supported.

Same

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A problem that needs to be fixed for the feature to function as intended. msal-angular Related to @azure/msal-angular package public-client Issues regarding PublicClientApplications
Projects
None yet
Development

No branches or pull requests

4 participants