[java-identity-provider] branch master updated: IDP-1507 - Increase flexibility around session address checking
Scott Cantor
cantor.2 at osu.edu
Mon Oct 7 18:52:16 EDT 2019
This is an automated email from the git hooks/post-receive script.
scantor pushed a commit to branch master
in repository java-identity-provider.
View the commit online:
http://git.shibboleth.net/view/?p=java-identity-provider.git;a=commit;h=7bf9783a8fb8fa29faf4342bdd70b9f014584b2e
The following commit(s) were added to refs/heads/master by this push:
new 7bf9783 IDP-1507 - Increase flexibility around session address checking
7bf9783 is described below
commit 7bf9783a8fb8fa29faf4342bdd70b9f014584b2e
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Mon Oct 7 18:52:10 2019 -0400
IDP-1507 - Increase flexibility around session address checking
https://issues.shibboleth.net/jira/browse/IDP-1507
Wire in predicate support.
---
.../system/conf/session-manager-system.xml | 3 +-
.../shibboleth/idp/session/AbstractIdPSession.java | 66 ++++++++---------
.../idp/session/impl/StorageBackedIdPSession.java | 28 ++++++--
.../session/impl/StorageBackedSessionManager.java | 83 ++++++++++++++++------
4 files changed, 121 insertions(+), 59 deletions(-)
diff --git a/idp-conf/src/main/resources/system/conf/session-manager-system.xml b/idp-conf/src/main/resources/system/conf/session-manager-system.xml
index 1e62555..7e53e67 100644
--- a/idp-conf/src/main/resources/system/conf/session-manager-system.xml
+++ b/idp-conf/src/main/resources/system/conf/session-manager-system.xml
@@ -66,7 +66,8 @@
p:authenticationFlowDescriptors-ref="shibboleth.AvailableAuthenticationFlows"
p:cookieManager-ref="shibboleth.CookieManager"
p:storageService-ref="#{'%{idp.session.StorageService:shibboleth.ClientSessionStorageService}'.trim()}"
- p:consistentAddress="%{idp.session.consistentAddress:true}"
+ p:consistentAddressCondition="#{getObject('%{idp.session.consistentAddressCondition:}'.trim()) ?:
+ T(net.shibboleth.idp.session.impl.StorageBackedSessionManager.DefaultConsistentAddressConditionFactory).getDefaultConsistentAddressCondition(%{idp.session.consistentAddress:true})}"
p:sessionTimeout="%{idp.session.timeout:PT60M}"
p:sessionSlop="%{idp.session.slop:PT0S}"
p:maskStorageFailure="%{idp.session.maskStorageFailure:false}"
diff --git a/idp-session-api/src/main/java/net/shibboleth/idp/session/AbstractIdPSession.java b/idp-session-api/src/main/java/net/shibboleth/idp/session/AbstractIdPSession.java
index 78b7b6e..08f6381 100644
--- a/idp-session-api/src/main/java/net/shibboleth/idp/session/AbstractIdPSession.java
+++ b/idp-session-api/src/main/java/net/shibboleth/idp/session/AbstractIdPSession.java
@@ -160,6 +160,33 @@ public abstract class AbstractIdPSession implements IdPSession {
lastActivityInstant = Constraint.isNotNull(instant, "Last activity instant cannot be null");
}
+ /** {@inheritDoc} */
+ @Override
+ public boolean checkAddress(@Nonnull @NotEmpty final String address) throws SessionException {
+ final AddressFamily family = getAddressFamily(address);
+ if (family == AddressFamily.UNKNOWN) {
+ log.warn("Address {} is of unknown type", address);
+ return false;
+ }
+ final String bound = getAddress(family);
+ if (bound != null) {
+ if (!bound.equals(address)) {
+ log.warn("Client address is {} but session {} already bound to {}", address, id, bound);
+ return false;
+ }
+ } else {
+ log.info("Session {} not yet locked to a {} address, locking it to {}", id, family, address);
+ try {
+ bindToAddress(address);
+ } catch (final SessionException e) {
+ log.error("Unable to bind session {} to address {}", id, address);
+ return false;
+ }
+ }
+
+ return true;
+ }
+
/**
* Get an address to which this session is bound.
*
@@ -214,6 +241,12 @@ public abstract class AbstractIdPSession implements IdPSession {
}
/** {@inheritDoc} */
+ public boolean checkTimeout() throws SessionException {
+ setLastActivityInstant(Instant.now());
+ return true;
+ }
+
+ /** {@inheritDoc} */
@Nonnull @NonnullElements @NotLive @Unmodifiable public Set<AuthenticationResult> getAuthenticationResults() {
return ImmutableSet.copyOf(Optional.presentInstances(authenticationResults.values()));
}
@@ -350,39 +383,6 @@ public abstract class AbstractIdPSession implements IdPSession {
}
/** {@inheritDoc} */
- @Override
- public boolean checkAddress(@Nonnull @NotEmpty final String address) throws SessionException {
- final AddressFamily family = getAddressFamily(address);
- if (family == AddressFamily.UNKNOWN) {
- log.warn("Address {} is of unknown type", address);
- return false;
- }
- final String bound = getAddress(family);
- if (bound != null) {
- if (!bound.equals(address)) {
- log.warn("Client address is {} but session {} already bound to {}", address, id, bound);
- return false;
- }
- } else {
- log.info("Session {} not yet locked to a {} address, locking it to {}", id, family, address);
- try {
- bindToAddress(address);
- } catch (final SessionException e) {
- log.error("Unable to bind session {} to address {}", id, address);
- return false;
- }
- }
-
- return true;
- }
-
- /** {@inheritDoc} */
- public boolean checkTimeout() throws SessionException {
- setLastActivityInstant(Instant.now());
- return true;
- }
-
- /** {@inheritDoc} */
public boolean equals(final Object obj) {
if (obj == null) {
return false;
diff --git a/idp-session-impl/src/main/java/net/shibboleth/idp/session/impl/StorageBackedIdPSession.java b/idp-session-impl/src/main/java/net/shibboleth/idp/session/impl/StorageBackedIdPSession.java
index 0efab2d..ef9ecd0 100644
--- a/idp-session-impl/src/main/java/net/shibboleth/idp/session/impl/StorageBackedIdPSession.java
+++ b/idp-session-impl/src/main/java/net/shibboleth/idp/session/impl/StorageBackedIdPSession.java
@@ -103,15 +103,35 @@ public class StorageBackedIdPSession extends AbstractIdPSession {
/** {@inheritDoc} */
@Override
public boolean checkAddress(@Nonnull @NotEmpty final String address) throws SessionException {
- return sessionManager.isConsistentAddress() ? super.checkAddress(address) : true;
+
+ final AddressFamily family = getAddressFamily(address);
+ if (family == AddressFamily.UNKNOWN) {
+ log.warn("Address {} is of unknown type", address);
+ return false;
+ }
+
+ final String bound = getAddress(family);
+ if (bound != null) {
+ if (!sessionManager.getConsistentAddressCondition().test(bound, address)) {
+ log.warn("Client address {} invalid for session {} bound to {}", address, getId(), bound);
+ return false;
+ }
+ } else {
+ log.info("Session {} not yet locked to a {} address, locking it to {}", getId(), family, address);
+ try {
+ bindToAddress(address);
+ } catch (final SessionException e) {
+ log.error("Unable to bind session {} to address {}", getId(), address);
+ return false;
+ }
+ }
+
+ return true;
}
/** {@inheritDoc} */
@Override
public void bindToAddress(@Nonnull @NotEmpty final String address) throws SessionException {
- if (!sessionManager.isConsistentAddress()) {
- return;
- }
// Update ourselves and then attempt to write back.
super.bindToAddress(address);
diff --git a/idp-session-impl/src/main/java/net/shibboleth/idp/session/impl/StorageBackedSessionManager.java b/idp-session-impl/src/main/java/net/shibboleth/idp/session/impl/StorageBackedSessionManager.java
index 57d2bb0..56037cf 100644
--- a/idp-session-impl/src/main/java/net/shibboleth/idp/session/impl/StorageBackedSessionManager.java
+++ b/idp-session-impl/src/main/java/net/shibboleth/idp/session/impl/StorageBackedSessionManager.java
@@ -23,6 +23,8 @@ import java.time.Instant;
import java.util.HashMap;
import java.util.Iterator;
import java.util.Map;
+import java.util.Objects;
+import java.util.function.BiPredicate;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
@@ -139,8 +141,8 @@ public class StorageBackedSessionManager extends AbstractIdentifiableInitializab
/** Indicates whether to secondary-index SPSessions. */
private boolean secondaryServiceIndex;
- /** Indicates whether sessions are bound to client addresses. */
- private boolean consistentAddress;
+ /** Indicates how bound session addresses and client addresses are compared. */
+ @Nonnull private BiPredicate<String,String> consistentAddressCondition;
/** Manages creation of cookies. */
@NonnullAfterInit private CookieManager cookieManager;
@@ -175,7 +177,8 @@ public class StorageBackedSessionManager extends AbstractIdentifiableInitializab
sessionSlop = Duration.ZERO;
serializer = new StorageBackedIdPSessionSerializer(this, null);
flowDescriptorMap = new HashMap<>();
- consistentAddress = true;
+ consistentAddressCondition =
+ DefaultConsistentAddressConditionFactory.getDefaultConsistentAddressCondition(true);
cookieName = DEFAULT_COOKIE_NAME;
storageServiceThreshold = 1024 * 1024;
}
@@ -311,23 +314,40 @@ public class StorageBackedSessionManager extends AbstractIdentifiableInitializab
}
/**
- * Get whether sessions are bound to client addresses.
+ * Get condition to evaluate bound session and client addresses for consistency.
*
- * @return true iff sessions should be bound to client addresses
+ * @return condition
+ *
+ * @since 4.0.0
*/
- public boolean isConsistentAddress() {
- return consistentAddress;
+ @Nonnull public BiPredicate<String,String> getConsistentAddressCondition() {
+ return consistentAddressCondition;
}
/**
- * Set whether sessions are bound to client addresses.
+ * Set whether sessions are bound to client addresses either via disabling the comparison
+ * or testing simple equality.
*
* @param flag flag to set
*/
public void setConsistentAddress(final boolean flag) {
ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
- consistentAddress = flag;
+ consistentAddressCondition =
+ DefaultConsistentAddressConditionFactory.getDefaultConsistentAddressCondition(flag);
+ }
+
+ /**
+ * Set condition to evaluate bound session and client addresses for consistency.
+ *
+ * @param condition condition to set
+ *
+ * @since 4.0.0
+ */
+ public void setConsistentAddressCondition(@Nonnull final BiPredicate<String,String> condition) {
+ ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
+
+ consistentAddressCondition = Constraint.isNotNull(condition, "Consistent address condition cannot be null");
}
/**
@@ -497,15 +517,13 @@ public class StorageBackedSessionManager extends AbstractIdentifiableInitializab
throws SessionException {
ComponentSupport.ifNotInitializedThrowUninitializedComponentException(this);
- String remoteAddr = null;
- if (consistentAddress) {
- if (httpRequest == null) {
- throw new SessionException("No HttpServletRequest available, can't bind to client address");
- }
- remoteAddr = StringSupport.trimOrNull(httpRequest.getRemoteAddr());
- if (remoteAddr == null) {
- throw new SessionException("No client address to bind");
- }
+ if (httpRequest == null) {
+ throw new SessionException("No HttpServletRequest available, can't bind to client address");
+ }
+
+ final String remoteAddr = StringSupport.trimOrNull(httpRequest.getRemoteAddr());
+ if (remoteAddr == null) {
+ throw new SessionException("No client address to bind");
}
final String sessionId = idGenerator.generateIdentifier(false);
@@ -515,9 +533,7 @@ public class StorageBackedSessionManager extends AbstractIdentifiableInitializab
final StorageBackedIdPSession newSession =
new StorageBackedIdPSession(this, sessionId, principalName, Instant.now());
- if (remoteAddr != null) {
- newSession.doBindToAddress(remoteAddr);
- }
+ newSession.doBindToAddress(remoteAddr);
try {
if (!storageService.create(sessionId, SESSION_MASTER_KEY, newSession, serializer,
@@ -884,4 +900,29 @@ public class StorageBackedSessionManager extends AbstractIdentifiableInitializab
return builder.build();
}
+
+ /**
+ * Simplifies Spring wiring of a true/false condition for the consistentAddress feature.
+ *
+ * @since 4.0.0
+ */
+ public static class DefaultConsistentAddressConditionFactory {
+
+ /**
+ * Returns a suitable {@link BiPredicate} to satisfy a simple true/false value for the
+ * consistentAddress feature.
+ *
+ * @param flag true/false value for the feature
+ *
+ * @return a {@link BiPredicate} that satisfies the input
+ */
+ @Nonnull public static BiPredicate<String,String> getDefaultConsistentAddressCondition(final boolean flag) {
+ if (flag) {
+ return (A,B) -> Objects.equals(A,B);
+ }
+
+ return (A,B) -> true;
+ }
+ }
+
}
\ 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