[
https://issues.apache.org/jira/browse/OAK-2872?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14552282#comment-14552282
]
angela edited comment on OAK-2872 at 5/20/15 1:04 PM:
------------------------------------------------------
IMHO the state must be cleared both in LoginModule#commit() and
LoginModule#abort() in case of unsuccessful authentication. the abort method is
covered by the base class, but i am not totally sure if the fix committed at
http://svn.apache.org/r1679503 doesn't 100% correct as the state is not cleared
in all cases where the commit method returns false:
{code}
public boolean commit() throws LoginException {
if (externalUser == null) {
// login attempt in this login module was not successful
clearState();
return false;
}
Set<? extends Principal> principals =
getPrincipals(externalUser.getId());
if (!principals.isEmpty()) {
if (!subject.isReadOnly()) {
subject.getPrincipals().addAll(principals);
if (credentials != null) {
subject.getPublicCredentials().add(credentials);
}
setAuthInfo(createAuthInfo(externalUser.getId(), principals),
subject);
} else {
log.debug("Could not add information to read only subject {}",
subject);
}
return true;
}
return false; // NOTE: state not cleared in this case ... right?
}
{code}
was (Author: anchela):
IMHO the state must be cleared both in {@code LoginModule#commit()} and {@code
LoginModule#abort()} in case of unsuccessful authentication. the abort method
is covered by the base class, but i am not totally sure if the fix committed at
http://svn.apache.org/r1679503 doesn't 100% correct as the state is not cleared
in all cases where the commit method returns false:
{code}
public boolean commit() throws LoginException {
if (externalUser == null) {
// login attempt in this login module was not successful
clearState();
return false;
}
Set<? extends Principal> principals =
getPrincipals(externalUser.getId());
if (!principals.isEmpty()) {
if (!subject.isReadOnly()) {
subject.getPrincipals().addAll(principals);
if (credentials != null) {
subject.getPublicCredentials().add(credentials);
}
setAuthInfo(createAuthInfo(externalUser.getId(), principals),
subject);
} else {
log.debug("Could not add information to read only subject {}",
subject);
}
return true;
}
return false; // NOTE: state not cleared in this case ... right?
}
{code}
> ExternalLoginModule should clear state when login was not successful
> --------------------------------------------------------------------
>
> Key: OAK-2872
> URL: https://issues.apache.org/jira/browse/OAK-2872
> Project: Jackrabbit Oak
> Issue Type: Bug
> Components: auth-external
> Reporter: Alex Parvulescu
> Assignee: Alex Parvulescu
> Fix For: 1.3.0
>
>
> As discussed in [1], it looks like the ExternalLoginModule ignores cleaning
> up its internal state when login was not successful.
> What I assume happens next is the old session (probably the initial one
> created on the very first login call) would be reused throughout the module's
> lifetime, which would in the end result in the SNFEs post compaction.
> [1] http://markmail.org/thread/pcmlz74ngxl7sqfy
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
