some_pre_step doesn't seem very descriptive ...

You probably want to also check what the storageScheme is after the delete, and if userpassword setting still works.

some_pre_step doesn't seem very descriptive ...

Done

You probably want to also check what the storageScheme is after the delete, and if userpassword setting still works.

Done

You misunderstand. You need to check the cn=config passwordStorageScheme after you disable this to see what it's set to - it still could be pbkdf2 ....

You misunderstand. You need to check the cn=config passwordStorageScheme after you disable this to see what it's set to - it still could be pbkdf2 ....

Now config part check is also added

Do ... you not see the bug here? If you disable PBKDF2Plugin, the scheme set as PBKDF2_SHA256 won't be found, so creating new users/passwords will fail.

The second assert here is pointless - because we changed the scheme in cn=config, that does NOT mean we re-hash anything, so the userPassword will ofcourse stay as pkbdf2. This line can be removed, because it's really doing nothing.

Do ... you not see the bug here? If you disable PBKDF2Plugin, the scheme set as PBKDF2_SHA256 won't be found, so creating new users/passwords will fail.

That's not true :) Apparently, PBKDF2 plugin is special and can't be disabled/removed.
PBKDF2 plugin is added back during the startup: https://pagure.io/389-ds-base/blob/master/f/ldap/servers/slapd/fedse.c#_127

We tested the following scenarios:
1. password storage scheme is set to PBKDF2_SHA256, plugin is disabled (with dynamic plugins on and off) - > we were able to create userPassword attribute with PBKDF2_SHA256 storage scheme successfully.
2. password storage scheme is set to PBKDF2_SHA256, plugin is disabled and removed (with dynamic plugins on and off) -> we were able to create userPassword attribute with PBKDF2_SHA256 storage scheme successfully.
3. password storage scheme is set to PBKDF2_SHA256, plugin is removed, server is restarted -> Restart is successful, we were able to create userPassword attribute with PBKDF2_SHA256 storage scheme successfully.

If we change the storage scheme to SSHA512, then all of these fail. And with removed plugin entry server refuses to start.

The second assert here is pointless - because we changed the scheme in cn=config, that does NOT mean we re-hash anything, so the userPassword will ofcourse stay as pkbdf2. This line can be removed, because it's really doing nothing.

Done

Do ... you not see the bug here? If you disable PBKDF2Plugin, the scheme set as PBKDF2_SHA256 won't be found, so creating new users/passwords will fail.

That's not true :) Apparently, PBKDF2 plugin is special and can't be disabled/removed.
PBKDF2 plugin is added back during the startup: https://pagure.io/389-ds-base/blob/master/f/ldap/servers/slapd/fedse.c#_127

But that's only after a restart. While the server is running if you disable the plugin, it still has the storage scheme set to PBKDF2 and so online further changes would fail.

We tested the following scenarios:
1. password storage scheme is set to PBKDF2_SHA256, plugin is disabled (with dynamic plugins on and off) - > we were able to create userPassword attribute with PBKDF2_SHA256 storage scheme successfully.
2. password storage scheme is set to PBKDF2_SHA256, plugin is disabled and removed (with dynamic plugins on and off) -> we were able to create userPassword attribute with PBKDF2_SHA256 storage scheme successfully.

But ... it's disabled? :|

1. password storage scheme is set to PBKDF2_SHA256, plugin is removed, server is restarted -> Restart is successful, we were able to create userPassword attribute with PBKDF2_SHA256 storage scheme successfully.
   If we change the storage scheme to SSHA512, then all of these fail. And with removed plugin entry server refuses to start.

Honestly, password schemes as plugins is cursed and evil, but I can't change the mistakes of the past .... The whole nature of turning these on/off is not good ....

So what do you think here then? Should we direct people to just change the storage scheme instead of enable/disable the plugins? Should we harden the password plugin areas to prevent them being disabled? Shoud we try and make this (hopefully uncommon) scenario work?

The original test case was testing default password storage scheme plugin (was SSHA at the time). It was asserting that if you remove the plugin entry, keeping password storage scheme pointing to that plugin, server would refuse to start. This makes sense and it's expected, since it's a mandatory plugin for the server to function.

When we updated the test case to use the current default password storage scheme (PBKDF2_SHA256), the original assumption was no longer valid, since PBKDF2 plugin entry is added on startup if it's not there (this was done on purpose as part of script-less upgrade).

So I'd suggest to have one test case for non-PBKDF2 storage scheme with the assumption that restart would fail without the corresponding plugin entry, and one for PBKDF2 storage scheme with the assumption that server would start successfully.

Should we direct people to just change the storage scheme instead of enable/disable the plugins?

Our documentation says only about changing nsslapd-pwstoragescheme or nsslapd-rootpwstoragescheme, it doesn't mention enabling/disabling password storage scheme plugins. So we're good there.

Should we harden the password plugin areas to prevent them being disabled? Shoud we try and make this (hopefully uncommon) scenario work?

I think preventing disabling the plugin that is used currently by nsslapd-pwstoragescheme or nsslapd-rootpwstoragescheme is a 'nice to have' feature. But it's a very uncommon scenario.

Okay, so maybe there is no further action here then?

@aborah, could you please add a separate test case for SSHA512 with the assertion that the server won't start without the SSHA512 plugin, if SSHA512 is set as a password storage scheme?
Thanks.

@aborah, could you please add a separate test case for SSHA512 with the assertion that the server won't start without the SSHA512 plugin, if SSHA512 is set as a password storage scheme?
Thanks.

DOne

Please fix the test case name, it's 'ssha512', not 'shaa256'.

Please update the docstring and change :id: to a unique value.

Please fix the test case name, it's 'ssha512', not 'shaa256'.

Please update the docstring and change 🆔 to a unique value.

Done

Thanks, LTGM! Please rebase and fix up commits.

Thanks, LTGM! Please rebase and fix up commits.

Done

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This pull request has been cloned to Github as issue and is available here:
- https://github.com/389ds/389-ds-base/issues/3856

If you want to continue to work on the PR, please navigate to the github issue,
download the patch from the attachments and file a new pull request.

Thank you for understanding. We apologize for all inconvenience.