[ 
https://issues.apache.org/jira/browse/TOMEE-4648?focusedWorklogId=1032352&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-1032352
 ]

ASF GitHub Bot logged work on TOMEE-4648:
-----------------------------------------

                Author: ASF GitHub Bot
            Created on: 27/Jul/26 08:13
            Start Date: 27/Jul/26 08:13
    Worklog Time Spent: 10m 
      Work Description: rzo1 commented on PR #2851:
URL: https://github.com/apache/tomee/pull/2851#issuecomment-5088904358

   Diagnosis is right and matches the RI — Soteria's 
`BasicAuthenticationMechanism`
   constructs `new UsernamePasswordCredential(credentials[0], new 
Password(credentials[1]))`
   for exactly this reason. Verified the test fails without the production hunk
   (`expected:<200> but was:<401>`) and the module is green with it.
   
   Three things before merge:
   
   1. This is not behaviour-preserving in one direction. An application store 
that
      declares `validate(BasicAuthenticationCredential)` was previously 
dispatched to
      by the exact-match rule and now never is — the default 
`validate(Credential)`
      throws `NoSuchMethodException`, it's swallowed into 
`NOT_VALIDATED_RESULT`, and
      the user gets a 401 with nothing logged anywhere. Matching Soteria is 
still the
      right call, but this needs a line in the JIRA / release notes.
   
      Separately, and maybe as a follow-up: 
`TomEEIdentityStoreHandler.validate` could
      log at debug when every store returns NOT_VALIDATED. Today this whole 
class of
      failure is completely silent, which is why TOMEE-4648 was hard to pin 
down.
   
   2. `TestIdentityStore` is `@ApplicationScoped` and 
`src/test/resources/META-INF/beans.xml`
      is a bare `<beans/>`, while `AbstractTomEESecurityTest` deploys the whole
      test-classes tree as one webapp. So this store becomes an active 
authentication
      store for every test in tomee-security and is consulted on every 
BASIC/FORM login
      in the module. It's harmless today because it returns `INVALID_RESULT` 
and the
      handler falls through to `TomEEDefaultIdentityStore`, but it's easy to 
trip over
      for whoever adds the next test here. Please scope it (`@Vetoed` + explicit
      registration, or a caller check that can't collide).
   
   3. The servlet writes the `kaz` role line and the test never asserts it — 
that's the
      negative case, worth asserting `false`.
   
   Follow-up JIRA worth filing: 
`OpenIdAuthenticationMechanism.handleTokenResponse` has
   the same problem. It passes `TomEEOpenIdCredential`, which lives in a 
TomEE-internal
   package, so given the same exact-parameter-type dispatch an application 
store on the
   OpenID path can only be invoked by declaring `validate(Credential)` or 
importing a
   TomEE-internal class. Not a regression and not something this PR must fix.
   




Issue Time Tracking
-------------------

    Worklog Id:     (was: 1032352)
    Time Spent: 20m  (was: 10m)

> Jakarta Security: BASIC mechanism rejects valid credentials with an 
> application IdentityStore
> ---------------------------------------------------------------------------------------------
>
>                 Key: TOMEE-4648
>                 URL: https://issues.apache.org/jira/browse/TOMEE-4648
>             Project: TomEE
>          Issue Type: Bug
>            Reporter: Markus Jung
>            Assignee: Markus Jung
>            Priority: Major
>          Time Spent: 20m
>  Remaining Estimate: 0h
>
> When the BASIC authentication mechanism validates credentials against an 
> application-supplied {{IdentityStore}}, TomEE answers 401 even when the 
> caller sends valid credentials. This shows up in the plain BASIC test and in 
> the decorated and custom-handler variants 
> ({{AppCustomAuthenticationMechanismHandler2IT}}), so the problem is not about 
> wrapping the mechanism.
> h2. Root cause
> The BASIC mechanism passed a {{BasicAuthenticationCredential}} to 
> {{IdentityStoreHandler.validate(Credential)}}. The spec's default 
> {{IdentityStore.validate(Credential)}} dispatches to a {{validate(...)}} 
> overload only on an _exact_ parameter-type match (its javadoc explicitly 
> states it does not look for the most specific overload). Because 
> {{BasicAuthenticationCredential extends UsernamePasswordCredential}}, a store 
> declaring the idiomatic {{validate(UsernamePasswordCredential)}} overload — 
> exactly what the TCK's {{TestIdentityStore}} does — was never invoked and 
> returned {{NOT_VALIDATED}}, producing a 401 for correct credentials. Built-in 
> stores (e.g. Tomcat users) were unaffected, which is why it only surfaced 
> with an application store.
> Fix: the BASIC mechanism now hands the identity store a plain 
> {{UsernamePasswordCredential}} while still parsing the header via 
> {{BasicAuthenticationCredential}}.
> h2. Steps to reproduce / TCK reference
> Run the Jakarta Security 4.0 TCK reactor against TomEE Plus (Java 21) through 
> the {{security}} runner in {{runner-standalone}}. The following tests fail 
> and are excluded in {{runner-standalone/exclusions/security.txt}} in the 
> apache/tomee-tck harness repo:
> * {{AppCustomAuthenticationMechanismHandler2IT}} (3 failing methods)
> * {{AppMemBasicDecorateIT#testAuthenticated}}
> * {{AppMemBasicIT#testAuthenticated}}
> Remove the matching lines from {{security.txt}} once fixed, then re-run the 
> {{security}} runner to confirm all three test classes pass.
> h2. Note on the OpenID modules (not a TomEE bug)
> {{OpenId2DefaultIT}} and {{OpenId3DefaultIT}} were previously listed here as 
> token-validation failures. Investigation showed this was a test-harness 
> environment problem, not a TomEE defect: these modules start their bundled 
> OpenID provider through Tomcat's {{startup.sh}}, which requires 
> {{JAVA_HOME}}/{{JRE_HOME}} and ignores {{PATH}}. With those unset the 
> provider never started, the client's {{.well-known}} discovery fetch was 
> refused, and the tests failed downstream in a way that looked like a token 
> check. With {{JAVA_HOME}} set, both OpenID modules pass unchanged. The runner 
> has been fixed in apache/tomee-tck to derive {{JAVA_HOME}} when unset; the 
> two OpenID entries can be dropped from {{security.txt}} once a corrected CI 
> run confirms them.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to