diff --git a/tomee/tomee-security/src/main/java/org/apache/tomee/security/cdi/BasicAuthenticationMechanism.java b/tomee/tomee-security/src/main/java/org/apache/tomee/security/cdi/BasicAuthenticationMechanism.java index 00c00375865..6c9d68c866d 100644 --- a/tomee/tomee-security/src/main/java/org/apache/tomee/security/cdi/BasicAuthenticationMechanism.java +++ b/tomee/tomee-security/src/main/java/org/apache/tomee/security/cdi/BasicAuthenticationMechanism.java @@ -24,6 +24,7 @@ import jakarta.security.enterprise.authentication.mechanism.http.HttpAuthenticationMechanism; import jakarta.security.enterprise.authentication.mechanism.http.HttpMessageContext; import jakarta.security.enterprise.credential.BasicAuthenticationCredential; +import jakarta.security.enterprise.credential.UsernamePasswordCredential; import jakarta.security.enterprise.identitystore.CredentialValidationResult; import jakarta.security.enterprise.identitystore.IdentityStoreHandler; import jakarta.servlet.http.HttpServletRequest; @@ -53,7 +54,12 @@ public AuthenticationStatus validateRequest(final HttpServletRequest request, try { final BasicAuthenticationCredential credential = parseAuthenticationHeader(request.getHeader(AUTHORIZATION)); - final CredentialValidationResult result = identityStoreHandler.validate(credential); + + // IdentityStore#validate(Credential) dispatches to an overload only on an *exact* parameter + // type match, so a store declaring the idiomatic validate(UsernamePasswordCredential) would + // never be called with the BasicAuthenticationCredential subclass. Hand it the base type. + final CredentialValidationResult result = identityStoreHandler.validate( + new UsernamePasswordCredential(credential.getCaller(), credential.getPassword())); if (result.getStatus().equals(VALID)) { return httpMessageContext.notifyContainerAboutLogin(result); diff --git a/tomee/tomee-security/src/main/java/org/apache/tomee/security/identitystore/TomEEIdentityStoreHandler.java b/tomee/tomee-security/src/main/java/org/apache/tomee/security/identitystore/TomEEIdentityStoreHandler.java index 693de9e2d07..99ee30d6308 100644 --- a/tomee/tomee-security/src/main/java/org/apache/tomee/security/identitystore/TomEEIdentityStoreHandler.java +++ b/tomee/tomee-security/src/main/java/org/apache/tomee/security/identitystore/TomEEIdentityStoreHandler.java @@ -16,6 +16,9 @@ */ package org.apache.tomee.security.identitystore; +import org.apache.openejb.util.LogCategory; +import org.apache.openejb.util.Logger; + import jakarta.annotation.PostConstruct; import jakarta.enterprise.context.ApplicationScoped; import jakarta.enterprise.inject.Instance; @@ -40,6 +43,9 @@ @ApplicationScoped public class TomEEIdentityStoreHandler implements IdentityStoreHandler { + private static final Logger LOGGER = Logger.getInstance( + LogCategory.TOMEE_SECURITY, TomEEIdentityStoreHandler.class); + @Inject private Instance identityStores; @@ -88,6 +94,21 @@ public CredentialValidationResult validate(final Credential credential) { return INVALID_RESULT; } else { + // No store validated the credential at all. The usual cause is that every store + // fell through to IdentityStore#validate(Credential), which dispatches to a + // validate(...) overload only on an exact parameter type match - so a store + // declaring an overload for a super- or subclass of the credential is silently + // never called. Without this the caller just sees a 401 with nothing logged. + if (LOGGER.isDebugEnabled()) { + LOGGER.debug("No IdentityStore validated a credential of type " + credential.getClass().getName() + + "; consulted " + authenticationStores.size() + " store(s): " + + authenticationStores.stream() + .map(store -> store.getClass().getName()) + .collect(Collectors.joining(", ")) + + ". Check that a store declares validate(" + credential.getClass().getSimpleName() + + ") or validate(Credential)."); + } + return NOT_VALIDATED_RESULT; } diff --git a/tomee/tomee-security/src/test/java/org/apache/tomee/security/servlet/Tomee4648BasicGroupsTest.java b/tomee/tomee-security/src/test/java/org/apache/tomee/security/servlet/Tomee4648BasicGroupsTest.java new file mode 100644 index 00000000000..7f8d69610ca --- /dev/null +++ b/tomee/tomee-security/src/test/java/org/apache/tomee/security/servlet/Tomee4648BasicGroupsTest.java @@ -0,0 +1,136 @@ +/* + * 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.servlet; + +import org.apache.tomee.security.AbstractTomEESecurityTest; +import org.apache.tomee.security.client.BasicAuthFilter; +import org.junit.Test; + +import jakarta.annotation.security.DeclareRoles; +import jakarta.enterprise.context.ApplicationScoped; +import jakarta.enterprise.util.AnnotationLiteral; +import jakarta.inject.Qualifier; +import jakarta.security.enterprise.authentication.mechanism.http.BasicAuthenticationMechanismDefinition; +import jakarta.security.enterprise.credential.UsernamePasswordCredential; +import jakarta.security.enterprise.identitystore.CredentialValidationResult; +import jakarta.security.enterprise.identitystore.IdentityStore; +import jakarta.servlet.ServletException; +import jakarta.servlet.annotation.HttpConstraint; +import jakarta.servlet.annotation.ServletSecurity; +import jakarta.servlet.annotation.WebServlet; +import jakarta.servlet.http.HttpServlet; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; +import jakarta.ws.rs.client.ClientBuilder; +import jakarta.ws.rs.core.Response; +import java.io.IOException; +import java.lang.annotation.Retention; +import java.lang.annotation.Target; +import java.util.HashSet; + +import static java.lang.annotation.ElementType.FIELD; +import static java.lang.annotation.ElementType.METHOD; +import static java.lang.annotation.ElementType.PARAMETER; +import static java.lang.annotation.ElementType.TYPE; +import static java.lang.annotation.RetentionPolicy.RUNTIME; +import static java.util.Arrays.asList; +import static jakarta.security.enterprise.identitystore.CredentialValidationResult.INVALID_RESULT; +import static jakarta.security.enterprise.identitystore.CredentialValidationResult.NOT_VALIDATED_RESULT; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +/** + * Mirrors the Jakarta Security TCK app-mem-basic module: an application supplied + * {@link IdentityStore} that only declares the {@code validate(UsernamePasswordCredential)} + * overload and returns the caller groups from that validation. + */ +public class Tomee4648BasicGroupsTest extends AbstractTomEESecurityTest { + + @Test + public void authenticatedCallerKeepsGroups() { + final Response response = ClientBuilder.newBuilder() + .register(new BasicAuthFilter("reza", "secret1")) + .build() + .target(getAppUrl() + "/tomee4648") + .request() + .get(); + + assertEquals(200, response.getStatus()); + + final String body = response.readEntity(String.class); + assertTrue(body, body.contains("web username: reza")); + assertTrue(body, body.contains("web user has role \"foo\": true")); + assertTrue(body, body.contains("web user has role \"bar\": true")); + // the store grants foo and bar only, so kaz must not be granted + assertTrue(body, body.contains("web user has role \"kaz\": false")); + } + + /** + * {@code src/test/resources/META-INF/beans.xml} is a bare {@code } and + * {@link AbstractTomEESecurityTest} deploys the whole test-classes tree as a single + * webapp, so this store is an active authentication store for every test in the + * module. It therefore only ever answers for {@value #CALLER}, a caller that exists + * in no other test and in no {@code conf/tomcat-users.xml} entry, and returns + * {@code NOT_VALIDATED_RESULT} for anything else so the handler keeps consulting the + * remaining stores exactly as it would if this one were not deployed. + */ + @ApplicationScoped + public static class TestIdentityStore implements IdentityStore { + private static final String CALLER = "reza"; + + public CredentialValidationResult validate(final UsernamePasswordCredential credential) { + if (!CALLER.equals(credential.getCaller())) { + return NOT_VALIDATED_RESULT; + } + + if (credential.compareTo(CALLER, "secret1")) { + return new CredentialValidationResult(CALLER, new HashSet<>(asList("foo", "bar"))); + } + + return INVALID_RESULT; + } + } + + @Qualifier + @Retention(RUNTIME) + @Target({FIELD, METHOD, TYPE, PARAMETER}) + public @interface Tomee4648Mechanism { + final class Literal extends AnnotationLiteral implements Tomee4648Mechanism { + private static final long serialVersionUID = 1L; + public static final Literal INSTANCE = new Literal(); + } + } + + @WebServlet(urlPatterns = "/tomee4648") + @DeclareRoles({"foo", "bar", "kaz"}) + @ServletSecurity(@HttpConstraint(rolesAllowed = "foo")) + @BasicAuthenticationMechanismDefinition( + realmName = "test realm", + qualifiers = Tomee4648Mechanism.class) + public static class TestServlet extends HttpServlet { + @Override + protected void doGet(final HttpServletRequest req, final HttpServletResponse resp) + throws ServletException, IOException { + resp.getWriter().write("This is a servlet \n"); + resp.getWriter().write("web username: " + + (req.getUserPrincipal() == null ? null : req.getUserPrincipal().getName()) + "\n"); + resp.getWriter().write("web user has role \"foo\": " + req.isUserInRole("foo") + "\n"); + resp.getWriter().write("web user has role \"bar\": " + req.isUserInRole("bar") + "\n"); + resp.getWriter().write("web user has role \"kaz\": " + req.isUserInRole("kaz") + "\n"); + } + } +}