-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Support azure.keyvault.access-token in azure-security-keyvault-jca
#47850
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
base: main
Are you sure you want to change the base?
Conversation
…ken property Co-authored-by: rujche <171773178+rujche@users.noreply.github.com>
Co-authored-by: rujche <171773178+rujche@users.noreply.github.com>
Co-authored-by: rujche <171773178+rujche@users.noreply.github.com>
…mprove docs Co-authored-by: rujche <171773178+rujche@users.noreply.github.com>
...y-keyvault-jca/src/main/java/com/azure/security/keyvault/jca/KeyVaultLoadStoreParameter.java
Outdated
Show resolved
Hide resolved
…ameter Co-authored-by: rujche <171773178+rujche@users.noreply.github.com>
…w feedback Co-authored-by: rujche <171773178+rujche@users.noreply.github.com>
...eyvault-jca/src/main/java/com/azure/security/keyvault/jca/implementation/KeyVaultClient.java
Show resolved
Hide resolved
...eyvault-jca/src/main/java/com/azure/security/keyvault/jca/implementation/KeyVaultClient.java
Outdated
Show resolved
Hide resolved
…ble naming Co-authored-by: rujche <171773178+rujche@users.noreply.github.com>
…ed refresh attempts Co-authored-by: rujche <171773178+rujche@users.noreply.github.com>
...eyvault-jca/src/main/java/com/azure/security/keyvault/jca/implementation/KeyVaultClient.java
Outdated
Show resolved
Hide resolved
Co-authored-by: rujche <171773178+rujche@users.noreply.github.com>
...ult-jca/src/test/java/com/azure/security/keyvault/jca/implementation/KeyVaultClientTest.java
Show resolved
Hide resolved
|
/azp run java - keyvault tests |
|
No pipelines are associated with this pull request. |
azure.keyvault.access-token in azure-security-keyvault-jca
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request adds support for bearer token authentication via the new azure.keyvault.access-token system property to the Azure Key Vault JCA library. This enhancement enables users to provide pre-obtained access tokens for authentication without requiring client credentials, supporting multi-factor authentication scenarios.
Changes:
- Added bearer token authentication support with proper authentication priority: Managed Identity > Access Token > Client Credentials
- Implemented token lifecycle management where provided tokens don't auto-refresh, and expiration is handled by Azure returning authentication errors
- Consolidated and improved test coverage for Key Vault client authentication scenarios
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| sdk/keyvault/azure-security-keyvault-jca/src/main/java/com/azure/security/keyvault/jca/implementation/KeyVaultClient.java | Added providedAccessToken field and authentication priority logic in obtainAccessToken method; renamed getAccessTokenByHttpRequest to obtainAccessToken |
| sdk/keyvault/azure-security-keyvault-jca/src/main/java/com/azure/security/keyvault/jca/KeyVaultLoadStoreParameter.java | Added accessToken field and constructor parameter to support passing access tokens |
| sdk/keyvault/azure-security-keyvault-jca/src/main/java/com/azure/security/keyvault/jca/KeyVaultKeyStore.java | Updated to read azure.keyvault.access-token system property and pass to KeyVaultCertificates |
| sdk/keyvault/azure-security-keyvault-jca/src/main/java/com/azure/security/keyvault/jca/implementation/certificates/KeyVaultCertificates.java | Added accessToken parameter to constructors and updateKeyVaultClient methods |
| sdk/keyvault/azure-security-keyvault-jca/src/main/java/com/azure/security/keyvault/jca/implementation/model/AccessToken.java | Added constructor accepting accessToken and expiresIn parameters for creating AccessToken instances from provided tokens |
| sdk/keyvault/azure-security-keyvault-jca/src/test/java/com/azure/security/keyvault/jca/implementation/KeyVaultClientTest.java | Added comprehensive tests for access token authentication and authentication priority; consolidated integration tests from removed file |
| sdk/keyvault/azure-security-keyvault-jca/src/test/java/com/azure/security/keyvault/jca/KeyVaultClientTest.java | Removed duplicate test file; tests consolidated into implementation package |
| sdk/keyvault/azure-security-keyvault-jca/README.md | Added documentation for the new azure.keyvault.access-token property |
| sdk/keyvault/azure-security-keyvault-jca/CHANGELOG.md | Added feature entry describing bearer token authentication support |
|
|
||
| @EnabledIfEnvironmentVariable(named = "AZURE_KEYVAULT_CERTIFICATE_NAME", matches = "myalias") | ||
| @Test | ||
| public void testKeuVaultClients() { |
Copilot
AI
Jan 29, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The method name contains a typo. "Keu" should be "Key".
| public void testKeuVaultClients() { | |
| public void testKeyVaultClients() { |
| public void testAuthenticationPriority() { | ||
| try (MockedStatic<HttpUtil> httpUtilMockedStatic = Mockito.mockStatic(HttpUtil.class); | ||
| MockedStatic<AccessTokenUtil> tokenUtilMockedStatic = Mockito.mockStatic(AccessTokenUtil.class)) { | ||
|
|
||
| httpUtilMockedStatic.when(() -> HttpUtil.validateUri(anyString(), anyString())).thenCallRealMethod(); | ||
| httpUtilMockedStatic.when(() -> HttpUtil.addTrailingSlashIfRequired(anyString())).thenCallRealMethod(); | ||
|
|
||
| AccessToken accessToken = new AccessToken("fake-token", 3600); | ||
| tokenUtilMockedStatic.when(() -> AccessTokenUtil.getAccessToken(anyString(), anyString())) | ||
| .thenReturn(accessToken); | ||
|
|
||
| CertificateItem fakeCertificateItem = new CertificateItem(); | ||
| fakeCertificateItem.setId("certificates/fakeCertificateItem"); | ||
|
|
||
| CertificateListResult certificateListResult = new CertificateListResult(); | ||
| certificateListResult.setValue(Arrays.asList(fakeCertificateItem)); | ||
|
|
||
| String certificateListResultString = JsonConverterUtil.toJson(certificateListResult); | ||
| httpUtilMockedStatic.when(() -> HttpUtil.get(anyString(), anyMap())) | ||
| .thenReturn(certificateListResultString); | ||
|
|
||
| // Test 1: Managed Identity should take priority over access token | ||
| KeyVaultClient client1 | ||
| = new KeyVaultClient(KEY_VAULT_TEST_URI_GLOBAL, null, null, null, "managed-id", "bearer-token", false); | ||
| client1.getAliases(); | ||
| tokenUtilMockedStatic.verify(() -> AccessTokenUtil.getAccessToken(anyString(), eq("managed-id")), times(1)); | ||
|
|
||
| // Test 2: Access token should be used when managed identity is not set | ||
| KeyVaultClient client2 | ||
| = new KeyVaultClient(KEY_VAULT_TEST_URI_GLOBAL, null, null, null, null, "bearer-token", false); | ||
| List<String> result = client2.getAliases(); | ||
| assertEquals(1, result.size()); | ||
| assertTrue(result.contains("fakeCertificateItem")); | ||
| } | ||
| } |
Copilot
AI
Jan 29, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The authentication priority test only verifies two scenarios: managed identity over access token, and access token alone. However, it's missing a test case to verify that access token takes priority over client credentials (client ID/secret). Consider adding a third test case with client credentials present alongside an access token to ensure the access token is used instead of client credentials.
| @Test | ||
| public void testAccessTokenAuthentication() { | ||
| try (MockedStatic<HttpUtil> httpUtilMockedStatic = Mockito.mockStatic(HttpUtil.class)) { | ||
| httpUtilMockedStatic.when(() -> HttpUtil.validateUri(anyString(), anyString())).thenCallRealMethod(); | ||
| httpUtilMockedStatic.when(() -> HttpUtil.addTrailingSlashIfRequired(anyString())).thenCallRealMethod(); | ||
|
|
||
| CertificateItem fakeCertificateItem = new CertificateItem(); | ||
| fakeCertificateItem.setId("certificates/fakeCertificateItem"); | ||
|
|
||
| CertificateListResult certificateListResult = new CertificateListResult(); | ||
| certificateListResult.setValue(Arrays.asList(fakeCertificateItem)); | ||
|
|
||
| String certificateListResultString = JsonConverterUtil.toJson(certificateListResult); | ||
| httpUtilMockedStatic.when(() -> HttpUtil.get(anyString(), anyMap())) | ||
| .thenReturn(certificateListResultString); | ||
|
|
||
| // Create client with access token | ||
| String testAccessToken = "test-bearer-token-12345"; | ||
| KeyVaultClient keyVaultClient | ||
| = new KeyVaultClient(KEY_VAULT_TEST_URI_GLOBAL, null, null, null, null, testAccessToken, false); | ||
|
|
||
| List<String> result = keyVaultClient.getAliases(); | ||
|
|
||
| // Verify that the access token was used | ||
| assertEquals(1, result.size()); | ||
| assertTrue(result.contains("fakeCertificateItem")); | ||
| } | ||
| } |
Copilot
AI
Jan 29, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding a test case to verify that an empty string access token is treated as if no access token was provided. This would ensure the empty string validation at line 230 in KeyVaultClient.java works correctly and falls back to client credentials when available.
| @@ -51,10 +51,16 @@ public final class KeyVaultCertificates implements AzureCertificates { | |||
|
|
|||
| public KeyVaultCertificates(long refreshInterval, String keyVaultUri, String tenantId, String clientId, | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we remove the old one?
| @@ -75,10 +81,27 @@ public KeyVaultCertificates(long refreshInterval, KeyVaultClient keyVaultClient) | |||
| */ | |||
| public void updateKeyVaultClient(String keyVaultUri, String tenantId, String clientId, String clientSecret, | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we remove the old one?



Implementation Complete: ****** Authentication Support
Summary
This PR adds support for bearer token authentication via the new
azure.keyvault.access-tokensystem property. This enhancement enables users to:Changes Made
Core Implementation:
providedAccessTokenfield toKeyVaultClientwith proper authentication priority logicLong.MAX_VALUE / 1000(effectively infinite) to prevent futile auto-refresh attemptsKeyVaultLoadStoreParameterto accept and pass access tokens through the main constructorKeyVaultCertificatesandKeyVaultKeyStoreto support the new authentication methodAccessTokenmodel for creating instances from provided tokensgetAccessTokenByHttpRequesttoobtainAccessTokento accurately reflect its behaviorDocumentation:
Testing:
testAccessTokenAuthentication)testAuthenticationPriority)Authentication Priority
The implementation ensures the following priority order in code:
azure.keyvault.managed-identity) - Highest priorityazure.keyvault.access-token) - Middle priorityazure.keyvault.client-id+azure.keyvault.client-secret) - Lowest priorityToken Lifecycle
Provided access tokens do not auto-refresh. The token is set with effectively infinite expiration to prevent the code from attempting to refresh it. When the actual token expires on Azure's side, authentication requests will fail with runtime exceptions, which correctly informs users they need to provide a fresh token through the
azure.keyvault.access-tokensystem property.Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.