From 10578449dfb39c9da5ba70a9372f2a995e505d3f Mon Sep 17 00:00:00 2001 From: "p.zahnen" Date: Thu, 22 Jan 2026 18:58:47 +0000 Subject: [PATCH 1/3] refactor and enhance authentication and OIDC handling --- .../auth/fallback/app/OidcFallback.java | 2 +- .../auth/app/InternalBearerAuthProvider.java | 2 +- .../auth/app/InternalUserAuthenticator.java | 101 ++++++++---------- .../auth/app/JwtTokenHandler.java | 5 +- .../xtraplatform/auth/app/PasswordHash.java | 19 +++- .../app/SplitCookieCredentialAuthFilter.java | 4 +- .../auth/app/SplitCookieResponseFilter.java | 5 +- .../de/ii/xtraplatform/auth/app/User.java | 5 +- .../external/ExternalBearerAuthProvider.java | 2 +- .../external/ExternalDynamicAuthFilter.java | 70 ++++++------ .../auth/app/external/OidcImpl.java | 87 ++++++++------- .../app/InternalUserAuthenticatorSpec.groovy | 2 +- 12 files changed, 153 insertions(+), 151 deletions(-) diff --git a/xtraplatform-auth-fallback/src/main/java/de/ii/xtraplatform/auth/fallback/app/OidcFallback.java b/xtraplatform-auth-fallback/src/main/java/de/ii/xtraplatform/auth/fallback/app/OidcFallback.java index 0b8c65dc..d2a414b5 100644 --- a/xtraplatform-auth-fallback/src/main/java/de/ii/xtraplatform/auth/fallback/app/OidcFallback.java +++ b/xtraplatform-auth-fallback/src/main/java/de/ii/xtraplatform/auth/fallback/app/OidcFallback.java @@ -61,7 +61,7 @@ public Optional getClientSecret() { @Override public Map getSigningKeys() { - return null; + return java.util.Collections.emptyMap(); } @Override diff --git a/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/app/InternalBearerAuthProvider.java b/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/app/InternalBearerAuthProvider.java index dd4e00b8..011370d0 100644 --- a/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/app/InternalBearerAuthProvider.java +++ b/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/app/InternalBearerAuthProvider.java @@ -56,7 +56,7 @@ public AuthFilter getAuthFilter() { JwtTokenAuthenticator tokenAuthenticator = new JwtTokenAuthenticator(tokenHandler); CachingAuthenticator cachingAuthenticator = - new CachingAuthenticator( + new CachingAuthenticator<>( metricRegistry, tokenAuthenticator, CaffeineSpec.parse("maximumSize=10000, expireAfterAccess=10m")); diff --git a/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/app/InternalUserAuthenticator.java b/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/app/InternalUserAuthenticator.java index 0cd2aff9..aa2be740 100644 --- a/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/app/InternalUserAuthenticator.java +++ b/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/app/InternalUserAuthenticator.java @@ -12,7 +12,6 @@ import de.ii.xtraplatform.auth.domain.Role; import de.ii.xtraplatform.auth.domain.User; import de.ii.xtraplatform.auth.domain.UserAuthenticator; -import de.ii.xtraplatform.base.domain.AppContext; import de.ii.xtraplatform.entities.domain.EntityData; import de.ii.xtraplatform.entities.domain.EntityDataStore; import java.time.Instant; @@ -41,11 +40,8 @@ public class InternalUserAuthenticator implements UserAuthenticator { private final EntityDataStore userRepository; @Inject - public InternalUserAuthenticator(AppContext appContext, EntityDataStore entityRepository) { + public InternalUserAuthenticator(EntityDataStore entityRepository) { this.isAccessRestricted = true; - /*Optional.ofNullable(appContext.getConfiguration().getAuth()) - .map(authConfig -> !authConfig.allowAnonymousAccess) - .orElse(true);*/ this.userRepository = ((EntityDataStore) entityRepository) .forType(de.ii.xtraplatform.auth.app.User.UserData.class); @@ -53,29 +49,8 @@ public InternalUserAuthenticator(AppContext appContext, EntityDataStore entit @Override public Optional authenticate(String username, String password) { - if (userRepository.has(username)) { - - de.ii.xtraplatform.auth.app.User.UserData userData = userRepository.get(username); - - if (PasswordHash.validatePassword(password, userData.getPassword())) { - if (LOGGER.isTraceEnabled()) { - LOGGER.trace( - "Authenticated {} {} {}", - userData.getId(), - userData.getRole(), - PasswordHash.createHash(password)); - } - - long now = Instant.now().toEpochMilli(); - - return Optional.of( - ImmutableUser.builder() - .name(userData.getId()) - .role(userData.getRole()) - .forceChangePassword(userData.getPasswordExpiresAt().orElse(now) < now) - .build()); - } + return authenticateExistingUser(username, password); } // TODO: for ldproxy make admin password settable via env variable, get from bundlecontext @@ -83,40 +58,56 @@ public Optional authenticate(String username, String password) { // ) // no user exists yet - if (userRepository.ids().isEmpty()) { - if (!isAccessRestricted) { - return Optional.of( - ImmutableUser.builder().name(SUPER_ADMIN.getId()).role(SUPER_ADMIN.getRole()).build()); - } - - if (Objects.equals(username, SUPER_ADMIN.getId()) - && PasswordHash.validatePassword(password, SUPER_ADMIN.getPassword())) { - if (LOGGER.isTraceEnabled()) { - LOGGER.trace( - "Authenticated {} {} {}", - SUPER_ADMIN.getId(), - SUPER_ADMIN.getRole(), - PasswordHash.createHash(password)); - } + if (userRepository.ids().isEmpty()) { + return authenticateFirstUser(username, password); + } - try { - de.ii.xtraplatform.auth.app.User.UserData firstUser = - userRepository.put(SUPER_ADMIN.getId(), SUPER_ADMIN).get(); + return Optional.empty(); + } - return Optional.of( - ImmutableUser.builder() - .name(firstUser.getId()) - .role(firstUser.getRole()) - .forceChangePassword(true) - .build()); + private Optional authenticateExistingUser(String username, String password) { + de.ii.xtraplatform.auth.app.User.UserData userData = userRepository.get(username); + if (!PasswordHash.validatePassword(password, userData.getPassword())) { + return Optional.empty(); + } + logAuthenticated(SUPER_ADMIN.getId(), SUPER_ADMIN.getRole().toString(), password); + long now = Instant.now().toEpochMilli(); + return Optional.of( + ImmutableUser.builder() + .name(userData.getId()) + .role(userData.getRole()) + .forceChangePassword(userData.getPasswordExpiresAt().orElse(now) < now) + .build()); + } - } catch (InterruptedException | ExecutionException e) { - // ignore - } + private Optional authenticateFirstUser(String username, String password) { + if (!isAccessRestricted) { + return Optional.of( + ImmutableUser.builder().name(SUPER_ADMIN.getId()).role(SUPER_ADMIN.getRole()).build()); + } + if (Objects.equals(username, SUPER_ADMIN.getId()) + && PasswordHash.validatePassword(password, SUPER_ADMIN.getPassword())) { + logAuthenticated(SUPER_ADMIN.getId(), SUPER_ADMIN.getRole().toString(), password); + try { + de.ii.xtraplatform.auth.app.User.UserData firstUser = + userRepository.put(SUPER_ADMIN.getId(), SUPER_ADMIN).get(); + return Optional.of( + ImmutableUser.builder() + .name(firstUser.getId()) + .role(firstUser.getRole()) + .forceChangePassword(true) + .build()); + } catch (InterruptedException | ExecutionException e) { + // ignore } } - return Optional.empty(); } + + private void logAuthenticated(String id, String role, String password) { + if (LOGGER.isTraceEnabled()) { + LOGGER.trace("Authenticated {} {} {}", id, role, PasswordHash.createHash(password)); + } + } } diff --git a/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/app/JwtTokenHandler.java b/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/app/JwtTokenHandler.java index 073cd306..633b4ffc 100644 --- a/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/app/JwtTokenHandler.java +++ b/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/app/JwtTokenHandler.java @@ -60,6 +60,7 @@ @Singleton @AutoBind +@SuppressWarnings({"PMD.GodClass", "PMD.TooManyMethods"}) public class JwtTokenHandler implements TokenHandler, AppLifeCycle { private static final Logger LOGGER = LoggerFactory.getLogger(JwtTokenHandler.class); @@ -162,7 +163,7 @@ private static boolean isComplex(String claim) { } private static String baseKey(String claim) { - return isComplex(claim) ? claim.substring(0, claim.indexOf(".")) : claim; + return isComplex(claim) ? claim.substring(0, claim.indexOf('.')) : claim; } private static Map> listType(String claim) { @@ -173,6 +174,7 @@ private String read(Claims claims, String claim) { return claims.get(claim, String.class); } + @SuppressWarnings("PMD.CognitiveComplexity") private List readList(Claims claims, String claim) { boolean isComplex = isComplex(claim); String baseKey = baseKey(claim); @@ -230,6 +232,7 @@ private Map> readListPerApi(Claims claims, String claim) { return lists; } + @SuppressWarnings("PMD.AvoidInstantiatingObjectsInLoops") private List getClaimNames(Map claims) { List keys = new ArrayList<>(); diff --git a/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/app/PasswordHash.java b/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/app/PasswordHash.java index 82b8198e..aeaeee03 100644 --- a/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/app/PasswordHash.java +++ b/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/app/PasswordHash.java @@ -47,7 +47,8 @@ * Author: havoc AT defuse.ca * www: http://crackstation.net/hashing-security.htm */ -public class PasswordHash { +public final class PasswordHash { + public static final String PBKDF2_ALGORITHM = "PBKDF2WithHmacSHA1"; // The following constants may be changed without breaking existing hashes. @@ -59,6 +60,8 @@ public class PasswordHash { public static final int SALT_INDEX = 1; public static final int PBKDF2_INDEX = 2; + private PasswordHash() {} + /** * Returns a salted PBKDF2 hash of the password. * @@ -79,6 +82,7 @@ public static String createHash(String password) { * @param password the password to hash * @return a salted PBKDF2 hash of the password */ + @SuppressWarnings("PMD.UseVarargs") public static String createHash(char[] password) throws NoSuchAlgorithmException, InvalidKeySpecException { // Generate a random salt @@ -140,7 +144,9 @@ public static boolean validatePassword(char[] password, String correctHash) */ private static boolean slowEquals(byte[] a, byte[] b) { int diff = a.length ^ b.length; - for (int i = 0; i < a.length && i < b.length; i++) diff |= a[i] ^ b[i]; + for (int i = 0; i < a.length && i < b.length; i++) { + diff |= a[i] ^ b[i]; + } return diff == 0; } @@ -183,9 +189,12 @@ private static byte[] fromHex(String hex) { private static String toHex(byte[] array) { BigInteger bi = new BigInteger(1, array); String hex = bi.toString(16); - int paddingLength = (array.length * 2) - hex.length(); - if (paddingLength > 0) return String.format("%0" + paddingLength + "d", 0) + hex; - else return hex; + int paddingLength = array.length * 2 - hex.length(); + if (paddingLength > 0) { + return String.format("%0" + paddingLength + "d", 0) + hex; + } else { + return hex; + } } /** diff --git a/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/app/SplitCookieCredentialAuthFilter.java b/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/app/SplitCookieCredentialAuthFilter.java index faed8b47..d17f119b 100644 --- a/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/app/SplitCookieCredentialAuthFilter.java +++ b/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/app/SplitCookieCredentialAuthFilter.java @@ -20,8 +20,10 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +@SuppressWarnings("PMD.MissingStaticMethodInNonInstantiatableClass") @Priority(Priorities.AUTHENTICATION) -public class SplitCookieCredentialAuthFilter

extends AuthFilter { +public final class SplitCookieCredentialAuthFilter

+ extends AuthFilter { private static final Logger LOGGER = LoggerFactory.getLogger(SplitCookieCredentialAuthFilter.class); diff --git a/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/app/SplitCookieResponseFilter.java b/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/app/SplitCookieResponseFilter.java index 415112d3..655ba9c5 100644 --- a/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/app/SplitCookieResponseFilter.java +++ b/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/app/SplitCookieResponseFilter.java @@ -46,12 +46,11 @@ public SplitCookieResponseFilter( EntityDataStore entityRepository) { this.servicesUri = servicesContext.getUri(); this.tokenHandler = tokenHandler; - this.userRepository = - ((EntityDataStore) entityRepository) - .forType(de.ii.xtraplatform.auth.app.User.UserData.class); + this.userRepository = ((EntityDataStore) entityRepository).forType(UserData.class); } @Override + @SuppressWarnings("PMD.AvoidDeeplyNestedIfStmts") public void filter( ContainerRequestContext requestContext, ContainerResponseContext responseContext) throws IOException { diff --git a/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/app/User.java b/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/app/User.java index c7618b34..66b4d581 100644 --- a/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/app/User.java +++ b/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/app/User.java @@ -51,9 +51,10 @@ abstract class Builder implements EntityDataBuilder { public abstract Builder role(Role role); + @Override public ImmutableUserData.Builder fillRequiredFieldsWithPlaceholders() { - return ((ImmutableUserData.Builder) - this.id("__DEFAULT__").password("__DEFAULT__").role(Role.NONE)); + return (ImmutableUserData.Builder) + this.id("__DEFAULT__").password("__DEFAULT__").role(Role.NONE); } } diff --git a/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/app/external/ExternalBearerAuthProvider.java b/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/app/external/ExternalBearerAuthProvider.java index 08246aff..963ff18c 100644 --- a/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/app/external/ExternalBearerAuthProvider.java +++ b/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/app/external/ExternalBearerAuthProvider.java @@ -60,7 +60,7 @@ public AuthFilter getAuthFilter() { TokenAuthenticator tokenAuthenticator = new TokenAuthenticator(authConfig, httpClient); CachingAuthenticator cachingAuthenticator = - new CachingAuthenticator( + new CachingAuthenticator<>( metricRegistry, tokenAuthenticator, CaffeineSpec.parse("maximumSize=10000, expireAfterAccess=10m")); diff --git a/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/app/external/ExternalDynamicAuthFilter.java b/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/app/external/ExternalDynamicAuthFilter.java index 92d2232a..18e7eded 100644 --- a/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/app/external/ExternalDynamicAuthFilter.java +++ b/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/app/external/ExternalDynamicAuthFilter.java @@ -42,6 +42,7 @@ * @author zahnen */ @PreMatching +@SuppressWarnings("PMD.SingularField") public class ExternalDynamicAuthFilter

extends AuthFilter { private static final Logger LOGGER = LoggerFactory.getLogger(ExternalDynamicAuthFilter.class); @@ -57,6 +58,8 @@ public class ExternalDynamicAuthFilter

extends AuthFilter delegate; + // TODO: cfg.yml + private static final List METHODS = ImmutableList.of("GET", "POST", "PUT", "DELETE"); ExternalDynamicAuthFilter( String edaUrl, @@ -88,15 +91,11 @@ private MediaType parse(String xacmlJsonMediaType) { } } - // TODO: cfg.yml - static List METHODS = ImmutableList.of("GET", "POST", "PUT", "DELETE"); - // TODO: either interface that is implemented in ogcapi to which we pass service id // or add xacml request as callback to user @Override + @SuppressWarnings("PMD.AvoidDeeplyNestedIfStmts") public void filter(ContainerRequestContext requestContext) throws IOException { - SecurityContext oldSecurityContext = requestContext.getSecurityContext(); - delegate.filter(requestContext); if (METHODS.contains(requestContext.getMethod())) { @@ -123,18 +122,22 @@ public void filter(ContainerRequestContext requestContext) throws IOException { requestContext.setSecurityContext( new SecurityContext() { + @Override public Principal getUserPrincipal() { return user; } + @Override public boolean isUserInRole(String role) { return requestContext.getSecurityContext().isUserInRole(role); } + @Override public boolean isSecure() { return requestContext.getSecurityContext().isSecure(); } + @Override public String getAuthenticationScheme() { return requestContext.getSecurityContext().getAuthenticationScheme(); } @@ -156,18 +159,15 @@ public String getAuthenticationScheme() { } private void postProcess(ContainerRequestContext requestContext, byte[] body) { - if (requestContext.getMethod().equals("POST") || requestContext.getMethod().equals("PUT")) { - try { - - InputStream processedBody = - httpClient.postAsInputStream( - ppUrl, body, GEOJSON.withCharset("utf-8"), Map.of("Accept", GEOJSON.toString())); + if ("POST".equals(requestContext.getMethod()) || "PUT".equals(requestContext.getMethod())) { + try (InputStream processedBody = + httpClient.postAsInputStream( + ppUrl, body, GEOJSON.withCharset("utf-8"), Map.of("Accept", GEOJSON.toString()))) { putEntityBody(requestContext, processedBody); } catch (Throwable e) { // ignore - boolean stop = true; } } } @@ -177,18 +177,18 @@ private PolicyDecision askPDP(String user, String method, String path, byte[] bo LOGGER.debug("EDA {} {} {} {}", user, method, path, new String(body, Charset.forName("utf-8"))); try { - byte[] xacmlRequest = getXacmlRequest(user, method, path, body); + byte[] xacmlRequest = getXacmlRequest(); - InputStream response = + try (InputStream response = httpClient.postAsInputStream( - edaUrl, xacmlRequest, mediaType, Map.of("Accept", mediaTypeAccept.toString())); + edaUrl, xacmlRequest, mediaType, Map.of("Accept", mediaTypeAccept.toString()))) { - XacmlResponse xacmlResponse = getXacmlResponse(response); - - return xacmlResponse.isAllowed() - ? PolicyDecision.PERMIT - : xacmlResponse.isNotApplicable() ? PolicyDecision.NOT_APPLICABLE : PolicyDecision.DENY; + XacmlResponse xacmlResponse = getXacmlResponse(response); + return xacmlResponse.isAllowed() + ? PolicyDecision.PERMIT + : xacmlResponse.isNotApplicable() ? PolicyDecision.NOT_APPLICABLE : PolicyDecision.DENY; + } } catch (Throwable e) { // ignore LogContext.errorAsDebug(LOGGER, e, "Error requesting a policy decision"); @@ -198,8 +198,7 @@ private PolicyDecision askPDP(String user, String method, String path, byte[] bo } // TODO - private byte[] getXacmlRequest(String user, String method, String path, byte[] body) - throws JsonProcessingException { + private byte[] getXacmlRequest() throws JsonProcessingException { Object xacmlRequest = null; /*xacmlJson10 ? new XacmlRequest( @@ -221,8 +220,10 @@ private byte[] getXacmlRequest(String user, String method, String path, byte[] b Optional.ofNullable(body), Map.of());*/ - LOGGER.debug( - "XACML {}", JSON.writerWithDefaultPrettyPrinter().writeValueAsString(xacmlRequest)); + if (LOGGER.isDebugEnabled()) { + LOGGER.debug( + "XACML {}", JSON.writerWithDefaultPrettyPrinter().writeValueAsString(xacmlRequest)); + } return JSON.writeValueAsBytes(xacmlRequest); } @@ -230,32 +231,21 @@ private byte[] getXacmlRequest(String user, String method, String path, byte[] b private XacmlResponse getXacmlResponse(InputStream response) throws IOException { XacmlResponse xacmlResponse = JSON.readValue(response, XacmlResponse.class); - LOGGER.debug( - "XACML R {}", JSON.writerWithDefaultPrettyPrinter().writeValueAsString(xacmlResponse)); + if (LOGGER.isDebugEnabled()) { + LOGGER.debug( + "XACML R {}", JSON.writerWithDefaultPrettyPrinter().writeValueAsString(xacmlResponse)); + } return xacmlResponse; } private byte[] getEntityBody(ContainerRequestContext requestContext) { ByteArrayOutputStream out = new ByteArrayOutputStream(); - InputStream in = requestContext.getEntityStream(); - - // final StringBuilder b = new StringBuilder(); - try { + try (InputStream in = requestContext.getEntityStream()) { ReaderWriter.writeTo(in, out); - byte[] requestEntity = out.toByteArray(); - /*if (requestEntity.length == 0) { - b.append("") - .append("\n"); - } else { - b.append(new String(requestEntity)) - .append("\n"); - }*/ requestContext.setEntityStream(new ByteArrayInputStream(requestEntity)); - return requestEntity; - } catch (IOException ex) { // Handle logging error } diff --git a/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/app/external/OidcImpl.java b/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/app/external/OidcImpl.java index eff2e81f..c537ff8f 100644 --- a/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/app/external/OidcImpl.java +++ b/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/app/external/OidcImpl.java @@ -55,6 +55,13 @@ public class OidcImpl extends AbstractVolatile implements Oidc, AppLifeCycle, SigningKeyResolver { private static final Logger LOGGER = LoggerFactory.getLogger(OidcImpl.class); + private final AuthConfiguration authConfig; + private final HttpClient httpClient; + private final ObjectMapper objectMapper; + private Optional oidcConfiguration; + private Map signingKeys; + private boolean enabled; + private static final String KEY_KID = "kid"; @Value.Immutable @Value.Style(builder = "new") @@ -86,13 +93,6 @@ interface OidcCerts { List> getKeys(); } - private final AuthConfiguration authConfig; - private final HttpClient httpClient; - private final ObjectMapper objectMapper; - private Optional oidcConfiguration; - private Map signingKeys; - private boolean enabled; - @Inject public OidcImpl(AppContext appContext, Http http, VolatileRegistry volatileRegistry) { super(volatileRegistry, "app/oidc"); @@ -117,8 +117,7 @@ public CompletionStage onStart(boolean isStartupAsync) { if (authConfig.getOidc().isPresent()) { String endpoint = authConfig.getOidc().get().getEndpoint(); - try { - InputStream inputStream = httpClient.getAsInputStream(endpoint); + try (InputStream inputStream = httpClient.getAsInputStream(endpoint)) { OidcConfiguration oidcConfiguration1 = objectMapper.readValue(inputStream, OidcConfiguration.class); @@ -148,46 +147,54 @@ public CompletionStage onStart(boolean isStartupAsync) { return CompletableFuture.completedFuture(null); } + private boolean isValidKeyDef(Map keyDef) { + return keyDef.containsKey(KEY_KID) + && keyDef.containsKey("kty") + && keyDef.containsKey("use") + && keyDef.containsKey("n") + && keyDef.containsKey("e"); + } + + private Map.Entry toSigningKey(Map keyDef) + throws InvalidKeySpecException, NoSuchAlgorithmException { + if (!isValidKeyDef(keyDef)) { + return null; + } + if (!Objects.equals(keyDef.get("kty"), "RSA")) { + if (LOGGER.isDebugEnabled()) { + LOGGER.debug( + "Skipping OIDC key '{}' with type '{}', only 'RSA' is supported.", + keyDef.get(KEY_KID), + keyDef.get("kty")); + } + return null; + } + if (!Objects.equals(keyDef.get("use"), "sig")) { + if (LOGGER.isDebugEnabled()) { + LOGGER.debug( + "Skipping OIDC key '{}' with use '{}', only 'sig' is supported.", + keyDef.get(KEY_KID), + keyDef.get("use")); + } + return null; + } + return Map.entry( + (String) keyDef.get(KEY_KID), + parseRsaKey((String) keyDef.get("n"), (String) keyDef.get("e"))); + } + private boolean loadSigningKeys() { if (oidcConfiguration.isPresent()) { String certsEndpoint = oidcConfiguration.get().getJwksEndpoint().toString(); - try { - InputStream inputStream = httpClient.getAsInputStream(certsEndpoint); + try (InputStream inputStream = httpClient.getAsInputStream(certsEndpoint)) { OidcCerts oidcCerts = objectMapper.readValue(inputStream, OidcCerts.class); this.signingKeys = oidcCerts.getKeys().stream() - .map( - mayThrow( - keyDef -> { - if (!keyDef.containsKey("kid") - || !keyDef.containsKey("kty") - || !keyDef.containsKey("use") - || !keyDef.containsKey("n") - || !keyDef.containsKey("e")) { - return null; - } - if (!Objects.equals(keyDef.get("kty"), "RSA")) { - LOGGER.debug( - "Skipping OIDC key '{}' with type '{}', only 'RSA' is supported.", - keyDef.get("kid"), - keyDef.get("kty")); - return null; - } - if (!Objects.equals(keyDef.get("use"), "sig")) { - LOGGER.debug( - "Skipping OIDC key '{}' with use '{}', only 'sig' is supported.", - keyDef.get("kid"), - keyDef.get("use")); - return null; - } - - return Map.entry( - (String) keyDef.get("kid"), - parseRsaKey((String) keyDef.get("n"), (String) keyDef.get("e"))); - })) + .map(mayThrow(this::toSigningKey)) .filter(Objects::nonNull) .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); + } catch (Throwable e) { LogContext.error( LOGGER, e, "Could not parse OpenID Connect certificates at {}", certsEndpoint); diff --git a/xtraplatform-auth/src/test/groovy/de/ii/xtraplatform/auth/app/InternalUserAuthenticatorSpec.groovy b/xtraplatform-auth/src/test/groovy/de/ii/xtraplatform/auth/app/InternalUserAuthenticatorSpec.groovy index 6de89197..d5058aa8 100644 --- a/xtraplatform-auth/src/test/groovy/de/ii/xtraplatform/auth/app/InternalUserAuthenticatorSpec.groovy +++ b/xtraplatform-auth/src/test/groovy/de/ii/xtraplatform/auth/app/InternalUserAuthenticatorSpec.groovy @@ -88,7 +88,7 @@ class InternalUserAuthenticatorSpec extends Specification { getConfiguration() >> config } EntityDataStore entityDataStore = new EntityDataStoreTest() - return new InternalUserAuthenticator(ac, entityDataStore) + return new InternalUserAuthenticator(entityDataStore) } class EntityDataStoreTest implements EntityDataStore { From 5263751230fc012350f7413504c54b0666e811d8 Mon Sep 17 00:00:00 2001 From: "p.zahnen" Date: Fri, 23 Jan 2026 13:02:47 +0000 Subject: [PATCH 2/3] pmd warnings --- .../app/external/PolicyDeciderXacmlJson.java | 32 ++++---- .../auth/app/external/TokenAuthenticator.java | 40 +++++----- .../auth/app/external/XacmlRequest.java | 29 +++---- .../auth/app/external/XacmlResponse.java | 76 +++++++++---------- .../de/ii/xtraplatform/auth/domain/Role.java | 2 +- .../xtraplatform/auth/domain/SplitCookie.java | 29 +++---- .../auth/domain/TokenResponse.java | 4 +- .../auth/infra/rest/TokenEndpoint.java | 2 +- 8 files changed, 106 insertions(+), 108 deletions(-) diff --git a/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/app/external/PolicyDeciderXacmlJson.java b/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/app/external/PolicyDeciderXacmlJson.java index 1a5f4410..f86a47d5 100644 --- a/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/app/external/PolicyDeciderXacmlJson.java +++ b/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/app/external/PolicyDeciderXacmlJson.java @@ -68,7 +68,7 @@ public class PolicyDeciderXacmlJson implements PolicyDecider { } @Override - public de.ii.xtraplatform.auth.domain.PolicyDecision request( + public PolicyDecision request( String resourceId, Map resourceAttributes, String actionId, @@ -85,20 +85,20 @@ public de.ii.xtraplatform.auth.domain.PolicyDecision request( String url = pdpUrl.replaceAll("\\{\\{apiId\\}\\}", (String) resourceAttributes.get("ldproxy:api:id")); - InputStream response = + try (InputStream response = httpClient.postAsInputStream( - url, xacmlRequest, mediaTypeContent, Map.of("Accept", mediaTypeAccept.toString())); + url, xacmlRequest, mediaTypeContent, Map.of("Accept", mediaTypeAccept.toString()))) { - XacmlResponse xacmlResponse = getXacmlResponse(response); - - return evaluate(xacmlResponse); + XacmlResponse xacmlResponse = getXacmlResponse(response); + return evaluate(xacmlResponse); + } } catch (Throwable e) { // ignore LogContext.error(LOGGER, e, "Error requesting a policy decision"); } - return de.ii.xtraplatform.auth.domain.PolicyDecision.deny(); + return PolicyDecision.deny(); } private byte[] getXacmlRequest( @@ -112,9 +112,11 @@ private byte[] getXacmlRequest( new XacmlRequest( version, resourceId, resourceAttributes, actionId, actionAttributes, user, geoXacml); - LOGGER.debug( - "XACML {}", JSON_MAPPER.writerWithDefaultPrettyPrinter().writeValueAsString(xacmlRequest)); - + if (LOGGER.isDebugEnabled()) { + LOGGER.debug( + "XACML {}", + JSON_MAPPER.writerWithDefaultPrettyPrinter().writeValueAsString(xacmlRequest)); + } return JSON_MAPPER.writeValueAsBytes(xacmlRequest); } @@ -123,10 +125,12 @@ private XacmlResponse getXacmlResponse(InputStream response) throws IOException XacmlResponse xacmlResponse = JSON_MAPPER.readValue(s, XacmlResponse.class); - LOGGER.debug("XACML R {}", s); - LOGGER.debug( - "XACML R {}", - JSON_MAPPER.writerWithDefaultPrettyPrinter().writeValueAsString(xacmlResponse)); + if (LOGGER.isDebugEnabled()) { + LOGGER.debug("XACML R {}", s); + LOGGER.debug( + "XACML R {}", + JSON_MAPPER.writerWithDefaultPrettyPrinter().writeValueAsString(xacmlResponse)); + } return xacmlResponse; } diff --git a/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/app/external/TokenAuthenticator.java b/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/app/external/TokenAuthenticator.java index c170b353..bffb5f17 100644 --- a/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/app/external/TokenAuthenticator.java +++ b/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/app/external/TokenAuthenticator.java @@ -51,29 +51,31 @@ public Optional authenticate(String token) throws AuthenticationException try { String url = userInfoProvider.getEndpoint().replace("{{token}}", token).replace("{token}", token); - InputStream response = - httpClient.getAsInputStream(url, Map.of("Authorization", "Bearer " + token)); + try (InputStream response = + httpClient.getAsInputStream(url, Map.of("Authorization", "Bearer " + token))) { - Map userInfo = MAPPER.readValue(response, TYPE_REF); + Map userInfo = MAPPER.readValue(response, TYPE_REF); - LOGGER.debug("USERINFO {}", userInfo); + LOGGER.debug("USERINFO {}", userInfo); - String name = (String) userInfo.get(userInfoProvider.getClaims().getUserName()); - Role role = - Role.fromString( - Optional.ofNullable( - (String) - userInfo.get(authConfig.getJwt().get().getClaims().getPermissions())) - .orElse("USER")); - List scopes = - userInfo.get(userInfoProvider.getClaims().getPermissions()) instanceof String - ? SPLITTER.splitToList( - (String) userInfo.get(userInfoProvider.getClaims().getPermissions())) - : Optional.ofNullable( - (List) userInfo.get(userInfoProvider.getClaims().getPermissions())) - .orElse(List.of()); + String name = (String) userInfo.get(userInfoProvider.getClaims().getUserName()); + Role role = + Role.fromString( + Optional.ofNullable( + (String) + userInfo.get(authConfig.getJwt().get().getClaims().getPermissions())) + .orElse("USER")); + List scopes = + userInfo.get(userInfoProvider.getClaims().getPermissions()) instanceof String + ? SPLITTER.splitToList( + (String) userInfo.get(userInfoProvider.getClaims().getPermissions())) + : Optional.ofNullable( + (List) + userInfo.get(userInfoProvider.getClaims().getPermissions())) + .orElse(List.of()); - return Optional.of(ImmutableUser.builder().name(name).role(role).scopes(scopes).build()); + return Optional.of(ImmutableUser.builder().name(name).role(role).scopes(scopes).build()); + } } catch (Throwable e) { if (LOGGER.isTraceEnabled()) { LOGGER.trace("Error validating token", e); diff --git a/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/app/external/XacmlRequest.java b/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/app/external/XacmlRequest.java index 65cc62a9..4a34d62d 100644 --- a/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/app/external/XacmlRequest.java +++ b/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/app/external/XacmlRequest.java @@ -26,7 +26,8 @@ public class XacmlRequest { // TODO private static final String PREFIX_GEO = "ldproxy:feature:geometry"; - public final Map Request; + public final Map request; + private static final String ATTRIBUTE = "Attribute"; public XacmlRequest( XacmlJsonVersion version, @@ -64,7 +65,7 @@ public XacmlRequest( .add(new Attribute("urn:oasis:names:tc:xacml:1.0:action:action-id", actionId)); actionAttributes.forEach((id, value) -> add(id, value, action, geoXacmlVersion)); - Request = + request = version == XacmlJsonVersion._1_0 ? request10(subject.build(), resource.build(), action.build()) : request11(subject.build(), resource.build(), action.build()); @@ -91,9 +92,9 @@ private static void add( private static Map request11( List subject, List resource, List action) { return ImmutableMap.of( - "AccessSubject", ImmutableMap.of("Attribute", subject), - "Resource", ImmutableMap.of("Attribute", resource), - "Action", ImmutableMap.of("Attribute", action)); + "AccessSubject", ImmutableMap.of(ATTRIBUTE, subject), + "Resource", ImmutableMap.of(ATTRIBUTE, resource), + "Action", ImmutableMap.of(ATTRIBUTE, action)); } private static Map request10( @@ -104,24 +105,24 @@ private static Map request10( ImmutableMap.of( "CategoryId", "urn:oasis:names:tc:xacml:1.0:subject-category:access-subject", - "Attribute", + ATTRIBUTE, subject), ImmutableMap.of( "CategoryId", "urn:oasis:names:tc:xacml:3.0:attribute-category:resource", - "Attribute", + ATTRIBUTE, resource), ImmutableMap.of( "CategoryId", "urn:oasis:names:tc:xacml:3.0:attribute-category:action", - "Attribute", + ATTRIBUTE, action))); } static class Attribute { - public final String AttributeId; - public final Object Value; - public final String DataType; + public final String attributeId; + public final Object value; + public final String dataType; Attribute(String attributeId, String value) { this(attributeId, value, "http://www.w3.org/2001/XMLSchema#string"); @@ -132,9 +133,9 @@ static class Attribute { } Attribute(String attributeId, Object value, String dataType) { - AttributeId = attributeId; - Value = value; - DataType = dataType; + this.attributeId = attributeId; + this.value = value; + this.dataType = dataType; } private static String inferType(Object value) { diff --git a/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/app/external/XacmlResponse.java b/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/app/external/XacmlResponse.java index 9cd10a6b..a8b3a008 100644 --- a/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/app/external/XacmlResponse.java +++ b/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/app/external/XacmlResponse.java @@ -1,5 +1,5 @@ /* - * Copyright 2018-2020 interactive instruments GmbH + * Copyright 2026 interactive instruments GmbH * * This Source Code Form is subject to the terms of the Mozilla Public * License, v. 2.0. If a copy of the MPL was not distributed with this @@ -13,54 +13,53 @@ import java.util.Map; import java.util.Objects; -/** - * @author zahnen - */ +@SuppressWarnings("PMD.CognitiveComplexity") public class XacmlResponse { - public List Response; + public List response; boolean isAllowed() { - return Objects.nonNull(Response) - && !Response.isEmpty() - && Strings.nullToEmpty(Response.get(0).Decision).equals("Permit"); + return Objects.nonNull(response) + && !response.isEmpty() + && "Permit".equals(Strings.nullToEmpty(response.get(0).decision)); } boolean isNotApplicable() { - return Objects.nonNull(Response) - && !Response.isEmpty() - && Strings.nullToEmpty(Response.get(0).Decision).equals("NotApplicable"); + return Objects.nonNull(response) + && !response.isEmpty() + && "NotApplicable".equals(Strings.nullToEmpty(response.get(0).decision)); } boolean isIndeterminate() { - return Objects.nonNull(Response) - && !Response.isEmpty() - && Strings.nullToEmpty(Response.get(0).Decision).equals("Indeterminate"); + return Objects.nonNull(response) + && !response.isEmpty() + && "Indeterminate".equals(Strings.nullToEmpty(response.get(0).decision)); } String getStatus() { - if (Objects.nonNull(Response) - && !Response.isEmpty() - && Objects.nonNull(Response.get(0).Status)) { + if (Objects.nonNull(response) + && !response.isEmpty() + && Objects.nonNull(response.get(0).status)) { String code = - Objects.nonNull(Response.get(0).Status.StatusCode) - ? Strings.nullToEmpty(Response.get(0).Status.StatusCode.Value) + Objects.nonNull(response.get(0).status.statusCode) + ? Strings.nullToEmpty(response.get(0).status.statusCode.value) : ""; return String.format( - "%s: %s", code, Strings.nullToEmpty(Response.get(0).Status.StatusMessage)); + "%s: %s", code, Strings.nullToEmpty(response.get(0).status.statusMessage)); } return null; } + @SuppressWarnings(("PMD.AvoidDeeplyNestedIfStmts")) Map getObligations() { - if (Objects.nonNull(Response) - && !Response.isEmpty() - && Objects.nonNull(Response.get(0).Obligations)) { + if (Objects.nonNull(response) + && !response.isEmpty() + && Objects.nonNull(response.get(0).obligations)) { Map obligations = new LinkedHashMap<>(); - for (Obligation obligation : Response.get(0).Obligations) { - if (Objects.nonNull(obligation.AttributeAssignment)) { - for (Attribute attribute : obligation.AttributeAssignment) { - if (Objects.nonNull(attribute.AttributeId) && Objects.nonNull(attribute.Value)) { - obligations.put(attribute.AttributeId, attribute.Value); + for (Obligation obligation : response.get(0).obligations) { + if (Objects.nonNull(obligation.attributeAssignment)) { + for (Attribute attribute : obligation.attributeAssignment) { + if (Objects.nonNull(attribute.attributeId) && Objects.nonNull(attribute.value)) { + obligations.put(attribute.attributeId, attribute.value); } } } @@ -71,28 +70,27 @@ Map getObligations() { } static class Response { - public String Decision; - public Status Status; - public List Obligations; + public String decision; + public Status status; + public List obligations; } static class Status { - public StatusCode StatusCode; - public String StatusMessage; + public StatusCode statusCode; + public String statusMessage; } static class StatusCode { - public String Value; + public String value; } static class Obligation { - public String Id; - public List AttributeAssignment; + public String id; + public List attributeAssignment; } static class Attribute { - public String AttributeId; - public String DataType; - public String Value; + public String attributeId; + public String value; } } diff --git a/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/domain/Role.java b/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/domain/Role.java index 16d51923..3b43e328 100644 --- a/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/domain/Role.java +++ b/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/domain/Role.java @@ -19,7 +19,7 @@ public enum Role { public static Role fromString(String role) { for (Role v : Role.values()) { - if (v.toString().toLowerCase().equals(role.toLowerCase())) { + if (v.toString().equalsIgnoreCase(role)) { return v; } } diff --git a/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/domain/SplitCookie.java b/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/domain/SplitCookie.java index 20e877f3..6ea68d27 100644 --- a/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/domain/SplitCookie.java +++ b/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/domain/SplitCookie.java @@ -8,23 +8,19 @@ package de.ii.xtraplatform.auth.domain; import com.google.common.collect.ImmutableList; -import de.ii.xtraplatform.auth.app.SplitCookieCredentialAuthFilter; import java.util.List; import java.util.Map; import java.util.Optional; import javax.ws.rs.core.Cookie; import javax.ws.rs.core.NewCookie; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; -public class SplitCookie { - - private static final Logger LOGGER = - LoggerFactory.getLogger(SplitCookieCredentialAuthFilter.class); +public final class SplitCookie { private static final String TOKEN_COOKIE_NAME = "xtraplatform-token"; private static final String SIGNATURE_COOKIE_NAME = "xtraplatform-signature"; + private SplitCookie() {} + public static Optional readToken(Map cookies) { Optional tokenCookie = Optional.ofNullable(cookies.get(TOKEN_COOKIE_NAME)); Optional signatureCookie = Optional.ofNullable(cookies.get(SIGNATURE_COOKIE_NAME)); @@ -40,35 +36,32 @@ public static Optional readToken(Map cookies) { public static List writeToken( String token, String domain, boolean secure, boolean rememberMe) { - int lastDot = token.lastIndexOf("."); + int lastDot = token.lastIndexOf('.'); String headerPayload = token.substring(0, lastDot); String signature = token.substring(lastDot + 1); // with rememberMe a user will be logged out after 30 days of inactivity // without rememberMe a user will be logged out after 1 hour of inactivity or a session end - int payloadExpires = rememberMe ? 2592000 : 3600; - int signatureExpires = rememberMe ? 2592000 : -1; + int payloadExpires = rememberMe ? 2_592_000 : 3_600; + int signatureExpires = rememberMe ? 2_592_000 : -1; return ImmutableList.of( - getPayloadCookie(headerPayload, domain, secure, payloadExpires), - getSignatureCookie(signature, domain, secure, signatureExpires)); + getPayloadCookie(headerPayload, secure, payloadExpires), + getSignatureCookie(signature, secure, signatureExpires)); } public static List deleteToken(String domain, boolean secure) { - return ImmutableList.of( - getPayloadCookie(null, domain, secure, 0), getSignatureCookie(null, domain, secure, 0)); + return ImmutableList.of(getPayloadCookie(null, secure, 0), getSignatureCookie(null, secure, 0)); } - private static String getPayloadCookie( - String payload, String domain, boolean secure, int expires) { + private static String getPayloadCookie(String payload, boolean secure, int expires) { NewCookie newCookie = new NewCookie(TOKEN_COOKIE_NAME, payload, "/", null, "", expires, secure, false); return String.format("%s;SameSite=strict", newCookie); } - private static String getSignatureCookie( - String signature, String domain, boolean secure, int expires) { + private static String getSignatureCookie(String signature, boolean secure, int expires) { NewCookie newCookie = new NewCookie(SIGNATURE_COOKIE_NAME, signature, "/", null, "", expires, secure, true); diff --git a/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/domain/TokenResponse.java b/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/domain/TokenResponse.java index ec7d8a90..5a6510f0 100644 --- a/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/domain/TokenResponse.java +++ b/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/domain/TokenResponse.java @@ -12,10 +12,10 @@ @Value.Immutable public interface TokenResponse { - String getAccess_token(); + String getAccessToken(); @Value.Default - default int getExpires_in() { + default int getExpiresIn() { return 60; } } diff --git a/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/infra/rest/TokenEndpoint.java b/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/infra/rest/TokenEndpoint.java index 6da9f6f0..b59ed0c9 100644 --- a/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/infra/rest/TokenEndpoint.java +++ b/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/infra/rest/TokenEndpoint.java @@ -113,7 +113,7 @@ public Response authorize(@Context HttpServletRequest request, Credentials body) Response.ResponseBuilder response = Response.ok() .entity( - ImmutableTokenResponse.builder().access_token(token).expires_in(expiresIn).build()); + ImmutableTokenResponse.builder().accessToken(token).expiresIn(expiresIn).build()); if (!body.noCookie) { String domain = null; // request.getServerName(); From cd6db756143122b43f59fcc7e3a3c09c348ddb6e Mon Sep 17 00:00:00 2001 From: "p.zahnen" Date: Fri, 23 Jan 2026 13:32:44 +0000 Subject: [PATCH 3/3] refactor endpoints and improve error handling --- .../auth/infra/rest/OidcEndpoint.java | 1 + .../auth/infra/rest/OidcView.java | 1 + .../auth/infra/rest/TokenEndpoint.java | 26 +++++++------------ .../auth/infra/rest/UserAdminEndpoint.java | 12 +++++---- 4 files changed, 19 insertions(+), 21 deletions(-) diff --git a/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/infra/rest/OidcEndpoint.java b/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/infra/rest/OidcEndpoint.java index 1092ed09..49904882 100644 --- a/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/infra/rest/OidcEndpoint.java +++ b/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/infra/rest/OidcEndpoint.java @@ -100,6 +100,7 @@ private String getAssetsPrefix(ContainerRequestContext containerRequestContext) } @Override + @SuppressWarnings("PMD.UseObjectForClearerAPI") public Response handle( ContainerRequestContext containerRequestContext, String redirectUri, diff --git a/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/infra/rest/OidcView.java b/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/infra/rest/OidcView.java index 6857820d..8d4fd8ea 100644 --- a/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/infra/rest/OidcView.java +++ b/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/infra/rest/OidcView.java @@ -10,6 +10,7 @@ import com.google.common.base.Charsets; import io.dropwizard.views.common.View; +@SuppressWarnings("PMD.DataClass") public class OidcView extends View { public final String oidcUri; public final String callbackUri; diff --git a/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/infra/rest/TokenEndpoint.java b/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/infra/rest/TokenEndpoint.java index b59ed0c9..c8b46496 100644 --- a/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/infra/rest/TokenEndpoint.java +++ b/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/infra/rest/TokenEndpoint.java @@ -14,8 +14,6 @@ import de.ii.xtraplatform.auth.domain.TokenHandler; import de.ii.xtraplatform.auth.domain.User; import de.ii.xtraplatform.auth.domain.UserAuthenticator; -import de.ii.xtraplatform.base.domain.AppContext; -import de.ii.xtraplatform.base.domain.AuthConfiguration; import de.ii.xtraplatform.services.domain.ServicesContext; import de.ii.xtraplatform.web.domain.Endpoint; import io.swagger.v3.oas.annotations.media.Content; @@ -43,31 +41,27 @@ @Path("/auth") public class TokenEndpoint implements Endpoint { - private static final int DEFAULT_EXPIRY = 2592000; + private static final int DEFAULT_EXPIRY = 2_592_000; + private final UserAuthenticator authenticator; + private final TokenHandler tokenGenerator; + private final URI servicesUri; public static class Credentials { @JsonProperty public String user; @JsonProperty public String password; @JsonProperty public int expiration = DEFAULT_EXPIRY; - @JsonProperty public boolean rememberMe = false; - @JsonProperty public boolean noCookie = false; + @JsonProperty public boolean rememberMe; + @JsonProperty public boolean noCookie; } - private final UserAuthenticator authenticator; - private final TokenHandler tokenGenerator; - private final URI servicesUri; - private final AuthConfiguration authConfig; - @Inject public TokenEndpoint( UserAuthenticator authenticator, TokenHandler tokenGenerator, - AppContext appContext, ServicesContext servicesContext) { this.authenticator = authenticator; this.tokenGenerator = tokenGenerator; this.servicesUri = servicesContext.getUri(); - this.authConfig = appContext.getConfiguration().getAuth(); } @RequestBody( @@ -75,8 +69,7 @@ public TokenEndpoint( content = @Content( examples = { - @ExampleObject( - value = "{\"user\": \"admin\", \"password\": \"admin\", \"noCookie\": true}") + @ExampleObject("{\"user\": \"admin\", \"password\": \"admin\", \"noCookie\": true}") }, schema = @Schema(implementation = Credentials.class))) @POST @@ -103,10 +96,10 @@ public Response authorize(@Context HttpServletRequest request, Credentials body) } return null; }) - .orElse(DEFAULT_EXPIRY)*/ ; + .orElse(DEFAULT_EXPIRY)*/ boolean rememberMe = body.rememberMe; - ; // Boolean.parseBoolean(body.get("rememberMe")); + // Boolean.parseBoolean(body.get("rememberMe")) String token = tokenGenerator.generateToken(user.get(), expiresIn, rememberMe); @@ -128,6 +121,7 @@ public Response authorize(@Context HttpServletRequest request, Credentials body) // TODO: instead of external url, get request url // TODO: but we want to access view action links with same token, would that work? + @SuppressWarnings("PMD.UnusedPrivateMethod") private String getDomain() { return getExternalUri().getHost(); } diff --git a/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/infra/rest/UserAdminEndpoint.java b/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/infra/rest/UserAdminEndpoint.java index 4f530b16..20109c16 100644 --- a/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/infra/rest/UserAdminEndpoint.java +++ b/xtraplatform-auth/src/main/java/de/ii/xtraplatform/auth/infra/rest/UserAdminEndpoint.java @@ -55,6 +55,7 @@ public class UserAdminEndpoint implements Endpoint { private static final Logger LOGGER = LoggerFactory.getLogger(UserAdminEndpoint.class); + private static final String FIELD_NEW_PASSWORD = "newPassword"; private final EntityDataStore userRepository; @Inject @@ -100,12 +101,13 @@ public User.UserData addUser( } catch (IllegalStateException | InterruptedException | ExecutionException e) { LogContext.error(LOGGER, e, "Error adding user"); - throw new BadRequestException(); + throw new BadRequestException(e); } } @Path("/{id}") @POST + @SuppressWarnings({"PMD.CyclomaticComplexity", "PMD.PreserveStackTrace"}) public Response updateUser( @Auth de.ii.xtraplatform.auth.domain.User user, @PathParam("id") String id, @@ -116,9 +118,9 @@ public Response updateUser( } if (!request.containsKey("oldPassword") - || !request.containsKey("newPassword") + || !request.containsKey(FIELD_NEW_PASSWORD) || !request.containsKey("newPasswordRepeat") - || !Objects.equals(request.get("newPassword"), request.get("newPassword"))) { + || !Objects.equals(request.get(FIELD_NEW_PASSWORD), request.get(FIELD_NEW_PASSWORD))) { throw new BadRequestException(); } @@ -131,7 +133,7 @@ public Response updateUser( User.UserData updated = new ImmutableUserData.Builder() .from(userData) - .password(PasswordHash.createHash(request.get("newPassword"))) + .password(PasswordHash.createHash(request.get(FIELD_NEW_PASSWORD))) .passwordExpiresAt(OptionalLong.empty()) .build(); @@ -143,7 +145,7 @@ public Response updateUser( if (e.getCause() instanceof IllegalArgumentException) { throw new BadRequestException(e.getCause().getMessage()); } - throw new InternalServerErrorException(); + throw new InternalServerErrorException(e); } }