[Identity] Azure Identity should expose the original MSAL error

[jiasli]

Is your feature request related to a problem? Please describe.

When AuthenticationRequiredError is raised, the original MSAL result (error dict) is discarded:

azure-sdk-for-python/sdk/identity/azure-identity/azure/identity/_internal/interactive.py

Lines 198 to 202 in ef46a5c

	# if we get this far, result is either None or the content of an AAD error response
	if result:
	details = result.get("error_description") or result.get("error")
	raise AuthenticationRequiredError(scopes, error_details=details)
	raise AuthenticationRequiredError(scopes)

This makes aad_exception_handler (Azure/azure-cli#17072) impossible to handle MSAL error directly.

Describe the solution you'd like

AuthenticationRequiredError should have an attribute referring to the original MSAL result.

[chlowell]

What do you mean by "handle MSAL error directly"? It's azure.identity.AuthenticationRequiredError an application must handle, by calling credential.authenticate() or abandoning, perhaps temporarily, whatever action it needed authorization for.

[jiasli]

Only result['error_description'] is preserved by AuthenticationRequiredError, but there are actually more keys in result.

ADAL and MSAL returns dicts with the same structure.

Exposing the original error makes Azure CLI's error handler much easier, as the error handler can be used for

• Azure Identity
• MSAL
• ADAL

at the same time.

+ @rayluo

[rayluo]

Perhaps @jiasli can specify a smaller subset of those raw fields, so that @chlowell can consider satisfy that need yet still maintain a tight control on minimal exposure on those raw fields.

[jiasli]

We can't really tell which fields will be needed in the future.

If MSAL raised an Exception instead of an error dict, we would be supposed to use Exception Chaining (also proposed in Azure/azure-cli#16348 (comment)):

raise RuntimeError from exc

So I think the same should apply to the error dict.

[rayluo]

If MSAL raised an Exception instead of an error dict, we would be supposed to use Exception Chaining

From MSAL's perspective, an error response is equally likely as a successful response, so the return value is a dict in both cases. But feel free to wrap it inside an exception as you see fit.

By the way, that Exception Chaining was introduced since Python 3.3. This might not be a problem for Azure CLI as an application, but historically Azure Identity and MSAL work with Python 2.7 too.

specify a smaller subset of those raw fields

Maybe we can start with error and error_description, which are mentioned in the OAuth2 specs. I don't see them going away in the future. The correlation_id is an AAD practice that could also be helpful during troubleshooting, if the customer does not have other ways to obtain it.

[joshfree]

@chlowell would it be possible to patch this in an offcycle release for the CAE e2e private testing?

[jiasli]

I forgot error_description already has trace_id, correlation_id and timestamp:

AADSTS700082: The refresh token has expired due to inactivity. The token was issued on 2020-04-15T15:46:11.2854629Z and was inactive for 90.00:00:00.\r\nTrace ID: e8776bd1-3736-4a37-9535-d2c84015f900\r\nCorrelation ID: 2a52f32e-3bac-4bdf-83d3-7431d171baba\r\nTimestamp: 2021-03-08 17:22:18Z

So for Azure CLI's displaying purpose, we are good for now.

error and suberror may still need to be exposed, because as discussed with Azure PowerShell team, we will decide whether we need to instruct the user to run az login again based on the error (like interaction_required) and suberror.

---

As a side note:

It would still be better to expose trace_id, correlation_id and timestamp, rather than forcing the user to parse the error_description output. Reason:

• error_description containing those info is a coincidence.
• The format of error_description is undocumented, and is likely changed by service side at any time, without notice.
• Programmatic access would better use those info originally available in their dedicated fields.

As a conclusion:

1. It is OK to expose error_description as-is on the screen for human eyes. That's what error_description is defined for.
2. But parsing error_description get each info is not a good practice.

[chlowell]

As discussed offline, I've opened a PR to address this by attaching Azure AD's HTTP response to authentication exceptions whenever possible. That will provide all the information returned by MSAL after an auth failure while remaining consistent with exceptions raised by credentials that do not use MSAL.