[jboss-jira] [JBoss JIRA] (WFLY-3518) JASPIAuthenticationMechanism#authenticate doesn't check if AuthenticatedSession is null

[arjan tijms (JIRA)]

    [ https://issues.jboss.org/browse/WFLY-3518?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13023412#comment-13023412 ] 

arjan tijms commented on WFLY-3518:
-----------------------------------

Just wondering if there's any update for this issue. 

We've been using the extra null check in production since June. At least for our use case we didn't see any side-effects.

> JASPIAuthenticationMechanism#authenticate doesn't check if AuthenticatedSession is null
> ---------------------------------------------------------------------------------------
>
>                 Key: WFLY-3518
>                 URL: https://issues.jboss.org/browse/WFLY-3518
>             Project: WildFly
>          Issue Type: Bug
>          Components: Security
>    Affects Versions: 8.1.0.Final
>            Reporter: arjan tijms
>            Assignee: Darran Lofthouse
>              Labels: jaspic
>
> In {{org.wildfly.extension.undertow.security.jaspi.JASPIAuthenticationMechanism#authenticate}} the variable {{authSession}} in the fragment below is frequently null, leading to null pointer exceptions:
> {code}
>   if (sessionManager != null) {
>             AuthenticatedSessionManager.AuthenticatedSession authSession = sessionManager.lookupSession(exchange);
>             cachedAccount = authSession.getAccount(); // NPE HAPPENS HERE
>             // if there is a cached account we set it in the security context so that the principal is available to
>             // SAM modules via request.getUserPrincipal().
>             if (cachedAccount !=  null) {
>                 jaspicSecurityContext.setCachedAuthenticatedAccount(cachedAccount);
>             }
>         }
> {code}
> At another place in Undertow where {{AuthenticatedSession}} is used, there's an extra null check (See {{io.undertow.security.impl.CachedAuthenticatedSessionMechanism#runCached}}).
> I patched the code locally to add an extra null check:
> {code}
>         if (sessionManager != null) {
>             AuthenticatedSessionManager.AuthenticatedSession authSession = sessionManager.lookupSession(exchange);
>             cachedAccount = authSession == null? null : authSession.getAccount();
>             // if there is a cached account we set it in the security context so that the principal is available to
>             // SAM modules via request.getUserPrincipal().
>             if (cachedAccount !=  null) {
>                 jaspicSecurityContext.setCachedAuthenticatedAccount(cachedAccount);
>             }
>         }
> {code}
> After a short amount of testing everything seems to be okay with that extra check.



--
This message was sent by Atlassian JIRA
(v6.3.8#6338)