jungm opened a new pull request, #2851:
URL: https://github.com/apache/tomee/pull/2851

   ## What
   
   Fixes [TOMEE-4648](https://issues.apache.org/jira/browse/TOMEE-4648): the 
BASIC authentication mechanism answered `401` for valid credentials when 
validation went against an application-supplied `IdentityStore`.
   
   ## Why
   
   `BasicAuthenticationMechanism` 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 states explicitly that 
it does not look for the most specific overload. Since 
`BasicAuthenticationCredential extends UsernamePasswordCredential`, a store 
declaring the idiomatic `validate(UsernamePasswordCredential)` overload 
(exactly what the Jakarta Security TCK's `TestIdentityStore` does) was never 
invoked and returned `NOT_VALIDATED`, producing a 401.
   
   Built-in stores (e.g. Tomcat users) declare their own credential handling 
and were unaffected, which is why this only surfaced with an application store 
— and why the plain, decorated, and custom-handler TCK variants all failed the 
same `testAuthenticated` check.
   
   ## Change
   
   `BasicAuthenticationMechanism.validateRequest` still parses the 
`Authorization` header via `BasicAuthenticationCredential` (which does the 
base64 decoding), but now hands the identity store a plain 
`UsernamePasswordCredential`, reusing the already-parsed `Password` (no 
plaintext round-trip). The empty/malformed-header path is unchanged — the 
constructor still throws `IllegalArgumentException`, caught as before to fall 
through to the challenge.
   
   ## Tests
   
   - New `Tomee4648BasicGroupsTest` mirrors the TCK `app-mem-basic` module (an 
`@ApplicationScoped IdentityStore` with only the 
`validate(UsernamePasswordCredential)` overload). It fails with 401 before the 
change and passes after, asserting caller name and both groups.
   - Full `tomee-security` module is green (108 tests), including the existing 
BASIC negative cases (wrong password, unknown user, missing role).
   
   ## TCK impact
   
   Removes the cause of these exclusions in the apache/tomee-tck `security` 
runner:
   - `AppMemBasicIT#testAuthenticated`
   - `AppMemBasicDecorateIT#testAuthenticated`
   - `AppCustomAuthenticationMechanismHandler2IT`
   
   ## Note for reviewers
   
   The two OpenID TCK modules originally bundled into TOMEE-4648 were **not** a 
TomEE bug — they were a test-harness environment issue (the bundled OpenID 
provider's `startup.sh` needs `JAVA_HOME`, which was unset). Both pass 
unchanged once `JAVA_HOME` is set; that fix lives in apache/tomee-tck and is 
out of scope for this PR. The ticket has been updated accordingly.
   
   🤖 Generated with [Claude Code](https://claude.com/claude-code)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to