From d8dae65780ecda2032a08be24bf4ab7debab52a7 Mon Sep 17 00:00:00 2001 From: Bill Wert Date: Wed, 31 Jan 2024 15:17:42 -0800 Subject: [PATCH] Move cache miss errors to info level (#38502) Customer reports spurious error messages for simple cache misses from MSAL. We'll catch those errors and log them at the INFO level instead of ERROR. This only applies to sync cases. In async cases we already suppress the error entirely. Fixes #38300 --- .../identity/ClientAssertionCredential.java | 6 ++-- .../identity/ClientCertificateCredential.java | 6 ++-- .../identity/ClientSecretCredential.java | 6 ++-- .../azure/identity/DeviceCodeCredential.java | 6 +++- .../InteractiveBrowserCredential.java | 6 +++- .../azure/identity/OnBehalfOfCredential.java | 6 ++-- .../identity/UsernamePasswordCredential.java | 6 +++- .../implementation/IdentitySyncClient.java | 36 +++++++++++++++---- 8 files changed, 61 insertions(+), 17 deletions(-) diff --git a/sdk/identity/azure-identity/src/main/java/com/azure/identity/ClientAssertionCredential.java b/sdk/identity/azure-identity/src/main/java/com/azure/identity/ClientAssertionCredential.java index b3fc2f8df28e..bf788fcad345 100644 --- a/sdk/identity/azure-identity/src/main/java/com/azure/identity/ClientAssertionCredential.java +++ b/sdk/identity/azure-identity/src/main/java/com/azure/identity/ClientAssertionCredential.java @@ -119,8 +119,10 @@ public Mono getToken(TokenRequestContext request) { public AccessToken getTokenSync(TokenRequestContext request) { try { AccessToken token = identitySyncClient.authenticateWithConfidentialClientCache(request); - LoggingUtil.logTokenSuccess(LOGGER, request); - return token; + if (token != null) { + LoggingUtil.logTokenSuccess(LOGGER, request); + return token; + } } catch (Exception e) { } try { diff --git a/sdk/identity/azure-identity/src/main/java/com/azure/identity/ClientCertificateCredential.java b/sdk/identity/azure-identity/src/main/java/com/azure/identity/ClientCertificateCredential.java index 1418b1d5e58f..471725f45fd8 100644 --- a/sdk/identity/azure-identity/src/main/java/com/azure/identity/ClientCertificateCredential.java +++ b/sdk/identity/azure-identity/src/main/java/com/azure/identity/ClientCertificateCredential.java @@ -148,8 +148,10 @@ public Mono getToken(TokenRequestContext request) { public AccessToken getTokenSync(TokenRequestContext request) { try { AccessToken token = identitySyncClient.authenticateWithConfidentialClientCache(request); - LoggingUtil.logTokenSuccess(LOGGER, request); - return token; + if (token != null) { + LoggingUtil.logTokenSuccess(LOGGER, request); + return token; + } } catch (Exception e) { } try { diff --git a/sdk/identity/azure-identity/src/main/java/com/azure/identity/ClientSecretCredential.java b/sdk/identity/azure-identity/src/main/java/com/azure/identity/ClientSecretCredential.java index a0c8619a6fa7..3450e1067666 100644 --- a/sdk/identity/azure-identity/src/main/java/com/azure/identity/ClientSecretCredential.java +++ b/sdk/identity/azure-identity/src/main/java/com/azure/identity/ClientSecretCredential.java @@ -126,8 +126,10 @@ public Mono getToken(TokenRequestContext request) { public AccessToken getTokenSync(TokenRequestContext request) { try { AccessToken token = identitySyncClient.authenticateWithConfidentialClientCache(request); - LoggingUtil.logTokenSuccess(LOGGER, request); - return token; + if (token != null) { + LoggingUtil.logTokenSuccess(LOGGER, request); + return token; + } } catch (Exception e) { } try { diff --git a/sdk/identity/azure-identity/src/main/java/com/azure/identity/DeviceCodeCredential.java b/sdk/identity/azure-identity/src/main/java/com/azure/identity/DeviceCodeCredential.java index 2b60d1ab7118..60fb3819f691 100644 --- a/sdk/identity/azure-identity/src/main/java/com/azure/identity/DeviceCodeCredential.java +++ b/sdk/identity/azure-identity/src/main/java/com/azure/identity/DeviceCodeCredential.java @@ -157,7 +157,11 @@ public Mono getToken(TokenRequestContext request) { public AccessToken getTokenSync(TokenRequestContext request) { if (cachedToken.get() != null) { try { - return identitySyncClient.authenticateWithPublicClientCache(request, cachedToken.get()); + MsalToken token = identitySyncClient.authenticateWithPublicClientCache(request, cachedToken.get()); + if (token != null) { + LoggingUtil.logTokenSuccess(LOGGER, request); + return token; + } } catch (Exception e) { } } try { diff --git a/sdk/identity/azure-identity/src/main/java/com/azure/identity/InteractiveBrowserCredential.java b/sdk/identity/azure-identity/src/main/java/com/azure/identity/InteractiveBrowserCredential.java index e11804a5e739..7d759b93e0e2 100644 --- a/sdk/identity/azure-identity/src/main/java/com/azure/identity/InteractiveBrowserCredential.java +++ b/sdk/identity/azure-identity/src/main/java/com/azure/identity/InteractiveBrowserCredential.java @@ -162,7 +162,11 @@ public Mono getToken(TokenRequestContext request) { public AccessToken getTokenSync(TokenRequestContext request) { if (cachedToken.get() != null) { try { - return identitySyncClient.authenticateWithPublicClientCache(request, cachedToken.get()); + MsalToken token = identitySyncClient.authenticateWithPublicClientCache(request, cachedToken.get()); + if (token != null) { + LoggingUtil.logTokenSuccess(LOGGER, request); + return token; + } } catch (Exception e) { } } try { diff --git a/sdk/identity/azure-identity/src/main/java/com/azure/identity/OnBehalfOfCredential.java b/sdk/identity/azure-identity/src/main/java/com/azure/identity/OnBehalfOfCredential.java index 263bba6a2e9f..4d369ea8d607 100644 --- a/sdk/identity/azure-identity/src/main/java/com/azure/identity/OnBehalfOfCredential.java +++ b/sdk/identity/azure-identity/src/main/java/com/azure/identity/OnBehalfOfCredential.java @@ -92,8 +92,10 @@ public Mono getToken(TokenRequestContext request) { public AccessToken getTokenSync(TokenRequestContext request) { try { AccessToken token = identitySyncClient.authenticateWithConfidentialClientCache(request); - LoggingUtil.logTokenSuccess(LOGGER, request); - return token; + if (token != null) { + LoggingUtil.logTokenSuccess(LOGGER, request); + return token; + } } catch (Exception e) { } try { diff --git a/sdk/identity/azure-identity/src/main/java/com/azure/identity/UsernamePasswordCredential.java b/sdk/identity/azure-identity/src/main/java/com/azure/identity/UsernamePasswordCredential.java index 2d77dc100d79..cfad0d11b119 100644 --- a/sdk/identity/azure-identity/src/main/java/com/azure/identity/UsernamePasswordCredential.java +++ b/sdk/identity/azure-identity/src/main/java/com/azure/identity/UsernamePasswordCredential.java @@ -127,7 +127,11 @@ public Mono getToken(TokenRequestContext request) { public AccessToken getTokenSync(TokenRequestContext request) { if (cachedToken.get() != null) { try { - return identitySyncClient.authenticateWithPublicClientCache(request, cachedToken.get()); + MsalToken token = identitySyncClient.authenticateWithPublicClientCache(request, cachedToken.get()); + if (token != null) { + LoggingUtil.logTokenSuccess(LOGGER, request); + return token; + } } catch (Exception e) { } } diff --git a/sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/IdentitySyncClient.java b/sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/IdentitySyncClient.java index 8ab4dfbb89f8..dbceb3b7a69e 100644 --- a/sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/IdentitySyncClient.java +++ b/sdk/identity/azure-identity/src/main/java/com/azure/identity/implementation/IdentitySyncClient.java @@ -166,7 +166,13 @@ public AccessToken authenticateWithManagedIdentityConfidentialClient(TokenReques } } - + /** + * Acquire a token from the confidential client. + * + * @param request the details of the token request + * @return An access token, or null if no token exists in the cache. + */ + @SuppressWarnings("deprecation") public AccessToken authenticateWithConfidentialClientCache(TokenRequestContext request) { ConfidentialClientApplication confidentialClientApplication = getConfidentialClientInstance(request).getValue(); SilentParameters.SilentParametersBuilder parametersBuilder = SilentParameters.builder(new HashSet<>(request.getScopes())) @@ -189,17 +195,23 @@ public AccessToken authenticateWithConfidentialClientCache(TokenRequestContext r } catch (MalformedURLException e) { throw LOGGER.logExceptionAsError(new RuntimeException(e.getMessage(), e)); } catch (ExecutionException | InterruptedException e) { - throw LOGGER.logExceptionAsError(new ClientAuthenticationException(e.getMessage(), null, e)); + // Cache misses should not throw an exception, but should log. + if (e.getMessage().contains("Token not found in the cache")) { + LOGGER.verbose("Token not found in the MSAL cache."); + return null; + } else { + throw LOGGER.logExceptionAsError(new ClientAuthenticationException(e.getMessage(), null, e)); + } } } /** - * Asynchronously acquire a token from the currently logged in client. + * Acquire a token from the currently logged in client. * * @param request the details of the token request * @param account the account used to log in to acquire the last token - * @return a Publisher that emits an AccessToken + * @return An access token, or null if no token exists in the cache. */ @SuppressWarnings("deprecation") public MsalToken authenticateWithPublicClientCache(TokenRequestContext request, IAccount account) { @@ -226,7 +238,13 @@ public MsalToken authenticateWithPublicClientCache(TokenRequestContext request, } catch (MalformedURLException e) { throw LOGGER.logExceptionAsError(new RuntimeException(e.getMessage(), e)); } catch (ExecutionException | InterruptedException e) { - throw LOGGER.logExceptionAsError(new ClientAuthenticationException(e.getMessage(), null, e)); + // Cache misses should not throw an exception, but should log. + if (e.getMessage().contains("Token not found in the cache")) { + LOGGER.verbose("Token not found in the MSAL cache."); + return null; + } else { + throw LOGGER.logExceptionAsError(new ClientAuthenticationException(e.getMessage(), null, e)); + } } SilentParameters.SilentParametersBuilder forceParametersBuilder = SilentParameters.builder( @@ -248,7 +266,13 @@ public MsalToken authenticateWithPublicClientCache(TokenRequestContext request, } catch (MalformedURLException e) { throw LOGGER.logExceptionAsError(new RuntimeException(e.getMessage(), e)); } catch (ExecutionException | InterruptedException e) { - throw LOGGER.logExceptionAsError(new ClientAuthenticationException(e.getMessage(), null, e)); + // Cache misses should not throw an exception, but should log. + if (e.getMessage().contains("Token not found in the cache")) { + LOGGER.verbose("Token not found in the MSAL cache."); + return null; + } else { + throw LOGGER.logExceptionAsError(new ClientAuthenticationException(e.getMessage(), null, e)); + } } }