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)_


-- 
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