Delete forceRefreshTime related content. (Azure#22637)

Author: Rujun Chen

sdk/keyvault/azure-security-keyvault-jca/src/main/java/com/azure/security/keyvault/jca/KeyVaultCertificates.java

@@ -39,11 +39,6 @@ public class KeyVaultCertificates implements AzureCertificates {
      */
     private Date lastRefreshTime;
 
-    /**
-     * Stores the last force refresh time.
-     */
-    private static volatile Date forceRefreshTime = new Date();
-
     private KeyVaultClient keyVaultClient;
 
     private final long refreshInterval;
@@ -88,13 +83,8 @@ boolean certificatesNeedRefresh() {
         if (keyVaultClient == null) {
             return false;
         }
-        if (lastRefreshTime == null || forceRefreshTime.after(lastRefreshTime)) {
-            return true;
-        }
-        if (refreshInterval > 0) {
-            return lastRefreshTime.getTime() + refreshInterval < new Date().getTime();
-        }
-        return false;
+        return refreshInterval > 0
+            && (lastRefreshTime == null || lastRefreshTime.getTime() + refreshInterval < new Date().getTime());
     }
 
     /**
@@ -132,14 +122,19 @@ public Map<String, Key> getCertificateKeys() {
 
     private void refreshCertificatesIfNeeded() {
         if (certificatesNeedRefresh()) { // Avoid acquiring the lock as much as possible.
-            refreshCertificates();
+            synchronized (this) {
+                if (certificatesNeedRefresh()) { // After obtaining the lock, avoid doing too many operations.
+                    refreshCertificates();
+                }
+            }
         }
     }
 
-    private synchronized void refreshCertificates() {
-        if (!certificatesNeedRefresh()) {
-            return; // After obtaining the lock, avoid doing too many operations.
-        }
+    /**
+     * Refresh certificates. Including certificates, aliases, certificate keys.
+     *
+     */
+    public synchronized void refreshCertificates() {
         // When refreshing certificates, the update of the 3 variables should be an atomic operation.
         aliases = keyVaultClient.getAliases();
         certificateKeys.clear();
@@ -166,7 +161,7 @@ private synchronized void refreshCertificates() {
      * @return certificate' alias if exist.
      */
     String refreshAndGetAliasByCertificate(Certificate certificate) {
-        updateForceRefreshTime();
+        refreshCertificates();
         return getCertificates().entrySet()
                                 .stream()
                                 .filter(entry -> certificate.equals(entry.getValue()))
@@ -190,11 +185,4 @@ public void deleteEntry(String alias) {
         certificateKeys.remove(alias);
     }
 
-    /**
-     * Overall refresh certificates' info
-     */
-    public static void updateForceRefreshTime() {
-        forceRefreshTime = new Date();
-    }
-
 }

sdk/keyvault/azure-security-keyvault-jca/src/main/java/com/azure/security/keyvault/jca/KeyVaultKeyStore.java

@@ -164,7 +164,7 @@ public Certificate engineGetCertificate(String alias) {
                                                  .orElse(null);
 
         if (refreshCertificatesWhenHaveUnTrustCertificate && certificate == null) {
-            KeyVaultCertificates.updateForceRefreshTime();
+            keyVaultCertificates.refreshCertificates();
             certificate = keyVaultCertificates.getCertificates().get(alias);
         }
         return certificate;

sdk/keyvault/azure-security-keyvault-jca/src/test/java/com/azure/security/keyvault/jca/KeyVaultCertificatesTest.java

@@ -26,13 +26,12 @@ public class KeyVaultCertificatesTest {
 
     @BeforeEach
     public void beforeEach() {
-
         List<String> aliases = new ArrayList<>();
         aliases.add("myalias");
         when(keyVaultClient.getAliases()).thenReturn(aliases);
         when(keyVaultClient.getKey("myalias", null)).thenReturn(key);
         when(keyVaultClient.getCertificate("myalias")).thenReturn(certificate);
-        keyVaultCertificates = new KeyVaultCertificates(0, keyVaultClient);
+        keyVaultCertificates = new KeyVaultCertificates(60_000, keyVaultClient);
     }
 
     @Test
@@ -55,9 +54,8 @@ public void testRefreshAndGetAliasByCertificate() {
         Assertions.assertEquals(keyVaultCertificates.refreshAndGetAliasByCertificate(certificate), "myalias");
         Assertions.assertEquals(keyVaultCertificates.getCertificates().get("myalias"), certificate);
         when(keyVaultClient.getAliases()).thenReturn(null);
-        // TODO (rujche) Fix this
-        // Assertions.assertNotEquals(keyVaultCertificates.refreshAndGetAliasByCertificate(certificate), "myalias");
-        // Assertions.assertNull(keyVaultCertificates.getCertificates().get("myalias"));
+        Assertions.assertNotEquals(keyVaultCertificates.refreshAndGetAliasByCertificate(certificate), "myalias");
+        Assertions.assertNull(keyVaultCertificates.getCertificates().get("myalias"));
     }
 
     @Test
@@ -67,17 +65,4 @@ public void testDeleteAlias() {
         Assertions.assertFalse(keyVaultCertificates.getAliases().contains("myalias"));
     }
 
-    @Test
-    public void testCertificatesNeedRefresh() throws InterruptedException {
-        keyVaultCertificates = new KeyVaultCertificates(1000, keyVaultClient);
-        Assertions.assertTrue(keyVaultCertificates.certificatesNeedRefresh());
-        keyVaultCertificates.getAliases();
-        Assertions.assertFalse(keyVaultCertificates.certificatesNeedRefresh());
-        KeyVaultCertificates.updateForceRefreshTime();
-        keyVaultCertificates.getAliases();
-        Assertions.assertFalse(keyVaultCertificates.certificatesNeedRefresh());
-        Thread.sleep(2000);
-        Assertions.assertTrue(keyVaultCertificates.certificatesNeedRefresh());
-    }
-
 }

sdk/keyvault/azure-security-test-keyvault-jca/src/test/java/com/azure/security/keyvault/jca/test/KeyVaultCertificatesTest.java

@@ -3,7 +3,6 @@
 
 package com.azure.security.keyvault.jca.test;
 
-import com.azure.security.keyvault.jca.KeyVaultCertificates;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.condition.EnabledIfEnvironmentVariable;
@@ -94,14 +93,4 @@ public void testCertificatesRefreshInterval() throws Exception {
         assertNotNull(keyStore.getCertificate(certificateName));
     }
 
-    @Test
-    public void testUpdateLastForceRefreshTime() throws Exception {
-        KeyStore keyStore = PropertyConvertorUtils.getKeyVaultKeyStore();
-        assertNotNull(keyStore.getCertificate(certificateName));
-        keyStore.deleteEntry(certificateName);
-        assertNull(keyStore.getCertificate(certificateName));
-        Thread.sleep(10);
-        KeyVaultCertificates.updateForceRefreshTime();
-        assertNotNull(keyStore.getCertificate(certificateName));
-    }
 }

sdk/spring/azure-spring-boot-starter-keyvault-certificates/README.md

@@ -281,40 +281,33 @@ spring:
           useInsecureTrustManager: true
 ```
 
-### Refresh certificates when have un trust certificate
+### Refresh certificate periodically
 
-When the inbound certificate is not trusted, the KeyVaultKeyStore can fetch 
-certificates from KeyVault if the following property is configured:
+KeyVaultKeyStore can fetch certificates from KeyVault periodically if following property is configured:
 
 ```yaml
 azure:
   keyvault:
     jca:
-      refresh-certificates-when-have-un-trust-certificate: true
+       certificates-refresh-interval: 1800000
 ```
 
-Note: If you set refresh-certificates-when-have-un-trust-certificate=true, your server will be vulnerable
-to attack, because every untrusted certificate will cause your application to send a re-acquire certificate request.
+Its value is 0(ms) by default, and certificate will not automatically refresh when its value <= 0.
 
-### Refresh certificate periodically
+### Refresh certificates when have un trust certificate
 
-KeyVaultKeyStore can fetch certificates from KeyVault periodically if following property is configured:
+When the inbound certificate is not trusted, the KeyVaultKeyStore can fetch 
+certificates from KeyVault if the following property is configured:
 
 ```yaml
 azure:
   keyvault:
     jca:
-       certificates-refresh-interval: 1800000
+      refresh-certificates-when-have-un-trust-certificate: true
 ```
 
-Its value is 0(ms) by default, and certificate will not automatically refresh when its value <= 0.
-
-### Refresh certificate by java code
-
-You can also manually refresh the certificate by calling this method:
-```java
-KeyVaultCertificates.refreshCertsInfo();
-```
+Note: If you set refresh-certificates-when-have-un-trust-certificate=true, your server will be vulnerable
+to attack, because every untrusted certificate will cause your application to send a re-acquire certificate request.
 
 ### Specific path certificates
 AzureKeyVault keystore will load certificates in the specific path: