[Feature Request] deprecate `data` out of `kwargs` and make SSHCertificate functionality public.

[jiasli]

MSAL client type

Public, Confidential

Problem Statement

data has been used for a long time for getting SSH certificates, but it is still not publicly documented and is part of kwargs:

microsoft-authentication-library-for-python/msal/application.py

Line 2106 in bb6d872

data = kwargs.pop("data", {})

microsoft-authentication-library-for-python/msal/application.py

Line 1560 in bb6d872

data = kwargs.get("data", {})

Azure CLI currently passes kwargs received by get_token() directly to MSAL. Since SDK is adding more keyword arguments, such as enable_cae (Azure/azure-sdk-for-python#37358), this will cause failure as enable_cae is not recognized by MSAL.

Azure CLI is considering making data an official keyword argument of get_token() so that only known arguments (scopes, claims_challenge, data) are passed to MSAL and kwargs received by get_token() is no longer directly passed to MSAL.

Proposed solution

Move data out of kwargs and make it a public keyword argument.

[jiasli]

Consider it another way: Since MSAL.NET has made getting SSH certificate a public interface via WithSSHCertificateAuthenticationScheme, MSAL.PY should do the same and have a similar public interface for getting SSH certificate.

3 years ago, when Azure CLI was migrated to MSAL, we had the assumption that kwargs received by get_token() was dedicated to MSAL and that's why we pass kwargs directly to MSAL. However, SDK keeps adding more arguments which are also received by kwargs in get_token(), thus breaking our assumption.

If MSAL exposed data as a public interface, we could have avoided the kwargs assumption from the very beginning.

[rayluo]

Proposed solution

Move data out of kwargs and make it a public keyword argument.

To summarize, Azure CLI is not blocked by the proposal above, and now the PR in Azure CLI has been merged.

I would suggest close this feature request as Won't-fix. We can revisit the topic of exposing the SSH Cert feature (when/if other apps also needs it), but its official api will unlikely be a generic data.

[ashok672]

@localden - We need to triage this tomorrow.

[bgavrilMS]

Consider it another way: Since MSAL.NET has made getting SSH certificate a public interface via WithSSHCertificateAuthenticationScheme, MSAL.PY should do the same and have a similar public interface for getting SSH certificate.

3 years ago, when Azure CLI was migrated to MSAL, we had the assumption that kwargs received by get_token() was dedicated to MSAL and that's why we pass kwargs directly to MSAL. However, SDK keeps adding more arguments which are also received by kwargs in get_token(), thus breaking our assumption.

If MSAL exposed data as a public interface, we could have avoided the kwargs assumption from the very beginning.

I am not aware of a single 3p customer using this. Only 1-2 internal teams use this for very specific sceanrios, i.e. provision Linux VMs. As such I do not think it qualifies for a fully fledged API. I am sure there is server telemetry for this.

In hindsight, it was a mistake to create an explicit public API in MSAL.NET without a good E2E scenario & docs & motivation.

[rayluo]

I am not aware of a single 3p customer using this. Only 1-2 internal teams use this for very specific sceanrios, i.e. provision Linux VMs. As such I do not think it qualifies for a fully fledged API.

Fully agreed. Also, in the case of SSH Certificate, there is only one internal team using it, and that is the Azure CLI team. The current undocumented API was designed for that, will continue to work, and I don't think it is worth becoming a documented public API.

[ashok672]

This is being tracked as a feature in ADO.