[java-opensaml] branch maint-4 updated: Add unrevoke method and clean up component calls.

Scott Cantor cantor.2 at osu.edu
Wed Aug 3 15:11:51 UTC 2022


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

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

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

The following commit(s) were added to refs/heads/maint-4 by this push:
     new 8bd7b6f61 Add unrevoke method and clean up component calls.
8bd7b6f61 is described below

commit 8bd7b6f6153104be9f95d75d439f2dd1256f01d0
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 | 50 ++++++++++++++++-
 .../opensaml/storage/impl/RevocationCacheTest.java | 65 +++++++++++++---------
 2 files changed, 87 insertions(+), 28 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 3b704b707..0fa615180 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
@@ -76,6 +76,7 @@ public class RevocationCache extends AbstractIdentifiableInitializableComponent
      */
     public void setEntryExpiration(@Positive final Duration entryExpiration) {
         ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
+        ComponentSupport.ifDestroyedThrowDestroyedComponentException(this);
         
         Constraint.isTrue(entryExpiration != null && !entryExpiration.isNegative() && !entryExpiration.isZero(),
                 "Revocation cache default entry expiration must be greater than 0");
@@ -98,6 +99,7 @@ public class RevocationCache extends AbstractIdentifiableInitializableComponent
      */
     public void setStorage(@Nonnull final StorageService storageService) {
         ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
+        ComponentSupport.ifDestroyedThrowDestroyedComponentException(this);
 
         storage = Constraint.isNotNull(storageService, "StorageService cannot be null");
         final StorageCapabilities caps = storage.getCapabilities();
@@ -122,6 +124,7 @@ public class RevocationCache extends AbstractIdentifiableInitializableComponent
      */
     public void setStrict(final boolean flag) {
         ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
+        ComponentSupport.ifDestroyedThrowDestroyedComponentException(this);
 
         strict = flag;
     }
@@ -129,6 +132,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");
         }
@@ -177,7 +182,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();
@@ -206,6 +213,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.
@@ -216,6 +256,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()) {
@@ -237,7 +280,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;
         }
@@ -260,6 +303,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()) {
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