You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tomee.apache.org by rz...@apache.org on 2026/04/23 13:50:54 UTC
(tomee) 16/23: security/cdi: fail deployment on duplicate mechanism qualifier sets
This is an automated email from the ASF dual-hosted git repository.
rzo1 pushed a commit to branch security-4.0
in repository https://gitbox.apache.org/repos/asf/tomee.git
commit 1ddd2388b21dce0724b6cabfd759373ba4493a38
Author: Richard Zowalla <rz...@apache.org>
AuthorDate: Mon Apr 20 21:09:32 2026 +0200
security/cdi: fail deployment on duplicate mechanism qualifier sets
C1.5 made ProcessBean idempotent so IDENTICAL mechanism definitions
collapse. Two DIFFERENT @*AuthenticationMechanismDefinition
instances that share the same qualifier set (e.g. two
@BasicAuthenticationMechanismDefinition with the same default
BasicAuthenticationMechanism qualifier but different realmName) are
not equal, stay as separate entries, and both register beans with
the same qualifier set -- OWB then throws AmbiguousResolutionException
on first access at runtime.
Observe AfterDeploymentValidation and group each mechanism type's
collected definitions by their effective qualifier set (wrapped in
a LinkedHashSet so intra-array order and duplicates don't mask
clashes). Any group with size > 1 becomes one
afterDeploymentValidation.addDeploymentProblem(DeploymentException)
carrying the annotation simple name, the conflicting qualifier
classes, the offending instances' toString, and a hint pointing at
qualifiers = { ... }. All four mechanism types are checked in one
observer so the error surfaces every violation at once.
Test coverage (5 cases in TomEESecurityExtensionQualifierValidationTest)
exercises the extracted package-private helper
validateQualifierUniqueness: single def passes, two defs with same
default qualifier and different realmName fails with both realms
named, distinct qualifier sets pass, reordered qualifier arrays
still collide, empty input is a no-op.
---
.../tomee/security/cdi/TomEESecurityExtension.java | 76 ++++++++++
...EESecurityExtensionQualifierValidationTest.java | 154 +++++++++++++++++++++
2 files changed, 230 insertions(+)
diff --git a/tomee/tomee-security/src/main/java/org/apache/tomee/security/cdi/TomEESecurityExtension.java b/tomee/tomee-security/src/main/java/org/apache/tomee/security/cdi/TomEESecurityExtension.java
index bd38a5cd78..3cddca0253 100644
--- a/tomee/tomee-security/src/main/java/org/apache/tomee/security/cdi/TomEESecurityExtension.java
+++ b/tomee/tomee-security/src/main/java/org/apache/tomee/security/cdi/TomEESecurityExtension.java
@@ -42,11 +42,13 @@ import jakarta.enterprise.event.Observes;
import jakarta.enterprise.inject.Any;
import jakarta.enterprise.inject.Default;
import jakarta.enterprise.inject.spi.AfterBeanDiscovery;
+import jakarta.enterprise.inject.spi.AfterDeploymentValidation;
import jakarta.enterprise.inject.spi.Annotated;
import jakarta.enterprise.inject.spi.AnnotatedType;
import jakarta.enterprise.inject.spi.BeanAttributes;
import jakarta.enterprise.inject.spi.BeanManager;
import jakarta.enterprise.inject.spi.BeforeBeanDiscovery;
+import jakarta.enterprise.inject.spi.DeploymentException;
import jakarta.enterprise.inject.spi.Extension;
import jakarta.enterprise.inject.spi.ProcessBean;
import jakarta.enterprise.util.TypeLiteral;
@@ -62,11 +64,17 @@ import jakarta.security.enterprise.identitystore.LdapIdentityStoreDefinition;
import java.lang.annotation.Annotation;
import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.LinkedHashMap;
import java.util.LinkedHashSet;
import java.util.List;
+import java.util.Map;
import java.util.Set;
import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.Function;
import java.util.function.Supplier;
+import java.util.stream.Collectors;
public class TomEESecurityExtension implements Extension {
// LinkedHashSet: ProcessBean fires for each bean *and* any derived observer/interceptor beans on the
@@ -455,6 +463,74 @@ public class TomEESecurityExtension implements Extension {
applicationAuthenticationMechanisms;
}
+ // Fail deployment early when two *AuthenticationMechanismDefinition annotations would register
+ // beans with the same qualifier set but different content (e.g. two @BasicAuthenticationMechanismDefinition
+ // with identical default qualifiers but different realmName). Without this the clash only surfaces
+ // at first injection as an AmbiguousResolutionException, which is hard to diagnose.
+ void validateMechanismDefinitionUniqueness(@Observes final AfterDeploymentValidation afterDeploymentValidation) {
+ final List<String> problems = new ArrayList<>();
+
+ problems.addAll(validateQualifierUniqueness(basicMechanismDefinitions,
+ BasicAuthenticationMechanismDefinition.class,
+ BasicAuthenticationMechanismDefinition::qualifiers));
+ problems.addAll(validateQualifierUniqueness(formMechanismDefinitions,
+ FormAuthenticationMechanismDefinition.class,
+ FormAuthenticationMechanismDefinition::qualifiers));
+ problems.addAll(validateQualifierUniqueness(customMechanismDefinitions,
+ CustomFormAuthenticationMechanismDefinition.class,
+ CustomFormAuthenticationMechanismDefinition::qualifiers));
+ problems.addAll(validateQualifierUniqueness(oidcMechanismDefinitions,
+ OpenIdAuthenticationMechanismDefinition.class,
+ OpenIdAuthenticationMechanismDefinition::qualifiers));
+
+ if (!problems.isEmpty()) {
+ final String message = "Ambiguous authentication mechanism definitions detected:\n "
+ + String.join("\n ", problems);
+ afterDeploymentValidation.addDeploymentProblem(new DeploymentException(message));
+ }
+ }
+
+ // Package-private for focused unit tests. Groups the annotations by the effective qualifier set
+ // (order-insensitive, duplicates collapsed) and returns a problem message for each set that has
+ // more than one distinct definition. Returns an empty list when the definitions are unambiguous.
+ static <T extends Annotation> List<String> validateQualifierUniqueness(
+ final Collection<T> definitions,
+ final Class<T> annotationType,
+ final Function<T, Class<?>[]> qualifiersAccessor) {
+
+ if (definitions == null || definitions.size() < 2) {
+ return List.of();
+ }
+
+ // LinkedHashMap preserves discovery order so error messages are deterministic.
+ final Map<Set<Class<?>>, List<T>> byQualifierSet = new LinkedHashMap<>();
+ for (final T definition : definitions) {
+ final Class<?>[] qualifiers = qualifiersAccessor.apply(definition);
+ final Set<Class<?>> key = new LinkedHashSet<>(Arrays.asList(qualifiers));
+ byQualifierSet.computeIfAbsent(key, k -> new ArrayList<>()).add(definition);
+ }
+
+ final List<String> problems = new ArrayList<>();
+ for (final Map.Entry<Set<Class<?>>, List<T>> entry : byQualifierSet.entrySet()) {
+ if (entry.getValue().size() <= 1) {
+ continue;
+ }
+ final String qualifierList = entry.getKey().isEmpty()
+ ? "{}"
+ : entry.getKey().stream()
+ .map(Class::getSimpleName)
+ .collect(Collectors.joining(", ", "{", "}"));
+ final String conflicting = entry.getValue().stream()
+ .map(Object::toString)
+ .collect(Collectors.joining("; "));
+ problems.add("@" + annotationType.getSimpleName()
+ + " declared " + entry.getValue().size() + " times with qualifier set " + qualifierList
+ + " but different content [" + conflicting + "]; "
+ + "declare distinct qualifiers via qualifiers={...} or pick a single definition");
+ }
+ return problems;
+ }
+
private Supplier<LoginToContinue> createFormLoginToContinueSupplier(final FormAuthenticationMechanismDefinition definition,
final BeanManager beanManager) {
return () -> {
diff --git a/tomee/tomee-security/src/test/java/org/apache/tomee/security/cdi/TomEESecurityExtensionQualifierValidationTest.java b/tomee/tomee-security/src/test/java/org/apache/tomee/security/cdi/TomEESecurityExtensionQualifierValidationTest.java
new file mode 100644
index 0000000000..a7e975359f
--- /dev/null
+++ b/tomee/tomee-security/src/test/java/org/apache/tomee/security/cdi/TomEESecurityExtensionQualifierValidationTest.java
@@ -0,0 +1,154 @@
+/*
+ * 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.tomee.security.cdi;
+
+import jakarta.security.enterprise.authentication.mechanism.http.BasicAuthenticationMechanismDefinition;
+import org.junit.Test;
+
+import java.lang.annotation.Annotation;
+import java.util.Arrays;
+import java.util.List;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+/**
+ * Focused unit tests for {@link TomEESecurityExtension#validateQualifierUniqueness}. This covers the
+ * specific failure mode where two @*AuthenticationMechanismDefinition annotations share the same
+ * qualifier set but carry different attribute values -- a clash that would otherwise only surface at
+ * first injection as an AmbiguousResolutionException. We exercise the helper directly because
+ * spinning up Arquillian (or even tomee-embedded) for a negative-deployment assertion is
+ * significantly heavier than is warranted for the pure validation logic.
+ */
+public class TomEESecurityExtensionQualifierValidationTest {
+
+ @Test
+ public void identicalDefinitionsWithSameQualifiersProduceNoProblem() {
+ final BasicAuthenticationMechanismDefinition a =
+ basicDef("realm-a", BasicAuthenticationMechanism.class);
+ // The caller already dedups identical annotations via LinkedHashSet (see C1.5 in the extension),
+ // so this method never sees exact duplicates; passing distinct definitions that share a realm
+ // still collapses on the Map-grouping side because group.size() == 1 after dedup.
+ final List<String> problems = TomEESecurityExtension.validateQualifierUniqueness(
+ List.of(a),
+ BasicAuthenticationMechanismDefinition.class,
+ BasicAuthenticationMechanismDefinition::qualifiers);
+ assertTrue("single definition must never produce a problem: " + problems, problems.isEmpty());
+ }
+
+ @Test
+ public void twoDefinitionsWithSameDefaultQualifiersAndDifferentRealmsProduceProblem() {
+ final BasicAuthenticationMechanismDefinition a =
+ basicDef("realm-a", BasicAuthenticationMechanism.class);
+ final BasicAuthenticationMechanismDefinition b =
+ basicDef("realm-b", BasicAuthenticationMechanism.class);
+
+ final List<String> problems = TomEESecurityExtension.validateQualifierUniqueness(
+ List.of(a, b),
+ BasicAuthenticationMechanismDefinition.class,
+ BasicAuthenticationMechanismDefinition::qualifiers);
+
+ assertEquals("expected exactly one conflict to be reported: " + problems, 1, problems.size());
+ final String message = problems.get(0);
+ assertTrue("message should name the annotation: " + message,
+ message.contains("@BasicAuthenticationMechanismDefinition"));
+ assertTrue("message should include the default qualifier simple name: " + message,
+ message.contains("BasicAuthenticationMechanism"));
+ assertTrue("message should include the first conflicting realm: " + message,
+ message.contains("realm-a"));
+ assertTrue("message should include the second conflicting realm: " + message,
+ message.contains("realm-b"));
+ assertTrue("message should include the remediation hint: " + message,
+ message.contains("declare distinct qualifiers"));
+ }
+
+ @Test
+ public void differentQualifierSetsAreNotReportedAsAConflict() {
+ final BasicAuthenticationMechanismDefinition a =
+ basicDef("realm-a", BasicAuthenticationMechanism.class);
+ final BasicAuthenticationMechanismDefinition b =
+ basicDef("realm-b", MarkerQualifier.class);
+
+ final List<String> problems = TomEESecurityExtension.validateQualifierUniqueness(
+ List.of(a, b),
+ BasicAuthenticationMechanismDefinition.class,
+ BasicAuthenticationMechanismDefinition::qualifiers);
+
+ assertTrue("distinct qualifier sets must not collide: " + problems, problems.isEmpty());
+ }
+
+ @Test
+ public void qualifierArrayOrderDoesNotMaskAConflict() {
+ final BasicAuthenticationMechanismDefinition a =
+ basicDef("realm-a", BasicAuthenticationMechanism.class, MarkerQualifier.class);
+ final BasicAuthenticationMechanismDefinition b =
+ basicDef("realm-b", MarkerQualifier.class, BasicAuthenticationMechanism.class);
+
+ final List<String> problems = TomEESecurityExtension.validateQualifierUniqueness(
+ List.of(a, b),
+ BasicAuthenticationMechanismDefinition.class,
+ BasicAuthenticationMechanismDefinition::qualifiers);
+
+ assertEquals("different qualifier ordering still resolves to the same key set: " + problems,
+ 1, problems.size());
+ }
+
+ @Test
+ public void emptyInputProducesNoProblem() {
+ assertFalse(TomEESecurityExtension.validateQualifierUniqueness(
+ List.of(),
+ BasicAuthenticationMechanismDefinition.class,
+ BasicAuthenticationMechanismDefinition::qualifiers)
+ .iterator().hasNext());
+ }
+
+ // --- helpers ---
+
+ /**
+ * Marker used only to exercise the "different qualifier set" path. Its contents are irrelevant
+ * to the validator -- only Class identity and Arrays.asList equality matter.
+ */
+ private @interface MarkerQualifier {
+ }
+
+ private static BasicAuthenticationMechanismDefinition basicDef(final String realmName,
+ final Class<?>... qualifiers) {
+ return new BasicAuthenticationMechanismDefinition() {
+ @Override
+ public Class<? extends Annotation> annotationType() {
+ return BasicAuthenticationMechanismDefinition.class;
+ }
+
+ @Override
+ public String realmName() {
+ return realmName;
+ }
+
+ @Override
+ public Class<?>[] qualifiers() {
+ return qualifiers;
+ }
+
+ @Override
+ public String toString() {
+ return "@BasicAuthenticationMechanismDefinition(realmName=\"" + realmName
+ + "\", qualifiers=" + Arrays.toString(qualifiers) + ")";
+ }
+ };
+ }
+}