[jboss-jira] [JBoss JIRA] Commented: (SECURITY-339) ClientLoginModule improperly handles SecurityAssociation stack in abort()

[Marco Schulze (JIRA)]

    [ https://jira.jboss.org/jira/browse/SECURITY-339?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12478968#action_12478968 ] 

Marco Schulze commented on SECURITY-339:
----------------------------------------

I just wanted to add one thought:

If it is possible in certain scenarios that both commit() and abort() are called, then the login module must track in a boolean field, if commit() was called. Only then, it must pop the principal in its abort() method.

I never had this scenario, hence I'm not sure if this can really ever happen. But it is definitely wrong to always pop in abort(), because at least in all situations, which I debugged, the abort() method was called without commit() happening before.

> ClientLoginModule improperly handles SecurityAssociation stack in abort()
> -------------------------------------------------------------------------
>
>                 Key: SECURITY-339
>                 URL: https://jira.jboss.org/jira/browse/SECURITY-339
>             Project: JBoss Security and Identity Management
>          Issue Type: Bug
>      Security Level: Public(Everyone can see) 
>          Components: JBossSX
>            Reporter: Marco Schulze
>            Assignee: Anil Saldhana
>             Fix For: JBossSecurity_2.0.4
>
>
> The abort() method calls SecurityAssociationActions.popPrincipalInfo() even though the corresponding push happens in commit() [via SecurityAssociationActions.setPrincipalInfo(loginPrincipal, loginCredential, subject)]. That means, whenever a login fails, the commit is not called (thus nothing pushed), but the abort pops out an element from the stack. This should not be done. IMHO the abort() method should look like this:
>   public boolean abort() throws LoginException
>    {
>       if( trace )
>          log.trace("abort");
>       if( restoreLoginIdentity == false )
>       {
>          // Clear the entire security association stack
>          SecurityAssociationActions.clear();         
>       }
>       return true;
>    }

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: https://jira.jboss.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira