jackrabbit-oak-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "angela (JIRA)" <j...@apache.org>
Subject [jira] [Comment Edited] (OAK-8408) UserImporter must not trigger creation of rep:pwd node unless included in xml
Date Wed, 19 Jun 2019 07:09:00 GMT

    [ 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 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}}

> 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)

Mime
View raw message