From 3f8ac4360a0019e9ca25bacc838030630bd6dece Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Fri, 30 Oct 2026 14:03:46 +0000 Subject: [PATCH 1/4] Include the RDN attribute on LDAP Add and honor AD rDNAttID NewEntryDialog.doCreate sent only MUST fields, so an AD group named with cn (MAY via top) was added without cn and the server rejected it. Always attach the RDN attribute=value unless a MUST field already has it. SchemaService now reads classSchema rDNAttID alongside X-NDS_NAMING so the naming combo lists it first. Co-authored-by: Jerry Combs --- .../arborj/service/SchemaService.java | 105 ++++++++++- .../arborj/view/NewEntryDialog.java | 83 +++++++-- .../arborj/service/SchemaServiceTest.java | 166 ++++++++++++++++++ .../arborj/view/NewEntryDialogTest.java | 164 +++++++++++++++++ 4 files changed, 499 insertions(+), 19 deletions(-) create mode 100644 src/test/java/com/pointbluetech/arborj/service/SchemaServiceTest.java create mode 100644 src/test/java/com/pointbluetech/arborj/view/NewEntryDialogTest.java diff --git a/src/main/java/com/pointbluetech/arborj/service/SchemaService.java b/src/main/java/com/pointbluetech/arborj/service/SchemaService.java index 1def7b4..6cf6ff1 100644 --- a/src/main/java/com/pointbluetech/arborj/service/SchemaService.java +++ b/src/main/java/com/pointbluetech/arborj/service/SchemaService.java @@ -38,6 +38,8 @@ public class SchemaService { private Set containerClasses; private Map attributeMap; private Map objectClassMap; + /** lDAPDisplayName (lower) → rDNAttID (lower) from AD classSchema. Empty when not AD. */ + private Map adRdnAttIds = Map.of(); public SchemaService(LDAPService ldapService) { this.ldapService = ldapService; @@ -103,9 +105,11 @@ public void loadSchema() throws LDAPException { } // Fetch subschema ONCE and reuse for all loading var schema = fetchSubschema(); + adRdnAttIds = Map.of(); containerClasses = loadContainerClasses(schema); attributeMap = loadAttributeMap(schema); objectClassMap = loadObjectClassMap(schema); + applyRDNAttIDs(objectClassMap, adRdnAttIds); } public Set getContainerClasses() { @@ -168,11 +172,13 @@ private void loadADContainerClasses(Set result) throws LDAPException { String schemaDN = firstValue(rootDSE, "schemaNamingContext"); if (schemaDN == null) return; - // Search for all classSchema objects with possSuperiors + // Search for all classSchema objects with possSuperiors and rDNAttID List classSchemas = ldapService.search(schemaDN, com.pointbluetech.arborj.model.SearchScope.SUBTREE, "(objectClass=classSchema)", - "lDAPDisplayName", "possSuperiors", "systemPossSuperiors"); + "lDAPDisplayName", "possSuperiors", "systemPossSuperiors", "rDNAttID"); + + adRdnAttIds = collectRDNAttIDs(classSchemas); // Any class that appears as a possSuperior is a container for (LDAPEntry entry : classSchemas) { @@ -406,6 +412,101 @@ private List resolveNaming(String className, Map> n return null; } + /** + * Reads AD classSchema rDNAttID values. Keys and values are lowercased. + * OID-looking values are skipped so the combo is not filled with + * {@code 2.5.4.3} when the server does not return the LDAP display name. + */ + static Map collectRDNAttIDs(List classSchemas) { + Map result = new LinkedHashMap<>(); + if (classSchemas == null) return result; + for (LDAPEntry entry : classSchemas) { + if (entry == null) continue; + String name = entry.getFirstValue("lDAPDisplayName"); + String rdn = entry.getFirstValue("rDNAttID"); + if (name == null || name.isBlank() || rdn == null || rdn.isBlank()) continue; + String trimmed = rdn.trim(); + if (looksLikeOid(trimmed)) continue; + result.put(name.trim().toLowerCase(Locale.ROOT), trimmed.toLowerCase(Locale.ROOT)); + } + return result; + } + + /** + * Sets each class's naming attribute from AD rDNAttID (direct or inherited + * via SUP), putting it first alongside any existing X-NDS_NAMING list. + */ + static void applyRDNAttIDs(Map objectClassMap, + Map rdnAttIds) { + if (objectClassMap == null || objectClassMap.isEmpty() + || rdnAttIds == null || rdnAttIds.isEmpty()) { + return; + } + for (String className : new ArrayList<>(objectClassMap.keySet())) { + LDAPObjectClassInfo info = objectClassMap.get(className); + if (info == null) continue; + String rdn = resolveRDNAttID(className, objectClassMap, rdnAttIds, new HashSet<>()); + if (rdn == null || rdn.isBlank()) continue; + List naming = mergeNamingAttribute(info.getNamingAttributes(), rdn); + if (namingEquals(info.getNamingAttributes(), naming)) continue; + objectClassMap.put(className, new LDAPObjectClassInfo( + info.getName(), + info.getMustAttributes(), + info.getMayAttributes(), + info.getSuperClass(), + naming + )); + } + } + + private static String resolveRDNAttID(String className, + Map objectClassMap, + Map rdnAttIds, + Set visited) { + String current = className; + while (current != null) { + String key = current.toLowerCase(Locale.ROOT); + if (!visited.add(key)) break; + String rdn = rdnAttIds.get(key); + if (rdn != null && !rdn.isBlank()) return rdn; + LDAPObjectClassInfo info = objectClassMap.get(key); + current = info != null ? info.getSuperClass() : null; + } + return null; + } + + private static List mergeNamingAttribute(List existing, String rdnAttId) { + Map merged = new LinkedHashMap<>(); + merged.put(rdnAttId.toLowerCase(Locale.ROOT), rdnAttId); + if (existing != null) { + for (String name : existing) { + if (name == null || name.isBlank()) continue; + merged.putIfAbsent(name.toLowerCase(Locale.ROOT), name); + } + } + return new ArrayList<>(merged.values()); + } + + private static boolean namingEquals(List a, List b) { + if (a == b) return true; + if (a == null || b == null) return false; + return a.equals(b); + } + + /** True for values like {@code 2.5.4.3} (digits and dots only, with at least one of each). */ + static boolean looksLikeOid(String value) { + if (value == null || value.isEmpty()) return false; + boolean hasDigit = false; + boolean hasDot = false; + for (int i = 0; i < value.length(); i++) { + char c = value.charAt(i); + if (c >= '0' && c <= '9') hasDigit = true; + else if (c == '.') hasDot = true; + else return false; + } + return hasDigit && hasDot; + } + private static String firstValue(Map> attrs, String key) { List values = attrs.get(key); if (values != null && !values.isEmpty()) return values.getFirst(); diff --git a/src/main/java/com/pointbluetech/arborj/view/NewEntryDialog.java b/src/main/java/com/pointbluetech/arborj/view/NewEntryDialog.java index 422be76..e47fd4f 100644 --- a/src/main/java/com/pointbluetech/arborj/view/NewEntryDialog.java +++ b/src/main/java/com/pointbluetech/arborj/view/NewEntryDialog.java @@ -303,25 +303,18 @@ private void updateNamingAttributes(ComboBox combo) { Map ocMap = controller.getSchemaService().getObjectClassMap(); if (ocMap == null) return; - Set namingAttrs = new LinkedHashSet<>(); + List selected = new ArrayList<>(); for (String ocName : selectedClasses) { - LDAPObjectClassInfo info = ocMap.get(ocName); - if (info != null && info.getNamingAttributes() != null) { - namingAttrs.addAll(info.getNamingAttributes()); - } - } - - // Fallback: common naming attributes - if (namingAttrs.isEmpty()) { - namingAttrs.addAll(List.of("cn", "ou", "o", "c", "dc", "uid")); + selected.add(ocMap.get(ocName)); } + List namingAttrs = namingAttributeChoices(selected); String current = combo.getValue(); combo.getItems().setAll(namingAttrs); if (current != null && namingAttrs.contains(current)) { combo.setValue(current); } else if (!namingAttrs.isEmpty()) { - combo.setValue(namingAttrs.iterator().next()); + combo.setValue(namingAttrs.getFirst()); } } @@ -363,21 +356,77 @@ private void doCreate() { } } - // Build attribute map - Map> attributes = new LinkedHashMap<>(); + Map fieldValues = new LinkedHashMap<>(); for (var entry : attributeFields.entrySet()) { - String value = entry.getValue().getText().trim(); - if (!value.isEmpty()) { - attributes.put(entry.getKey(), List.of(value)); - } + fieldValues.put(entry.getKey(), entry.getValue().getText()); } + String rdnAttr = rdnAttrCombo.getValue(); + String rdnValue = rdnField.getText(); + Map> attributes = buildCreateAttributes(fieldValues, rdnAttr, rdnValue); + List objectClasses = new ArrayList<>(selectedClasses); controller.createEntry(parentDN, rdn, objectClasses, attributes); close(); } + /** + * Builds the LDAP Add attribute map from MUST field values plus the RDN. + * The RDN attribute is always included even when it is MAY (not a MUST + * field) — Active Directory requires the naming attribute on Add. + * If the RDN attribute is already present (case-insensitive), the field + * value is kept and it is not duplicated. + */ + static Map> buildCreateAttributes(Map fieldValues, + String rdnAttr, String rdnValue) { + Map> attributes = new LinkedHashMap<>(); + if (fieldValues != null) { + for (var entry : fieldValues.entrySet()) { + if (entry.getKey() == null || entry.getKey().isBlank()) continue; + String value = entry.getValue() == null ? "" : entry.getValue().trim(); + if (!value.isEmpty()) { + attributes.put(entry.getKey(), List.of(value)); + } + } + } + + String attr = (rdnAttr == null || rdnAttr.isBlank()) ? "cn" : rdnAttr.trim(); + String value = rdnValue == null ? "" : rdnValue.trim(); + if (!value.isEmpty() && !containsAttrIgnoreCase(attributes, attr)) { + attributes.put(attr, List.of(value)); + } + return attributes; + } + + /** + * RDN combo choices for the selected objectClasses: schema naming + * attributes (X-NDS_NAMING / AD rDNAttID) in encounter order — so the + * class's naming attribute is first — then the conventional cn/ou/o/c/dc/uid + * fallback when the schema has none. + */ + static List namingAttributeChoices(Iterable selectedClassInfos) { + Set namingAttrs = new LinkedHashSet<>(); + if (selectedClassInfos != null) { + for (LDAPObjectClassInfo info : selectedClassInfos) { + if (info != null && info.getNamingAttributes() != null) { + namingAttrs.addAll(info.getNamingAttributes()); + } + } + } + if (namingAttrs.isEmpty()) { + namingAttrs.addAll(List.of("cn", "ou", "o", "c", "dc", "uid")); + } + return new ArrayList<>(namingAttrs); + } + + private static boolean containsAttrIgnoreCase(Map attributes, String attr) { + for (String key : attributes.keySet()) { + if (key.equalsIgnoreCase(attr)) return true; + } + return false; + } + private void showAlert(Alert.AlertType type, String title, String message) { Alert alert = new Alert(type); alert.setTitle(title); diff --git a/src/test/java/com/pointbluetech/arborj/service/SchemaServiceTest.java b/src/test/java/com/pointbluetech/arborj/service/SchemaServiceTest.java new file mode 100644 index 0000000..e317eec --- /dev/null +++ b/src/test/java/com/pointbluetech/arborj/service/SchemaServiceTest.java @@ -0,0 +1,166 @@ +package com.pointbluetech.arborj.service; + +import com.pointbluetech.arborj.model.LDAPEntry; +import com.pointbluetech.arborj.model.LDAPObjectClassInfo; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; + +import static org.junit.jupiter.api.Assertions.*; + +class SchemaServiceTest { + + @Test + @DisplayName("collectRDNAttIDs maps classSchema lDAPDisplayName to rDNAttID") + void collectRdnAttIdsFromClassSchema() { + LDAPEntry group = classSchema("group", "cn"); + LDAPEntry ou = classSchema("organizationalUnit", "ou"); + LDAPEntry noRdn = classSchema("top", null); + + Map ids = SchemaService.collectRDNAttIDs(List.of(group, ou, noRdn)); + + assertEquals("cn", ids.get("group")); + assertEquals("ou", ids.get("organizationalunit")); + assertFalse(ids.containsKey("top")); + assertEquals(2, ids.size()); + } + + @Test + @DisplayName("collectRDNAttIDs lowercases names and skips blank / OID values") + void collectRdnAttIdsNormalizesAndSkipsOids() { + LDAPEntry mixed = classSchema("Group", "CN"); + LDAPEntry oid = classSchema("person", "2.5.4.3"); + LDAPEntry blank = classSchema("computer", " "); + + Map ids = SchemaService.collectRDNAttIDs(List.of(mixed, oid, blank)); + + assertEquals(Map.of("group", "cn"), ids); + } + + @Test + @DisplayName("collectRDNAttIDs is empty for null or empty input") + void collectRdnAttIdsEmptyInput() { + assertTrue(SchemaService.collectRDNAttIDs(null).isEmpty()); + assertTrue(SchemaService.collectRDNAttIDs(List.of()).isEmpty()); + } + + @Test + @DisplayName("looksLikeOid detects numeric dotted OIDs only") + void looksLikeOid() { + assertTrue(SchemaService.looksLikeOid("2.5.4.3")); + assertTrue(SchemaService.looksLikeOid("1.2.840.113556.1.4.750")); + assertFalse(SchemaService.looksLikeOid("cn")); + assertFalse(SchemaService.looksLikeOid("ou")); + assertFalse(SchemaService.looksLikeOid("123")); + assertFalse(SchemaService.looksLikeOid("")); + assertFalse(SchemaService.looksLikeOid(null)); + } + + @Test + @DisplayName("applyRDNAttIDs sets group naming to cn when schema had none") + void applyRdnAttIdWhenNamingMissing() { + Map map = new LinkedHashMap<>(); + map.put("group", oc("group", "top", List.of("grouptype"), List.of("cn"), null)); + map.put("top", oc("top", null, List.of("objectclass"), List.of("cn"), null)); + + SchemaService.applyRDNAttIDs(map, Map.of("group", "cn")); + + assertEquals(List.of("cn"), map.get("group").getNamingAttributes()); + assertNull(map.get("top").getNamingAttributes()); + } + + @Test + @DisplayName("applyRDNAttIDs puts rDNAttID first alongside existing X-NDS_NAMING") + void applyRdnAttIdPrependsExistingNaming() { + Map map = new LinkedHashMap<>(); + map.put("user", oc("user", "top", List.of("cn"), List.of(), List.of("cn", "uid"))); + + SchemaService.applyRDNAttIDs(map, Map.of("user", "cn")); + + assertEquals(List.of("cn", "uid"), map.get("user").getNamingAttributes()); + } + + @Test + @DisplayName("applyRDNAttIDs prepends a distinct rDNAttID ahead of X-NDS_NAMING") + void applyRdnAttIdAlongsideNdsNaming() { + Map map = new LinkedHashMap<>(); + map.put("org", oc("org", "top", List.of(), List.of(), List.of("o"))); + + SchemaService.applyRDNAttIDs(map, Map.of("org", "cn")); + + assertEquals(List.of("cn", "o"), map.get("org").getNamingAttributes()); + } + + @Test + @DisplayName("applyRDNAttIDs inherits rDNAttID from the SUP chain") + void applyRdnAttIdInheritsFromSuperclass() { + Map map = new LinkedHashMap<>(); + map.put("group", oc("group", "top", List.of("grouptype"), List.of("cn"), null)); + map.put("top", oc("top", null, List.of("objectclass"), List.of("cn"), null)); + + SchemaService.applyRDNAttIDs(map, Map.of("top", "cn")); + + assertEquals(List.of("cn"), map.get("group").getNamingAttributes()); + assertEquals(List.of("cn"), map.get("top").getNamingAttributes()); + } + + @Test + @DisplayName("applyRDNAttIDs prefers the class's own rDNAttID over the superclass") + void applyRdnAttIdPrefersDirectOverInherited() { + Map map = new LinkedHashMap<>(); + map.put("organizationalunit", oc("organizationalUnit", "top", + List.of("ou"), List.of(), null)); + map.put("top", oc("top", null, List.of(), List.of(), null)); + + SchemaService.applyRDNAttIDs(map, Map.of( + "top", "cn", + "organizationalunit", "ou")); + + assertEquals(List.of("ou"), map.get("organizationalunit").getNamingAttributes()); + assertEquals(List.of("cn"), map.get("top").getNamingAttributes()); + } + + @Test + @DisplayName("applyRDNAttIDs is a no-op when the rDNAttID map is empty") + void applyRdnAttIdNoOpWhenEmpty() { + Map map = new LinkedHashMap<>(); + LDAPObjectClassInfo original = oc("group", "top", List.of("grouptype"), List.of("cn"), null); + map.put("group", original); + + SchemaService.applyRDNAttIDs(map, Map.of()); + SchemaService.applyRDNAttIDs(map, null); + + assertSame(original, map.get("group")); + } + + @Test + @DisplayName("applyRDNAttIDs does not loop on a cyclic SUP chain") + void applyRdnAttIdCyclicSup() { + Map map = new LinkedHashMap<>(); + map.put("a", oc("a", "b", List.of(), List.of(), null)); + map.put("b", oc("b", "a", List.of(), List.of(), null)); + + SchemaService.applyRDNAttIDs(map, Map.of("c", "cn")); + + assertNull(map.get("a").getNamingAttributes()); + assertNull(map.get("b").getNamingAttributes()); + } + + private static LDAPEntry classSchema(String displayName, String rdnAttId) { + Map> attrs = new LinkedHashMap<>(); + attrs.put("lDAPDisplayName", List.of(displayName)); + if (rdnAttId != null) { + attrs.put("rDNAttID", List.of(rdnAttId)); + } + return new LDAPEntry("CN=" + displayName + ",CN=Schema,CN=Configuration,DC=lab", attrs); + } + + private static LDAPObjectClassInfo oc(String name, String sup, + List must, List may, + List naming) { + return new LDAPObjectClassInfo(name, must, may, sup, naming); + } +} diff --git a/src/test/java/com/pointbluetech/arborj/view/NewEntryDialogTest.java b/src/test/java/com/pointbluetech/arborj/view/NewEntryDialogTest.java new file mode 100644 index 0000000..ab129b9 --- /dev/null +++ b/src/test/java/com/pointbluetech/arborj/view/NewEntryDialogTest.java @@ -0,0 +1,164 @@ +package com.pointbluetech.arborj.view; + +import com.pointbluetech.arborj.model.LDAPObjectClassInfo; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; + +import static org.junit.jupiter.api.Assertions.*; + +/** + * Pure-logic tests for New Entry create-payload and RDN combo helpers. + * Does not construct a Dialog, so no JavaFX runtime is required. + */ +class NewEntryDialogTest { + + @Test + @DisplayName("create payload includes MAY RDN (cn) plus MUST groupType") + void createPayloadIncludesMayRdnAndMust() { + Map fields = new LinkedHashMap<>(); + fields.put("groupType", "-2147483646"); + + Map> attrs = + NewEntryDialog.buildCreateAttributes(fields, "cn", "gr2"); + + assertEquals(List.of("gr2"), attrs.get("cn")); + assertEquals(List.of("-2147483646"), attrs.get("groupType")); + assertEquals(2, attrs.size()); + } + + @Test + @DisplayName("create payload does not duplicate RDN when it is already a MUST field") + void createPayloadDoesNotDuplicateMustRdn() { + Map fields = new LinkedHashMap<>(); + fields.put("cn", "Alice"); + fields.put("sn", "Smith"); + + Map> attrs = + NewEntryDialog.buildCreateAttributes(fields, "cn", "Alice"); + + assertEquals(List.of("Alice"), attrs.get("cn")); + assertEquals(List.of("Smith"), attrs.get("sn")); + assertEquals(2, attrs.size()); + } + + @Test + @DisplayName("create payload treats CN and cn as the same attribute") + void createPayloadDedupesRdnCaseInsensitive() { + Map fields = new LinkedHashMap<>(); + fields.put("CN", "Alice"); + + Map> attrs = + NewEntryDialog.buildCreateAttributes(fields, "cn", "Alice"); + + assertEquals(List.of("Alice"), attrs.get("CN")); + assertFalse(attrs.containsKey("cn")); + assertEquals(1, attrs.size()); + } + + @Test + @DisplayName("create payload keeps the MUST field value when RDN casing differs") + void createPayloadKeepsExistingFieldValue() { + Map fields = new LinkedHashMap<>(); + fields.put("cn", "from-field"); + + Map> attrs = + NewEntryDialog.buildCreateAttributes(fields, "cn", "from-rdn"); + + assertEquals(List.of("from-field"), attrs.get("cn")); + } + + @Test + @DisplayName("create payload with no MUST fields still sends the RDN attribute") + void createPayloadRdnOnly() { + Map> attrs = + NewEntryDialog.buildCreateAttributes(Map.of(), "ou", "Sales"); + + assertEquals(Map.of("ou", List.of("Sales")), attrs); + } + + @Test + @DisplayName("blank RDN attribute name defaults to cn") + void createPayloadDefaultRdnAttrIsCn() { + Map> attrs = + NewEntryDialog.buildCreateAttributes(Map.of(), " ", "gr2"); + + assertEquals(List.of("gr2"), attrs.get("cn")); + } + + @Test + @DisplayName("empty MUST field values are omitted") + void createPayloadOmitsEmptyFields() { + Map fields = new LinkedHashMap<>(); + fields.put("description", " "); + fields.put("groupType", "2"); + + Map> attrs = + NewEntryDialog.buildCreateAttributes(fields, "cn", "gr2"); + + assertFalse(attrs.containsKey("description")); + assertEquals(List.of("2"), attrs.get("groupType")); + assertEquals(List.of("gr2"), attrs.get("cn")); + } + + @Test + @DisplayName("null field map is treated as empty") + void createPayloadNullFields() { + Map> attrs = + NewEntryDialog.buildCreateAttributes(null, "cn", "gr2"); + + assertEquals(Map.of("cn", List.of("gr2")), attrs); + } + + @Test + @DisplayName("empty RDN value does not add a blank attribute") + void createPayloadSkipsEmptyRdnValue() { + Map fields = new LinkedHashMap<>(); + fields.put("groupType", "2"); + + Map> attrs = + NewEntryDialog.buildCreateAttributes(fields, "cn", " "); + + assertEquals(Map.of("groupType", List.of("2")), attrs); + } + + @Test + @DisplayName("schema naming attributes (rDNAttID) come first in the combo") + void namingChoicesPreferSchemaRdnAttId() { + LDAPObjectClassInfo group = new LDAPObjectClassInfo( + "group", List.of("grouptype"), List.of("cn", "member"), + "top", List.of("cn")); + + List choices = NewEntryDialog.namingAttributeChoices(List.of(group)); + + assertEquals(List.of("cn"), choices); + assertFalse(choices.contains("grouptype")); + } + + @Test + @DisplayName("fallback cn/ou/o/c/dc/uid is used when schema has no naming attributes") + void namingChoicesFallbackWhenSchemaEmpty() { + LDAPObjectClassInfo group = new LDAPObjectClassInfo( + "group", List.of("grouptype"), List.of("cn"), "top", null); + + assertEquals(List.of("cn", "ou", "o", "c", "dc", "uid"), + NewEntryDialog.namingAttributeChoices(List.of(group))); + assertEquals(List.of("cn", "ou", "o", "c", "dc", "uid"), + NewEntryDialog.namingAttributeChoices(List.of())); + } + + @Test + @DisplayName("person MUST cn still yields cn (regression)") + void namingChoicesPersonAndGroup() { + LDAPObjectClassInfo person = new LDAPObjectClassInfo( + "person", List.of("cn", "sn"), List.of(), "top", List.of("cn")); + LDAPObjectClassInfo group = new LDAPObjectClassInfo( + "group", List.of("grouptype"), List.of("cn"), "top", List.of("cn")); + + assertEquals(List.of("cn"), + NewEntryDialog.namingAttributeChoices(List.of(person, group))); + } +} From 529e86c34df19b78b0c243db1378819d5f8b4c34 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Fri, 30 Oct 2026 14:08:14 +0000 Subject: [PATCH 2/4] Escape RDN values in the DN and send the raw value on Add Treat the New Entry RDN field as the unescaped attribute value. formatRDN uses UnboundID RDN (RFC 4514) so comma, plus, spaces, hash, and backslash are escaped in the DN; buildCreateAttributes sends the same raw text. Plus is a value character, not a multi-valued RDN. A MUST field that disagrees with the RDN is rejected, and namingEquals is now Objects.equals. Co-authored-by: Jerry Combs --- .../arborj/service/SchemaService.java | 8 +- .../arborj/view/NewEntryDialog.java | 81 +++++++++++++------ .../arborj/view/NewEntryDialogTest.java | 76 +++++++++++++++-- 3 files changed, 128 insertions(+), 37 deletions(-) diff --git a/src/main/java/com/pointbluetech/arborj/service/SchemaService.java b/src/main/java/com/pointbluetech/arborj/service/SchemaService.java index 6cf6ff1..abbdde3 100644 --- a/src/main/java/com/pointbluetech/arborj/service/SchemaService.java +++ b/src/main/java/com/pointbluetech/arborj/service/SchemaService.java @@ -448,7 +448,7 @@ static void applyRDNAttIDs(Map objectClassMap, String rdn = resolveRDNAttID(className, objectClassMap, rdnAttIds, new HashSet<>()); if (rdn == null || rdn.isBlank()) continue; List naming = mergeNamingAttribute(info.getNamingAttributes(), rdn); - if (namingEquals(info.getNamingAttributes(), naming)) continue; + if (Objects.equals(info.getNamingAttributes(), naming)) continue; objectClassMap.put(className, new LDAPObjectClassInfo( info.getName(), info.getMustAttributes(), @@ -487,12 +487,6 @@ private static List mergeNamingAttribute(List existing, String r return new ArrayList<>(merged.values()); } - private static boolean namingEquals(List a, List b) { - if (a == b) return true; - if (a == null || b == null) return false; - return a.equals(b); - } - /** True for values like {@code 2.5.4.3} (digits and dots only, with at least one of each). */ static boolean looksLikeOid(String value) { if (value == null || value.isEmpty()) return false; diff --git a/src/main/java/com/pointbluetech/arborj/view/NewEntryDialog.java b/src/main/java/com/pointbluetech/arborj/view/NewEntryDialog.java index e47fd4f..8bb60d8 100644 --- a/src/main/java/com/pointbluetech/arborj/view/NewEntryDialog.java +++ b/src/main/java/com/pointbluetech/arborj/view/NewEntryDialog.java @@ -2,6 +2,7 @@ import com.pointbluetech.arborj.controller.MainController; import com.pointbluetech.arborj.model.LDAPObjectClassInfo; +import com.unboundid.ldap.sdk.RDN; import javafx.collections.FXCollections; import javafx.collections.ObservableList; import javafx.collections.transformation.FilteredList; @@ -228,9 +229,9 @@ private void populateAttributeFields() { mustAttrsByLower.remove("objectclass"); Collection mustAttrs = mustAttrsByLower.values(); - // Parse RDN attribute name and value + // Prefill the RDN attribute from the (unescaped) RDN field value String rdnAttr = rdnAttrCombo.getValue(); - String rdnValue = rdnField.getText().trim(); + String rdnValue = rdnField.getText() == null ? "" : rdnField.getText(); if (rdnAttr == null || rdnAttr.isBlank()) rdnAttr = "cn"; Font mono = Font.font("monospaced", 12); @@ -319,15 +320,12 @@ private void updateNamingAttributes(ComboBox combo) { } private String getRDN() { - String attr = rdnAttrCombo.getValue(); - String value = rdnField.getText().trim(); - if (attr == null || attr.isBlank()) attr = "cn"; - return attr + "=" + value; + return formatRDN(rdnAttrCombo.getValue(), rdnField.getText()); } private boolean validateStep1() { - String value = rdnField.getText().trim(); - if (value.isEmpty()) { + String value = rdnField.getText(); + if (value == null || value.isBlank()) { showAlert(Alert.AlertType.WARNING, "Validation", "Please enter an RDN value."); return false; @@ -343,6 +341,8 @@ private boolean validateStep1() { // ---- Create entry ---- private void doCreate() { + String rdnAttr = rdnAttrCombo.getValue(); + String rdnValue = rdnField.getText() == null ? "" : rdnField.getText(); String rdn = getRDN(); // Validate required fields are filled @@ -361,8 +361,15 @@ private void doCreate() { fieldValues.put(entry.getKey(), entry.getValue().getText()); } - String rdnAttr = rdnAttrCombo.getValue(); - String rdnValue = rdnField.getText(); + String conflict = rdnFieldConflict(fieldValues, rdnAttr, rdnValue); + if (conflict != null) { + showAlert(Alert.AlertType.WARNING, "Validation", + "Required attribute '" + conflict + "' must match the RDN value."); + TextField field = attributeFields.get(conflict); + if (field != null) field.requestFocus(); + return; + } + Map> attributes = buildCreateAttributes(fieldValues, rdnAttr, rdnValue); List objectClasses = new ArrayList<>(selectedClasses); @@ -371,34 +378,65 @@ private void doCreate() { close(); } + /** + * Builds {@code attributeType=} plus an RFC 4514-escaped value. The RDN + * text field is the unescaped attribute value; {@code +} is escaped as + * part of that value (multi-valued RDNs are not supported). + */ + static String formatRDN(String attr, String unescapedValue) { + if (attr == null || attr.isBlank()) attr = "cn"; + else attr = attr.trim(); + String value = unescapedValue == null ? "" : unescapedValue; + return new RDN(attr, value).toString(); + } + /** * Builds the LDAP Add attribute map from MUST field values plus the RDN. * The RDN attribute is always included even when it is MAY (not a MUST * field) — Active Directory requires the naming attribute on Add. - * If the RDN attribute is already present (case-insensitive), the field - * value is kept and it is not duplicated. + * The Add value is the unescaped RDN text (not the DN-escaped form). + * If the RDN attribute is already a MUST field, the RDN value wins so + * the Add cannot disagree with the DN. */ static Map> buildCreateAttributes(Map fieldValues, String rdnAttr, String rdnValue) { + String attr = (rdnAttr == null || rdnAttr.isBlank()) ? "cn" : rdnAttr.trim(); + String value = rdnValue == null ? "" : rdnValue; + Map> attributes = new LinkedHashMap<>(); if (fieldValues != null) { for (var entry : fieldValues.entrySet()) { if (entry.getKey() == null || entry.getKey().isBlank()) continue; - String value = entry.getValue() == null ? "" : entry.getValue().trim(); - if (!value.isEmpty()) { - attributes.put(entry.getKey(), List.of(value)); + if (entry.getKey().equalsIgnoreCase(attr)) continue; + String fieldValue = entry.getValue() == null ? "" : entry.getValue().trim(); + if (!fieldValue.isEmpty()) { + attributes.put(entry.getKey(), List.of(fieldValue)); } } } - String attr = (rdnAttr == null || rdnAttr.isBlank()) ? "cn" : rdnAttr.trim(); - String value = rdnValue == null ? "" : rdnValue.trim(); - if (!value.isEmpty() && !containsAttrIgnoreCase(attributes, attr)) { + if (!value.isBlank()) { attributes.put(attr, List.of(value)); } return attributes; } + /** + * Returns the MUST field name when it is the RDN attribute but holds a + * different value, or {@code null} when they agree or the RDN is not a MUST. + */ + static String rdnFieldConflict(Map fieldValues, String rdnAttr, String rdnValue) { + if (fieldValues == null) return null; + String attr = (rdnAttr == null || rdnAttr.isBlank()) ? "cn" : rdnAttr.trim(); + String expected = rdnValue == null ? "" : rdnValue; + for (var entry : fieldValues.entrySet()) { + if (entry.getKey() == null || !entry.getKey().equalsIgnoreCase(attr)) continue; + String fieldValue = entry.getValue() == null ? "" : entry.getValue(); + return fieldValue.equals(expected) ? null : entry.getKey(); + } + return null; + } + /** * RDN combo choices for the selected objectClasses: schema naming * attributes (X-NDS_NAMING / AD rDNAttID) in encounter order — so the @@ -420,13 +458,6 @@ static List namingAttributeChoices(Iterable selecte return new ArrayList<>(namingAttrs); } - private static boolean containsAttrIgnoreCase(Map attributes, String attr) { - for (String key : attributes.keySet()) { - if (key.equalsIgnoreCase(attr)) return true; - } - return false; - } - private void showAlert(Alert.AlertType type, String title, String message) { Alert alert = new Alert(type); alert.setTitle(title); diff --git a/src/test/java/com/pointbluetech/arborj/view/NewEntryDialogTest.java b/src/test/java/com/pointbluetech/arborj/view/NewEntryDialogTest.java index ab129b9..de52227 100644 --- a/src/test/java/com/pointbluetech/arborj/view/NewEntryDialogTest.java +++ b/src/test/java/com/pointbluetech/arborj/view/NewEntryDialogTest.java @@ -54,21 +54,38 @@ void createPayloadDedupesRdnCaseInsensitive() { Map> attrs = NewEntryDialog.buildCreateAttributes(fields, "cn", "Alice"); - assertEquals(List.of("Alice"), attrs.get("CN")); - assertFalse(attrs.containsKey("cn")); + assertEquals(List.of("Alice"), attrs.get("cn")); + assertFalse(attrs.containsKey("CN")); assertEquals(1, attrs.size()); } @Test - @DisplayName("create payload keeps the MUST field value when RDN casing differs") - void createPayloadKeepsExistingFieldValue() { + @DisplayName("create payload uses the RDN value when a MUST field disagrees") + void createPayloadRdnValueWinsOverMustField() { Map fields = new LinkedHashMap<>(); fields.put("cn", "from-field"); Map> attrs = NewEntryDialog.buildCreateAttributes(fields, "cn", "from-rdn"); - assertEquals(List.of("from-field"), attrs.get("cn")); + assertEquals(List.of("from-rdn"), attrs.get("cn")); + } + + @Test + @DisplayName("rdnFieldConflict is null when MUST matches RDN or RDN is not MUST") + void rdnFieldConflictNone() { + assertNull(NewEntryDialog.rdnFieldConflict(Map.of("cn", "Alice"), "cn", "Alice")); + assertNull(NewEntryDialog.rdnFieldConflict(Map.of("groupType", "2"), "cn", "gr2")); + assertNull(NewEntryDialog.rdnFieldConflict(null, "cn", "gr2")); + } + + @Test + @DisplayName("rdnFieldConflict returns the MUST field name when it disagrees with the RDN") + void rdnFieldConflictWhenDifferent() { + assertEquals("cn", NewEntryDialog.rdnFieldConflict( + Map.of("cn", "from-field"), "cn", "from-rdn")); + assertEquals("CN", NewEntryDialog.rdnFieldConflict( + Map.of("CN", "x"), "cn", "y")); } @Test @@ -161,4 +178,53 @@ void namingChoicesPersonAndGroup() { assertEquals(List.of("cn"), NewEntryDialog.namingAttributeChoices(List.of(person, group))); } + + @Test + @DisplayName("comma is escaped in the DN and sent raw in the Add") + void rdnEscapesComma() { + assertEscapedRdn("Smith, John", "cn=Smith\\, John"); + } + + @Test + @DisplayName("plus is escaped as a value character, not a multi-valued RDN") + void rdnEscapesPlus() { + assertEscapedRdn("Smith+John", "cn=Smith\\+John"); + } + + @Test + @DisplayName("leading space is escaped in the DN and preserved in the Add") + void rdnEscapesLeadingSpace() { + assertEscapedRdn(" Alice", "cn=\\ Alice"); + } + + @Test + @DisplayName("trailing space is escaped in the DN and preserved in the Add") + void rdnEscapesTrailingSpace() { + assertEscapedRdn("Alice ", "cn=Alice\\ "); + } + + @Test + @DisplayName("leading hash is escaped in the DN and sent raw in the Add") + void rdnEscapesLeadingHash() { + assertEscapedRdn("#hash", "cn=\\#hash"); + } + + @Test + @DisplayName("backslash is escaped in the DN and sent raw in the Add") + void rdnEscapesBackslash() { + assertEscapedRdn("foo\\bar", "cn=foo\\\\bar"); + } + + @Test + @DisplayName("plain RDN value is not escaped") + void rdnPlainValueUnchanged() { + assertEscapedRdn("gr2", "cn=gr2"); + } + + /** DN uses RFC 4514 escapes; Add attributes keep the unescaped field text. */ + private static void assertEscapedRdn(String raw, String expectedRdn) { + assertEquals(expectedRdn, NewEntryDialog.formatRDN("cn", raw)); + assertEquals(List.of(raw), + NewEntryDialog.buildCreateAttributes(Map.of(), "cn", raw).get("cn")); + } } From 16240bdd7fc0f2791575ec27ec2bb1d045790008 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Fri, 30 Oct 2026 14:11:19 +0000 Subject: [PATCH 3/4] Trim the RDN value and lock the prefilled MUST field MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit validateStep1 trims the RDN text and writes it back so Create, the DN, and the Add all use the same value (" Bob " → cn=Bob). The Step 2 MUST field for that attribute is read-only, matching Arbor, so the conflict reject path is gone. The editable RDN attribute combo is checked against an LDAP descr or numeric OID. Co-authored-by: Jerry Combs --- .../arborj/view/NewEntryDialog.java | 60 +++++++++++-------- .../arborj/view/NewEntryDialogTest.java | 55 +++++++++-------- 2 files changed, 63 insertions(+), 52 deletions(-) diff --git a/src/main/java/com/pointbluetech/arborj/view/NewEntryDialog.java b/src/main/java/com/pointbluetech/arborj/view/NewEntryDialog.java index 8bb60d8..029b0d6 100644 --- a/src/main/java/com/pointbluetech/arborj/view/NewEntryDialog.java +++ b/src/main/java/com/pointbluetech/arborj/view/NewEntryDialog.java @@ -13,6 +13,7 @@ import javafx.scene.text.Font; import java.util.*; +import java.util.regex.Pattern; /** * Two-step wizard dialog for creating a new LDAP entry. @@ -52,6 +53,11 @@ public class NewEntryDialog extends Dialog { private int currentStep = 1; + /** RFC 4512 descr: ALPHA *( ALPHA / DIGIT / HYPHEN ). */ + private static final Pattern ATTR_DESCR = Pattern.compile("[A-Za-z][A-Za-z0-9-]*"); + /** RFC 4512 numericoid: number *( DOT number ). */ + private static final Pattern ATTR_OID = Pattern.compile("[0-9]+(?:\\.[0-9]+)*"); + public NewEntryDialog(MainController controller, String parentDN) { this(controller, parentDN, null); } @@ -244,9 +250,11 @@ private void populateAttributeFields() { TextField field = new TextField(); field.setFont(mono); - // Pre-populate the RDN attribute + // Prefill the RDN attribute from Step 1 and keep it read-only so + // the Add cannot disagree with the DN (Arbor does the same). if (attr.equalsIgnoreCase(rdnAttr)) { field.setText(rdnValue != null ? rdnValue : ""); + field.setEditable(false); } attributeGrid.add(label, 0, row); @@ -324,12 +332,27 @@ private String getRDN() { } private boolean validateStep1() { - String value = rdnField.getText(); - if (value == null || value.isBlank()) { + String value = rdnField.getText() == null ? "" : rdnField.getText().trim(); + if (value.isEmpty()) { showAlert(Alert.AlertType.WARNING, "Validation", "Please enter an RDN value."); return false; } + rdnField.setText(value); + + String attr = rdnAttrCombo.getValue(); + if (attr == null || attr.isBlank()) { + rdnAttrCombo.setValue("cn"); + } else { + attr = attr.trim(); + if (!isValidAttributeType(attr)) { + showAlert(Alert.AlertType.WARNING, "Validation", + "RDN attribute must be an LDAP name such as cn, or a numeric OID such as 2.5.4.3."); + return false; + } + rdnAttrCombo.setValue(attr); + } + if (selectedClasses.isEmpty()) { showAlert(Alert.AlertType.WARNING, "Validation", "Please select at least one objectClass."); @@ -361,15 +384,6 @@ private void doCreate() { fieldValues.put(entry.getKey(), entry.getValue().getText()); } - String conflict = rdnFieldConflict(fieldValues, rdnAttr, rdnValue); - if (conflict != null) { - showAlert(Alert.AlertType.WARNING, "Validation", - "Required attribute '" + conflict + "' must match the RDN value."); - TextField field = attributeFields.get(conflict); - if (field != null) field.requestFocus(); - return; - } - Map> attributes = buildCreateAttributes(fieldValues, rdnAttr, rdnValue); List objectClasses = new ArrayList<>(selectedClasses); @@ -386,7 +400,7 @@ private void doCreate() { static String formatRDN(String attr, String unescapedValue) { if (attr == null || attr.isBlank()) attr = "cn"; else attr = attr.trim(); - String value = unescapedValue == null ? "" : unescapedValue; + String value = unescapedValue == null ? "" : unescapedValue.trim(); return new RDN(attr, value).toString(); } @@ -401,7 +415,7 @@ static String formatRDN(String attr, String unescapedValue) { static Map> buildCreateAttributes(Map fieldValues, String rdnAttr, String rdnValue) { String attr = (rdnAttr == null || rdnAttr.isBlank()) ? "cn" : rdnAttr.trim(); - String value = rdnValue == null ? "" : rdnValue; + String value = rdnValue == null ? "" : rdnValue.trim(); Map> attributes = new LinkedHashMap<>(); if (fieldValues != null) { @@ -422,19 +436,13 @@ static Map> buildCreateAttributes(Map field } /** - * Returns the MUST field name when it is the RDN attribute but holds a - * different value, or {@code null} when they agree or the RDN is not a MUST. + * True for an RFC 4512 attribute descr ({@code cn}, {@code telephoneNumber}) + * or a numeric OID ({@code 2.5.4.3}). */ - static String rdnFieldConflict(Map fieldValues, String rdnAttr, String rdnValue) { - if (fieldValues == null) return null; - String attr = (rdnAttr == null || rdnAttr.isBlank()) ? "cn" : rdnAttr.trim(); - String expected = rdnValue == null ? "" : rdnValue; - for (var entry : fieldValues.entrySet()) { - if (entry.getKey() == null || !entry.getKey().equalsIgnoreCase(attr)) continue; - String fieldValue = entry.getValue() == null ? "" : entry.getValue(); - return fieldValue.equals(expected) ? null : entry.getKey(); - } - return null; + static boolean isValidAttributeType(String attr) { + if (attr == null || attr.isBlank()) return false; + String trimmed = attr.trim(); + return ATTR_DESCR.matcher(trimmed).matches() || ATTR_OID.matcher(trimmed).matches(); } /** diff --git a/src/test/java/com/pointbluetech/arborj/view/NewEntryDialogTest.java b/src/test/java/com/pointbluetech/arborj/view/NewEntryDialogTest.java index de52227..e95444a 100644 --- a/src/test/java/com/pointbluetech/arborj/view/NewEntryDialogTest.java +++ b/src/test/java/com/pointbluetech/arborj/view/NewEntryDialogTest.java @@ -71,23 +71,6 @@ void createPayloadRdnValueWinsOverMustField() { assertEquals(List.of("from-rdn"), attrs.get("cn")); } - @Test - @DisplayName("rdnFieldConflict is null when MUST matches RDN or RDN is not MUST") - void rdnFieldConflictNone() { - assertNull(NewEntryDialog.rdnFieldConflict(Map.of("cn", "Alice"), "cn", "Alice")); - assertNull(NewEntryDialog.rdnFieldConflict(Map.of("groupType", "2"), "cn", "gr2")); - assertNull(NewEntryDialog.rdnFieldConflict(null, "cn", "gr2")); - } - - @Test - @DisplayName("rdnFieldConflict returns the MUST field name when it disagrees with the RDN") - void rdnFieldConflictWhenDifferent() { - assertEquals("cn", NewEntryDialog.rdnFieldConflict( - Map.of("cn", "from-field"), "cn", "from-rdn")); - assertEquals("CN", NewEntryDialog.rdnFieldConflict( - Map.of("CN", "x"), "cn", "y")); - } - @Test @DisplayName("create payload with no MUST fields still sends the RDN attribute") void createPayloadRdnOnly() { @@ -192,15 +175,11 @@ void rdnEscapesPlus() { } @Test - @DisplayName("leading space is escaped in the DN and preserved in the Add") - void rdnEscapesLeadingSpace() { - assertEscapedRdn(" Alice", "cn=\\ Alice"); - } - - @Test - @DisplayName("trailing space is escaped in the DN and preserved in the Add") - void rdnEscapesTrailingSpace() { - assertEscapedRdn("Alice ", "cn=Alice\\ "); + @DisplayName("padded RDN value is trimmed: \" Bob \" becomes DN cn=Bob and Add Bob") + void rdnValueIsTrimmed() { + assertEquals("cn=Bob", NewEntryDialog.formatRDN("cn", " Bob ")); + assertEquals(List.of("Bob"), + NewEntryDialog.buildCreateAttributes(Map.of(), "cn", " Bob ").get("cn")); } @Test @@ -221,6 +200,30 @@ void rdnPlainValueUnchanged() { assertEscapedRdn("gr2", "cn=gr2"); } + @Test + @DisplayName("attribute type accepts LDAP descr and numeric OID") + void validAttributeTypes() { + assertTrue(NewEntryDialog.isValidAttributeType("cn")); + assertTrue(NewEntryDialog.isValidAttributeType("telephoneNumber")); + assertTrue(NewEntryDialog.isValidAttributeType("CN")); + assertTrue(NewEntryDialog.isValidAttributeType(" ou ")); + assertTrue(NewEntryDialog.isValidAttributeType("2.5.4.3")); + assertTrue(NewEntryDialog.isValidAttributeType("0.9.2342.19200300.100.1.1")); + } + + @Test + @DisplayName("attribute type rejects empty, underscore, and junk") + void invalidAttributeTypes() { + assertFalse(NewEntryDialog.isValidAttributeType(null)); + assertFalse(NewEntryDialog.isValidAttributeType("")); + assertFalse(NewEntryDialog.isValidAttributeType(" ")); + assertFalse(NewEntryDialog.isValidAttributeType("cn=Bob")); + assertFalse(NewEntryDialog.isValidAttributeType("_cn")); + assertFalse(NewEntryDialog.isValidAttributeType("cn_name")); + assertFalse(NewEntryDialog.isValidAttributeType("1cn")); + assertFalse(NewEntryDialog.isValidAttributeType(".2.5.4.3")); + } + /** DN uses RFC 4514 escapes; Add attributes keep the unescaped field text. */ private static void assertEscapedRdn(String raw, String expectedRdn) { assertEquals(expectedRdn, NewEntryDialog.formatRDN("cn", raw)); From 1835a7c50aaf5c521433e131419b6a5ad4d15274 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Fri, 30 Oct 2026 14:41:15 +0000 Subject: [PATCH 4/4] Use the RDN combo editor text as the attribute source of truth JavaFX ComboBox.getValue() can lag typed text until Enter or focus loss, so typing ou and clicking Next could still emit cn. Resolve from the editor first, fall back to getValue, and use that one value for validation, the DN, and the Add. Co-authored-by: Jerry Combs --- .../arborj/view/NewEntryDialog.java | 55 ++++++++++++++----- .../arborj/view/NewEntryDialogTest.java | 23 ++++++++ 2 files changed, 63 insertions(+), 15 deletions(-) diff --git a/src/main/java/com/pointbluetech/arborj/view/NewEntryDialog.java b/src/main/java/com/pointbluetech/arborj/view/NewEntryDialog.java index 029b0d6..9fac978 100644 --- a/src/main/java/com/pointbluetech/arborj/view/NewEntryDialog.java +++ b/src/main/java/com/pointbluetech/arborj/view/NewEntryDialog.java @@ -236,9 +236,8 @@ private void populateAttributeFields() { Collection mustAttrs = mustAttrsByLower.values(); // Prefill the RDN attribute from the (unescaped) RDN field value - String rdnAttr = rdnAttrCombo.getValue(); + String rdnAttr = resolvedRdnAttr(); String rdnValue = rdnField.getText() == null ? "" : rdnField.getText(); - if (rdnAttr == null || rdnAttr.isBlank()) rdnAttr = "cn"; Font mono = Font.font("monospaced", 12); int row = 0; @@ -328,7 +327,38 @@ private void updateNamingAttributes(ComboBox combo) { } private String getRDN() { - return formatRDN(rdnAttrCombo.getValue(), rdnField.getText()); + return formatRDN(resolvedRdnAttr(), rdnField.getText()); + } + + /** + * Editor text is the source of truth for the editable RDN combo: + * {@link ComboBox#getValue()} can lag typed text until Enter or focus loss. + */ + private String resolvedRdnAttr() { + String editorText = null; + if (rdnAttrCombo != null && rdnAttrCombo.isEditable()) { + TextField editor = rdnAttrCombo.getEditor(); + if (editor != null) editorText = editor.getText(); + } + String value = rdnAttrCombo == null ? null : rdnAttrCombo.getValue(); + String resolved = resolveComboText(editorText, value); + return resolved.isEmpty() ? "cn" : resolved; + } + + /** + * Prefers trimmed editor text over a stale committed combo value. + * Returns empty when both are blank (callers default to {@code cn}). + */ + static String resolveComboText(String editorText, String value) { + if (editorText != null) { + String trimmed = editorText.trim(); + if (!trimmed.isEmpty()) return trimmed; + } + if (value != null) { + String trimmed = value.trim(); + if (!trimmed.isEmpty()) return trimmed; + } + return ""; } private boolean validateStep1() { @@ -340,18 +370,13 @@ private boolean validateStep1() { } rdnField.setText(value); - String attr = rdnAttrCombo.getValue(); - if (attr == null || attr.isBlank()) { - rdnAttrCombo.setValue("cn"); - } else { - attr = attr.trim(); - if (!isValidAttributeType(attr)) { - showAlert(Alert.AlertType.WARNING, "Validation", - "RDN attribute must be an LDAP name such as cn, or a numeric OID such as 2.5.4.3."); - return false; - } - rdnAttrCombo.setValue(attr); + String attr = resolvedRdnAttr(); + if (!isValidAttributeType(attr)) { + showAlert(Alert.AlertType.WARNING, "Validation", + "RDN attribute must be an LDAP name such as cn, or a numeric OID such as 2.5.4.3."); + return false; } + rdnAttrCombo.setValue(attr); if (selectedClasses.isEmpty()) { showAlert(Alert.AlertType.WARNING, "Validation", @@ -364,7 +389,7 @@ private boolean validateStep1() { // ---- Create entry ---- private void doCreate() { - String rdnAttr = rdnAttrCombo.getValue(); + String rdnAttr = resolvedRdnAttr(); String rdnValue = rdnField.getText() == null ? "" : rdnField.getText(); String rdn = getRDN(); diff --git a/src/test/java/com/pointbluetech/arborj/view/NewEntryDialogTest.java b/src/test/java/com/pointbluetech/arborj/view/NewEntryDialogTest.java index e95444a..237742c 100644 --- a/src/test/java/com/pointbluetech/arborj/view/NewEntryDialogTest.java +++ b/src/test/java/com/pointbluetech/arborj/view/NewEntryDialogTest.java @@ -200,6 +200,29 @@ void rdnPlainValueUnchanged() { assertEscapedRdn("gr2", "cn=gr2"); } + @Test + @DisplayName("typed combo editor text wins over a stale committed value") + void resolveComboTextPrefersEditor() { + assertEquals("ou", NewEntryDialog.resolveComboText("ou", "cn")); + assertEquals("ou", NewEntryDialog.resolveComboText(" ou ", "cn")); + assertEquals("cn", NewEntryDialog.resolveComboText("", "cn")); + assertEquals("cn", NewEntryDialog.resolveComboText(" ", "cn")); + assertEquals("cn", NewEntryDialog.resolveComboText(null, "cn")); + assertEquals("", NewEntryDialog.resolveComboText(null, null)); + assertEquals("", NewEntryDialog.resolveComboText("", " ")); + } + + @Test + @DisplayName("typed ou with stale cn value produces ou DN and Add, not cn") + void typedEditorAttrUsedForDnAndAdd() { + String attr = NewEntryDialog.resolveComboText("ou", "cn"); + assertEquals("ou=Sales", NewEntryDialog.formatRDN(attr, "Sales")); + Map> attrs = + NewEntryDialog.buildCreateAttributes(Map.of(), attr, "Sales"); + assertEquals(List.of("Sales"), attrs.get("ou")); + assertFalse(attrs.containsKey("cn")); + } + @Test @DisplayName("attribute type accepts LDAP descr and numeric OID") void validAttributeTypes() {