Added support for Client Assertion (Workload identity federation)

[kratkyzobak]

Add support for https://docs.microsoft.com/en-us/azure/active-directory/develop/workload-identity-federation JWT tokens in azidentity package. These tokens are called assertions in AzureAD REST API and MSAL for Go library, so same name used in this merge request.

I was not able to create test to correctly auth using Assertion as configuration to Live AzureAD needs to be done.

• The purpose of this PR is explained in this or a referenced issue.
• The PR does not update generated files.
  • These files are managed by the codegen framework at Azure/autorest.go.
• Tests are included and/or updated for code changes.
• Updates to CHANGELOG.md are included.
• MIT license headers are included in each file.

[ghost]

Thank you for your contribution kratkyzobak! We will review the pull request and get back to you soon.

[ghost]

All CLA requirements met.

[chlowell]

Thanks for doing this! It looks pretty good at first glance. I'll have a closer look after I fix our CI. Just FYI, we're actually working on a design for this already; the feature is on our roadmap for a beta release around April.

[chlowell]

Thanks for doing this! CI is fixed on main, so if you rebase onto that, validation can pass.

The implementation looks good. However, because assertions can expire, our design would have NewClientAssertionCredential take a callback instead of a static string (see for example the Python and .NET implementations). For Go, I think we want a func (context.Context) (string, error). That's awkward to implement today because MSAL takes a static assertion and provides no way to update it. This credential would need to track assertion changes and create new clients as needed. That's feasible, and you're welcome to implement it if you like, but the effort may be better spent on MSAL. I opened AzureAD/microsoft-authentication-library-for-go#292 to track simplifying this in MSAL but don't know when it can be done. In any case this PR is close to what we want to release, so I think it's alright to leave it open for now and finish it up once we know more about MSAL's roadmap.

[kratkyzobak]

Thanks for pointing this out. I did not realize this problem and I would ran into issue if you would not write this, so thanks!

I would propose temporary solution (until MSAL will support it) to add call for "refetch assertion" inside this branch of code
https://github.com/Azure/azure-sdk-for-go/pull/16902/files#diff-6445e06e4b5109d2b036219e9c14ec64d3233e5d03460dd61bde062fbbd5ff29R72

If I understand it correctly, it's after original OAuth token expired and also probably assertion expired as login via credential failed. I would make it one try with refreshed token and try to call AcquireTokenByCredential again.

You suggested reacting to change of assertion itself, but I think, it's needed only to refetch current assertion only if previous assertion does not work anymore.

Is this solution ok for you? If so, I would make changes in code and update this PR.

[chlowell]

If I understand it correctly, it's after original OAuth token expired and also probably assertion expired as login via credential failed.

Access token and assertion lifetimes are independent.

You suggested reacting to change of assertion itself, but I think, it's needed only to refetch current assertion only if previous assertion does not work anymore.

That could work but in my imagination isn't much simpler. You would have to be careful to "retry" at most once per GetToken() call and only when Azure AD rejects a formerly valid assertion. There would also be untidy side effects, for example the credential would swallow errors from some failed authentications but the associated HTTP response would still be logged, perhaps causing users to pointlessly investigate.

[ghost]

Hi @kratkyzobak. Thank you for your interest in helping to improve the Azure SDK experience and for your contribution. We've noticed that there hasn't been recent engagement on this pull request. If this is still an active work stream, please let us know by pushing some changes or leaving a comment. Otherwise, we'll close this out in 7 days.

[ghost]

Hi @kratkyzobak. Thank you for your contribution. Since there hasn't been recent engagement, we're going to close this out. Feel free to respond with a comment containing "/reopen" if you'd like to continue working on these changes. Please be sure to use the command to reopen or remove the "no-recent-activity" label; otherwise, this is likely to be closed again with the next cleanup pass.