ActiveDirectoryDsc icon indicating copy to clipboard operation
ActiveDirectoryDsc copied to clipboard

BREAKING CHANGE: xADUser: Suggest renaming parameter PasswordNeverResets to EnforcePassword

Open johlju opened this issue 5 years ago • 9 comments

Details of the scenario you tried and the problem that is occurring

Currently the parameter PasswordNeverResets is not very intuitive of what it meant to do, to enforce the password for an account or not. Suggest renaming the parameter to EnforcePassword to clearly show what it is meant to do.

Verbose logs showing the problem

n/a

Suggested solution to the issue

n/a

The DSC configuration that is used to reproduce the issue (as detailed as possible)

n/a

The operating system the target node is running

n/a

Version and build of PowerShell the target node is running

n/a

Version of the DSC module that was used

v6.1.0-preview0005

johlju avatar Aug 25 '20 14:08 johlju

Adding this to track this as discussed in the Slack #DSC channel.

johlju avatar Aug 25 '20 14:08 johlju

I would recommend changing to PasswordResets, PasswordChange or UpdatePassword instead and have 3 states.

Always OnCreate WhenDifferent

Always would always set the password OnCreate only resets when the account is created WhenDifferent sets the password back to the one from DSC (if it gets reset outside of DSC, reset it back)

Deprecate the PasswordNeverResets parameter. If this is set to true, then throw warning and set PasswordResets to OnCreate and if it's false, set to WhenDifferent.

Default option could probably be whatever. I would suggest OnCreate as I am just thinking this is probably about 90% of the use cases. I think right now the default is to reset if password is different. So if neither parameter is specified, it would default to this. Could also throw a deprecated warning here as well if the default behavior changes and to explicitly set what you want.

New parameter takes priority, so if both options are set, ignore PasswordNeverResets and use PasswordResets and throw deprecated warning.

This could allow for other options later on as well. Like WhenExpired, AfterDate, WhenMoonIsFull. You get the idea.

kungfoome avatar Aug 25 '20 14:08 kungfoome

I don't understand what would be the difference between Always and WhenDifferent.

X-Guardian avatar Aug 25 '20 15:08 X-Guardian

I don't understand what would be the difference between Always and WhenDifferent.

Always will always set the password no matter what (it will basically be out of state anytime you run the config). WhenDifferent only sets the password if it's different than the password that is being set from DSC. If the passwords match, it won't change it. Always will.

Always is useful if you need to force a password change though.

kungfoome avatar Aug 25 '20 15:08 kungfoome

Dsc resources should only apply changes when a resource is detected to be not in the desired state, so Always does not fit this pattern. I only see two options; only set the password when the user is initially created or always set the password when it is detected to not be in the desired state.

Making a breaking change to rename this parameter seems overkill to me. Much better to just add a better description for it in the help/Wiki.

X-Guardian avatar Aug 25 '20 16:08 X-Guardian

Dsc resources should only apply changes when a resource is detected to be not in the desired state, so Always does not fit this pattern. I only see two options; only set the password when the user is initially created or always set the password when it is detected to not be in the desired state.

Making a breaking change to rename this parameter seems overkill to me. Much better to just add a better description for it in the help/Wiki.

Makes sense. This is what ansible does or is supposed to do. Currently, 'Always' is broken and its the same two states you mentioned.

https://github.com/ansible/ansible/issues/58246

I'm indifferent to whatever is decided. The only major thing from me is just adding a state to only set the password when the account is created.

kungfoome avatar Aug 25 '20 16:08 kungfoome

That setting is already there @kungfu71186, use PasswordNeverResets=$true

X-Guardian avatar Aug 25 '20 16:08 X-Guardian

That setting is already there @kungfu71186, use PasswordNeverResets=$true

I see, so when this is true it only sets the password on create and when false it updates when the passwords differ. In that case, then yeah, it does make sense.

kungfoome avatar Aug 25 '20 16:08 kungfoome

Yes the two states already exist and works. It was just the naming of the parameter that I thought could be more intuitive.

But since that is a breaking change (eventually, first we could deprecate the old parameter name) we could do it when there are more breaking changes to be merged. But if we decide to just improve the documentation and provide a better example then that might be sufficient.

johlju avatar Aug 25 '20 17:08 johlju