[Identity] Correctly implement TokenCredential protocols

[Laia Patiño (laiapat)]

Description

Resolves #25175.

azure-identity credentials should all implement the TokenCredential protocol, but they technically don't at the moment. For both sync and async credentials, our get_token methods don't include all the parameters specified in the protocol. This PR update method signatures to match, and clarifies which parameters may be ignored in docstrings.

Validating live Key Vault pipeline run: https://dev.azure.com/azure-sdk/internal/_build/results?buildId=2958066&view=results

All SDK Contribution checklist:

• The pull request does not introduce [breaking changes]
• CHANGELOG is updated for new features, bug fixes or other significant changes.
• I have read the contribution guidelines.

General Guidelines and Best Practices

• Title of the pull request is clear and informative.
• There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For more information on cleaning up the commits in your PR, see this page.

Testing Guidelines

• Pull request includes test coverage for the included changes.

[Azure SDK Bot (azure-sdk)]

API change check

APIView has identified API level changes in this PR and created following API reviews.

azure-identity

[Xiang Yan (xiangyan99)]

Is it possible that claims/tenant_id are passed into transport?

(want to check if we need to add such protection in core)

[Laia Patiño (laiapat)]

Is it possible that claims/tenant_id are passed into transport?

(want to check if we need to add such protection in core)

This hasn't happened in live tests, but that doesn't give us full coverage of all credentials. I'm writing tests now to make sure the transport layer doesn't receive these.

EDIT: added in fc5410b

[Paul Van Eck (pvaneck)]

Thanks for doing this, @mccoyp! Just some minor doc fixes I think should go in.

With the recent support for CAE, all get_token methods for user credentials and service principal credentials should support claims (this is mainly any credential that uses AadClient or inherits MsalCredential, example of claims usage). So, I think it makes sense to update the docstrings for these credentials.

[Laia Patiño (laiapat)]

Xiang Yan (@xiangyan99) Paul Van Eck (@pvaneck) Do you think this can make it into next week's release?

[Paul Van Eck (pvaneck)]

I think it would be nice to have nice this in if possible provided we can get the additional API changes approved. One thing I noticed when looking at the apiview was the removal of the Any type from all the kwargs in get_token. What was the motivation for this? I see that the protocol method has the Any still.

[Laia Patiño (laiapat)]

I think it would be nice to have nice this in if possible provided we can get the additional API changes approved. One thing I noticed when looking at the apiview was the removal of the Any type from all the kwargs in get_token. What was the motivation for this? I see that the protocol method has the Any still.

I had removed them because of an impression that **kwargs should be left untyped, but a double-check with our static typing guide showed we should use Any. And that's true about the protocol's typing as well, so I just updated credential signatures to match 🙂