[java-opensaml] 01/02: Add unrevoke method and clean up component calls.

Scott Cantor cantor.2 at osu.edu
Wed Aug 3 15:16:55 UTC 2022


This is an automated email from the git hooks/post-receive script.

scantor pushed a commit to branch main
in repository java-opensaml.

View the commit online:
http://git.shibboleth.net/view/?p=java-opensaml.git;a=commit;h=e44e5386ab3d0efa64a50fd14615c5022ee19e49

commit e44e5386ab3d0efa64a50fd14615c5022ee19e49
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Wed Aug 3 11:11:48 2022 -0400

    Add unrevoke method and clean up component calls.
---
 .../java/org/opensaml/storage/RevocationCache.java | 49 +++++++++++++++-
 .../opensaml/storage/impl/RevocationCacheTest.java | 65 +++++++++++++---------
 2 files changed, 85 insertions(+), 29 deletions(-)

diff --git a/opensaml-storage-api/src/main/java/org/opensaml/storage/RevocationCache.java b/opensaml-storage-api/src/main/java/org/opensaml/storage/RevocationCache.java
index b4e0c7f7c..bad643e91 100644
--- a/opensaml-storage-api/src/main/java/org/opensaml/storage/RevocationCache.java
+++ b/opensaml-storage-api/src/main/java/org/opensaml/storage/RevocationCache.java
@@ -128,6 +128,8 @@ public class RevocationCache extends AbstractIdentifiableInitializableComponent
     /** {@inheritDoc} */
     @Override
     public void doInitialize() throws ComponentInitializationException {
+        super.doInitialize();
+        
         if (storage == null) {
             throw new ComponentInitializationException("StorageService cannot be null");
         }
@@ -176,7 +178,9 @@ public class RevocationCache extends AbstractIdentifiableInitializableComponent
      */
     public synchronized boolean revoke(@Nonnull @NotEmpty final String context, @Nonnull @NotEmpty final String s,
             @Nonnull @NotEmpty final String value, @Nonnull final Duration exp) {
-
+        ComponentSupport.ifNotInitializedThrowUninitializedComponentException(this);
+        ComponentSupport.ifDestroyedThrowDestroyedComponentException(this);
+        
         final String key;
 
         final StorageCapabilities caps = storage.getCapabilities();
@@ -205,6 +209,39 @@ public class RevocationCache extends AbstractIdentifiableInitializableComponent
             return !strict;
         }
     }
+    
+    /**
+     * Remove a revocation record.
+     * 
+     * @param context a context label to subdivide the cache
+     * @param s value to remove
+     * 
+     * @return true iff a record was removed
+     * 
+     * @since 4.3.0
+     */
+    public synchronized boolean unrevoke(@Nonnull @NotEmpty final String context, @Nonnull @NotEmpty final String s) {
+        ComponentSupport.ifNotInitializedThrowUninitializedComponentException(this);
+        ComponentSupport.ifDestroyedThrowDestroyedComponentException(this);
+
+        final String key;
+        final StorageCapabilities caps = storage.getCapabilities();
+        if (context.length() > caps.getContextSize()) {
+            log.error("context {} too long for StorageService (limit {})", context, caps.getContextSize());
+            return false;
+        } else if (s.length() > caps.getKeySize()) {
+            key = DigestUtils.sha1Hex(s);
+        } else {
+            key = s;
+        }
+
+        try {
+            return storage.delete(context, key);
+        } catch (final IOException e) {
+            log.error("Exception writing to storage service", e);
+            return false;
+        }
+    }
 
     /**
      * Returns true iff the value has been revoked.
@@ -215,6 +252,9 @@ public class RevocationCache extends AbstractIdentifiableInitializableComponent
      * @return true iff the check value is found in the cache
      */
     public synchronized boolean isRevoked(@Nonnull @NotEmpty final String context, @Nonnull @NotEmpty final String s) {
+        ComponentSupport.ifNotInitializedThrowUninitializedComponentException(this);
+        ComponentSupport.ifDestroyedThrowDestroyedComponentException(this);
+
         final String key;
         final StorageCapabilities caps = storage.getCapabilities();
         if (context.length() > caps.getContextSize()) {
@@ -236,7 +276,7 @@ public class RevocationCache extends AbstractIdentifiableInitializableComponent
             log.debug("Entry '{}' is revoked", s);
             return true;
         } catch (final IOException e) {
-            log.error("Exception reading/writing to storage service, indicating {}",
+            log.error("Exception reading  storage service, indicating {}",
                     strict ? "revoked" : "not revoked", e);
             return strict;
         }
@@ -259,6 +299,9 @@ public class RevocationCache extends AbstractIdentifiableInitializableComponent
      */
     @Nullable @NotEmpty public synchronized String getRevocationRecord(@Nonnull @NotEmpty final String context,
             @Nonnull @NotEmpty final String s) throws IOException {
+        ComponentSupport.ifNotInitializedThrowUninitializedComponentException(this);
+        ComponentSupport.ifDestroyedThrowDestroyedComponentException(this);
+
         final String key;
         final StorageCapabilities caps = storage.getCapabilities();
         if (context.length() > caps.getContextSize()) {
@@ -289,4 +332,4 @@ public class RevocationCache extends AbstractIdentifiableInitializableComponent
         }
     }
     
-}
\ No newline at end of file
+}
diff --git a/opensaml-storage-impl/src/test/java/org/opensaml/storage/impl/RevocationCacheTest.java b/opensaml-storage-impl/src/test/java/org/opensaml/storage/impl/RevocationCacheTest.java
index f245c83a4..b423af25a 100644
--- a/opensaml-storage-impl/src/test/java/org/opensaml/storage/impl/RevocationCacheTest.java
+++ b/opensaml-storage-impl/src/test/java/org/opensaml/storage/impl/RevocationCacheTest.java
@@ -17,6 +17,8 @@
 
 package org.opensaml.storage.impl;
 
+import static org.testng.Assert.*;
+
 import java.io.IOException;
 import java.nio.charset.Charset;
 import java.time.Duration;
@@ -30,7 +32,6 @@ import net.shibboleth.utilities.java.support.component.ComponentInitializationEx
 import net.shibboleth.utilities.java.support.logic.ConstraintViolationException;
 
 import org.testng.annotations.BeforeMethod;
-import org.testng.Assert;
 import org.opensaml.storage.RevocationCache;
 
 /**
@@ -51,6 +52,7 @@ public class RevocationCacheTest {
         storageService.initialize();
         
         revocationCache = new RevocationCache();
+        revocationCache.setId("test");
         revocationCache.setEntryExpiration(Duration.ofMillis(500));
         revocationCache.setStorage(storageService);
         revocationCache.initialize();
@@ -67,14 +69,14 @@ public class RevocationCacheTest {
         revocationCache = new RevocationCache();
         try {
             revocationCache.setStorage(null);
-            Assert.fail("Null StorageService should have caused constraint violation");
+            fail("Null StorageService should have caused constraint violation");
         } catch (final Exception e) {
         }
 
         try {
             revocationCache.setStorage(new ClientStorageService());
             
-            Assert.fail("ClientStorageService should have caused constraint violation");
+            fail("ClientStorageService should have caused constraint violation");
         } catch (final Exception e) {
         }
     }
@@ -82,12 +84,13 @@ public class RevocationCacheTest {
     
     @Test
     public void testStrictSetter() throws ComponentInitializationException {
-        Assert.assertFalse(revocationCache.isStrict());
+        assertFalse(revocationCache.isStrict());
         revocationCache = new RevocationCache();
+        revocationCache.setId("test");
         revocationCache.setStorage(storageService);
         revocationCache.setStrict(true);
         revocationCache.initialize();
-        Assert.assertTrue(revocationCache.isStrict());
+        assertTrue(revocationCache.isStrict());
     }
     
     @Test (expectedExceptions = ConstraintViolationException.class)
@@ -99,14 +102,14 @@ public class RevocationCacheTest {
     
     @Test 
     public void testStorageGetter() {
-        Assert.assertEquals(storageService, revocationCache.getStorage());
+        assertEquals(storageService, revocationCache.getStorage());
     }
     
     @Test 
     public void testRevocationSuccess() {
-        Assert.assertFalse(revocationCache.isRevoked("context", "item"));
-        Assert.assertTrue(revocationCache.revoke("context", "item"));
-        Assert.assertTrue(revocationCache.isRevoked("context", "item"));
+        assertFalse(revocationCache.isRevoked("context", "item"));
+        assertTrue(revocationCache.revoke("context", "item"));
+        assertTrue(revocationCache.isRevoked("context", "item"));
     }
     
     @Test 
@@ -118,15 +121,16 @@ public class RevocationCacheTest {
         storageService.initialize();
         
         revocationCache = new RevocationCache();
+        revocationCache.setId("test");
         revocationCache.setStorage(storageService);
         revocationCache.initialize();
         
         final byte[] array = new byte[storageService.getCapabilities().getContextSize()*2];
         new Random().nextBytes(array);
         final String context = new String(array, Charset.forName("UTF-8"));
-        Assert.assertTrue(context.length()>storageService.getCapabilities().getContextSize());
-        Assert.assertTrue(revocationCache.isRevoked(context, "item"));
-        Assert.assertFalse(revocationCache.revoke(context, "item"));
+        assertTrue(context.length()>storageService.getCapabilities().getContextSize());
+        assertTrue(revocationCache.isRevoked(context, "item"));
+        assertFalse(revocationCache.revoke(context, "item"));
     }
     
     @Test 
@@ -137,38 +141,47 @@ public class RevocationCacheTest {
         storageService.setKeySize(50);
         storageService.initialize();
         revocationCache = new RevocationCache();
+        revocationCache.setId("test");
         revocationCache.setStorage(storageService);
         revocationCache.initialize();
         final byte[] array = new byte[storageService.getCapabilities().getKeySize()*2];
         new Random().nextBytes(array);
         final String item = new String(array, Charset.forName("UTF-8"));
-        Assert.assertTrue(item.length()>storageService.getCapabilities().getKeySize());
-        Assert.assertFalse(revocationCache.isRevoked("context", item));
-        Assert.assertTrue(revocationCache.revoke("context", item));
-        Assert.assertTrue(revocationCache.isRevoked("context", item));
+        assertTrue(item.length()>storageService.getCapabilities().getKeySize());
+        assertFalse(revocationCache.isRevoked("context", item));
+        assertTrue(revocationCache.revoke("context", item));
+        assertTrue(revocationCache.isRevoked("context", item));
     }
     
     @Test 
     public void testRevocationExpirationSuccess() throws InterruptedException {
         //Test expiration of entry (500ms)
-        Assert.assertFalse(revocationCache.isRevoked("context", "item"));
-        Assert.assertTrue(revocationCache.revoke("context", "item"));
+        assertFalse(revocationCache.isRevoked("context", "item"));
+        assertTrue(revocationCache.revoke("context", "item"));
         Thread.sleep(600L);
-        Assert.assertFalse(revocationCache.isRevoked("context", "item"));
+        assertFalse(revocationCache.isRevoked("context", "item"));
         //Test rolling window, second revoke updates expiration past original 500ms
-        Assert.assertTrue(revocationCache.revoke("context", "item"));
+        assertTrue(revocationCache.revoke("context", "item"));
         Thread.sleep(300L);
-        Assert.assertTrue(revocationCache.revoke("context", "item"));
+        assertTrue(revocationCache.revoke("context", "item"));
         Thread.sleep(300L);
-        Assert.assertTrue(revocationCache.isRevoked("context", "item"));
+        assertTrue(revocationCache.isRevoked("context", "item"));
     }
 
     @Test
     public void testRevokedRecordFetch() throws IOException {
-        Assert.assertTrue(revocationCache.revoke("context", "item", "value", Duration.ofHours(1)));
-        Assert.assertNull(revocationCache.getRevocationRecord("context", "item2"));
-        Assert.assertEquals(revocationCache.getRevocationRecord("context", "item"), "value");
+        assertTrue(revocationCache.revoke("context", "item", "value", Duration.ofHours(1)));
+        assertNull(revocationCache.getRevocationRecord("context", "item2"));
+        assertEquals(revocationCache.getRevocationRecord("context", "item"), "value");
         
     }
-    
+
+    @Test
+    public void testRevokedRecordDelete() throws IOException {
+        assertTrue(revocationCache.revoke("context", "item", "value", Duration.ofHours(1)));
+        assertEquals(revocationCache.getRevocationRecord("context", "item"), "value");
+        assertTrue(revocationCache.unrevoke("context", "item"));
+        assertNull(revocationCache.getRevocationRecord("context", "item"));
+    }
+
 }
\ No newline at end of file

-- 
To stop receiving notification emails like this one, please contact
the administrator of this repository.


More information about the commits mailing list