[PUP-6586] User resource always reports password changed when account disabled Created: 2016/08/04 Updated: 2018/04/03 Resolved: 2016/08/24 |
|
Status: | Closed |
Project: | Puppet |
Component/s: | Windows |
Affects Version/s: | PUP 4.5.3 |
Fix Version/s: | PUP 4.6.1 |
Type: | Bug | Priority: | Normal |
Reporter: | Adam Bottchen | Assignee: | Rob Reynolds |
Resolution: | Fixed | Votes: | 0 |
Labels: | customer-escalation | ||
Remaining Estimate: | Not Specified | ||
Time Spent: | Not Specified | ||
Original Estimate: | Not Specified |
Attachments: |
![]() |
||||||||||||||||||||
Issue Links: |
|
||||||||||||||||||||
Template: | customfield_10700 145290 | ||||||||||||||||||||
Acceptance Criteria: | Make sure that we don't report a password change when the account is disabled when managing the password. |
||||||||||||||||||||
Story Points: | 2 | ||||||||||||||||||||
Sprint: | Windows 2016-08-24 | ||||||||||||||||||||
Method Found: | Customer Feedback | ||||||||||||||||||||
Release Notes: | Bug Fix | ||||||||||||||||||||
Release Notes Summary: | Previously, when Puppet is managing accounts that are disabled on Windows, it would attempt to change the password every time as it didn't recognize that the logon failure was due to the account being disabled. This causes both reporting and event log messages surrounding these changes. Now Puppet recognizes the account is disabled and no longer makes attempts to change the password while the account is not enabled. |
Description |
On Windows 2008R2, when managing a local user the password will always report as changed if the user's account is disabled. This appears to be due to the provider not checking for the ERROR_ACCOUNT_DISABLED return code as outlined in To recreate: 1. Create a new local user, "testuser", on Win 2k8R2 (also seen on 2012) and set the account to disabled. "`. Note that no matter how many times it is run, the output will report "Notice: /Stage[main]/Main/User[testuser]/password: changed password". "`. Note that the notice is no longer shown. |
Comments |
Comment by Moses Mendoza [ 2016/08/04 ] | |||||||
reproduced in puppet/stable as of 70426ea1effce8aaa545db911c3f340c09c25f56 | |||||||
Comment by Ethan Brown [ 2016/08/04 ] | |||||||
Due to the way that passwords are checked with the LogonUser API, an account has to be in an enabled state for Puppet to successfully login. Based on a successful login, it knows the current password is correct - but it currently only uses success as an indicator. So after a quick test, it looks like we have a pretty good option. ERROR_ACCOUNT_DISABLED - 1331 (from
This would be the simplest fix for us. Other options for the sake of discussion if for some reason the above doesn't work out:
| |||||||
Comment by Moses Mendoza [ 2016/08/05 ] | |||||||
In this option would we (can we) first do a check for if an account is disabled before actually attempting the logon? If so, I think this may also be one to really consider - maybe it depends on how much it matters that we generate failed login attempts in the security log. This option won't generate spurious/recurring logon failures in the security log (which e.g., potentially obscure our user's audit(s), security measures, etc). | |||||||
Comment by Ethan Brown [ 2016/08/05 ] | |||||||
We can indeed verify disabled status on the ADSI User with the AccountDisabled property. Of course, we're hoping to move away from ADSI in the future to something based more on the Windows API - but that's still a bit out. | |||||||
Comment by Moses Mendoza [ 2016/08/10 ] | |||||||
I think this may not be the best path after all - I think we should still do the password validation even when the account is disabled. Their use-case and the spirit of the ticket is that they want to make sure the password is managed even though the account is disabled. We're not really managing the password if we check and skip the validation if the account is disabled. I can see a world where the account is disabled, someone (outside of puppet) changes the password, and then later a walmart admin enables the account, tries to log in, and the password isn't the one puppet was "managing" - at least until puppet runs again. This would be quite surprising. I think the best path is to validate the password and handle that error so that we don't try the reset. | |||||||
Comment by Moses Mendoza [ 2016/08/12 ] | |||||||
WIP pushed https://github.com/MosesMendoza/puppet/commit/6f91c87de98f6884f39cf32f7cd9c37980815541. still needs tests | |||||||
Comment by Ethan Brown [ 2016/08/19 ] | |||||||
Merged to stable in https://github.com/puppetlabs/puppet/commit/352dde3557a8ca1854f411501fefda7c3ffcd40e | |||||||
Comment by Glenn Sarti [ 2016/08/19 ] | |||||||
Merged to master at; | |||||||
Comment by Ethan Brown [ 2016/08/20 ] | |||||||
Stable has passed through the Jenkins suite at https://jenkins.puppetlabs.com/view/puppet-agent/view/stable/view/puppet-agent/job/platform_puppet-agent_intn-van-sys_suite-daily-puppet-stable/182/ against all platforms That CI run contained puppet-agent SHA ad760e51b586b6e0a6dc919610f330ef3a3b76f8 - https://github.com/puppetlabs/puppet-agent/commit/ad760e51b586b6e0a6dc919610f330ef3a3b76f8 Which includes Puppet SHA - 4e7bbb02b80f382011d4e63d40d9292ad7a4614f - https://github.com/puppetlabs/puppet/commits/4e7bbb02b80f382011d4e63d40d9292ad7a4614f Similarly it passed through the full puppet suite at https://jenkins.puppetlabs.com/view/puppet-agent/view/stable/view/puppet/job/platform_puppet_pkg-van-promote_stable/178/ based on the same Puppet SHA 4e7bbb02b80f382011d4e63d40d9292ad7a4614f This includes specs run against Ruby 2.3 - https://jenkins.puppetlabs.com/view/puppet-agent/view/stable/view/puppet/job/platform_puppet_unit-ruby-win_stable/ SHA a0516659fc55a614984ed74cf68857aee9497d78 of Puppet - at This is ready for any additional FR | |||||||
Comment by Kenn Hussey [ 2016/08/24 ] | |||||||
Ethan Brown this issue needs release notes as well... | |||||||
Comment by Rob Reynolds [ 2016/08/24 ] | |||||||
Verified as fixed. Now there is only one call that states Account currently disabled. It does not make calls to attempt to change the password. Puppet also does not report any change. | |||||||
Comment by Rob Reynolds [ 2016/08/24 ] | |||||||
Release notes have been added. |