[jira] [Commented] (OAK-8408) UserImporter must not trigger creation of rep:pwd node unless included in xml

["angela (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/OAK-8408?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16867273#comment-16867273 ] 

angela commented on OAK-8408:
-----------------------------

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 just stating {{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.}}.

> 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: Bug
>          Components: core, security
>            Reporter: angela
>            Assignee: angela
>            Priority: Minor
>         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)