Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[keyvault] add cae support #23543

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

gracewilcox
Copy link
Member

Closes https://github.com/Azure/azure-sdk-for-go-pr/issues/43

Adding CAE support to key vault libraries:

  • Upgrade to the latest version of azcore with CAE support
  • Add tests to the key vault challenge policy to ensure that it's compatible with CAE

@gracewilcox gracewilcox added KeyVault Client This issue points to a problem in the data-plane of the library. labels Oct 3, 2024
@gracewilcox gracewilcox marked this pull request as ready for review October 14, 2024 18:18
default:
t.Fatal("unexpected token request")
}
return azcore.AccessToken{Token: accessTk, ExpiresOn: time.Now().Add(time.Hour)}, nil
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please also check that the policy uses the correct access token after handling the CAE challenge? Something simple like this should do in the credential:

Suggested change
return azcore.AccessToken{Token: accessTk, ExpiresOn: time.Now().Add(time.Hour)}, nil
return azcore.AccessToken{Token: fmt.Sprint(tkReqs), ExpiresOn: time.Now().Add(time.Hour)}, nil

...then a predicate in the AppendResponse calls can compare the actual value of the Authorization header

Comment on lines +104 to +105
accessTk = "***"
kvChallenge = `Bearer authorization="https://login.microsoftonline.com/tenant", resource="https://vault.azure.net"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider sharing these constants with the test above

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Client This issue points to a problem in the data-plane of the library. KeyVault
Projects
Status: Untriaged
Development

Successfully merging this pull request may close these issues.

2 participants