InteractionRequiredAuthError doesn't include the claims when implementing conditional access

salman90 commented 1 year ago

Core Library

MSAL Node (@azure/msal-node)

Core Library Version

1.14.5

Wrapper Library

N/A

Wrapper Library Version

0

Public or Confidential Client?

Confidential

Description

When trying to Implement conditional access (Require MFA) in a ConfidentialClientApplication using the acquireTokenOnBehalfOf API, we are not receiving the claims in the error object to acquire a new access token with the required claims.

{
  "errorCode": "invalid_grant",
  "errorMessage": "AADSTS50076: Due to a configuration change made by your administrator, or because you moved to a new location, you must use multi-factor authentication to access ''<recourse-id>.\r\nTrace ID: <app-id>\r\nCorrelation ID: <Correlation-ID>\r\nTimestamp: 2022-12-19 22:37:41Z",
  "subError": "basic_action",
  "name": "InteractionRequiredAuthError"
}

When looking at the raw response on fiddler, we receive the claims from the token endpoint as shown below:

{
    "error": "invalid_grant",
    "error_description": "AADSTS50076: Due to a configuration change made by your administrator, or because you moved to a new location, you must use multi-factor authentication to access '<recourse-id>'.\r\nTrace ID: <Trace-id>\r\nCorrelation ID: <Correlation-id>\r\nTimestamp: 2022-12-19 22:36:37Z",
    "error_codes": [
        50076
    ],
    "timestamp": "2022-12-19 22:36:37Z",
    "trace_id": "trace_id",
    "correlation_id": "correlation_id",
    "error_uri": "https://login.microsoftonline.com/error?code=50076",
    "suberror": "basic_action",
    "claims": "{\"access_token\":{\"capolids\":{\"essential\":true,\"values\":[\"GUID\"]}}}"
}

The issue is when instantiating the InteractionRequiredAuthError class, the claims are not passed to the class. Is there a way to receive the complete error response from msal?

Error Message

{
  "errorCode": "invalid_grant",
  "errorMessage": "AADSTS50076: Due to a configuration change made by your administrator, or because you moved to a new location, you must use multi-factor authentication to access ''<recourse-id>.\r\nTrace ID: <app-id>\r\nCorrelation ID: <Correlation-ID>\r\nTimestamp: 2022-12-19 22:37:41Z",
  "subError": "basic_action",
  "name": "InteractionRequiredAuthError"
}

Msal Logs

[Tue, 20 Dec 2022 01:28:53 GMT] : @azure/msal-node@1.14.5 : Info - acquireTokenOnBehalfOf called [Tue, 20 Dec 2022 01:28:53 GMT] : [ed732fe7-13fd-4e60-a6e4-719fd554968f] : @azure/msal-common@9.0.1 : Info - SilentFlowClient:acquireCachedToken - No access token found in cache for the given properties.

MSAL Configuration

const msalConfig = {
    auth: {
        clientId: config.credentials.clientID,
        authority: `https://${config.metadata.authority}/${config.credentials.tenantID}`,
        clientSecret: config.credentials.clientSecret,
        clientCapabilities: ["CP1"],
    },
    system: {
        proxyUrl: "http://localhost:8866",
        loggerOptions: {
            loggerCallback(loglevel, message, containsPii) {
                console.log(message);
            },
            piiLoggingEnabled: false,
            logLevel: msal.LogLevel.Info,
        },
    },
};

Relevant Code Snippets

const msal = require('@azure/msal-node');
const config = require('./authConfig');

const msalConfig = {
    auth: {
        clientId: config.credentials.clientID,
        authority: `https://${config.metadata.authority}/${config.credentials.tenantID}`,
        clientSecret: config.credentials.clientSecret,
        clientCapabilities: ["CP1"],
    },
    system: {
        proxyUrl: "http://localhost:8866",
        loggerOptions: {
            loggerCallback(loglevel, message, containsPii) {
                console.log(message);
            },
            piiLoggingEnabled: false,
            logLevel: msal.LogLevel.Info,
        },
    },
};

// Create msal application object
const cca = new msal.ConfidentialClientApplication(msalConfig);

const getOboToken = async (oboAssertion) => {
    const oboRequest = {
        oboAssertion: oboAssertion,
        scopes: config.resources.downstreamAPI.scopes,
    };

    try {
        const response = await cca.acquireTokenOnBehalfOf(oboRequest);
        return response.accessToken;
    } catch (error) {
        console.log(error)
        throw error;
    }
};

Reproduction Steps

Create a ConfidentialClientApplication.
Use acquireTokenOnBehalfOf API to get an access token for a recourse.
Create a CA policy to enable MFA for the recourse.
Check the InteractionRequiredAuthError.

Expected Behavior

Receiving the claims with the InteractionRequiredAuthError to acquire an access token.

Identity Provider

Azure AD / MSA

Browsers Affected (Select all that apply)

Chrome

Regression

No response

Source

Internal (Microsoft)

tnorling commented 1 year ago

I think this is only relevant for OBO so I'll let @bgavrilMS address this

bgavrilMS commented 1 year ago

Thanks @tnorling . This is about exposing "Claims" field that comes with AAD errors. This is confidential client specific, it covers public client flows as well.

The flow is:

AAD signals that "a more powerful" token is required (when redeeming an RT or when doing OBO). They send back error invalid_grant + claims
MSAL should throw appropriate error (UserInteractionRequired) and expose the claims sent by AAD
app developer calls AcquireTokenInteractive / AcquireTokenByOBO and adds claims
MSAL bypasses the cache and fetches new token

There seems to be several issues related to handling claims and CAE in MSAL JS:

bgavrilMS commented 1 year ago

@Robbie-Microsoft - I think it makes sense to tackle claim handling next, as it blocks scenarios like CAE

tnorling commented 1 year ago

Thanks @tnorling . This is about exposing "Claims" field that comes with AAD errors. This is confidential client specific, it covers public client flows as well.

@bgavrilMS Fixing this will likely address it for all flows anyway but I am curious, what is the public client scenario where AAD would throw claims back to the caller rather than just handling it on their own? My thought was that the absence of claims on the error object is only an issue for OBO because the caller in that scenario is not capable of invoking interaction on its own. I'm not sure I understand why a claims challenge from AAD (rather than a resource) would be necessary in public clients?

bgavrilMS commented 1 year ago

Thanks @tnorling . This is about exposing "Claims" field that comes with AAD errors. This is confidential client specific, it covers public client flows as well.

@bgavrilMS Fixing this will likely address it for all flows anyway but I am curious, what is the public client scenario where AAD would throw claims back to the caller rather than just handling it on their own? My thought was that the absence of claims on the error object is only an issue for OBO because the caller in that scenario is not capable of invoking interaction on its own. I'm not sure I understand why a claims challenge from AAD (rather than a resource) would be necessary in public clients?

Excellent question. Apart from OBO there are 2 more scenarios documented here: https://learn.microsoft.com/en-us/azure/active-directory/develop/v2-conditional-access-dev-guide#scenario-app-accessing-multiple-services - one is around SPAs and one is "accessing multiple services" - the docs mention a web app calling web apis, but this can very well be a mobile app accessing multiple web apis. CC @jmprieur to keep me honest on this.

tnorling commented 1 year ago

@bgavrilMS Thanks for sharing that. After reading through it, it's still not clear to me why the claims are important to the caller in a web app scenario (I also don't see the claims called out explicitly like they are in the OBO section).

Let's ignore silent calls for a moment, if I make an interactive token request in a web app and AAD determines that claims are needed I would expect it to present any prompts it needs to and then return the token with the needed claims. In what scenario would it throw the claims back to the caller and expect the caller to make another interactive request with the claims it just threw?

Now if we agree that AAD should "do the right thing" in an interactive request regardless of whether or not claims are included, then the claims aren't really relevant in a web app at all, even if they are returned in a silent request error, since the "interaction_required" error is enough information for the caller to know what to do to resolve it.

bgavrilMS commented 1 year ago

I am not sure about this point @tnorling , hoping that @jmprieur or @salman90 can clarify. It may be a limitation of CA?

The docs and diagrams do show that AAD will reply with an error + claims and that there is an expectation for the app developer to include the claims in a new token request.

ghost commented 1 year ago

@salman90 This issue has been automatically marked as stale because it is marked as requiring author feedback but has not had any activity for 5 days. If your issue has been resolved please let us know by closing the issue. If your issue has not been resolved please leave a comment to keep this open. It will be closed automatically in 7 days if it remains stale.

derisen commented 1 year ago

Just to chime in, so far we didn't run into an issue with any of the CA scenarios we tried using a public client -since STS knows which claims are required, it knows how to get a token with those claims. So @tnorling is right on that account.

Coming back to the silent calls however, since msal.js caches/distinguishes tokens on the basis of claims in addition to scopes and claims are needed in silent requests (as of msal-common@v6), what happens if devs want to retrieve a token from the cache with claims as opposed to going to the network? (my memory is failing me here, as we tried most CA scenarios before this change -@salman90 would be good check this out)

On a more general note, I'm not sure if the SDKs should hide something that the platform itself choses to disclose. If the fact that the claims will be exposed to the app is shared knowledge, there might be features in future that rely on this (though perhaps this is a YAGNI case..)

salman90 commented 1 year ago

@bgavrilMS @tnorling I tried both scenarios: SPA calling CA-protected web API using MSAL.js and a web app accessing multiple services (one service is CA protected). In both cases, an interaction_required is enough for the CA to work as expected; as @tnorling said, Azure AD does the right thing with or without a claim.

@derisen in the SPA case, we are not hiding anything because the claim is not sent with the interaction_required error from the auth server. I still need to confirm that the same behavior is happening in the web app scenario. I will debug the app further and confirm.

salman90 commented 1 year ago

@bgavrilMS @tnorling I finish debugging the web app scenario; we do receive the claims in the error object:

{"error":"invalid_grant","error_description":"AADSTS50076: Due to a configuration change made by your administrator, or because you moved to a new location, you must use multi-factor authentication to access 'GUID'.\r\nTrace ID: 64ab9ad5-447a-432c-ba42-83784ad85800\r\nCorrelation ID: GUID\r\nTimestamp: 2023-01-04 19:44:18Z","error_codes":[50076],"timestamp":"2023-01-04 19:44:18Z","trace_id":"GUID","correlation_id":"GUID","error_uri":"https://login.microsoftonline.com/error?code=50076","suberror":"basic_action","claims":"{\"access_token\":{\"capolids\":{\"essential\":true,\"values\":[\"GUID\"]}}}"}

The scenario works fine with just an interaction, but I think @derisen is correct that we shouldn't hide something (claims) that the platform provides.

bgavrilMS commented 1 year ago

I agree, we need to expose the claims as part of the exception data. We have been doing this for years in ADAL and MSAL .NET

tnorling commented 1 year ago

Agreed, if it's returned from the service we should return it as well. I started this discussion mostly for my own education so thanks for indulging me. To summarize my understanding of the above: we do not have a functional miss in PCAs today and addressing this bug would just serve to future-proof or add completeness to our interaction_required errors (and address a gap in OBO scenarios).

bgavrilMS commented 1 year ago

@Robbie-Microsoft - please pick this up next.

AzureAD / microsoft-authentication-library-for-js