[
https://issues.apache.org/jira/browse/OAK-1942?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14052281#comment-14052281
]
angela commented on OAK-1942:
-----------------------------
thanks for the patch.
> Additionally, upon encountering a userId/password mismatch, no exception is
> thrown but instead false is returned
that's not quite true...
anyway: using more specific LoginExceptions looks good to me. but i am not
convinced about changing the behavior for no-such-authorizable. the
loginmodules are expected to be stacked and it might well be that another
loginmodule actually does know the user with the given id. in fact that's a
very common scenario in a setup with defaultloginmodule + externalloginmodule
and letting the loginmodule return false instead of throwing LoginException
looks better to me, though you are right wrt the Authentication api contract.
furthermore: the patch now throws FailedLoginException if the credentials are
not supported. IMO this violates the contract and doesn't make sense from the
way the loginmodules work.
> UserAuthentication: enhance login states with relevant exceptions
> -----------------------------------------------------------------
>
> Key: OAK-1942
> URL: https://issues.apache.org/jira/browse/OAK-1942
> Project: Jackrabbit Oak
> Issue Type: Improvement
> Components: security
> Affects Versions: 1.0, 1.0.1
> Reporter: Dominique Jäggi
> Assignee: angela
> Priority: Minor
> Fix For: 1.1
>
> Attachments:
> OAK-1942_-_UserAuthentication_enhance_login_states_with_relevant_exceptions.patch
>
>
> Currently _UserAuthentication_ throws generalized _LoginException_s upon
> encountering certain login states: user is disabled, user is a group.
> Additionally, upon encountering a userId/password mismatch, no exception is
> thrown but instead false is returned (Causing the login module to again throw
> a LoginException). This is contrary to the API contract of the _authenticate_
> method which states "true if the validation was successful; false if the
> specified credentials are not supported and this authentication
> implementation cannot verify their validity.". A userId/password mismatch
> means that the credentials are supported and *have been* verified and found
> invalid.
> I therefore suggest to detail login states and fix the contract issue by
> throwing relevant exceptions (e.g. _AccountNotFoundException_,
> _FailedLoginException_, et al).
> Through the exceptions consumers can react to various login states in a more
> detailed fashion and support the user through differentiated processes.
> Deeper analysis of how this affects various login modules may be required
> with corresponding test coverage.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
