client_assertion can accept a callback

[rayluo]

• client_assertion can accept a callback.
• Also expose a AutoRefresher and document their usage. The new doc is staged here, just search the keyword "Supporting raw assertion obtained from elsewhere"
• Deprecate client_assertion as a string

This will close #705 and close #746

[bgavrilMS]

Suggested change

	"Please use a callback instead of a string. See also "
	"Use a callback instead of a string. MSAL will invoke the callback every time it needs to fetch a token from the identity provider. The implementer is responsible for returning non-expired assertion and for caching assertions. See also "

[rayluo]

Will accept this suggestion in my next commit.

We might as well also append "... with the help of a helper AutoRefresher. See also ...".

I'm providing more explanation about the AutoRefresher in the last comment below.

[bgavrilMS]

Can you add claims as input to this callback? This will help implementers of FIC perform token revocation, where the claims need to be take into account by the assertion provider as well.

May be separate PR, but want to make sure adding parametewrs to this is not a breaking change.

[rayluo]

An adjustable callback can be created as a class, so that it can maintain its state. The crux is that the MSAL code path calling this callback does not know what proper values to feed in. So, the app developer - or a higher-level API - will need to expect an assertion error from eSTS, and then adjust that callback instance accordingly.

class AdjustsableCallback:
    extra_claims = None

    def callback(self):
        return "A new assertion based on extra claims: " + self.extra_claims

ac = AdjustableCallback()
cca = ConfidentialClientApplication(..., client_credential={"client_assertion": ac.callback})

result = cca.acquire_token_for_client(...)
if result.get("error"):
    ac.extra_claims = get_new_claims_somehow()
    result = cca.acquire_token_for_client(...)

Do you want me to also document this advance scenario into the docs this time?

[bgavrilMS]

The claims have to flow from MSAL's acquire_token_for_client call into the callback. An example would be good.

Here's the equivalent .NET code

// singleton
var cca = ConfidentialCLientApp(...)
                .WithClientAssertion(
                        (input) => {
                              string claims = input.Claims; 
                              GetAssertionFromMsiOrWhatever(claims);
                  });


// later in the code
var authResult = cca.acquireTokenForClient();  // ok, callback invoked, claims is empty

// call downstream API
// 401 + WWW-AUthenticate returned
// parse header and extract claims

// get new token with claims challenge
authResult = cca.acquireTokenForClient(claims); // ok, callback is invoked, claims is not empty

[bgavrilMS]

Not sure what this auto refresher is. We should mention FIC scenarios in the docs instead.

[rayluo]

Not sure what this auto refresher is.

Naming is hard, and new name suggestions are welcome (perhaps CallbackCache instead?). But the functionality behind the "auto refresher" was needed since day 1, because even when in the legit authenticated-by-a-cert scenario, MSAL Python internally has (1) a helper function to use that cert to create a short-lived assertion, and (2) a helper class AutoRefresher to cache the callback outcome and only call the callback again after a predefined expiry time elapsed. Such a cache behavior is desirable for the #2 scenario described in this comment.

In this PR, we repurpose the #1 callback behavior to accept any callback; and expose the #2 cache behavior so that app developers don't need to reinvent the wheel.

We should mention FIC scenarios in the docs instead.

I suppose, the #1 callback can be used in any scenario including FIC, but the #2 cache concept shall always be applicable.

[bgavrilMS]

FIC example would be good. You can use example from here: https://review.learn.microsoft.com/en-us/identity/microsoft-identity-platform/federated-identity-credentials?branch=main

To fix the problem, we should avoid logging the sensitive secret_name directly. Instead, we can log a generic message that indicates the presence of a secret without revealing its actual value. This approach maintains the usefulness of the log for debugging purposes while protecting sensitive information.

• Replace the logging statement that logs secret_name with a more generic message.
• Ensure that the new log message does not contain any sensitive information.

To fix the problem, we should avoid logging the sensitive raw_url directly. Instead, we can log a sanitized version of the URL that does not include the sensitive parts. One approach is to log only the non-sensitive components of the URL, such as the scheme, netloc, and path, while omitting the query parameters and fragment.

• Modify the logging statement on line 1051 to exclude sensitive information.
• Introduce a helper function to sanitize the URL before logging.

[Robbie-Microsoft]

Does msal Python utilize CI pipeline? Can you get these test certs from the lab instead of committing them here?

[rayluo]

Those certs are not certs for any real services. We just created some dummy certs for testing the cert loading code path. Thanks for your review, though. :)

[rayluo]

Note: As an aftermath of all commits being rewritten, this PR currently contains +16,009 −1,044 lines of changes from 250 commits happened in last 2 years, plus a merge conflict. This is un-review-able. Please do not review until I fix it.