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