[java-plugin-shibd] branch main updated: Unit tests and fixes to session cache flow.

Codeberg noreply at shibboleth.net
Sat Dec 13 01:09:11 UTC 2025


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

codeberg pushed a commit to branch main
in repository java-plugin-shibd.

View the commit online:
https://codeberg.org/Shibboleth/java-plugin-shibd/commit/28ea40c4734ad0f7fcd144438cb3220efa6959e9

The following commit(s) were added to refs/heads/main by this push:
     new 28ea40c  Unit tests and fixes to session cache flow.
28ea40c is described below

commit 28ea40c4734ad0f7fcd144438cb3220efa6959e9
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Fri Dec 12 18:06:21 2025 -0700

    Unit tests and fixes to session cache flow.
---
 .../shibboleth/sp/flows/SessionCacheFlowTest.java  | 486 +++++++++++++++++++++
 ...SPEnvironmentApplicationContextInitializer.java |   1 +
 .../sp/profile/impl/DoSessionCacheOperation.java   |  83 ++--
 3 files changed, 516 insertions(+), 54 deletions(-)

diff --git a/sp-conf-impl/src/test/java/net/shibboleth/sp/flows/SessionCacheFlowTest.java b/sp-conf-impl/src/test/java/net/shibboleth/sp/flows/SessionCacheFlowTest.java
new file mode 100644
index 0000000..79e3e7b
--- /dev/null
+++ b/sp-conf-impl/src/test/java/net/shibboleth/sp/flows/SessionCacheFlowTest.java
@@ -0,0 +1,486 @@
+/*
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package net.shibboleth.sp.flows;
+
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.time.Instant;
+
+import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
+
+import org.opensaml.profile.action.EventIds;
+import org.opensaml.storage.StorageRecord;
+import org.opensaml.storage.StorageService;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.beans.factory.annotation.Qualifier;
+import org.springframework.webflow.executor.FlowExecutionResult;
+import org.testng.Assert;
+import org.testng.annotations.AfterMethod;
+import org.testng.annotations.Test;
+
+import net.shibboleth.shared.codec.Base64Support;
+import net.shibboleth.shared.codec.DecodingException;
+import net.shibboleth.shared.codec.EncodingException;
+import net.shibboleth.sp.ddf.DDF;
+import net.shibboleth.sp.profile.impl.DoSessionCacheOperation;
+
+/**
+ * Unit test for the SP sealer flow.
+ */
+public class SessionCacheFlowTest extends AbstractSPFlowTest {
+    
+    /** Flow ID. */
+    @Nonnull public static final String FLOW_ID = "sp/session-cache";
+
+    /** Test context. */
+    @Nonnull public static final String TEST_CONTEXT = DoSessionCacheOperation.STORAGE_CONTEXT + '!' + AGENT_ID;
+    
+    @Autowired
+    @Qualifier("shibboleth.StorageService")
+    @Nullable StorageService storageService;
+
+    protected SessionCacheFlowTest() {
+        super(FLOW_ID);
+    }
+    
+    /**
+     * Get the auto-wired storage service.
+     * 
+     * @return storage service
+     */
+    @Nonnull public StorageService getStorageService() {
+        assert storageService != null;
+        return storageService;
+    }
+    
+    /**
+     * Clear the storage service between tests.
+     * 
+     * @throws IOException
+     */
+    @AfterMethod
+    public void clearStorage() throws IOException {
+        getStorageService().deleteContext(TEST_CONTEXT);
+    }
+
+    /**
+     * Test no operation.
+     * 
+     * @throws IOException 
+     */
+    @Test
+    public void noOperation() throws IOException {
+        setDefaultAuth();
+        setRequest("POST", new DDF(null));
+        final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
+        assertFlowExecutionResult(result, FLOW_ID);
+        assertFlowExecutionOutcome(result.getOutcome());
+        assertOutputMessageEvent(result, EventIds.INVALID_MESSAGE);
+    }
+
+    /**
+     * Test read with no key input.
+     * 
+     * @throws IOException 
+     */
+    @Test
+    public void testReadMissingKey() throws IOException {
+        setDefaultAuth();
+
+        final DDF input = new DDF().structure();
+        input.addmember(DoSessionCacheOperation.OP).string("R");
+        setRequest("POST", input);
+
+        final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
+        assertFlowExecutionResult(result, FLOW_ID);
+        assertFlowExecutionOutcome(result.getOutcome());
+        assertOutputMessageEvent(result, EventIds.INVALID_MESSAGE);
+    }
+    
+    /**
+     * Test read with no record.
+     * 
+     * @throws IOException 
+     */
+    @Test
+    public void testReadMissingSession() throws IOException {
+        setDefaultAuth();
+
+        final DDF input = new DDF().structure();
+        input.addmember(DoSessionCacheOperation.OP).string("R");
+        input.addmember(DoSessionCacheOperation.KEY).string("foo");
+        setRequest("POST", input);
+
+        final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
+        assertFlowExecutionResult(result, FLOW_ID);
+        assertFlowExecutionOutcome(result.getOutcome());
+        final DDF output = assertOutputMessageSuccess(result);
+        assert output != null;
+        Assert.assertTrue(output.getmember(DoSessionCacheOperation.SESSION).isnull());
+    }
+
+    /**
+     * Test read with bad record.
+     * 
+     * @throws IOException 
+     * @throws EncodingException 
+     */
+    @Test
+    public void testReadBadRecord() throws IOException, EncodingException {
+        setDefaultAuth();
+
+        final long exp = Instant.now().plusSeconds(900).toEpochMilli();
+        
+        getStorageService().create(TEST_CONTEXT, "foo", Base64Support.encode("foo".getBytes(StandardCharsets.UTF_8), false), exp);
+        
+        final DDF input = new DDF().structure();
+        input.addmember(DoSessionCacheOperation.OP).string("R");
+        input.addmember(DoSessionCacheOperation.KEY).string("foo");
+        setRequest("POST", input);
+
+        final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
+        assertFlowExecutionResult(result, FLOW_ID);
+        assertFlowExecutionOutcome(result.getOutcome());
+        assertOutputMessageEvent(result, DoSessionCacheOperation.INVALID_SESSION);
+    }
+    
+    /**
+     * Test successful read.
+     * 
+     * @throws IOException 
+     * @throws EncodingException 
+     */
+    @Test
+    public void testReadSuccess() throws IOException, EncodingException {
+        setDefaultAuth();
+
+        final long exp = Instant.now().plusSeconds(900).toEpochMilli();
+        
+        getStorageService().create(TEST_CONTEXT, "foo", getEncodedSession(), exp);
+        
+        final DDF input = new DDF().structure();
+        input.addmember(DoSessionCacheOperation.OP).string("R");
+        input.addmember(DoSessionCacheOperation.KEY).string("foo");
+        setRequest("POST", input);
+        
+        final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
+        assertFlowExecutionResult(result, FLOW_ID);
+        assertFlowExecutionOutcome(result.getOutcome());
+        final DDF output = assertOutputMessageSuccess(result);
+        assert output != null;
+        
+        Assert.assertEquals(output.getmember("session.foo").integer(), 42);
+    }
+
+    /**
+     * Test read with a timeout.
+     * 
+     * @throws IOException 
+     * @throws EncodingException 
+     * @throws InterruptedException 
+     */
+    @Test
+    public void testReadTimeout() throws IOException, EncodingException, InterruptedException {
+        setDefaultAuth();
+
+        final long exp = Instant.now().plusSeconds(900).toEpochMilli();
+        
+        getStorageService().create(TEST_CONTEXT, "foo", getEncodedSession(), exp);
+        
+        final DDF input = new DDF().structure();
+        input.addmember(DoSessionCacheOperation.OP).string("R");
+        input.addmember(DoSessionCacheOperation.KEY).string("foo");
+        input.addmember(DoSessionCacheOperation.STORAGE_TIMEOUT).longinteger(900);
+        input.addmember(DoSessionCacheOperation.TIMEOUT).integer(1);
+        setRequest("POST", input);
+        
+        Thread.sleep(2000);
+        
+        final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
+        assertFlowExecutionResult(result, FLOW_ID);
+        assertFlowExecutionOutcome(result.getOutcome());
+        assertOutputMessageEvent(result, DoSessionCacheOperation.EXPIRED_SESSION);
+    }
+    
+    /**
+     * Test missing delete.
+     * 
+     * @throws IOException 
+     */
+    @Test
+    public void testDeleteMissing() throws IOException {
+        setDefaultAuth();
+        
+        final DDF input = new DDF().structure();
+        input.addmember(DoSessionCacheOperation.OP).string("D");
+        input.addmember(DoSessionCacheOperation.KEY).string("foo");
+        setRequest("POST", input);
+
+        final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
+        assertFlowExecutionResult(result, FLOW_ID);
+        assertFlowExecutionOutcome(result.getOutcome());
+        assertOutputMessageEvent(result, DoSessionCacheOperation.MISSING_SESSION);
+    }
+    
+    /**
+     * Test successful delete.
+     * 
+     * @throws IOException 
+     * @throws EncodingException 
+     */
+    @Test
+    public void testDeleteSuccess() throws IOException, EncodingException  {
+        setDefaultAuth();
+        
+        final long exp = Instant.now().plusSeconds(900).toEpochMilli();
+
+        getStorageService().create(TEST_CONTEXT, "foo", getEncodedSession(), exp);
+        
+        final DDF input = new DDF().structure();
+        input.addmember(DoSessionCacheOperation.OP).string("D");
+        input.addmember(DoSessionCacheOperation.KEY).string("foo");
+        setRequest("POST", input);
+
+        final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
+        assertFlowExecutionResult(result, FLOW_ID);
+        assertFlowExecutionOutcome(result.getOutcome());
+        assertOutputMessageSuccess(result);
+        
+        Assert.assertNull(getStorageService().read(TEST_CONTEXT, "foo"));
+    }
+
+    /**
+     * Test successful create.
+     * 
+     * @throws IOException 
+     * @throws DecodingException 
+     */
+    @Test
+    public void testCreateMissingInput() throws IOException, DecodingException {
+        setDefaultAuth();
+        
+        final DDF input = new DDF().structure();
+        input.addmember(DoSessionCacheOperation.OP).string("C");
+        
+        setRequest("POST", input);
+        
+        final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
+        assertFlowExecutionResult(result, FLOW_ID);
+        assertFlowExecutionOutcome(result.getOutcome());
+        assertOutputMessageEvent(result, EventIds.INVALID_MESSAGE);
+    }
+    
+    /**
+     * Test successful create.
+     * 
+     * @throws IOException 
+     * @throws DecodingException 
+     */
+    @Test
+    public void testCreateSuccess() throws IOException, DecodingException {
+        setDefaultAuth();
+        
+        final DDF input = new DDF().structure();
+        input.addmember(DoSessionCacheOperation.OP).string("C");
+        input.addmember(DoSessionCacheOperation.SESSION).structure().addmember("foo").integer(42);
+        
+        setRequest("POST", input);
+        
+        final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
+        assertFlowExecutionResult(result, FLOW_ID);
+        assertFlowExecutionOutcome(result.getOutcome());
+        final DDF output = assertOutputMessageSuccess(result);
+        assert output != null;
+        
+        final String key = output.getmember(DoSessionCacheOperation.KEY).string();
+        Assert.assertNotNull(key);
+        
+        final StorageRecord<?> record = getStorageService().read(TEST_CONTEXT, key);
+        assert record != null;
+        Assert.assertEquals(record.getVersion(), 1);
+        final String encoded = record.getValue();
+        try (final ByteArrayInputStream src = new ByteArrayInputStream(Base64Support.decode(encoded))) {
+            DDF session = DDF.deserialize(src);
+            Assert.assertEquals(session.getmember("foo").integer(), 42);
+        }
+    }
+
+    /**
+     * Test successful update of expiration.
+     * 
+     * @throws IOException 
+     * @throws EncodingException 
+     */
+    @Test
+    public void testTouchMissingInput() throws IOException, EncodingException {
+        setDefaultAuth();
+        
+        final Instant exp = Instant.now().plusSeconds(900);
+        
+        getStorageService().create(TEST_CONTEXT, "foo", getEncodedSession(), exp.toEpochMilli());
+                
+        final DDF input = new DDF().structure();
+        input.addmember(DoSessionCacheOperation.OP).string("T");
+        setRequest("POST", input);
+        
+        final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
+        assertFlowExecutionResult(result, FLOW_ID);
+        assertFlowExecutionOutcome(result.getOutcome());
+        assertOutputMessageEvent(result, EventIds.INVALID_MESSAGE);
+    }
+
+    /**
+     * Test successful update of expiration.
+     * 
+     * @throws IOException 
+     * @throws EncodingException 
+     */
+    @Test
+    public void testTouchSuccess() throws IOException, EncodingException {
+        setDefaultAuth();
+        
+        final Instant exp = Instant.now().plusSeconds(900);
+        
+        getStorageService().create(TEST_CONTEXT, "foo", getEncodedSession(), exp.toEpochMilli());
+                
+        final DDF input = new DDF().structure();
+        input.addmember(DoSessionCacheOperation.OP).string("T");
+        input.addmember(DoSessionCacheOperation.KEY).string("foo");
+        input.addmember(DoSessionCacheOperation.STORAGE_TIMEOUT).longinteger(900);
+        setRequest("POST", input);
+        
+        final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
+        assertFlowExecutionResult(result, FLOW_ID);
+        assertFlowExecutionOutcome(result.getOutcome());
+        assertOutputMessageSuccess(result);
+        
+        final StorageRecord<?> record = getStorageService().read(TEST_CONTEXT, "foo");
+        assert record != null;
+        Assert.assertEquals(record.getVersion(), 1);
+        Assert.assertTrue(exp.isBefore(Instant.ofEpochMilli(record.getExpiration())));
+    }    
+
+    /**
+     * Test successful update.
+     * 
+     * @throws IOException 
+     * @throws EncodingException 
+     */
+    @Test
+    public void testUpdateSuccess() throws IOException, EncodingException {
+        setDefaultAuth();
+        
+        final long exp = Instant.now().plusSeconds(900).toEpochMilli();
+
+        getStorageService().create(TEST_CONTEXT, "foo", getEncodedSession(), exp);
+                
+        final DDF input = new DDF().structure();
+        input.addmember(DoSessionCacheOperation.OP).string("U");
+        input.addmember(DoSessionCacheOperation.KEY).string("foo");
+        input.addmember(DoSessionCacheOperation.VERSION).integer(1);
+        input.addmember(DoSessionCacheOperation.STORAGE_TIMEOUT).longinteger(900);
+        input.addmember(DoSessionCacheOperation.SESSION).structure().addmember("foo").integer(43);
+        setRequest("POST", input);
+        
+        final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
+        assertFlowExecutionResult(result, FLOW_ID);
+        assertFlowExecutionOutcome(result.getOutcome());
+        final DDF output = assertOutputMessageSuccess(result);
+        Assert.assertEquals(output.getmember(DoSessionCacheOperation.VERSION).integer(), 2);
+        
+        final StorageRecord<?> record = getStorageService().read(TEST_CONTEXT, "foo");
+        assert record != null;
+        Assert.assertEquals(record.getVersion(), 2);
+    }
+
+    /**
+     * Test failed update due to version mismatch.
+     * 
+     * @throws IOException 
+     * @throws EncodingException 
+     */
+    @Test
+    public void testUpdateVersionMismatch() throws IOException, EncodingException {
+        setDefaultAuth();
+        
+        final long exp = Instant.now().plusSeconds(900).toEpochMilli();
+
+        getStorageService().create(TEST_CONTEXT, "foo", getEncodedSession(), exp);
+
+        final DDF input = new DDF().structure();
+        input.addmember(DoSessionCacheOperation.OP).string("U");
+        input.addmember(DoSessionCacheOperation.KEY).string("foo");
+        input.addmember(DoSessionCacheOperation.VERSION).integer(2);
+        input.addmember(DoSessionCacheOperation.STORAGE_TIMEOUT).longinteger(900);
+        input.addmember(DoSessionCacheOperation.SESSION).structure().addmember("foo").integer(43);
+        setRequest("POST", input);
+        
+        final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
+        assertFlowExecutionResult(result, FLOW_ID);
+        assertFlowExecutionOutcome(result.getOutcome());
+        assertOutputMessageEvent(result, DoSessionCacheOperation.VERSION_MISMATCH);
+    }
+
+    /**
+     * Test failed update when record missing.
+     * 
+     * @throws IOException 
+     */
+    @Test
+    public void testUpdateMissing() throws IOException {
+        setDefaultAuth();
+        
+        final DDF input = new DDF().structure();
+        input.addmember(DoSessionCacheOperation.OP).string("U");
+        input.addmember(DoSessionCacheOperation.KEY).string("foo");
+        input.addmember(DoSessionCacheOperation.VERSION).longinteger(1);
+        input.addmember(DoSessionCacheOperation.STORAGE_TIMEOUT).longinteger(900);
+        input.addmember(DoSessionCacheOperation.SESSION).structure().addmember("foo").integer(43);
+        setRequest("POST", input);
+        
+        final FlowExecutionResult result = flowExecutor.launchExecution(FLOW_ID, null, externalContext);
+        assertFlowExecutionResult(result, FLOW_ID);
+        assertFlowExecutionOutcome(result.getOutcome());
+        final DDF output = assertOutputMessageSuccess(result);
+        
+        Assert.assertTrue(output.getmember(DoSessionCacheOperation.VERSION).isnull());
+    }
+    
+    /**
+     * Produce a base64 encoded session record for use in manually seeding storage.
+     * 
+     * @return encoded session record
+     * 
+     * @throws IOException 
+     * @throws EncodingException 
+     */
+    @Nonnull private String getEncodedSession() throws IOException, EncodingException {
+
+        final DDF session = new DDF().structure();
+        session.addmember("foo").integer(42);
+        
+        String value;
+        try (final ByteArrayOutputStream sink = new ByteArrayOutputStream()) {
+            session.serialize(sink);
+            value = Base64Support.encode(sink.toByteArray(), false);
+        }
+        
+        return value;
+    }
+    
+}
\ No newline at end of file
diff --git a/sp-conf-impl/src/test/java/net/shibboleth/sp/flows/TestSPEnvironmentApplicationContextInitializer.java b/sp-conf-impl/src/test/java/net/shibboleth/sp/flows/TestSPEnvironmentApplicationContextInitializer.java
index 123fd1d..31ecb19 100644
--- a/sp-conf-impl/src/test/java/net/shibboleth/sp/flows/TestSPEnvironmentApplicationContextInitializer.java
+++ b/sp-conf-impl/src/test/java/net/shibboleth/sp/flows/TestSPEnvironmentApplicationContextInitializer.java
@@ -34,6 +34,7 @@ public class TestSPEnvironmentApplicationContextInitializer extends TestEnvironm
     protected void addProperties(@Nonnull final MockPropertySource mock) {
         super.addProperties(mock);
         mock.setProperty("sp.storageService", "shibboleth.StorageService");
+        mock.setProperty("sp.session.storageService", "shibboleth.StorageService");
         mock.setProperty("idp.additionalProperties",
                 "/conf/ldap.properties, /conf/saml-nameid.properties, /conf/services.properties, /conf/admin/admin.properties, /conf/authn/authn.properties, /conf/c14n/subject-c14n.properties, /credentials/secrets.properties, /conf/sp/sp.properties");
     }
diff --git a/sp-server-impl/src/main/java/net/shibboleth/sp/profile/impl/DoSessionCacheOperation.java b/sp-server-impl/src/main/java/net/shibboleth/sp/profile/impl/DoSessionCacheOperation.java
index 240661b..ba1bbca 100644
--- a/sp-server-impl/src/main/java/net/shibboleth/sp/profile/impl/DoSessionCacheOperation.java
+++ b/sp-server-impl/src/main/java/net/shibboleth/sp/profile/impl/DoSessionCacheOperation.java
@@ -97,9 +97,6 @@ public class DoSessionCacheOperation extends AbstractAgentAction {
 
     /** Member for session timeout. */
     @Nonnull @NotEmpty public static final String TIMEOUT = "timeout";
-
-    /** Member for session creation timestamp. */
-    @Nonnull @NotEmpty public static final String TS = "ts";
     
     /** Member for session version. */
     @Nonnull @NotEmpty public static final String VERSION = "ver";
@@ -259,7 +256,7 @@ public class DoSessionCacheOperation extends AbstractAgentAction {
         int attempts = 0;
         do {
             final String key = identifierStrategy.generateIdentifier(false);
-            if (storageService.create(STORAGE_CONTEXT, ensureAgent().getId() + '!' + key, value, exp)) {
+            if (storageService.create(getStorageContext(), key, value, exp)) {
                 log.debug("{} Created session record ({}), expiration ({})", getLogPrefix(), key, Instant.ofEpochMilli(exp));
                 DDF out = new DDF(null);
                 out.addmember(KEY).string(key);
@@ -286,7 +283,6 @@ public class DoSessionCacheOperation extends AbstractAgentAction {
         // Input:
         // "key" - session key
         // "storage_timeout" - the storage timeout in seconds
-        // "lifetime" - session lifetime policy to apply
         // "timeout" - session timeout policy to apply
         
         // Output:
@@ -299,12 +295,10 @@ public class DoSessionCacheOperation extends AbstractAgentAction {
             return;
         }
         
-        final StorageRecord<?> record = storageService.read(STORAGE_CONTEXT, ensureAgent().getId() + '!' + key);
+        final StorageRecord<?> record = storageService.read(getStorageContext(), key);
         if (record == null) {
             // Send back an empty response.
             log.debug("{} No session found ({})", getLogPrefix(), key);
-            final DDF output = new DDF();
-            ensureAgentRequestContext().setOutput(output);
             return;
         }
         
@@ -320,11 +314,11 @@ public class DoSessionCacheOperation extends AbstractAgentAction {
         final Integer timeout = input.getmember(TIMEOUT).integer();
         if (timeout != null && timeout > 0) {
             // Recover last access time by backdating record expiration.
-            lastAccess -= getStorageTimeout();
+            lastAccess -= (getStorageTimeout() * 1000);
             if (Instant.ofEpochMilli(lastAccess).plusSeconds(timeout).isBefore(now)) {
                 log.info("{} Session record ({}) timed out, last use was {}, timeout policy was {}",
                         getLogPrefix(), key, Instant.ofEpochMilli(lastAccess), timeout);
-                storageService.delete(STORAGE_CONTEXT, ensureAgent().getId() + '!' + key);
+                storageService.delete(getStorageContext(), key);
                 ActionSupport.buildEvent(profileRequestContext, EXPIRED_SESSION);
                 return;
             }
@@ -338,30 +332,14 @@ public class DoSessionCacheOperation extends AbstractAgentAction {
             log.error("{} Unable to base64-decode session data", getLogPrefix(), e);
             ActionSupport.buildEvent(profileRequestContext, INVALID_SESSION);
             return;
+        } catch (final IOException e) {
+            log.error("{} Unable to deserialize session record", getLogPrefix(), e);
+            ActionSupport.buildEvent(profileRequestContext, INVALID_SESSION);
+            return;
         }
-        
-        // We have a deserialized session, need to enforce lifetime if specified.
-        
-        final Integer lifetime = input.getmember(LIFETIME).integer();
-        if (lifetime != null && lifetime > 0) {
-            final Long ts = sessionData.getmember(TS).longinteger();
-            if (ts == null) {
-                log.error("{} Session record ({}) missing creation timestamp", getLogPrefix(), key);
-                storageService.delete(STORAGE_CONTEXT, ensureAgent().getId() + '!' + key);
-                ActionSupport.buildEvent(profileRequestContext, INVALID_SESSION);
-                return;
-            }
-            
-            if (Instant.ofEpochSecond(ts).plusSeconds(lifetime).isBefore(now)) {
-                log.info("{} Session record ({}) expired, created {}, lifetime policy was {}",
-                        getLogPrefix(), key, Instant.ofEpochSecond(ts), lifetime);
-                storageService.delete(STORAGE_CONTEXT, ensureAgent().getId() + '!' + key);
-                ActionSupport.buildEvent(profileRequestContext, EXPIRED_SESSION);
-                return;
-            }
-        }
-                
+                        
         final DDF output = new DDF().structure();
+        sessionData.name(SESSION);
         output.add(sessionData);
         ensureAgentRequestContext().setOutput(output);
     }    
@@ -415,8 +393,7 @@ public class DoSessionCacheOperation extends AbstractAgentAction {
         // TODO: Encrypt?
         
         try {
-            Long newver = storageService.updateWithVersion(version, STORAGE_CONTEXT, ensureAgent().getId() + '!' + key,
-                    value, exp);
+            Long newver = storageService.updateWithVersion(version, getStorageContext(),key, value, exp);
             if (newver != null) {
                 log.debug("{} Updated session ({}) to version ({})", getLogPrefix(), key, version);
                 final DDF output = new DDF().structure();
@@ -426,11 +403,9 @@ public class DoSessionCacheOperation extends AbstractAgentAction {
             } else {
                 // Send back an empty response.
                 log.debug("{} No session found ({})", getLogPrefix(), key);
-                final DDF output = new DDF();
-                ensureAgentRequestContext().setOutput(output);
             }
         } catch (final VersionMismatchException e) {
-            log.info("{} Version mismatch for session ({}). requested update with older version ({})", getLogPrefix(),
+            log.info("{} Version mismatch for session ({}). requested update with non-matching version ({})", getLogPrefix(),
                     key, version);
             ActionSupport.buildEvent(profileRequestContext, VERSION_MISMATCH);
         }
@@ -466,12 +441,10 @@ public class DoSessionCacheOperation extends AbstractAgentAction {
         final Integer timeout = input.getmember(TIMEOUT).integer();
         if (timeout != null && timeout > 0) {
             // Read back record to check for timeout.
-            final StorageRecord<?> record = storageService.read(STORAGE_CONTEXT, ensureAgent().getId() + '!' + key);
+            final StorageRecord<?> record = storageService.read(getStorageContext(), key);
             if (record == null) {
                 // Send back an empty response.
                 log.debug("{} No session found ({})", getLogPrefix(), key);
-                final DDF output = new DDF();
-                ensureAgentRequestContext().setOutput(output);
                 return;
             }
         
@@ -484,24 +457,18 @@ public class DoSessionCacheOperation extends AbstractAgentAction {
         
         
             // Recover last access time by backdating record expiration.
-            lastAccess -= st;
+            lastAccess -= (st * 1000);
             if (Instant.ofEpochMilli(lastAccess).plusSeconds(timeout).isBefore(now)) {
                 log.info("{} Session record ({}) timed out, last use was {}, timeout policy was {}",
                         getLogPrefix(), key, Instant.ofEpochMilli(lastAccess), timeout);
-                storageService.delete(STORAGE_CONTEXT, ensureAgent().getId() + '!' + key);
+                storageService.delete(getStorageContext(), key);
                 ActionSupport.buildEvent(profileRequestContext, EXPIRED_SESSION);
                 return;
             }
         }
         
-        // Bump the expiration and return the version as a success indicator.
-        
-        if (storageService.updateExpiration(STORAGE_CONTEXT, ensureAgent().getId() + '!' + key,
-                now.plusSeconds(st).toEpochMilli())) {
-            final DDF output = new DDF();
-            output.addmember(KEY).string(key);
-            ensureAgentRequestContext().setOutput(output);
-        } else {
+        // Bump the expiration.
+        if (!storageService.updateExpiration(getStorageContext(), key, now.plusSeconds(st).toEpochMilli())) {
             // Record disappeared, so send back an empty response.
             log.debug("{} Session record ({}) disappeared before update?", getLogPrefix(), key);
             ActionSupport.buildEvent(profileRequestContext, MISSING_SESSION);
@@ -530,10 +497,8 @@ public class DoSessionCacheOperation extends AbstractAgentAction {
             return;
         }
         
-        if (storageService.delete(STORAGE_CONTEXT, ensureAgent().getId() + '!' + key)) {
+        if (storageService.delete(getStorageContext(), key)) {
             log.debug("{} Deleted session record ({})", getLogPrefix(), key);
-            final DDF output = new DDF();
-            ensureAgentRequestContext().setOutput(output);
         } else {
             log.debug("{} No session record ({})", getLogPrefix(), key);
             ActionSupport.buildEvent(profileRequestContext, MISSING_SESSION);
@@ -545,7 +510,7 @@ public class DoSessionCacheOperation extends AbstractAgentAction {
      * 
      * @return effective timeout in seconds
      */
-    @Nonnull Long getStorageTimeout() {
+    @Nonnull private Long getStorageTimeout() {
         
         Long st = input.getmember(STORAGE_TIMEOUT).longinteger();
         if (st != null) {
@@ -560,4 +525,14 @@ public class DoSessionCacheOperation extends AbstractAgentAction {
         return maxStorageTimeout.toSeconds();
     }
 
+    
+    /**
+     * Compute agent-specific storage context. 
+     *
+     * @return storage context with agent qualifier
+     */
+    @Nonnull private String getStorageContext() {
+        return STORAGE_CONTEXT + '!' + ensureAgent().getId();
+    }
+
 }
\ 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