[
https://issues.apache.org/jira/browse/OAK-2445?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14591925#comment-14591925
]
angela commented on OAK-2445:
-----------------------------
[~djaeggi], the patch looks great... as always a pleasure to review. there is
just one minor thing i am not fully convinced of: why do you think it's
necessary to store the password history entries in a separate node each?
wouldn't it be sufficient to have a single string property for that (either
single or multivalued? i started wondering because i spotted that
{{rep:PasswordHistoryEntry}} extends from {{nt:hierarchyNode}} (for the sake of
getting a {{jcr:created}} (but you additionally get {{jcr:createdBy}}). is it
by intention that you also want to record _who_ did the change?
without having thought about it very carefully (in contrast to you), i would
have expected this to be just store in a single property as i wouldn't have
expected to have thousands of entries in the pw-history. you could argue that
updating that property would fail if there was a concurrent pw-change but on
the other hand you may also end up with multiple nodes that have the exact same
{{jcr:createdBy}} property, which somehow would also leave you with a conflict,
wouldn't it? so maybe having those concurrent modifications immediately causing
a collision might even be the expected behavior.... wdyt? at least it was less
verbose, easier to implement and probably a bit faster to parse than creating a
tree structure of that.
> Password History Support
> ------------------------
>
> Key: OAK-2445
> URL: https://issues.apache.org/jira/browse/OAK-2445
> Project: Jackrabbit Oak
> Issue Type: New Feature
> Components: core
> Affects Versions: 1.1.5
> Reporter: Dominique Jäggi
> Assignee: Dominique Jäggi
> Attachments: OAK-2445_-_Password_History_Support.patch
>
>
> This is a patch that enhances oak security to support user user password
> history.
> details of the feature as per doc:
> {noformat}
> Password History
> --------------------------------------------------------------------------------
> ### General
> Oak provides functionality to remember a configurable number of
> passwords after password changes and to prevent a password to
> be set during changing a user's password if found in said history.
> ### Configuration
> An administrator may enable password history via the
> _org.apache.jackrabbit.oak.security.user.UserConfigurationImpl_
> OSGi configuration. By default the history is disabled.
> The following configuration option is supported:
> - Maximum Password History Size (_passwordHistorySize_, number of passwords):
> When greater 0 enables password
> history and sets feature to remember the specified number of passwords for
> a user.
> ### How it works
> #### Recording of Passwords
> If the feature is enabled, during a user changing her password, the old
> password
> hash is recorded in the password history.
> The old password hash is only recorded if a password was set (non-empty).
> Therefore setting a password for a user for the first time does not result
> in a history record, as there is no old password.
> The old password hash is copied to the password history *after* the provided
> new
> password has been validated (e.g. via the _PasswwordChangeAction_) but
> *before*
> the new password hash is written to the user's _rep:password_ property.
> The history operates as a FIFO list. A new password history record exceeding
> the
> configured max history size, results in the oldest recorded password from
> being
> removed from the history.
> Also, if the configuration parameter for the history
> size is changed to a non-zero but smaller value than before, upon the next
> password change the oldest records exceeding the new history size are removed.
> History password hashes are recorded as child nodes of a user's
> _rep:pwd/rep:pwdHistory_
> node. The _rep:pwdHistory_ node is governed by the _rep:PasswordHistory_ node
> type,
> its children by the _rep:PasswordHistoryEntry_ node type, allowing setting of
> the
> _rep:password_ property and its record date via _jcr:created_.
> ##### Node Type rep:PasswordHistory and rep:PasswordHistoryEntry
> [rep:PasswordHistory]
> + * (rep:PasswordHistoryEntry) = rep:PasswordHistoryEntry protected
> [rep:PasswordHistoryEntry] > nt:hierarchyNode
> - rep:password (STRING) protected
> ##### Nodes rep:pwd and rep:pwdHistory
> [rep:Password]
> + rep:pwdHistory (rep:PasswordHistory) = rep:PasswordHistory protected
> ...
> [rep:User] > rep:Authorizable, rep:Impersonatable
> + rep:pwd (rep:Password) = rep:Password protected
> ...
>
> The _rep:pwdHistory_ and history entry nodes and the _rep:password_ property
> are defined protected in order to guard against the user modifying
> (overcoming) her
> password history limitations. The new sub-nodes also has the advantage of
> allowing
> repository consumers to e.g. register specific commit hooks / actions on such
> a node.
> #### Evaluation of Password History
> Upon a user changing her password and if the password history feature is
> enabled
> (configured password history size > 0), the _PasswordChangeAction_ checks
> whether
> any of the password hashes recorded in the history matches the new password.
> If any record is a match, a _ConstraintViolationException_ is thrown and the
> user's password is *NOT* changed.
> #### Oak JCR XML Import
> When users are imported via the Oak JCR XML importer, password history is
> currently not supported (ignored).
> {noformat}
> [~anchela], if you could kindly review the patch.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
