[
https://issues.apache.org/jira/browse/OAK-8408?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16867273#comment-16867273
]
angela edited comment on OAK-8408 at 6/19/19 7:08 AM:
------------------------------------------------------
while working on a possible fix, i noticed that _initial-pw-change_ works
somewhat contrary to the _pw-expiry_ feature where {{rep:passwordLastModified}}
for the former must NOT be set unless the user logs in the first time, whereas
the latter should have a baseline date set upon user creation. when it comes to
user-import i decided to change the existing behavior (i.e. treating import the
same way as regular user-creation and pw-change api calls) as follows:
- _initial-pw-change_: don't set {{rep:passwordLastModified}} if the user tree
has either Status.NEW _or_ if {{UserManagerImpl.setPassword}} is call from an
import (i.e. adding the extra import condition. was only checking for NEW
before, which resulted in the original issue)
- _pw-expiry_: always set {{rep:passwordLastModified}} for all user mgt api
calls; however, for user-import only set upon user creation (i.e. user tree has
Status.NEW) in order to comply with {{UserManager.createUser}} but don't update
the last-mod date when an import modifies an existing user unless the XML to be
imported contains that information. (original behavior: always set
{{rep:passwordLastModified}} when pw-expiry is configured)
note: for the combination of _initial-pw-change_ and _pw-expiry_ the behavior
for _initial-pw-change_ applies (i.e. original behavior, i tried to make to
code a bit easier to read).
[~stillalex], since the proposed fix changes existing behavior and potentially
introduces regressions, i would like you to review the proposed changes and
help me assess the risk that comes with the changes. while there exists
documentation about importing the {{rep:pwd}} subtree it is a bit vague about
the exact behavior upon user import in general, it nevertheless states that the
importing for an existing user might have undesired effects:
{quote}
On the other hand, if the imported user already exists, potentially existing
rep:passwordLastModified properties will be overwritten with the value from the
import. If password expiry is enabled, this may cause passwords to expire
earlier or later than anticipated, governed by the new value. Also, an import
may create such a property where none previously existed, thus effectively
cancelling the need to change the password on first login - if the feature is
enabled.
Therefore customers using the importer in such fashion should be aware of the
potential need to enable password expiry/force initial password change for the
imported data to make sense, and/or the effect on already existing/overwritten
data.
{{quote}}
was (Author: anchela):
while working on a possible fix, i noticed that _initial-pw-change_ works
somewhat contrary to the _pw-expiry_ feature where {{rep:passwordLastModified}}
for the former must NOT be set unless the user logs in the first time, whereas
the latter should have a baseline date set upon user creation. when it comes to
user-import i decided to change the existing behavior (i.e. treating import the
same way as regular user-creation and pw-change api calls) as follows:
- _initial-pw-change_: don't set {{rep:passwordLastModified}} if the user tree
has either Status.NEW _or_ if {{UserManagerImpl.setPassword}} is call from an
import (i.e. adding the extra import condition. was only checking for NEW
before, which resulted in the original issue)
- _pw-expiry_: always set {{rep:passwordLastModified}} for all user mgt api
calls; however, for user-import only set upon user creation (i.e. user tree has
Status.NEW) in order to comply with {{UserManager.createUser}} but don't update
the last-mod date when an import modifies an existing user unless the XML to be
imported contains that information. (original behavior: always set
{{rep:passwordLastModified}} when pw-expiry is configured)
note: for the combination of _initial-pw-change_ and _pw-expiry_ the behavior
for _initial-pw-change_ applies (i.e. original behavior, i tried to make to
code a bit easier to read).
[~stillalex], since the fix changes existing and potentially introduces
regressions, i would like you to review the proposed changes and assess the
risk that comes with the changes. while there exists documentation about
importing the {{rep:pwd}} subtree it is a bit vague about the exact behavior
upon user import in general. but it nevertheless states that the importing for
an existing user might have undesired effects:
{quote}
On the other hand, if the imported user already exists, potentially existing
rep:passwordLastModified properties will be overwritten with the value from the
import. If password expiry is enabled, this may cause passwords to expire
earlier or later than anticipated, governed by the new value. Also, an import
may create such a property where none previously existed, thus effectively
cancelling the need to change the password on first login - if the feature is
enabled.
Therefore customers using the importer in such fashion should be aware of the
potential need to enable password expiry/force initial password change for the
imported data to make sense, and/or the effect on already existing/overwritten
data.
{{quote}}
> UserImporter must not trigger creation of rep:pwd node unless included in xml
> -----------------------------------------------------------------------------
>
> Key: OAK-8408
> URL: https://issues.apache.org/jira/browse/OAK-8408
> Project: Jackrabbit Oak
> Issue Type: Improvement
> Components: core, security
> Reporter: angela
> Assignee: angela
> Priority: Major
> Attachments: OAK-8408-tests.patch, OAK-8408.patch
>
>
> when xml-importing an existing user (i.e. {{Tree}} doesn't have status NEW
> upon import) calling {{UserManagerImpl.setPassword}} will force the creation
> of the {{rep:pwd}} node and {{rep:passwordLastModified}} property contained
> therein _if_ theinitial-password-change feature is enabled.
> imo the {{rep:pwd}} (and any properties contained therein) must not be
> auto-created by should only be imported if contained in the XML.
> proposed fix: {{UserManagerImpl.setPassword}} already contains special
> treatment for the password hashing triggered upon xml import -> renaming that
> flag and respect it for the handling of the pw last modified.
> [~stillalex], wdyt?
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
