[
https://issues.apache.org/jira/browse/TOMEE-4660?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Markus Jung updated TOMEE-4660:
-------------------------------
Description:
Follow-up to TOMEE-4648, raised in review of the fix for that issue.
{{OpenIdAuthenticationMechanism.handleTokenResponse}} builds a
{{TomEEOpenIdCredential}} and passes it straight to the identity store handler:
{code:java}
TomEEOpenIdCredential credential = new TomEEOpenIdCredential(tokenResponse,
httpMessageContext);
CredentialValidationResult validationResult =
identityStoreHandler.validate(credential);
{code}
{{TomEEOpenIdCredential}} lives in
{{org.apache.tomee.security.http.openid.model}}, a TomEE-internal package.
The spec's default {{IdentityStore.validate(Credential)}} dispatches to a
{{validate(...)}} overload only on an _exact_ parameter-type match. So on the
OpenID path an application-supplied {{IdentityStore}} can only be invoked by
either:
* declaring {{validate(Credential)}} - losing the typed overload convenience
the spec intends, or
* declaring {{validate(TomEEOpenIdCredential)}} - importing a TomEE-internal
class, which is not portable.
This is the same class of problem as TOMEE-4648 (which concerned the BASIC path
passing {{BasicAuthenticationCredential}} where stores declare
{{validate(UsernamePasswordCredential)}}), but on the OpenID path it is
arguably worse: there is no portable credential type an application can name at
all.
Not a regression, and not required for the TOMEE-4648 fix. Worth deciding
whether the OpenID path should pass a spec-defined credential type, or whether
{{TomEEOpenIdCredential}} should be promoted to a public/portable API type.
Note that since TOMEE-4648, {{TomEEIdentityStoreHandler.validate}} logs at
debug when no store validated a credential, which makes this failure mode
visible rather than a silent 401.
was:Follow-up to TOMEE-4648, raised in review of the fix for that
issue.\n\n{{OpenIdAuthenticationMechanism.handleTokenResponse}} builds a
{{TomEEOpenIdCredential}} and passes it straight to the identity store
handler:\n\n``{{java\nTomEEOpenIdCredential credential = new
TomEEOpenIdCredential(tokenResponse,
httpMessageContext);\nCredentialValidationResult validationResult =
identityStoreHandler.validate(credential);\n}}`{{\n\n}}TomEEOpenIdCredential{{
lives in }}org.apache.tomee.security.http.openid.model{{, a TomEE-internal
package.\n\nThe spec's default }}IdentityStore.validate(Credential){{
dispatches to a }}validate(...){{ overload only on an *exact* parameter-type
match. So on the OpenID path an application-supplied }}IdentityStore{{ can only
be invoked by either:\n- declaring }}validate(Credential){{ — losing the typed
overload convenience the spec intends, or\n- declaring
}}validate(TomEEOpenIdCredential){{ — importing a TomEE-internal class, which
is not portable.\n\nThis is the same class of problem as TOMEE-4648 (which
concerned the BASIC path passing }}BasicAuthenticationCredential{{ where stores
declare }}validate(UsernamePasswordCredential){{), but on the OpenID path it is
arguably worse: there is no portable credential type an application can name at
all.\n\nNot a regression, and not required for the TOMEE-4648 fix. Worth
deciding whether the OpenID path should pass a spec-defined credential type, or
whether }}TomEEOpenIdCredential{{ should be promoted to a public/portable API
type.\n\nNote that since TOMEE-4648, }}TomEEIdentityStoreHandler.validate` logs
at debug when no store validated a credential, which makes this failure mode
visible rather than a silent 401.
> Jakarta Security: OpenID path passes a TomEE-internal credential type to
> IdentityStore
> --------------------------------------------------------------------------------------
>
> Key: TOMEE-4660
> URL: https://issues.apache.org/jira/browse/TOMEE-4660
> Project: TomEE
> Issue Type: Bug
> Reporter: Markus Jung
> Priority: Major
>
> Follow-up to TOMEE-4648, raised in review of the fix for that issue.
> {{OpenIdAuthenticationMechanism.handleTokenResponse}} builds a
> {{TomEEOpenIdCredential}} and passes it straight to the identity store
> handler:
> {code:java}
> TomEEOpenIdCredential credential = new TomEEOpenIdCredential(tokenResponse,
> httpMessageContext);
> CredentialValidationResult validationResult =
> identityStoreHandler.validate(credential);
> {code}
> {{TomEEOpenIdCredential}} lives in
> {{org.apache.tomee.security.http.openid.model}}, a TomEE-internal package.
> The spec's default {{IdentityStore.validate(Credential)}} dispatches to a
> {{validate(...)}} overload only on an _exact_ parameter-type match. So on the
> OpenID path an application-supplied {{IdentityStore}} can only be invoked by
> either:
> * declaring {{validate(Credential)}} - losing the typed overload convenience
> the spec intends, or
> * declaring {{validate(TomEEOpenIdCredential)}} - importing a TomEE-internal
> class, which is not portable.
> This is the same class of problem as TOMEE-4648 (which concerned the BASIC
> path passing {{BasicAuthenticationCredential}} where stores declare
> {{validate(UsernamePasswordCredential)}}), but on the OpenID path it is
> arguably worse: there is no portable credential type an application can name
> at all.
> Not a regression, and not required for the TOMEE-4648 fix. Worth deciding
> whether the OpenID path should pass a spec-defined credential type, or
> whether {{TomEEOpenIdCredential}} should be promoted to a public/portable API
> type.
> Note that since TOMEE-4648, {{TomEEIdentityStoreHandler.validate}} logs at
> debug when no store validated a credential, which makes this failure mode
> visible rather than a silent 401.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)