From b571739af5e8c6be7e50aaa44b0b04f428b34fbb Mon Sep 17 00:00:00 2001 From: Rolain Djeumen Date: Mon, 24 Jun 2024 12:04:10 +0100 Subject: [PATCH] feat(jans-keycloak-integration): enhancements to jans-keycloak-integration #8614 * fixes suggested by static analyser Signed-off-by: Rolain Djeumen --- .../main/java/io/jans/kc/scheduler/App.java | 4 +- .../scheduler/TrustRelationshipSyncJob.java | 15 ------ .../scheduler/job/impl/QuartzJobWrapper.java | 2 - .../jans/kc/model/JansUserAttributeModel.java | 1 - .../java/io/jans/kc/model/JansUserModel.java | 51 ++++++++----------- .../io/jans/kc/model/internal/JansPerson.java | 14 ++--- .../java/io/jans/kc/oidc/OIDCAuthRequest.java | 4 +- .../io/jans/kc/oidc/OIDCMetaCacheKeys.java | 4 ++ .../kc/oidc/impl/HashBasedOIDCMetaCache.java | 40 +++++++-------- .../jans/kc/spi/auth/JansAuthenticator.java | 8 +-- 10 files changed, 55 insertions(+), 88 deletions(-) diff --git a/jans-keycloak-integration/job-scheduler/src/main/java/io/jans/kc/scheduler/App.java b/jans-keycloak-integration/job-scheduler/src/main/java/io/jans/kc/scheduler/App.java index c4d39157eee..db7c1593e4e 100644 --- a/jans-keycloak-integration/job-scheduler/src/main/java/io/jans/kc/scheduler/App.java +++ b/jans-keycloak-integration/job-scheduler/src/main/java/io/jans/kc/scheduler/App.java @@ -95,13 +95,15 @@ public static void main(String[] args) throws InterruptedException, ParserCreate } System.exit(-1); return; + }catch(InterruptedException e) { + log.error("Application interrupted",e); + throw e; }catch(Exception e) { log.error("Fatal error starting application",e); if(jobScheduler != null ) { jobScheduler.stop(); } System.exit(-1); - return; } } diff --git a/jans-keycloak-integration/job-scheduler/src/main/java/io/jans/kc/scheduler/TrustRelationshipSyncJob.java b/jans-keycloak-integration/job-scheduler/src/main/java/io/jans/kc/scheduler/TrustRelationshipSyncJob.java index d23120b7228..1e01ea212e9 100644 --- a/jans-keycloak-integration/job-scheduler/src/main/java/io/jans/kc/scheduler/TrustRelationshipSyncJob.java +++ b/jans-keycloak-integration/job-scheduler/src/main/java/io/jans/kc/scheduler/TrustRelationshipSyncJob.java @@ -207,14 +207,6 @@ private void addReleasedAttributesToManagedSamlClient(ManagedSamlClient client, List protmappers = releasedattributes.stream().map((r)-> { log.debug("Preparing to add released attribute {} to managed saml client with clientId {}",r.getName(),client.clientId()); - /*return ProtocolMapper - .samlUserAttributeMapper(samlUserAttributeMapperId) - .name(generateKeycloakUniqueProtocolMapperName(r)) - .userAttribute(r.getName()) - .friendlyName(r.getDisplayName()!=null?r.getDisplayName():r.getName()) - .attributeName(r.getSaml2Uri()) - .attributeNameFormatUriReference() - .build(); */ return ProtocolMapper .samlUserAttributeMapper(samlUserAttributeMapperId) .name(generateKeycloakUniqueProtocolMapperName(r)) @@ -228,13 +220,6 @@ private void addReleasedAttributesToManagedSamlClient(ManagedSamlClient client, private void updateManagedSamlClientProtocolMapper(ManagedSamlClient client, ProtocolMapper mapper, JansAttributeRepresentation releasedattribute) { log.debug("Updating managed client released attribute. Client id: {} / Attribute name: {}",client.clientId(),releasedattribute.getName()); - /*ProtocolMapper newmapper = ProtocolMapper - .samlUserAttributeMapper(mapper) - .userAttribute(releasedattribute.getName()) - .friendlyName(releasedattribute.getDisplayName()!=null?releasedattribute.getDisplayName():releasedattribute.getName()) - .attributeName(releasedattribute.getSaml2Uri()) - .attributeNameFormatUriReference() - .build(); */ ProtocolMapper newmapper = ProtocolMapper .samlUserAttributeMapper(mapper) .jansAttributeName(releasedattribute.getName()) diff --git a/jans-keycloak-integration/job-scheduler/src/main/java/io/jans/kc/scheduler/job/impl/QuartzJobWrapper.java b/jans-keycloak-integration/job-scheduler/src/main/java/io/jans/kc/scheduler/job/impl/QuartzJobWrapper.java index 5a7c5cde4a4..cf177c1484f 100644 --- a/jans-keycloak-integration/job-scheduler/src/main/java/io/jans/kc/scheduler/job/impl/QuartzJobWrapper.java +++ b/jans-keycloak-integration/job-scheduler/src/main/java/io/jans/kc/scheduler/job/impl/QuartzJobWrapper.java @@ -38,8 +38,6 @@ public void execute(JobExecutionContext context) throws JobExecutionException { io.jans.kc.scheduler.job.Job job = (io.jans.kc.scheduler.job.Job) constructor.newInstance(); ExecutionContext effectivecontext = new QuartzExecutionContext(context.getMergedJobDataMap()); job.run(effectivecontext); - } catch(ReflectiveOperationException e) { - throw new JobExecutionException("Failed to run job " + jobname,e); }catch(Exception e) { throw new JobExecutionException("Failed to run job " + jobname,e); } diff --git a/jans-keycloak-integration/spi/src/main/java/io/jans/kc/model/JansUserAttributeModel.java b/jans-keycloak-integration/spi/src/main/java/io/jans/kc/model/JansUserAttributeModel.java index cad9a2c2545..43d4296e6a4 100644 --- a/jans-keycloak-integration/spi/src/main/java/io/jans/kc/model/JansUserAttributeModel.java +++ b/jans-keycloak-integration/spi/src/main/java/io/jans/kc/model/JansUserAttributeModel.java @@ -8,7 +8,6 @@ import org.apache.commons.lang3.StringUtils; -import org.keycloak.dom.saml.v2.assertion.AttributeStatementType; import org.keycloak.dom.saml.v2.assertion.AttributeType; import org.keycloak.saml.common.constants.JBossSAMLURIConstants; diff --git a/jans-keycloak-integration/spi/src/main/java/io/jans/kc/model/JansUserModel.java b/jans-keycloak-integration/spi/src/main/java/io/jans/kc/model/JansUserModel.java index 8eb3664af83..fe9ce074ec5 100644 --- a/jans-keycloak-integration/spi/src/main/java/io/jans/kc/model/JansUserModel.java +++ b/jans-keycloak-integration/spi/src/main/java/io/jans/kc/model/JansUserModel.java @@ -1,7 +1,6 @@ package io.jans.kc.model; import io.jans.kc.model.internal.JansPerson; -import io.jans.orm.model.base.CustomObjectAttribute; import java.util.ArrayList; import java.util.HashMap; @@ -20,8 +19,6 @@ import org.keycloak.storage.ReadOnlyException; import org.keycloak.storage.StorageId; -import org.jboss.logging.Logger; - public class JansUserModel implements UserModel { private static final String INUM_ATTR_NAME = "inum"; @@ -31,18 +28,13 @@ public class JansUserModel implements UserModel { private static final String GIVEN_NAME_ATTR_NAME = "givenName"; private static final String MAIL_ATTR_NAME = "mail"; private static final String EMAIL_VERIFIED_ATTR_NAME = "emailVerified"; - - private static final Logger log = Logger.getLogger(JansUserModel.class); + private static final String USER_READ_ONLY_EXCEPTION_MSG = "User is read-only for this update"; private final JansPerson jansPerson; private final StorageId storageId; - private final ComponentModel storageProviderModel; - private final KeycloakSession session; public JansUserModel(KeycloakSession session, ComponentModel storageProviderModel, JansPerson jansPerson) { - this.session = session; - this.storageProviderModel = storageProviderModel; this.jansPerson = jansPerson; String userId = jansPerson.customAttributeValue(INUM_ATTR_NAME); this.storageId = new StorageId(storageProviderModel.getId(),userId); @@ -63,7 +55,7 @@ public String getUsername() { @Override public void setUsername(String username) { - throw new ReadOnlyException("User is read-only for this update"); + throw new ReadOnlyException(USER_READ_ONLY_EXCEPTION_MSG); } @Override @@ -83,7 +75,7 @@ public Long getCreatedTimestamp() { @Override public void setCreatedTimestamp(Long timestamp) { - throw new ReadOnlyException("User is read-only for this update"); + throw new ReadOnlyException(USER_READ_ONLY_EXCEPTION_MSG); } @Override @@ -93,31 +85,28 @@ public boolean isEnabled() { if(enabledStr == null) { return false; } - if("active".equals(enabledStr)) { - return true; - } - return false; + return "active".equals(enabledStr); } @Override public void setEnabled(boolean enabled) { - throw new ReadOnlyException("User is read-only for this update"); + throw new ReadOnlyException(USER_READ_ONLY_EXCEPTION_MSG); } @Override public void setSingleAttribute(String name, String value) { - throw new ReadOnlyException("User is read-only for this update"); + throw new ReadOnlyException(USER_READ_ONLY_EXCEPTION_MSG); } @Override public void setAttribute(String name, List value) { - throw new ReadOnlyException("User is read-only for this update"); + throw new ReadOnlyException(USER_READ_ONLY_EXCEPTION_MSG); } @Override public void removeAttribute(String name) { - throw new ReadOnlyException("User is read-only for this update"); + throw new ReadOnlyException(USER_READ_ONLY_EXCEPTION_MSG); } @@ -170,12 +159,12 @@ public Stream getRequiredActionsStream() { @Override public void addRequiredAction(String action) { - throw new ReadOnlyException("User is read-only for this update"); + throw new ReadOnlyException(USER_READ_ONLY_EXCEPTION_MSG); } @Override public void removeRequiredAction(String action) { - throw new ReadOnlyException("User is read-only for this update"); + throw new ReadOnlyException(USER_READ_ONLY_EXCEPTION_MSG); } @Override @@ -187,7 +176,7 @@ public String getFirstName() { @Override public void setFirstName(String firstName) { - throw new ReadOnlyException("User is read-only for this update"); + throw new ReadOnlyException(USER_READ_ONLY_EXCEPTION_MSG); } @Override @@ -199,7 +188,7 @@ public String getLastName() { @Override public void setLastName(String lastName) { - throw new ReadOnlyException("User is read-only for this update"); + throw new ReadOnlyException(USER_READ_ONLY_EXCEPTION_MSG); } @Override @@ -211,7 +200,7 @@ public String getEmail() { @Override public void setEmail(final String email) { - throw new ReadOnlyException("User is read-only for this update"); + throw new ReadOnlyException(USER_READ_ONLY_EXCEPTION_MSG); } @Override @@ -231,7 +220,7 @@ public boolean isEmailVerified() { @Override public void setEmailVerified(boolean verified) { - throw new ReadOnlyException("User is read-only for this update"); + throw new ReadOnlyException(USER_READ_ONLY_EXCEPTION_MSG); } @Override @@ -255,13 +244,13 @@ public long getGroupsCountByNameContaining(String search) { @Override public void joinGroup(GroupModel group) { - throw new ReadOnlyException("User is read-only for this update"); + throw new ReadOnlyException(USER_READ_ONLY_EXCEPTION_MSG); } @Override public void leaveGroup(GroupModel group) { - throw new ReadOnlyException("User is read-only for this update"); + throw new ReadOnlyException(USER_READ_ONLY_EXCEPTION_MSG); } @Override @@ -279,7 +268,7 @@ public String getFederationLink() { @Override public void setFederationLink(String link) { - throw new ReadOnlyException("User is read-only for this update"); + throw new ReadOnlyException(USER_READ_ONLY_EXCEPTION_MSG); } @Override @@ -291,7 +280,7 @@ public String getServiceAccountClientLink() { @Override public void setServiceAccountClientLink(String clientInternalId) { - throw new ReadOnlyException("User is read-only for this update"); + throw new ReadOnlyException(USER_READ_ONLY_EXCEPTION_MSG); } @Override @@ -321,7 +310,7 @@ public boolean hasRole(RoleModel role) { @Override public void grantRole(RoleModel role) { - throw new ReadOnlyException("User is in read-only for this update"); + throw new ReadOnlyException(USER_READ_ONLY_EXCEPTION_MSG); } @Override @@ -333,7 +322,7 @@ public Stream getRoleMappingsStream() { @Override public void deleteRoleMapping(RoleModel role) { - throw new ReadOnlyException("User is in read-only for this update"); + throw new ReadOnlyException(USER_READ_ONLY_EXCEPTION_MSG); } } diff --git a/jans-keycloak-integration/spi/src/main/java/io/jans/kc/model/internal/JansPerson.java b/jans-keycloak-integration/spi/src/main/java/io/jans/kc/model/internal/JansPerson.java index 86df86812c7..829f46a16e5 100644 --- a/jans-keycloak-integration/spi/src/main/java/io/jans/kc/model/internal/JansPerson.java +++ b/jans-keycloak-integration/spi/src/main/java/io/jans/kc/model/internal/JansPerson.java @@ -5,7 +5,6 @@ import java.util.ArrayList; import java.util.List; -import io.jans.model.JansAttribute; import io.jans.orm.annotation.*; import io.jans.orm.model.base.CustomObjectAttribute; @@ -21,11 +20,6 @@ public class JansPerson implements Serializable { @AttributesList(name="name",value="values",multiValued="multiValued") private List customAttributes = new ArrayList<>(); - - public JansPerson() { - - } - public String getDn() { return this.dn; @@ -72,7 +66,7 @@ public List customAttributeValues(final String name) { for(CustomObjectAttribute customAttribute : customAttributes) { if(customAttribute.getName().equals(name)) { List values = customAttribute.getValues(); - if(values == null || values.size() == 0) { + if(values == null || values.isEmpty()) { return new ArrayList<>(); } return convertToString(values); @@ -95,7 +89,7 @@ public String customAttributeValue(final String attributeName) { for(CustomObjectAttribute customAttribute : customAttributes) { if(customAttribute.getName().equals(attributeName)) { List values = customAttribute.getValues(); - if(values == null || values.size() == 0) { + if(values == null || values.isEmpty()) { return null; } List ret = convertToString(values); @@ -113,8 +107,8 @@ private List convertToString(List values) { List ret = new ArrayList<>(); for(Object val : values) { - if(val instanceof String) { - ret.add((String) val); + if(val instanceof String strval) { + ret.add((String) strval); }else { ret.add(val.toString()); } diff --git a/jans-keycloak-integration/spi/src/main/java/io/jans/kc/oidc/OIDCAuthRequest.java b/jans-keycloak-integration/spi/src/main/java/io/jans/kc/oidc/OIDCAuthRequest.java index 5a5b49b7cbd..489e7fa5a0f 100644 --- a/jans-keycloak-integration/spi/src/main/java/io/jans/kc/oidc/OIDCAuthRequest.java +++ b/jans-keycloak-integration/spi/src/main/java/io/jans/kc/oidc/OIDCAuthRequest.java @@ -17,8 +17,8 @@ public OIDCAuthRequest() { this.clientId = null; this.state = null; this.nonce = null; - this.scopes = new ArrayList(); - this.responseTypes = new ArrayList(); + this.scopes = new ArrayList<>(); + this.responseTypes = new ArrayList<>(); this.redirectUri = null; } diff --git a/jans-keycloak-integration/spi/src/main/java/io/jans/kc/oidc/OIDCMetaCacheKeys.java b/jans-keycloak-integration/spi/src/main/java/io/jans/kc/oidc/OIDCMetaCacheKeys.java index 3a64b29997d..f5b69c39de0 100644 --- a/jans-keycloak-integration/spi/src/main/java/io/jans/kc/oidc/OIDCMetaCacheKeys.java +++ b/jans-keycloak-integration/spi/src/main/java/io/jans/kc/oidc/OIDCMetaCacheKeys.java @@ -4,4 +4,8 @@ public class OIDCMetaCacheKeys { public static final String AUTHORIZATION_URL = "oidc.authorization.url"; public static final String TOKEN_URL = "oidc.token.url"; public static final String USERINFO_URL = "oidc.userinfo.url"; + + private OIDCMetaCacheKeys() { + //private constructor + } } diff --git a/jans-keycloak-integration/spi/src/main/java/io/jans/kc/oidc/impl/HashBasedOIDCMetaCache.java b/jans-keycloak-integration/spi/src/main/java/io/jans/kc/oidc/impl/HashBasedOIDCMetaCache.java index bf056a3e56e..8e2758938b9 100644 --- a/jans-keycloak-integration/spi/src/main/java/io/jans/kc/oidc/impl/HashBasedOIDCMetaCache.java +++ b/jans-keycloak-integration/spi/src/main/java/io/jans/kc/oidc/impl/HashBasedOIDCMetaCache.java @@ -2,16 +2,13 @@ import java.util.Map; -import org.jboss.logging.Logger; - import io.jans.kc.oidc.OIDCMetaCache; import java.util.HashMap; public class HashBasedOIDCMetaCache implements OIDCMetaCache{ - private static final Logger log = Logger.getLogger(HashBasedOIDCMetaCache.class); - private static final long DEFAULT_CACHE_TTL = 20*60; // 20 seconds + private static final long DEFAULT_CACHE_TTL = 20*60l; // 20 seconds private long cacheEntryTtl; @@ -28,7 +25,7 @@ public HashBasedOIDCMetaCache(long cacheEntryTtl) { this.cacheEntryTtl = DEFAULT_CACHE_TTL; } this.cacheEntryTtl = this.cacheEntryTtl * 1000; // convert to milliseconds - this.cacheEntries = new HashMap>(); + this.cacheEntries = new HashMap<>(); } @Override @@ -63,42 +60,41 @@ private boolean issuerCacheEntryIsMissing(String issuer) { private Object getIssuerCacheEntryValue(String issuer, String key) { - Map issuer_cache = cacheEntries.get(issuer); - return issuer_cache.get(key).getValue(); + Map issuerCache = cacheEntries.get(issuer); + return issuerCache.get(key).getValue(); } private void createIfNotExistIssuerCacheEntry(String issuer) { - if(!cacheEntries.containsKey(issuer)) { - cacheEntries.put(issuer,new HashMap()); - } + cacheEntries.computeIfAbsent(issuer, k-> new HashMap<>()); } private void addIssuerCacheEntry(String issuer,String key, Object value) { Map issuerCache = cacheEntries.get(issuer); - for(String existingkey : issuerCache.keySet()) { - if(existingkey.equalsIgnoreCase(key)) { + if(issuerCache == null) { + return; + } + + for(Map.Entry entry : issuerCache.entrySet()) { + if(entry.getKey().equalsIgnoreCase(key)) { //update cache entry - CacheEntry cache_entry = issuerCache.get(existingkey); - cache_entry.updateValue(value); + entry.getValue().updateValue(value); return; } } - issuerCache.put(key,new CacheEntry(cacheEntryTtl, value)); } private void performHouseCleaning() { for(String issuer: cacheEntries.keySet()) { - Map issuer_cache = cacheEntries.get(issuer); - for(String key :issuer_cache.keySet()) { - CacheEntry cache_entry = issuer_cache.get(key); - if(cache_entry.isExpired()) { - issuer_cache.remove(key); - } - } + Map issuerCache = cacheEntries.get(issuer); + for(Map.Entry entry : issuerCache.entrySet()) { + if(entry.getValue().isExpired()) { + issuerCache.remove(entry.getKey()); + } + } } } diff --git a/jans-keycloak-integration/spi/src/main/java/io/jans/kc/spi/auth/JansAuthenticator.java b/jans-keycloak-integration/spi/src/main/java/io/jans/kc/spi/auth/JansAuthenticator.java index 2fa0cea1a7c..aa686dbfcaa 100644 --- a/jans-keycloak-integration/spi/src/main/java/io/jans/kc/spi/auth/JansAuthenticator.java +++ b/jans-keycloak-integration/spi/src/main/java/io/jans/kc/spi/auth/JansAuthenticator.java @@ -5,12 +5,12 @@ import java.net.URISyntaxException; import java.net.URLDecoder; +import java.security.SecureRandom; import java.text.MessageFormat; import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Random; import java.util.ArrayList; import jakarta.ws.rs.core.Response; @@ -267,9 +267,9 @@ private Configuration pluginConfigurationFromContext(AuthenticationFlowContext c String extra_scopes = config.getConfig().get(JansAuthenticatorConfigProp.EXTRA_SCOPES.getName()); List parsed_extra_scopes = new ArrayList<>(); if(extra_scopes != null) { - String [] tokens = extra_scopes.split("\\s*,\\s*"); + String [] tokens = extra_scopes.split(","); for(String token : tokens) { - parsed_extra_scopes.add(token); + parsed_extra_scopes.add(token.trim()); } } @@ -302,7 +302,7 @@ private String generateRandomString(int length) { int leftlimit = 48; int rightlimit = 122; - return new Random().ints(leftlimit,rightlimit+1) + return new SecureRandom().ints(leftlimit,rightlimit+1) .filter(i -> (i <= 57 || i >= 65) && (i <= 90 || i >= 97)) .limit(length) .collect(StringBuilder::new, StringBuilder::appendCodePoint, StringBuilder::append)