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

Reply via email to