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

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

                Author: ASF GitHub Bot
            Created on: 28/Jul/26 19:34
            Start Date: 28/Jul/26 19:34
    Worklog Time Spent: 10m 
      Work Description: jungm commented on PR #2851:
URL: https://github.com/apache/tomee/pull/2851#issuecomment-5108806878

   Thanks — all three addressed in 6f0e1f9, plus the two follow-ups filed.
   
   **1. Behaviour change documented.** Added a "Behaviour change (release 
note)" section to 
[TOMEE-4648](https://issues.apache.org/jira/browse/TOMEE-4648) spelling out 
that a store declaring `validate(BasicAuthenticationCredential)` is no longer 
dispatched to on the BASIC path, that such stores must move to 
`validate(UsernamePasswordCredential)`/`validate(Credential)`, and that this 
aligns us with Soteria at the cost of being breaking for anyone relying on the 
old TomEE-specific behaviour.
   
   Took the debug-logging suggestion in this PR rather than deferring it: 
`TomEEIdentityStoreHandler.validate` now logs at debug when no store validated, 
naming the credential type and the stores consulted, and pointing at the 
exact-match dispatch rule. That's the branch that was silently returning 
`NOT_VALIDATED_RESULT`.
   
   **2. Test store scoped.** You're right that a bare `<beans/>` plus the 
whole-tree webapp deployment made it a module-wide authentication store. It now 
returns `NOT_VALIDATED_RESULT` for any caller other than `reza` — which appears 
in no other test and in no `conf/tomcat-users.xml` entry — so the handler falls 
through to the remaining stores exactly as if it weren't deployed. I went with 
the caller check rather than `@Vetoed` + explicit registration because 
`AbstractTomEESecurityTest` uses the embedded container rather than 
`ApplicationComposer`, so there's no per-test `@Classes` hook to register it 
back through; `@Vetoed` alone would remove the store the test needs. Happy to 
switch if you'd prefer a different shape.
   
   **3. `kaz` asserted** as `false`.
   
   Re-verified the test still fails without the production hunk 
(`expected:<200> but was:<401>`) after the strengthening, and the module is 
green at 108 tests.
   
   **Follow-up filed:** 
[TOMEE-4660](https://issues.apache.org/jira/browse/TOMEE-4660) for the OpenID 
path — confirmed `handleTokenResponse` passes `TomEEOpenIdCredential` from the 
internal `org.apache.tomee.security.http.openid.model` package, so a portable 
application store can't name a type to overload on at all. Left out of this PR 
as you suggested.
   
   _🤖 Addressed by [Claude Code](https://claude.com/claude-code)_




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

    Worklog Id:     (was: 1032654)
    Time Spent: 0.5h  (was: 20m)

> 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: 0.5h
>  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}}. This matches the Soteria RI, which 
> constructs {{new UsernamePasswordCredential(...)}} for the same reason.
> h2. Behaviour change (release note)
> This is _not_ behaviour-preserving in one direction. An application 
> {{IdentityStore}} that declares {{validate(BasicAuthenticationCredential)}} 
> was previously dispatched to by the exact-match rule and will no longer be 
> invoked on the BASIC path; the default {{validate(Credential)}} then applies 
> and the store returns {{NOT_VALIDATED}}, so the caller gets a 401. Such 
> stores must declare {{validate(UsernamePasswordCredential)}} (or 
> {{validate(Credential)}}) instead. This aligns TomEE with the Jakarta 
> Security RI, but it is a breaking change for any application relying on the 
> previous TomEE-specific behaviour.
> To make this class of failure diagnosable, 
> {{TomEEIdentityStoreHandler.validate}} now logs at debug when no store 
> validated the credential — previously the caller simply received a 401 with 
> nothing logged anywhere, which is what made this issue hard to pin down.
> 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