[java-identity-provider] 01/02: IDP-2069 Null handling
Rod Widdowson
rdw at steadingsoftware.com
Tue Feb 28 10:32:40 UTC 2023
This is an automated email from the git hooks/post-receive script.
rdw pushed a commit to branch main
in repository java-identity-provider.
View the commit online:
http://git.shibboleth.net/view/?p=java-identity-provider.git;a=commit;h=6081a53a0fb97644f37df7e088b1e7c4cea39508
commit 6081a53a0fb97644f37df7e088b1e7c4cea39508
Author: Rod Widdowson <rdw at steadingsoftware.com>
AuthorDate: Mon Feb 27 17:13:49 2023 +0000
IDP-2069 Null handling
https://shibboleth.atlassian.net/browse/IDP-2069
Cleanup idp-consent-impl test are tbd
---
idp-consent-impl/pom.xml | 5 +++
.../ar/impl/PopulateAttributeReleaseContext.java | 13 ++++--
.../consent/flow/ar/impl/ReleaseAttributes.java | 18 ++++++---
.../idp/consent/flow/impl/ExtractConsent.java | 4 +-
.../consent/flow/impl/PopulateConsentContext.java | 5 ++-
.../impl/AbstractConsentIndexedStorageAction.java | 47 +++++++++++++++-------
.../storage/impl/CreateGlobalConsentResult.java | 17 +++++---
.../consent/flow/storage/impl/CreateResult.java | 18 +++++++--
.../flow/storage/impl/ReadConsentFromStorage.java | 19 ++++++---
.../consent/flow/storage/impl/RevokeConsent.java | 5 ++-
.../consent/flow/storage/impl/UpdateCounter.java | 9 +++--
.../impl/AttributeDisplayDescriptionFunction.java | 2 +-
.../idp/consent/logic/impl/AttributePredicate.java | 15 +++----
.../logic/impl/AttributeValuesHashFunction.java | 1 +
.../logic/impl/CounterStorageKeyComparator.java | 6 ---
.../logic/impl/CounterStorageKeyFunction.java | 18 +++++----
.../impl/GlobalAttributeConsentPredicate.java | 3 --
.../logic/impl/IsAttributeRequiredPredicate.java | 5 ++-
.../consent/logic/impl/LocaleLookupFunction.java | 6 ++-
.../logic/impl/MessageSourceConsentFunction.java | 10 +++--
.../impl/MessageSourceConsentFunctionTest.java | 2 +
21 files changed, 149 insertions(+), 79 deletions(-)
diff --git a/idp-consent-impl/pom.xml b/idp-consent-impl/pom.xml
index 3eb5c0194..61b6c19b7 100644
--- a/idp-consent-impl/pom.xml
+++ b/idp-consent-impl/pom.xml
@@ -132,6 +132,11 @@
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>net.shibboleth.idp</groupId>
+ <artifactId>idp-consent-api</artifactId>
+ <version>5.0.0-SNAPSHOT</version>
+ </dependency>
</dependencies>
<scm>
diff --git a/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/flow/ar/impl/PopulateAttributeReleaseContext.java b/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/flow/ar/impl/PopulateAttributeReleaseContext.java
index baa1a98dc..afe89fe9c 100644
--- a/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/flow/ar/impl/PopulateAttributeReleaseContext.java
+++ b/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/flow/ar/impl/PopulateAttributeReleaseContext.java
@@ -29,7 +29,10 @@ import org.opensaml.profile.context.ProfileRequestContext;
import org.slf4j.Logger;
import net.shibboleth.idp.attribute.IdPAttribute;
+import net.shibboleth.idp.attribute.context.AttributeContext;
+import net.shibboleth.idp.consent.context.AttributeReleaseContext;
import net.shibboleth.idp.profile.context.ProfileInterceptorContext;
+import net.shibboleth.shared.annotation.constraint.NonnullAfterInit;
import net.shibboleth.shared.component.ComponentInitializationException;
import net.shibboleth.shared.logic.Constraint;
import net.shibboleth.shared.primitive.LoggerFactory;
@@ -48,7 +51,7 @@ public class PopulateAttributeReleaseContext extends AbstractAttributeReleaseAct
@Nonnull private final Logger log = LoggerFactory.getLogger(PopulateAttributeReleaseContext.class);
/** Predicate to determine whether consent should be obtained for an attribute. */
- @Nonnull private Predicate<IdPAttribute> attributePredicate;
+ @NonnullAfterInit private Predicate<IdPAttribute> attributePredicate;
/** Comparator used to sort attributes displayed to user. */
@Nullable private Comparator<String> attributeIdComparator;
@@ -84,7 +87,9 @@ public class PopulateAttributeReleaseContext extends AbstractAttributeReleaseAct
@Override protected void doExecute(@Nonnull final ProfileRequestContext profileRequestContext,
@Nonnull final ProfileInterceptorContext interceptorContext) {
- final Map<String, IdPAttribute> attributes = getAttributeContext().getIdPAttributes();
+ final AttributeContext attributeContext = getAttributeContext();
+ assert attributeContext != null;
+ final Map<String, IdPAttribute> attributes = attributeContext.getIdPAttributes();
final Map<String, IdPAttribute> consentableAttributes = new TreeMap<>(attributeIdComparator);
for (final IdPAttribute attribute : attributes.values()) {
@@ -93,7 +98,9 @@ public class PopulateAttributeReleaseContext extends AbstractAttributeReleaseAct
}
}
- getAttributeReleaseContext().getConsentableAttributes().putAll(consentableAttributes);
+ final AttributeReleaseContext releaseContext = getAttributeReleaseContext();
+ assert releaseContext != null;
+ releaseContext.getConsentableAttributes().putAll(consentableAttributes);
log.debug("{} Consentable attribute IDs '{}'", getLogPrefix(), consentableAttributes.keySet());
diff --git a/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/flow/ar/impl/ReleaseAttributes.java b/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/flow/ar/impl/ReleaseAttributes.java
index da6c3c249..120fad66b 100644
--- a/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/flow/ar/impl/ReleaseAttributes.java
+++ b/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/flow/ar/impl/ReleaseAttributes.java
@@ -29,7 +29,10 @@ import com.google.common.collect.MapDifference;
import com.google.common.collect.Maps;
import net.shibboleth.idp.attribute.IdPAttribute;
+import net.shibboleth.idp.attribute.context.AttributeContext;
import net.shibboleth.idp.consent.Consent;
+import net.shibboleth.idp.consent.context.AttributeReleaseContext;
+import net.shibboleth.idp.consent.context.ConsentContext;
import net.shibboleth.idp.profile.context.ProfileInterceptorContext;
import net.shibboleth.shared.primitive.LoggerFactory;
@@ -57,19 +60,22 @@ public class ReleaseAttributes extends AbstractAttributeReleaseAction {
/** {@inheritDoc} */
@Override protected void doExecute(@Nonnull final ProfileRequestContext profileRequestContext,
@Nonnull final ProfileInterceptorContext interceptorContext) {
+ final AttributeContext attributeContext = getAttributeContext();
+ final AttributeReleaseContext releaseContext = getAttributeReleaseContext();
+ final ConsentContext consentContext = getConsentContext();
+ assert attributeContext != null && releaseContext != null && consentContext!=null;
- final Map<String, Consent> consents =
- getConsentContext().getCurrentConsents().isEmpty() ? getConsentContext().getPreviousConsents()
- : getConsentContext().getCurrentConsents();
+ final Map<String, Consent>consents =
+ consentContext.getCurrentConsents().isEmpty() ? consentContext.getPreviousConsents() : consentContext.getCurrentConsents();
log.debug("{} Consents '{}'", getLogPrefix(), consents);
- final Map<String, IdPAttribute> attributes = getAttributeContext().getIdPAttributes();
+ final Map<String, IdPAttribute> attributes = attributeContext.getIdPAttributes();
log.debug("{} Attributes before release: {}", getLogPrefix(), attributes.keySet());
final Map<String, IdPAttribute> releasedAttributes = new HashMap<>(attributes.size());
for (final IdPAttribute attribute : attributes.values()) {
- if (!getAttributeReleaseContext().getConsentableAttributes().containsKey(attribute.getId())) {
+ if (!releaseContext.getConsentableAttributes().containsKey(attribute.getId())) {
log.debug("{} Attribute '{}' will be released because it is excluded from consent", getLogPrefix(),
attribute.getId());
releasedAttributes.put(attribute.getId(), attribute);
@@ -97,7 +103,7 @@ public class ReleaseAttributes extends AbstractAttributeReleaseAction {
log.debug("{} Not releasing attributes: {}", getLogPrefix(), diff.entriesOnlyOnLeft().keySet());
}
- getAttributeContext().setIdPAttributes(releasedAttributes.values());
+ attributeContext.setIdPAttributes(releasedAttributes.values());
}
}
diff --git a/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/flow/impl/ExtractConsent.java b/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/flow/impl/ExtractConsent.java
index 5dd454f04..19e76b22b 100644
--- a/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/flow/impl/ExtractConsent.java
+++ b/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/flow/impl/ExtractConsent.java
@@ -62,7 +62,7 @@ public class ExtractConsent extends AbstractConsentAction {
@Nonnull final ProfileInterceptorContext interceptorContext) {
final ConsentContext consentContext = getConsentContext();
-
+ assert consentContext != null;
final HttpServletRequest request = getHttpServletRequest();
if (request == null) {
log.debug("{} Profile action does not contain an HttpServletRequest", getLogPrefix());
@@ -81,7 +81,7 @@ public class ExtractConsent extends AbstractConsentAction {
log.debug("{} Extracted consent ids '{}' from request parameter '{}'", getLogPrefix(), consentIds,
CONSENT_IDS_REQUEST_PARAMETER);
- final Map<String, Consent> currentConsents = getConsentContext().getCurrentConsents();
+ final Map<String, Consent> currentConsents = consentContext.getCurrentConsents();
for (final Consent consent : currentConsents.values()) {
if (consentIds.contains(consent.getId())) {
consent.setApproved(Boolean.TRUE);
diff --git a/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/flow/impl/PopulateConsentContext.java b/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/flow/impl/PopulateConsentContext.java
index f656fa9c6..8203fd9d1 100644
--- a/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/flow/impl/PopulateConsentContext.java
+++ b/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/flow/impl/PopulateConsentContext.java
@@ -26,6 +26,7 @@ import org.opensaml.profile.context.ProfileRequestContext;
import org.slf4j.Logger;
import net.shibboleth.idp.consent.Consent;
+import net.shibboleth.idp.consent.context.ConsentContext;
import net.shibboleth.idp.profile.context.ProfileInterceptorContext;
import net.shibboleth.shared.logic.Constraint;
import net.shibboleth.shared.primitive.LoggerFactory;
@@ -64,7 +65,9 @@ public class PopulateConsentContext extends AbstractConsentAction {
log.debug("{} Populating consents: {}", getLogPrefix(), consents.keySet());
if (consents != null) {
- getConsentContext().getCurrentConsents().putAll(consents);
+ final ConsentContext consentContext = getConsentContext();
+ assert consentContext != null;
+ consentContext.getCurrentConsents().putAll(consents);
}
}
diff --git a/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/flow/storage/impl/AbstractConsentIndexedStorageAction.java b/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/flow/storage/impl/AbstractConsentIndexedStorageAction.java
index e51d05301..2d2a70520 100644
--- a/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/flow/storage/impl/AbstractConsentIndexedStorageAction.java
+++ b/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/flow/storage/impl/AbstractConsentIndexedStorageAction.java
@@ -32,6 +32,7 @@ import javax.annotation.Nullable;
import org.opensaml.profile.context.ProfileRequestContext;
import org.opensaml.storage.StorageRecord;
import org.opensaml.storage.StorageSerializer;
+import org.opensaml.storage.StorageService;
import org.slf4j.Logger;
import net.shibboleth.idp.consent.flow.impl.ConsentFlowDescriptor;
@@ -165,8 +166,11 @@ public class AbstractConsentIndexedStorageAction extends AbstractConsentStorageA
*/
@Nonnull @NonnullElements protected List<String> getStorageKeysFromIndex() throws IOException {
- final StorageRecord<Collection<String>> storageRecord =
- getStorageService().read(getStorageContext(), getStorageIndexKey());
+ final StorageService service = getStorageService();
+ final String context = getStorageContext();
+ final String indexKey = getStorageIndexKey();
+ assert service != null && indexKey != null && context!= null;
+ final StorageRecord<Collection<String>> storageRecord = service.read(context, indexKey);
log.debug("{} Read storage record '{}' with context '{}' and key '{}'", getLogPrefix(), storageRecord,
getStorageContext(), getStorageIndexKey());
@@ -175,8 +179,7 @@ public class AbstractConsentIndexedStorageAction extends AbstractConsentStorageA
return CollectionSupport.emptyList();
}
- return new ArrayList<>(storageRecord.getValue(getStorageKeysSerializer(), getStorageContext(),
- getStorageIndexKey()));
+ return new ArrayList<>(storageRecord.getValue(getStorageKeysSerializer(), context, indexKey));
}
/**
@@ -188,21 +191,23 @@ public class AbstractConsentIndexedStorageAction extends AbstractConsentStorageA
*/
protected boolean addKeyToStorageIndex(@Nonnull final String keyToAdd) throws IOException {
- final StorageRecord<?> storageRecord = getStorageService().read(getStorageContext(), getStorageIndexKey());
+ final StorageService service = getStorageService();
+ final String storageContext = getStorageContext();
+ final String indexKey = getStorageIndexKey();
+ assert service != null && indexKey != null && storageContext!= null;
+ final StorageRecord<?> storageRecord = service.read(storageContext, indexKey);
log.debug("{} Read storage record '{}' with context '{}' and key '{}'", getLogPrefix(), storageRecord,
getStorageContext(), getStorageIndexKey());
if (storageRecord == null) {
log.debug("{} Creating storage index with key '{}'", getLogPrefix(), keyToAdd);
- return getStorageService().create(getStorageContext(), getStorageIndexKey(),
- CollectionSupport.singletonList(keyToAdd), storageKeysSerializer, null);
+ return service.create(storageContext, indexKey, CollectionSupport.singletonList(keyToAdd), storageKeysSerializer, null);
}
final LinkedHashSet<String> keys = new LinkedHashSet<>(getStorageKeysFromIndex());
if (keys.add(keyToAdd)) {
log.debug("{} Updating storage index by adding key '{}'", getLogPrefix(), keyToAdd);
- return getStorageService().update(getStorageContext(), getStorageIndexKey(), keys,
- storageKeysSerializer, null);
+ return service.update(storageContext, indexKey, keys, storageKeysSerializer, null);
}
log.debug("{} Storage key '{}' already indexed, nothing to do", getLogPrefix(), keyToAdd);
@@ -218,7 +223,12 @@ public class AbstractConsentIndexedStorageAction extends AbstractConsentStorageA
*/
protected boolean removeKeyFromStorageIndex(@Nonnull final String keyToRemove) throws IOException {
- final StorageRecord<?> storageRecord = getStorageService().read(getStorageContext(), getStorageIndexKey());
+ final StorageService service = getStorageService();
+ final String storageContext = getStorageContext();
+ final String indexKey = getStorageIndexKey();
+ assert service != null && indexKey != null && storageContext!= null;
+
+ final StorageRecord<?> storageRecord = service.read(storageContext, indexKey);
log.debug("{} Read storage record '{}' with context '{}' and key '{}'", getLogPrefix(), storageRecord,
getStorageContext(), getStorageIndexKey());
@@ -231,7 +241,8 @@ public class AbstractConsentIndexedStorageAction extends AbstractConsentStorageA
final LinkedHashSet<String> keys = new LinkedHashSet<>(getStorageKeysFromIndex());
if (keys.remove(keyToRemove)) {
log.debug("{} Updating storage index by removing key '{}'", getLogPrefix(), keyToRemove);
- return getStorageService().update(getStorageContext(), storageIndexKey, keys, storageKeysSerializer,
+ assert storageIndexKey != null;
+ return service.update(storageContext, storageIndexKey, keys, storageKeysSerializer,
null);
}
@@ -256,8 +267,11 @@ public class AbstractConsentIndexedStorageAction extends AbstractConsentStorageA
protected void pruneStorageRecords(@Nonnull final ProfileRequestContext profileRequestContext) throws IOException {
final ConsentFlowDescriptor flowDescriptor = getConsentFlowDescriptor();
+ final StorageService service = getStorageService();
+ final String storageContext = getStorageContext();
+ assert service != null && flowDescriptor!=null &&storageContext!= null;
int maxStoredRecords = flowDescriptor.getMaximumNumberOfStoredRecords();
- if (getStorageService().getCapabilities().getValueSize() >= flowDescriptor.getExpandedStorageThreshold()) {
+ if (service.getCapabilities().getValueSize() >= flowDescriptor.getExpandedStorageThreshold()) {
maxStoredRecords = flowDescriptor.getExpandedNumberOfStoredRecords();
}
@@ -289,12 +303,13 @@ public class AbstractConsentIndexedStorageAction extends AbstractConsentStorageA
while (keysIterator.hasNext() && numberOfKeys >= maxStoredRecords) {
final String keyToDelete = keysIterator.next();
+ assert keyToDelete!=null;
log.debug("{} Pruning storage record with key '{}'. There are '{}' records of max '{}' ", getLogPrefix(),
keyToDelete, numberOfKeys, maxStoredRecords);
log.debug("{} Deleting storage record with context '{}' and key '{}'", getLogPrefix(), getStorageContext(),
keyToDelete);
- final boolean success = getStorageService().delete(getStorageContext(), keyToDelete);
+ final boolean success = service.delete(storageContext, keyToDelete);
if (success) {
numberOfKeys--;
@@ -324,11 +339,13 @@ public class AbstractConsentIndexedStorageAction extends AbstractConsentStorageA
int attempts = 10;
boolean success = false;
do {
- success = getStorageService().create(context, key, value,
+ final StorageService service = getStorageService();
+ assert service != null;
+ success = service.create(context, key, value,
expiration != null ? expiration.toEpochMilli() : null);
if (!success) {
// The record already exists, so we need to overwrite via an update.
- success = getStorageService().update(context, key, value,
+ success = service .update(context, key, value,
expiration != null ? expiration.toEpochMilli() : null);
}
} while (!success && attempts-- > 0);
diff --git a/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/flow/storage/impl/CreateGlobalConsentResult.java b/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/flow/storage/impl/CreateGlobalConsentResult.java
index 12e9e54e7..53c0870d2 100644
--- a/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/flow/storage/impl/CreateGlobalConsentResult.java
+++ b/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/flow/storage/impl/CreateGlobalConsentResult.java
@@ -20,7 +20,6 @@ package net.shibboleth.idp.consent.flow.storage.impl;
import java.io.IOException;
import java.time.Duration;
import java.time.Instant;
-import java.util.Collections;
import javax.annotation.Nonnull;
@@ -30,9 +29,11 @@ import org.opensaml.profile.context.ProfileRequestContext;
import org.slf4j.Logger;
import net.shibboleth.idp.consent.Consent;
+import net.shibboleth.idp.consent.flow.impl.ConsentFlowDescriptor;
import net.shibboleth.idp.consent.storage.impl.ConsentResult;
import net.shibboleth.idp.profile.context.ProfileInterceptorContext;
import net.shibboleth.idp.profile.interceptor.ProfileInterceptorResult;
+import net.shibboleth.shared.collection.CollectionSupport;
import net.shibboleth.shared.primitive.LoggerFactory;
/**
@@ -55,11 +56,15 @@ public class CreateGlobalConsentResult extends AbstractConsentIndexedStorageActi
final Consent globalConsent = new Consent();
globalConsent.setId(Consent.WILDCARD);
globalConsent.setApproved(true);
-
+ final String id = globalConsent.getId();
+ assert id!= null;
final String value =
- getStorageSerializer().serialize(Collections.singletonMap(globalConsent.getId(), globalConsent));
-
- final Duration lifetime = getConsentFlowDescriptor().getLifetime();
+ getStorageSerializer().serialize(CollectionSupport.singletonMap(id, globalConsent));
+ final ConsentFlowDescriptor flowDescriptor = getConsentFlowDescriptor();
+ final String storageContext = getStorageContext();
+ final String storageKey = getStorageKey();
+ assert flowDescriptor!=null && storageContext!= null &&storageKey!= null;
+ final Duration lifetime = flowDescriptor.getLifetime();
final Instant expiration;
if (lifetime == null) {
expiration = null;
@@ -68,7 +73,7 @@ public class CreateGlobalConsentResult extends AbstractConsentIndexedStorageActi
}
final ProfileInterceptorResult result =
- new ConsentResult(getStorageContext(), getStorageKey(), value, expiration);
+ new ConsentResult(storageContext, storageKey, value, expiration);
log.debug("{} Created global consent result '{}'", getLogPrefix(), result);
diff --git a/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/flow/storage/impl/CreateResult.java b/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/flow/storage/impl/CreateResult.java
index aa2ee42fd..0dc53a7d7 100644
--- a/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/flow/storage/impl/CreateResult.java
+++ b/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/flow/storage/impl/CreateResult.java
@@ -28,6 +28,8 @@ import org.opensaml.profile.context.ProfileRequestContext;
import org.slf4j.Logger;
import net.shibboleth.idp.consent.Consent;
+import net.shibboleth.idp.consent.context.ConsentContext;
+import net.shibboleth.idp.consent.flow.impl.ConsentFlowDescriptor;
import net.shibboleth.idp.consent.storage.impl.ConsentResult;
import net.shibboleth.idp.profile.context.ProfileInterceptorContext;
import net.shibboleth.idp.profile.interceptor.ProfileInterceptorResult;
@@ -54,8 +56,11 @@ public class CreateResult extends AbstractConsentIndexedStorageAction {
if (!super.doPreExecute(profileRequestContext, interceptorContext)) {
return false;
}
+ final ConsentContext context = getConsentContext();
+ assert context!= null;
- if (getConsentContext().getCurrentConsents().isEmpty()) {
+
+ if (context.getCurrentConsents().isEmpty()) {
log.debug("{} No result will be created because there are no current consents", getLogPrefix());
return false;
}
@@ -67,11 +72,16 @@ public class CreateResult extends AbstractConsentIndexedStorageAction {
@Override protected void doExecute(@Nonnull final ProfileRequestContext profileRequestContext,
@Nonnull final ProfileInterceptorContext interceptorContext) {
+ final ConsentContext consentContext = getConsentContext();
+ final ConsentFlowDescriptor flowDescriptor = getConsentFlowDescriptor();
+ final String storageContext = getStorageContext();
+ final String storageKey = getStorageKey();
+ assert consentContext!= null && flowDescriptor!=null && storageContext!=null && storageKey!=null;
try {
- final Map<String, Consent> currentConsents = getConsentContext().getCurrentConsents();
+ final Map<String, Consent> currentConsents = consentContext.getCurrentConsents();
final String value = getStorageSerializer().serialize(currentConsents);
- final Duration lifetime = getConsentFlowDescriptor().getLifetime();
+ final Duration lifetime = flowDescriptor.getLifetime();
final Instant expiration;
if (lifetime == null) {
expiration = null;
@@ -79,7 +89,7 @@ public class CreateResult extends AbstractConsentIndexedStorageAction {
expiration = Instant.now().plus(lifetime);
}
final ProfileInterceptorResult result =
- new ConsentResult(getStorageContext(), getStorageKey(), value, expiration);
+ new ConsentResult(storageContext, storageKey, value, expiration);
log.debug("{} Created consent result '{}'", getLogPrefix(), result);
diff --git a/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/flow/storage/impl/ReadConsentFromStorage.java b/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/flow/storage/impl/ReadConsentFromStorage.java
index 43117dff7..398cd6951 100644
--- a/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/flow/storage/impl/ReadConsentFromStorage.java
+++ b/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/flow/storage/impl/ReadConsentFromStorage.java
@@ -24,9 +24,12 @@ import javax.annotation.Nonnull;
import org.opensaml.profile.context.ProfileRequestContext;
import org.opensaml.storage.StorageRecord;
+import org.opensaml.storage.StorageSerializer;
+import org.opensaml.storage.StorageService;
import org.slf4j.Logger;
import net.shibboleth.idp.consent.Consent;
+import net.shibboleth.idp.consent.context.ConsentContext;
import net.shibboleth.idp.profile.context.ProfileInterceptorContext;
import net.shibboleth.shared.primitive.LoggerFactory;
/**
@@ -44,22 +47,26 @@ public class ReadConsentFromStorage extends AbstractConsentStorageAction {
@Override protected void doExecute(@Nonnull final ProfileRequestContext profileRequestContext,
@Nonnull final ProfileInterceptorContext interceptorContext) {
- final String context = getStorageContext();
+ final String storageContext = getStorageContext();
+ final ConsentContext consentContext = getConsentContext();
final String key = getStorageKey();
+ final StorageService service = getStorageService();
+ final StorageSerializer<Map<String, Consent>> storageSerializer = getStorageSerializer();
+ assert consentContext != null && service != null && key != null && storageContext!= null && storageSerializer!=null;
try {
- final StorageRecord<Map<String,Consent>> storageRecord = getStorageService().read(context, key);
+ final StorageRecord<Map<String,Consent>> storageRecord = service.read(storageContext, key);
log.debug("{} Read storage record '{}' with context '{}' and key '{}'", getLogPrefix(), storageRecord,
- context, key);
+ storageContext, key);
if (storageRecord == null) {
- log.debug("{} No storage record for context '{}' and key '{}'", getLogPrefix(), context, key);
+ log.debug("{} No storage record for context '{}' and key '{}'", getLogPrefix(), storageContext, key);
return;
}
- final Map<String,Consent> consents = storageRecord.getValue(getStorageSerializer(), context, key);
+ final Map<String,Consent> consents = storageRecord.getValue(storageSerializer, storageContext, key);
- getConsentContext().getPreviousConsents().putAll(consents);
+ consentContext.getPreviousConsents().putAll(consents);
} catch (final IOException e) {
log.error("{} Unable to read consent from storage", getLogPrefix(), e);
diff --git a/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/flow/storage/impl/RevokeConsent.java b/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/flow/storage/impl/RevokeConsent.java
index 7b8e36280..c345477d8 100644
--- a/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/flow/storage/impl/RevokeConsent.java
+++ b/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/flow/storage/impl/RevokeConsent.java
@@ -24,6 +24,7 @@ import javax.annotation.Nonnull;
import org.opensaml.profile.action.ActionSupport;
import org.opensaml.profile.action.EventIds;
import org.opensaml.profile.context.ProfileRequestContext;
+import org.opensaml.storage.StorageService;
import org.slf4j.Logger;
import net.shibboleth.idp.profile.context.ProfileInterceptorContext;
@@ -59,11 +60,13 @@ public class RevokeConsent extends AbstractConsentIndexedStorageAction {
final String context = getStorageContext();
final String key = getStorageKey();
+ final StorageService service = getStorageService();
+ assert context!=null && key!=null && service!=null;
log.debug("{} Attempting to delete consent storage record with context '{}' and key '{}'", getLogPrefix(),
context, key);
try {
- final boolean success = getStorageService().delete(context, key);
+ final boolean success = service.delete(context, key);
if (success) {
log.debug("{} Deleted consent storage record with context '{}' and key '{}'", getLogPrefix(), context,
key);
diff --git a/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/flow/storage/impl/UpdateCounter.java b/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/flow/storage/impl/UpdateCounter.java
index 0a722028e..2980d148c 100644
--- a/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/flow/storage/impl/UpdateCounter.java
+++ b/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/flow/storage/impl/UpdateCounter.java
@@ -23,6 +23,7 @@ import javax.annotation.Nonnull;
import org.opensaml.profile.context.ProfileRequestContext;
import org.opensaml.storage.StorageRecord;
+import org.opensaml.storage.StorageService;
import org.slf4j.Logger;
import net.shibboleth.shared.primitive.LoggerFactory;
@@ -49,17 +50,19 @@ public class UpdateCounter extends AbstractConsentStorageAction {
final String context = getStorageContext();
final String key = getStorageKey();
+ final StorageService service = getStorageService();
+ assert context!=null && key!=null && service!=null;
- final StorageRecord<?> storageRecord = getStorageService().read(context, key);
+ final StorageRecord<?> storageRecord = service.read(context, key);
log.debug("{} Read storage record '{}' with context '{}' and key '{}'", getLogPrefix(), storageRecord,
context, key);
if (storageRecord == null) {
log.debug("{} Creating counter with value '{}'", getLogPrefix(), value);
- getStorageService().create(context, key, value, null);
+ service.create(context, key, value, null);
} else {
log.debug("{} Updating counter with value '{}'", getLogPrefix(), value);
- getStorageService().update(context, key, value, null);
+ service.update(context, key, value, null);
}
} catch (final IOException e) {
diff --git a/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/logic/impl/AttributeDisplayDescriptionFunction.java b/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/logic/impl/AttributeDisplayDescriptionFunction.java
index 2f353414b..3fc0a4290 100644
--- a/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/logic/impl/AttributeDisplayDescriptionFunction.java
+++ b/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/logic/impl/AttributeDisplayDescriptionFunction.java
@@ -49,7 +49,7 @@ public class AttributeDisplayDescriptionFunction extends AbstractAttributeDispla
}
/** {@inheritDoc} */
- protected Map<Locale, String> getDisplayInfo( @Nonnull final AttributeTranscoderRegistry registry,
+ protected @Nonnull Map<Locale, String> getDisplayInfo( @Nonnull final AttributeTranscoderRegistry registry,
@Nonnull final IdPAttribute attribute) {
return registry.getDescriptions(attribute);
}
diff --git a/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/logic/impl/AttributePredicate.java b/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/logic/impl/AttributePredicate.java
index 77f7590e7..5f9b97fbb 100644
--- a/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/logic/impl/AttributePredicate.java
+++ b/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/logic/impl/AttributePredicate.java
@@ -18,9 +18,9 @@
package net.shibboleth.idp.consent.logic.impl;
import java.util.Collection;
-import java.util.Collections;
import java.util.HashSet;
import java.util.Set;
+import java.util.function.Predicate;
import java.util.regex.Pattern;
import javax.annotation.Nonnull;
@@ -30,9 +30,9 @@ import net.shibboleth.idp.attribute.EmptyAttributeValue;
import net.shibboleth.idp.attribute.IdPAttribute;
import net.shibboleth.idp.attribute.IdPAttributeValue;
import net.shibboleth.shared.annotation.constraint.NonnullElements;
+import net.shibboleth.shared.collection.CollectionSupport;
import net.shibboleth.shared.component.AbstractInitializableComponent;
import net.shibboleth.shared.primitive.StringSupport;
-import java.util.function.Predicate;
/**
* Predicate to determine whether consent should be obtained for an attribute.
@@ -50,8 +50,8 @@ public class AttributePredicate extends AbstractInitializableComponent implement
/** Constructor. */
public AttributePredicate() {
- promptedAttributeIds = Collections.emptySet();
- ignoredAttributeIds = Collections.emptySet();
+ promptedAttributeIds = CollectionSupport.emptySet();
+ ignoredAttributeIds = CollectionSupport.emptySet();
}
/**
@@ -100,18 +100,19 @@ public class AttributePredicate extends AbstractInitializableComponent implement
}
final String attributeId = input.getId();
+ final Pattern expression = matchExpression;
if (!promptedAttributeIds.isEmpty() && !promptedAttributeIds.contains(attributeId)) {
// Not in prompted set. Only prompt if a regexp applies.
- if (matchExpression == null) {
+ if (expression == null) {
return false;
}
- return matchExpression.matcher(attributeId).matches();
+ return expression.matcher(attributeId).matches();
}
// In prompted set (or none). Check unprompted set, and if necessary a regexp.
return !ignoredAttributeIds.contains(attributeId)
- && (matchExpression == null || matchExpression.matcher(attributeId).matches());
+ && (expression == null || expression.matcher(attributeId).matches());
}
/**
diff --git a/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/logic/impl/AttributeValuesHashFunction.java b/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/logic/impl/AttributeValuesHashFunction.java
index 85c502f8c..c984fd722 100644
--- a/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/logic/impl/AttributeValuesHashFunction.java
+++ b/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/logic/impl/AttributeValuesHashFunction.java
@@ -122,6 +122,7 @@ public class AttributeValuesHashFunction implements Function<Collection<IdPAttri
final MessageDigest digest = MessageDigest.getInstance("SHA-256");
final byte[] digestedBytes = digest.digest(byteArrayOutputStream.toByteArray());
+ assert digestedBytes!=null;
return Base64Support.encode(digestedBytes, false);
} catch (final IOException | NoSuchAlgorithmException | EncodingException e) {
diff --git a/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/logic/impl/CounterStorageKeyComparator.java b/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/logic/impl/CounterStorageKeyComparator.java
index 56716e0fa..008fb59d9 100644
--- a/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/logic/impl/CounterStorageKeyComparator.java
+++ b/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/logic/impl/CounterStorageKeyComparator.java
@@ -23,10 +23,7 @@ import java.util.Map;
import javax.annotation.Nonnull;
-import org.slf4j.Logger;
-
import net.shibboleth.shared.logic.Constraint;
-import net.shibboleth.shared.primitive.LoggerFactory;
/**
* A {@link Comparator} used to order storage keys so that the least used and oldest storage keys are returned first.
@@ -36,9 +33,6 @@ import net.shibboleth.shared.primitive.LoggerFactory;
*/
public class CounterStorageKeyComparator implements Comparator<String> {
- /** Class logger. */
- @Nonnull private final Logger log = LoggerFactory.getLogger(CounterStorageKeyComparator.class);
-
/** Storage keys in FIFO order. */
@Nonnull private final List<String> storageKeys;
diff --git a/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/logic/impl/CounterStorageKeyFunction.java b/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/logic/impl/CounterStorageKeyFunction.java
index c743dee3f..36136c0fd 100644
--- a/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/logic/impl/CounterStorageKeyFunction.java
+++ b/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/logic/impl/CounterStorageKeyFunction.java
@@ -66,8 +66,8 @@ public class CounterStorageKeyFunction extends AbstractInitializableComponent im
/** Constructor. */
public CounterStorageKeyFunction() {
- setInterceptorContextLookupStrategy(new ChildContextLookup<>(ProfileInterceptorContext.class));
- setStorageContextLookupStrategy(new FlowIdLookupFunction());
+ interceptorContextlookupStrategy = new ChildContextLookup<>(ProfileInterceptorContext.class);
+ storageContextLookupStrategy = new FlowIdLookupFunction();
}
/**
@@ -111,8 +111,7 @@ public class CounterStorageKeyFunction extends AbstractInitializableComponent im
Constraint.isNotNull(interceptorContext,
"Profile interceptor context not available from profile request context");
- final ProfileInterceptorFlowDescriptor flowDescriptor = interceptorContext.getAttemptedFlow();
- Constraint.isNotNull(flowDescriptor,
+ final ProfileInterceptorFlowDescriptor flowDescriptor = Constraint.isNotNull(interceptorContext.getAttemptedFlow(),
"Profile interceptor flow descriptor not available from profile interceptor context");
return Constraint.isNotNull(flowDescriptor.getStorageService(),
@@ -188,6 +187,7 @@ public class CounterStorageKeyFunction extends AbstractInitializableComponent im
final Map<String, Long> map = new LinkedHashMap<>();
for (final String storageKey : storageKeys) {
+ assert storageKey != null;
try {
map.put(storageKey, getStorageKeyCounter(storageService, storageContext, storageKey));
} catch (final NumberFormatException | IOException e) {
@@ -199,14 +199,16 @@ public class CounterStorageKeyFunction extends AbstractInitializableComponent im
/** {@inheritDoc} */
@Nullable public List<String> apply(@Nullable final Pair<ProfileRequestContext, List<String>> input) {
- if (input == null || input.getFirst() == null || input.getSecond() == null) {
+ if (input == null) {
+ return null;
+ }
+ final ProfileRequestContext profileRequestContext = input.getFirst();
+ final List<String> storageKeys = input.getSecond();
+ if (storageKeys==null || profileRequestContext==null) {
return null;
}
try {
- final ProfileRequestContext profileRequestContext = input.getFirst();
- final List<String> storageKeys = input.getSecond();
-
final Map<String, Long> keyToCounterMap = getStorageKeyCounters(profileRequestContext, storageKeys);
final Comparator<String> comparator = new CounterStorageKeyComparator(storageKeys, keyToCounterMap);
diff --git a/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/logic/impl/GlobalAttributeConsentPredicate.java b/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/logic/impl/GlobalAttributeConsentPredicate.java
index 008b71849..f9420f995 100644
--- a/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/logic/impl/GlobalAttributeConsentPredicate.java
+++ b/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/logic/impl/GlobalAttributeConsentPredicate.java
@@ -37,9 +37,6 @@ import net.shibboleth.shared.primitive.LoggerFactory;
*/
public class GlobalAttributeConsentPredicate implements Predicate<ProfileRequestContext> {
- /** Class logger. */
- @Nonnull private final Logger log = LoggerFactory.getLogger(GlobalAttributeConsentPredicate.class);
-
/** Strategy used to find the {@link ConsentContext} from the {@link ProfileRequestContext}. */
@Nonnull private Function<ProfileRequestContext, ConsentContext> consentContextlookupStrategy;
diff --git a/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/logic/impl/IsAttributeRequiredPredicate.java b/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/logic/impl/IsAttributeRequiredPredicate.java
index 8afd9f061..1b055a3c3 100644
--- a/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/logic/impl/IsAttributeRequiredPredicate.java
+++ b/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/logic/impl/IsAttributeRequiredPredicate.java
@@ -128,8 +128,9 @@ public class IsAttributeRequiredPredicate implements Predicate<IdPAttribute> {
/** {@inheritDoc} */
public boolean test(@Nullable final IdPAttribute input) {
- if (input != null && requestedAttributesMap != null && !requestedAttributesMap.isEmpty()) {
- final Collection<IdPAttribute> requestedAttrs = requestedAttributesMap.get(input.getId());
+ final Multimap<String,IdPAttribute> ram = requestedAttributesMap;
+ if (input != null && ram != null && !ram .isEmpty()) {
+ final Collection<IdPAttribute> requestedAttrs = ram .get(input.getId());
if (requestedAttrs != null) {
for (final IdPAttribute requestedAttr : requestedAttrs) {
if (requestedAttr instanceof IdPRequestedAttribute
diff --git a/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/logic/impl/LocaleLookupFunction.java b/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/logic/impl/LocaleLookupFunction.java
index ccc41a75c..69125dbb3 100644
--- a/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/logic/impl/LocaleLookupFunction.java
+++ b/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/logic/impl/LocaleLookupFunction.java
@@ -25,6 +25,7 @@ import javax.annotation.Nullable;
import net.shibboleth.idp.profile.context.SpringRequestContext;
import org.opensaml.profile.context.ProfileRequestContext;
+import org.springframework.webflow.execution.RequestContext;
/**
* Function which resolves the {@link Locale} from a {@link ProfileRequestContext}.
@@ -38,8 +39,9 @@ public class LocaleLookupFunction implements Function<ProfileRequestContext, Loc
}
final SpringRequestContext springSubcontext = input.getSubcontext(SpringRequestContext.class);
- if (springSubcontext != null && springSubcontext.getRequestContext() != null) {
- return springSubcontext.getRequestContext().getExternalContext().getLocale();
+ final RequestContext rc = springSubcontext != null ? springSubcontext.getRequestContext() : null;
+ if (rc != null) {
+ return rc.getExternalContext().getLocale();
}
return null;
diff --git a/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/logic/impl/MessageSourceConsentFunction.java b/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/logic/impl/MessageSourceConsentFunction.java
index 480110f85..a7e233c90 100644
--- a/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/logic/impl/MessageSourceConsentFunction.java
+++ b/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/logic/impl/MessageSourceConsentFunction.java
@@ -69,7 +69,7 @@ public class MessageSourceConsentFunction extends AbstractInitializableComponent
@Nonnull private Function<ProfileRequestContext,Locale> localeLookupStrategy;
/** MessageSource injected by Spring, typically the parent ApplicationContext itself. */
- @Nonnull private MessageSource messageSource;
+ @NonnullAfterInit private MessageSource messageSource;
/** Constructor. */
public MessageSourceConsentFunction() {
@@ -82,7 +82,7 @@ public class MessageSourceConsentFunction extends AbstractInitializableComponent
/** {@inheritDoc} */
@Override
- public void setMessageSource(final MessageSource source) {
+ public void setMessageSource(final @Nonnull MessageSource source) {
messageSource = source;
}
@@ -162,6 +162,9 @@ public class MessageSourceConsentFunction extends AbstractInitializableComponent
throw new ComponentInitializationException(e);
}
}
+ if (messageSource == null) {
+ throw new ComponentInitializationException("Message source not set up by Spring");
+ }
}
/** {@inheritDoc} */
@@ -171,6 +174,7 @@ public class MessageSourceConsentFunction extends AbstractInitializableComponent
}
final Locale locale = getLocale(input);
+ assert locale != null;
final String id = getConsentId(input, locale);
if (id != null) {
final Consent consent = new Consent();
@@ -247,7 +251,7 @@ public class MessageSourceConsentFunction extends AbstractInitializableComponent
*
* @return locale
*/
- @Nullable protected Locale getLocale(@Nonnull final ProfileRequestContext profileRequestContext) {
+ @Nullable Locale getLocale(@Nonnull final ProfileRequestContext profileRequestContext) {
return localeLookupStrategy.apply(profileRequestContext);
}
diff --git a/idp-consent-impl/src/test/java/net/shibboleth/idp/consent/logic/impl/MessageSourceConsentFunctionTest.java b/idp-consent-impl/src/test/java/net/shibboleth/idp/consent/logic/impl/MessageSourceConsentFunctionTest.java
index fa7dd1d77..b9c44e8e7 100644
--- a/idp-consent-impl/src/test/java/net/shibboleth/idp/consent/logic/impl/MessageSourceConsentFunctionTest.java
+++ b/idp-consent-impl/src/test/java/net/shibboleth/idp/consent/logic/impl/MessageSourceConsentFunctionTest.java
@@ -117,6 +117,7 @@ public class MessageSourceConsentFunctionTest {
setUpDescriptor(false);
function.setConsentKeyLookupStrategy(FunctionSupport.constant("key"));
+ function.setLocaleLookupStrategy(e -> new Locale("en"));
function.initialize();
final Consent consent = new Consent();
@@ -133,6 +134,7 @@ public class MessageSourceConsentFunctionTest {
setUpDescriptor(true);
function.setConsentKeyLookupStrategy(FunctionSupport.constant("key"));
+ function.setLocaleLookupStrategy(e -> new Locale("en"));
function.initialize();
final Consent consent = new Consent();
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list