Merge remote-tracking branch 'origin/develop' into fix-passwd-change

Author: strehle

model/src/main/java/org/cloudfoundry/identity/uaa/provider/SamlIdentityProviderDefinition.java

@@ -18,6 +18,7 @@
 import lombok.Data;
 import lombok.NoArgsConstructor;
 import org.cloudfoundry.identity.uaa.util.ObjectUtils;
+import org.cloudfoundry.identity.uaa.util.UaaStringUtils;
 import org.springframework.util.StringUtils;
 import org.xml.sax.InputSource;
 import org.xml.sax.SAXException;
@@ -91,7 +92,7 @@ public MetadataLocation getType() {
     }
 
     public static MetadataLocation getType(String urlOrXmlData) {
-        String trimmedValue = urlOrXmlData.trim();
+        String trimmedValue = stripLeadingCharacters(urlOrXmlData);
 
         if (trimmedValue.startsWith("<?xml") ||
                 trimmedValue.startsWith("<md:EntityDescriptor") ||
@@ -111,6 +112,20 @@ public static MetadataLocation getType(String urlOrXmlData) {
         return MetadataLocation.UNKNOWN;
     }
 
+    /**
+     * Trims whitespace and, if present, a leading byte order marker character. Some IdPs (e.g.
+     * Microsoft Entra ID's federation metadata endpoint) prepend a byte order marker to their
+     * XML response; whether it was originally a UTF-8, UTF-16LE, or UTF-16BE BOM, correctly
+     * decoded bytes leave this same character (U+FEFF) at the start of the resulting String,
+     * which would otherwise defeat the {@code startsWith} sniffing in {@link #getType(String)}.
+     */
+    private static String stripLeadingCharacters(String value) {
+        String trimmedValue = value.trim();
+        return trimmedValue.startsWith(UaaStringUtils.BYTE_ORDER_MARKER)
+                ? trimmedValue.substring(UaaStringUtils.BYTE_ORDER_MARKER.length()).trim()
+                : trimmedValue;
+    }
+
     @JsonIgnore
     public String getIdpEntityId() {
         return this.idpEntityId;

model/src/main/java/org/cloudfoundry/identity/uaa/util/UaaStringUtils.java

@@ -51,6 +51,14 @@ public final class UaaStringUtils {
 
     public static final String EMPTY_STRING = "";
 
+    /**
+     * The Unicode byte order marker (codepoint U+FEFF), as decoded into a Java String. This is
+     * the same character regardless of which encoding's BOM bytes produced it (UTF-8's EF BB BF,
+     * UTF-16LE's FF FE, UTF-16BE's FE FF, ...) once decoded with the correct charset for that
+     * encoding, or when a UTF-8 BOM survives an incorrect {@code new String(bytes, UTF_8)} decode.
+     */
+    public static final String BYTE_ORDER_MARKER = String.valueOf((char) 0xFEFF);
+
     public static final String DEFAULT_UAA_URL = "http://localhost:8080/uaa";
 
     private UaaStringUtils() {

model/src/test/java/org/cloudfoundry/identity/uaa/provider/saml/SamlIdentityProviderDefinitionTests.java

@@ -2,6 +2,7 @@
 
 import org.cloudfoundry.identity.uaa.provider.SamlIdentityProviderDefinition;
 import org.cloudfoundry.identity.uaa.util.JsonUtils;
+import org.cloudfoundry.identity.uaa.util.UaaStringUtils;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.springframework.util.ReflectionUtils;
@@ -141,6 +142,18 @@ void get_data_type_when_valid() {
         assertThat(definition.getType()).isEqualTo(DATA);
     }
 
+    @Test
+    void get_data_type_when_valid_with_leading_byte_order_marker() {
+        // Microsoft Entra ID's federation metadata endpoint prepends a byte order marker to its
+        // XML response. Whether the original bytes were a UTF-8, UTF-16LE, or UTF-16BE BOM,
+        // correctly decoded text has this same character (U+FEFF) leading it; String.trim() does
+        // not strip it, so metadata that is otherwise identical to get_data_type_when_valid()
+        // must still classify as DATA, not UNKNOWN.
+        String byteOrderMarkerPrefixedMetadata = UaaStringUtils.BYTE_ORDER_MARKER + IDP_METADATA;
+        definition.setMetaDataLocation(byteOrderMarkerPrefixedMetadata);
+        assertThat(definition.getType()).isEqualTo(DATA);
+    }
+
     public static final String ALIAS = "alias";
     public static final String IDP_METADATA = "<?xml version=\"1.0\"?>\n" +
             "<md:EntityDescriptor xmlns:md=\"urn:oasis:names:tc:SAML:2.0:metadata\" xmlns:ds=\"http://www.w3.org/2000/09/xmldsig#\" entityID=\"http://" + ALIAS + ".cfapps.io/saml2/idp/metadata.php\" ID=\"pfx06ad4153-c17c-d286-194c-dec30bb92796\"><ds:Signature>\n" +

server/src/main/java/org/cloudfoundry/identity/uaa/audit/event/AbstractUaaEvent.java

@@ -13,30 +13,35 @@
  *******************************************************************************/
 package org.cloudfoundry.identity.uaa.audit.event;
 
-import tools.jackson.core.type.TypeReference;
 import org.cloudfoundry.identity.uaa.audit.AuditEvent;
 import org.cloudfoundry.identity.uaa.audit.AuditEventType;
 import org.cloudfoundry.identity.uaa.audit.UaaAuditService;
+import org.cloudfoundry.identity.uaa.authentication.UaaAuthenticationDetails;
 import org.cloudfoundry.identity.uaa.oauth.UaaOauth2Authentication;
 import org.cloudfoundry.identity.uaa.oauth.jwt.Jwt;
 import org.cloudfoundry.identity.uaa.oauth.jwt.JwtHelper;
 import org.cloudfoundry.identity.uaa.oauth.provider.OAuth2Authentication;
 import org.cloudfoundry.identity.uaa.oauth.provider.authentication.OAuth2AuthenticationDetails;
 import org.cloudfoundry.identity.uaa.oauth.token.ClaimConstants;
 import org.cloudfoundry.identity.uaa.util.JsonUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 import org.springframework.context.ApplicationEvent;
 import org.springframework.security.core.Authentication;
 import org.springframework.security.core.GrantedAuthority;
 import org.springframework.security.core.context.SecurityContextHolder;
+import org.springframework.security.web.authentication.WebAuthenticationDetails;
+import org.springframework.util.StringUtils;
+import tools.jackson.core.type.TypeReference;
 
 import java.io.Serial;
 import java.security.Principal;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Map;
+import java.util.Optional;
 
 import static org.cloudfoundry.identity.uaa.util.UaaTokenUtils.isJwtToken;
-import static org.springframework.util.StringUtils.hasText;
 
 /**
  * Base class for UAA events that want to publish audit records.
@@ -49,6 +54,9 @@ public abstract class AbstractUaaEvent extends ApplicationEvent {
 
     @Serial
     private static final long serialVersionUID = -7639844193401892160L;
+
+    private static final Logger logger = LoggerFactory.getLogger(AbstractUaaEvent.class);
+
     private final transient String zoneId;
 
     private Authentication authentication;
@@ -115,35 +123,50 @@ private String getAuthenticationString(Authentication caller) {
             builder.append("caller=").append(caller.getName());
         }
 
-        if (caller.getDetails() != null) {
+        Object details = caller.getDetails();
+        if (details != null) {
             builder.append(", details=(");
-            try {
-                @SuppressWarnings("unchecked")
-                Map<String, Object> map =
-                        JsonUtils.readValue((String) caller.getDetails(), new TypeReference<Map<String, Object>>(){
-                        });
-                if (map.containsKey("remoteAddress")) {
-                    builder.append("remoteAddress=").append(map.get("remoteAddress")).append(", ");
-                }
-                builder.append("type=").append(caller.getDetails().getClass().getSimpleName());
-            } catch (Exception _) {
-                // ignore
-                builder.append(caller.getDetails());
-            }
+            extractRemoteAddress(details).ifPresent(address -> builder.append("remoteAddress=").append(address).append(", "));
+            builder.append("type=").append(details.getClass().getSimpleName());
             appendTokenDetails(caller, builder);
             builder.append(")");
         }
+
         return builder.toString();
     }
 
-    protected void appendTokenDetails(Authentication caller, StringBuilder builder) {
+    private Optional<String> extractRemoteAddress(Object details) {
+        return switch (details) {
+            case UaaAuthenticationDetails d -> Optional.ofNullable(d.getOrigin()).filter(StringUtils::hasText);
+            case OAuth2AuthenticationDetails d -> Optional.ofNullable(d.getRemoteAddress()).filter(StringUtils::hasText);
+            case WebAuthenticationDetails d -> Optional.ofNullable(d.getRemoteAddress()).filter(StringUtils::hasText);
+            case Map<?, ?> map -> Optional.ofNullable(map.get("remoteAddress")).map(Object::toString).filter(StringUtils::hasText);
+            case String jsonBlob -> extractRemoteAddressFromJson(jsonBlob);
+            default -> {
+                logger.warn("Unhandled Authentication.details type in audit origin: {}", details.getClass().getName());
+                yield Optional.empty();
+            }
+        };
+    }
+
+    private Optional<String> extractRemoteAddressFromJson(String jsonBlob) {
+        try {
+            Map<String, Object> map = JsonUtils.readValue(jsonBlob, new TypeReference<>() {
+            });
+            return map == null ? Optional.empty() : extractRemoteAddress(map);
+        } catch (JsonUtils.JsonUtilException _) {
+            return Optional.empty();
+        }
+    }
+
+    private void appendTokenDetails(Authentication caller, StringBuilder builder) {
         String tokenValue = null;
         if (caller instanceof UaaOauth2Authentication uaaOauth2Authentication) {
             tokenValue = uaaOauth2Authentication.getTokenValue();
         } else if (caller.getDetails() instanceof OAuth2AuthenticationDetails oAuth2AuthenticationDetails) {
             tokenValue = oAuth2AuthenticationDetails.getTokenValue();
         }
-        if (hasText(tokenValue)) {
+        if (StringUtils.hasText(tokenValue)) {
             if (isJwtToken(tokenValue)) {
                 try {
                     Jwt token = JwtHelper.decode(tokenValue);

server/src/main/java/org/cloudfoundry/identity/uaa/authentication/event/AbstractUaaAuthenticationEvent.java

@@ -17,6 +17,10 @@
 import org.cloudfoundry.identity.uaa.authentication.UaaAuthenticationDetails;
 import org.springframework.security.core.Authentication;
 
+import java.util.StringJoiner;
+
+import static org.springframework.util.StringUtils.hasText;
+
 /**
  * @author Luke Taylor
  */
@@ -27,7 +31,21 @@ public abstract class AbstractUaaAuthenticationEvent extends AbstractUaaEvent {
     }
 
     protected String getOrigin(UaaAuthenticationDetails details) {
-        return details == null ? "unknown" : details.toString();
+        if (details == null) {
+            return "unknown";
+        }
+
+        StringJoiner joiner = new StringJoiner(", ");
+
+        if (hasText(details.getOrigin())) {
+            joiner.add("remoteAddress=" + details.getOrigin());
+        }
+
+        if (hasText(details.getClientId())) {
+            joiner.add("clientId=" + details.getClientId());
+        }
+
+        return joiner.length() == 0 ? "unknown" : joiner.toString();
     }
 
     UaaAuthenticationDetails getAuthenticationDetails() {

server/src/main/java/org/cloudfoundry/identity/uaa/provider/saml/SamlIdentityProviderConfigurator.java

@@ -1,5 +1,6 @@
 package org.cloudfoundry.identity.uaa.provider.saml;
 
+import org.apache.commons.io.ByteOrderMark;
 import org.apache.commons.io.IOUtils;
 import org.apache.hc.core5.net.URIBuilder;
 import org.cloudfoundry.identity.uaa.constants.OriginKeys;
@@ -17,7 +18,9 @@
 
 import java.net.URI;
 import java.net.URISyntaxException;
+import java.nio.charset.Charset;
 import java.nio.charset.StandardCharsets;
+import java.util.Arrays;
 import java.util.List;
 
 import static org.springframework.util.StringUtils.hasText;
@@ -166,9 +169,31 @@ public String resolveMetadataXml(SamlIdentityProviderDefinition def) {
         try {
             String adjustedMetadataURIForPort = adjustURIForPort(metadataLocation);
             byte[] metadata = fixedHttpMetaDataProvider.fetchMetadata(adjustedMetadataURIForPort, def.isSkipSslValidation());
-            return new String(metadata, StandardCharsets.UTF_8);
+            return new String(metadata, detectCharset(metadata));
         } catch (URISyntaxException e) {
             throw new IllegalStateException("Invalid socket factory(invalid URI):" + metadataLocation, e);
         }
     }
+
+    /**
+     * Detects the charset of fetched metadata bytes from a leading byte order marker. Only
+     * UTF-16 needs to be sniffed explicitly here: a UTF-8 byte order marker, if present, already
+     * decodes correctly under the UTF-8 default and is stripped later by
+     * {@link SamlIdentityProviderDefinition#getType(String)}, since it decodes to the same
+     * character (U+FEFF) as a correctly-decoded UTF-16 BOM.
+     */
+    static Charset detectCharset(byte[] metadata) {
+        if (hasLeadingByteOrderMark(metadata, ByteOrderMark.UTF_16LE)) {
+            return StandardCharsets.UTF_16LE;
+        }
+        if (hasLeadingByteOrderMark(metadata, ByteOrderMark.UTF_16BE)) {
+            return StandardCharsets.UTF_16BE;
+        }
+        return StandardCharsets.UTF_8;
+    }
+
+    private static boolean hasLeadingByteOrderMark(byte[] data, ByteOrderMark byteOrderMark) {
+        byte[] bom = byteOrderMark.getBytes();
+        return data.length >= bom.length && Arrays.equals(data, 0, bom.length, bom, 0, bom.length);
+    }
 }