(directory-scimple) branch never-returned-regression-tests created (now 9071361c)

[bd...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

bdemers pushed a change to branch never-returned-regression-tests
in repository https://gitbox.apache.org/repos/asf/directory-scimple.git


      at 9071361c Guard Returned.NEVER attributes from leaking via toString() and responses

This branch includes the following new commits:

     new 9071361c Guard Returned.NEVER attributes from leaking via toString() and responses

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.

(directory-scimple) 01/01: Guard Returned.NEVER attributes from leaking via toString() and responses

[bd...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

bdemers pushed a commit to branch never-returned-regression-tests
in repository https://gitbox.apache.org/repos/asf/directory-scimple.git

commit 9071361cd104ead9992eb2a061e94500ab61d2a7
Author: Brian Demers <bd...@apache.org>
AuthorDate: Sat May 30 23:32:45 2026 -0400

    Guard Returned.NEVER attributes from leaking via toString() and responses
    
    Returned.NEVER attributes (in the core schema, only ScimUser.password) must
    never appear in a serialized SCIM response (RFC 7643 section 2.2 / 8.7.1) nor in
    a resource's toString() output, which scim-server logs at DEBUG. Both held in
    code but were not guarded by tests, so a future change (e.g. regenerating
    toString(), or adding a new NEVER attribute) could silently reintroduce a leak.
    
    scim-spec-schema (ScimUserTest, new): guards the toString() / logging channel.
    - VALUE-channel guard: the password value (a sentinel) must not appear.
    - SCHEMA/NAME-channel guard: schema-driven over every Returned.NEVER attribute,
      asserting its "name=" token is absent, so a future NEVER field is covered too.
    - null-password case: "password=" must be absent even when password is null.
    - sanity check: password is annotated Returned.NEVER in the schema.
    
    scim-server (AttributeUtilTest, +3 tests): guards the response channel through
    AttributeUtil.setAttributesForDisplay() — password stripped on display and even
    when explicitly requested, absent from the serialized JSON (with a present-in-raw
    baseline proving stripping is causal), and the original resource is not mutated.
    
    scim-test (ExampleObjectExtension): omit the Returned.NEVER valueNever field from
    toString(), mirroring ScimUser.toString() omitting password, so the example does
    not model a leak for implementers who copy it.
    
    Tests use JUnit 5 + AssertJ and a distinctive sentinel value.
---
 .../scim/server/rest/AttributeUtilTest.java        | 120 ++++++++++++++++
 .../scim/spec/resources/ScimUserTest.java          | 159 +++++++++++++++++++++
 .../scim/test/stub/ExampleObjectExtension.java     |   4 +-
 3 files changed, 282 insertions(+), 1 deletion(-)

diff --git a/scim-server/src/test/java/org/apache/directory/scim/server/rest/AttributeUtilTest.java b/scim-server/src/test/java/org/apache/directory/scim/server/rest/AttributeUtilTest.java
index 4a9b66fc..ba8a7792 100644
--- a/scim-server/src/test/java/org/apache/directory/scim/server/rest/AttributeUtilTest.java
+++ b/scim-server/src/test/java/org/apache/directory/scim/server/rest/AttributeUtilTest.java
@@ -23,6 +23,7 @@ import com.fasterxml.jackson.core.JsonGenerationException;
 import com.fasterxml.jackson.databind.JsonMappingException;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import org.apache.directory.scim.core.json.ObjectMapperFactory;
+import org.apache.directory.scim.server.exception.AttributeException;
 import org.apache.directory.scim.spec.resources.GroupMembership;
 import org.apache.directory.scim.spec.resources.ScimGroup;
 import org.apache.directory.scim.spec.schema.Meta;
@@ -380,6 +381,125 @@ public class AttributeUtilTest {
     return user;
   }
 
+  // -----------------------------------------------------------------------
+  // Returned.NEVER regression tests
+  //
+  // SECURITY REQUIREMENT: attributes annotated Returned.NEVER (e.g. ScimUser.password)
+  // must be stripped by setAttributesForDisplay() before a response leaves the server.
+  // If any test below fails with the sentinel "SECRET-do-not-log-uuid", a Returned.NEVER
+  // attribute is leaking into the serialized response. Fix AttributeUtil, NOT these tests.
+  // -----------------------------------------------------------------------
+
+  /**
+   * Guard: {@code password} (Returned.NEVER) must be null after
+   * {@link AttributeUtil#setAttributesForDisplay(ScimResource)} with no explicit
+   * attribute selection.
+   *
+   * <p>The sentinel value makes it unambiguous which field leaked if the assertion fails.
+   * The input object must not be mutated — stripping operates on a clone.
+   */
+  @Test
+  void passwordSentinelStrippedBySetAttributesForDisplay() throws AttributeException {
+    final String SENTINEL = "SECRET-do-not-log-uuid";
+
+    ScimUser user = new ScimUser();
+    user.setId("sentinel-test-user");
+    user.setUserName("sentinel-user");
+    user.setPassword(SENTINEL);
+
+    ScimUser result = attributeUtil.setAttributesForDisplay(user);
+
+    Assertions.assertThat(result.getPassword())
+        .as("ScimUser.password is annotated Returned.NEVER and must be null after " +
+            "setAttributesForDisplay(). Sentinel value that must NOT survive: " + SENTINEL)
+        .isNull();
+
+    // Stripping must operate on a clone — the input object must be unchanged.
+    Assertions.assertThat(user.getPassword())
+        .as("setAttributesForDisplay() must not mutate the original resource; " +
+            "user.password should still equal the sentinel after the call")
+        .isEqualTo(SENTINEL);
+  }
+
+  /**
+   * Guard: {@code password} (Returned.NEVER) must be absent from the serialized
+   * JSON produced by the object mapper after {@link AttributeUtil#setAttributesForDisplay(ScimResource)}.
+   *
+   * <p>This covers the full serialization channel end-to-end, not just the in-memory field.
+   * A baseline assertion first confirms the sentinel IS present in raw (pre-strip) JSON,
+   * proving that the subsequent absence is due to stripping rather than serializer omission.
+   */
+  @Test
+  void passwordSentinelAbsentFromSerializedResponse() throws IOException, AttributeException {
+    final String SENTINEL = "SECRET-do-not-log-uuid";
+
+    ScimUser user = new ScimUser();
+    user.setId("sentinel-json-test-user");
+    user.setUserName("sentinel-json-user");
+    user.setPassword(SENTINEL);
+
+    // Baseline: raw ScimUser JSON includes password — confirm the sentinel is present
+    // before stripping, so the subsequent absence proves stripping actually removed it.
+    StringWriter rawSw = new StringWriter();
+    objectMapper.writeValue(rawSw, user);
+    Assertions.assertThat(rawSw.toString())
+        .as("Baseline: raw (pre-strip) ScimUser JSON must contain the sentinel so we know " +
+            "stripping is what removes it, not a serializer no-op")
+        .contains(SENTINEL);
+
+    ScimUser result = attributeUtil.setAttributesForDisplay(user);
+
+    StringWriter sw = new StringWriter();
+    objectMapper.writeValue(sw, result);
+    String json = sw.toString();
+
+    Assertions.assertThat(json)
+        .as("Serialized SCIM response must not contain the Returned.NEVER password sentinel " +
+            "after setAttributesForDisplay(). Sentinel: " + SENTINEL)
+        .doesNotContain(SENTINEL);
+
+    // Stripping must operate on a clone — the input object must be unchanged.
+    Assertions.assertThat(user.getPassword())
+        .as("setAttributesForDisplay() must not mutate the original resource; " +
+            "user.password should still equal the sentinel after the call")
+        .isEqualTo(SENTINEL);
+  }
+
+  /**
+   * Guard: {@code password} (Returned.NEVER) must remain null after
+   * {@link AttributeUtil#setAttributesForDisplay(ScimResource, java.util.Set)}
+   * even when the caller explicitly requests the {@code password} attribute by name.
+   *
+   * <p>Per RFC 7643 §2.2: "never" attributes are never returned regardless of
+   * client requests.
+   */
+  @Test
+  void passwordSentinelStrippedEvenWhenExplicitlyRequested() throws AttributeException {
+    final String SENTINEL = "SECRET-do-not-log-uuid";
+
+    ScimUser user = new ScimUser();
+    user.setId("sentinel-explicit-test-user");
+    user.setUserName("sentinel-explicit-user");
+    user.setPassword(SENTINEL);
+
+    // Explicitly request "password" — must still be stripped per RFC 7643.
+    java.util.Set<AttributeReference> requested = new HashSet<>();
+    requested.add(new AttributeReference("password"));
+
+    ScimUser result = attributeUtil.setAttributesForDisplay(user, requested);
+
+    Assertions.assertThat(result.getPassword())
+        .as("ScimUser.password is Returned.NEVER and must be null after setAttributesForDisplay() " +
+            "even when the client requests it explicitly (RFC 7643 §2.2). Sentinel: " + SENTINEL)
+        .isNull();
+
+    // Stripping must operate on a clone — the input object must be unchanged.
+    Assertions.assertThat(user.getPassword())
+        .as("setAttributesForDisplay() must not mutate the original resource; " +
+            "user.password should still equal the sentinel after the call")
+        .isEqualTo(SENTINEL);
+  }
+
   private ScimGroup createScimGroup() {
     ScimGroup group = new ScimGroup();
     group.setId("g1");
diff --git a/scim-spec/scim-spec-schema/src/test/java/org/apache/directory/scim/spec/resources/ScimUserTest.java b/scim-spec/scim-spec-schema/src/test/java/org/apache/directory/scim/spec/resources/ScimUserTest.java
new file mode 100644
index 00000000..4e9e3c74
--- /dev/null
+++ b/scim-spec/scim-spec-schema/src/test/java/org/apache/directory/scim/spec/resources/ScimUserTest.java
@@ -0,0 +1,159 @@
+/*
+* Licensed to the Apache Software Foundation (ASF) under one
+* or more contributor license agreements.  See the NOTICE file
+* distributed with this work for additional information
+* regarding copyright ownership.  The ASF licenses this file
+* to you under the Apache License, Version 2.0 (the
+* "License"); you may not use this file except in compliance
+* with the License.  You may obtain a copy of the License at
+
+* http://www.apache.org/licenses/LICENSE-2.0
+
+* Unless required by applicable law or agreed to in writing,
+* software distributed under the License is distributed on an
+* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+* KIND, either express or implied.  See the License for the
+* specific language governing permissions and limitations
+* under the License.
+*/
+
+package org.apache.directory.scim.spec.resources;
+
+import org.apache.directory.scim.spec.schema.Schema;
+import org.apache.directory.scim.spec.schema.Schemas;
+import org.junit.jupiter.api.Test;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+/**
+ * Unit tests for {@link ScimUser}.
+ */
+public class ScimUserTest {
+
+  /** Distinctive password value so a substring assertion on toString() output is unambiguous. */
+  private static final String SENTINEL = "SECRET-do-not-log-uuid";
+
+  private ScimUser buildUserWithSentinelPassword() {
+    ScimUser user = new ScimUser();
+    user.setId("test-user-id-1");
+    user.setUserName("jdoe");
+    user.setDisplayName("John Doe");
+    user.setPassword(SENTINEL);
+
+    Name name = new Name();
+    name.setGivenName("John");
+    name.setFamilyName("Doe");
+    user.setName(name);
+
+    return user;
+  }
+
+  /**
+   * VALUE-channel guard: a {@code Returned.NEVER} value (the sentinel) must not appear in
+   * {@link ScimUser#toString()}. Pairs with {@link #allNeverReturnedAttributeNamesAbsentFromToString()}
+   * (the NAME-channel guard) — both intentionally fire on the same leak; do not consolidate them.
+   */
+  @Test
+  public void passwordSentinelAbsentFromToString() {
+    ScimUser user = buildUserWithSentinelPassword();
+
+    String toString = user.toString();
+
+    assertThat(toString)
+        .as("VALUE-channel guard: a Returned.NEVER attribute's VALUE (sentinel '%s') leaked " +
+            "into ScimUser.toString(). If this fails, toString() was regenerated to include " +
+            "'password'. Fix toString() to omit Returned.NEVER fields.", SENTINEL)
+        .doesNotContain(SENTINEL);
+  }
+
+  /**
+   * SCHEMA/NAME-channel guard: for every {@code Returned.NEVER} attribute in the
+   * {@link ScimUser} schema, its name token (e.g. {@code "password="}) must be absent from
+   * {@link ScimUser#toString()}. Being schema-driven, it also covers future
+   * {@code Returned.NEVER} fields and the {@code password=null} case. See the inline note
+   * for the schema-name/field-name assumption.
+   */
+  @Test
+  public void allNeverReturnedAttributeNamesAbsentFromToString() {
+    Schema userSchema = Schemas.schemaFor(ScimUser.class);
+
+    List<String> neverAttributes = new ArrayList<>();
+    for (Schema.Attribute attr : userSchema.getAttributes()) {
+      if (Schema.Attribute.Returned.NEVER.equals(attr.getReturned())) {
+        neverAttributes.add(attr.getName());
+      }
+    }
+
+    assertThat(neverAttributes)
+        .as("Expected at least 'password' to have Returned.NEVER in the ScimUser schema")
+        .isNotEmpty();
+
+    ScimUser user = buildUserWithSentinelPassword();
+    String toString = user.toString();
+
+    // Assert the attribute NAME token is absent. A regenerated toString() that includes
+    // password will produce "..., password=SECRET-do-not-log-uuid, ..." and this assertion
+    // catches it by the "password=" token independently of the value.
+    //
+    // Assumption: SCIM schema name == Java field name as rendered in toString().
+    // If a future Returned.NEVER field's schema name differs from its toString() rendering,
+    // add an explicit targeted test for that field alongside this schema-driven loop.
+    for (String attrName : neverAttributes) {
+      assertThat(toString)
+          .as("SCHEMA/NAME-channel guard: a Returned.NEVER attribute NAME ('%s=') is being " +
+              "rendered by ScimUser.toString(). Fix toString() to omit this field. " +
+              "Note: this guard assumes the SCIM schema name matches the Java field name " +
+              "as rendered by toString().",
+              attrName)
+          .doesNotContain(attrName + "=");
+    }
+  }
+
+  /**
+   * NAME-channel guard, null-value case: {@code "password="} must be absent from
+   * {@link ScimUser#toString()} even when password is null — which the sentinel guard
+   * cannot catch.
+   */
+  @Test
+  public void nullPasswordAbsentFromToString() {
+    ScimUser user = new ScimUser();
+    user.setId("test-user-id-null-pw");
+    user.setUserName("jdoe-null-pw");
+    user.setDisplayName("Jane Doe");
+    // password intentionally left null
+
+    Name name = new Name();
+    name.setGivenName("Jane");
+    name.setFamilyName("Doe");
+    user.setName(name);
+
+    String toString = user.toString();
+
+    assertThat(toString)
+        .as("NAME-channel guard (null-value case): 'password=' must not appear in " +
+            "ScimUser.toString() even when password is null. The VALUE-channel sentinel " +
+            "test would not catch this because the sentinel is absent when password=null.")
+        .doesNotContain("password=");
+  }
+
+  /**
+   * Sanity check: {@code password} is annotated {@code Returned.NEVER} in the schema, so
+   * the value-level guards above stay meaningful if the annotation is ever changed.
+   */
+  @Test
+  public void passwordAttributeIsReturnedNeverInSchema() {
+    Schema userSchema = Schemas.schemaFor(ScimUser.class);
+
+    Schema.Attribute passwordAttr = userSchema.getAttribute("password");
+    assertThat(passwordAttr)
+        .as("Expected 'password' attribute to be present in ScimUser schema")
+        .isNotNull();
+
+    assertThat(passwordAttr.getReturned())
+        .as("ScimUser.password must be annotated with Returned.NEVER per RFC 7643 §8.7.1")
+        .isEqualTo(Schema.Attribute.Returned.NEVER);
+  }
+}
diff --git a/scim-test/src/main/java/org/apache/directory/scim/test/stub/ExampleObjectExtension.java b/scim-test/src/main/java/org/apache/directory/scim/test/stub/ExampleObjectExtension.java
index 3d70dd4b..fdcdc97e 100644
--- a/scim-test/src/main/java/org/apache/directory/scim/test/stub/ExampleObjectExtension.java
+++ b/scim-test/src/main/java/org/apache/directory/scim/test/stub/ExampleObjectExtension.java
@@ -215,7 +215,9 @@ public class ExampleObjectExtension implements ScimExtension {
   }
 
   public String toString() {
-    return "ExampleObjectExtension(valueAlways=" + this.getValueAlways() + ", valueDefault=" + this.getValueDefault() + ", valueNever=" + this.getValueNever() + ", valueRequest=" + this.getValueRequest() + ", valueComplex=" + this.getValueComplex() + ", list=" + this.getList() + ", enumList=" + this.getEnumList() + ", subobject=" + this.getSubobject() + ")";
+    // valueNever is Returned.NEVER and is omitted here; Returned.NEVER fields should
+    // generally be kept out of toString() to avoid leaking them into logs.
+    return "ExampleObjectExtension(valueAlways=" + this.getValueAlways() + ", valueDefault=" + this.getValueDefault() + ", valueRequest=" + this.getValueRequest() + ", valueComplex=" + this.getValueComplex() + ", list=" + this.getList() + ", enumList=" + this.getEnumList() + ", subobject=" + this.getSubobject() + ")";
   }
 
   @XmlType