Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

xADUser: Add ChangePasswordAtLogon Parameter #371

Merged
merged 4 commits into from
Jun 15, 2019
Merged

xADUser: Add ChangePasswordAtLogon Parameter #371

merged 4 commits into from
Jun 15, 2019

Conversation

X-Guardian
Copy link
Contributor

@X-Guardian X-Guardian commented Jun 13, 2019

Pull Request (PR) description

This PR adds the ChangePasswordAtLogon parameter to the xADUser Resource.

It also adds an error exception to check whether the ChangePasswordAtLogon and PasswordNeverExpires properties have both been set to true, which is invalid.

This Pull Request (PR) fixes the following issues

This Pull Request (PR) partly fixes the following issues

Task list

  • Added an entry under the Unreleased section in the CHANGELOG.md.
    Entry should say what was changed, and how that affects users (if applicable).
  • Resource documentation added/updated in README.md.
  • Resource parameter descriptions added/updated in README.md, schema.mof
    and comment-based help.
  • Comment-based help added/updated.
  • Localization strings added/updated in all localization files as appropriate.
  • Examples appropriately added/updated.
  • Unit tests added/updated. See DSC Resource Testing Guidelines.
  • Integration tests added/updated (where possible). See DSC Resource Testing Guidelines.
  • New/changed code adheres to DSC Resource Style Guidelines and Best Practices.

This change is Reviewable

@codecov-io
Copy link

codecov-io commented Jun 13, 2019

Codecov Report

Merging #371 into dev will decrease coverage by <1%.
The diff coverage is 90%.

Impacted file tree graph

@@         Coverage Diff         @@
##            dev   #371   +/-   ##
===================================
- Coverage    91%    91%   -1%     
===================================
  Files        19     19           
  Lines      2307   2317   +10     
  Branches     10     10           
===================================
+ Hits       2115   2124    +9     
- Misses      182    183    +1     
  Partials     10     10

@X-Guardian X-Guardian marked this pull request as ready for review June 14, 2019 00:02
@johlju johlju added the needs review The pull request needs a code review. label Jun 15, 2019
Copy link
Member

@johlju johlju left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewed 3 of 4 files at r1, 2 of 2 files at r2.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@johlju
Copy link
Member

johlju commented Jun 15, 2019

@X-Guardian thank you for closing all these issue! 😃 🙇 Merging this as soon as the tests passes again (pushed the latest changes from dev).

@johlju johlju added ready for merge The pull request was approved by the community and is ready to be merged by a maintainer. and removed needs review The pull request needs a code review. labels Jun 15, 2019
@johlju johlju merged commit 5cd649a into dsccommunity:dev Jun 15, 2019
@johlju johlju removed the ready for merge The pull request was approved by the community and is ready to be merged by a maintainer. label Jun 15, 2019
@X-Guardian X-Guardian deleted the xADUser-ChangePasswordAtLogon-Add branch June 15, 2019 11:54
@X-Guardian
Copy link
Contributor Author

No problem @johlju. I need this for the current project I'm working on.

Looking at the code in the xADUser resource, there are issues with it processing parameters that are set to null or an empty string. (A common problem in DSC resources from what I've seen). I'll create an issue for this and raise a pull request to fix it.
I'm also generalising the code that was added to support the ServicePrincipalNames array property so that it can be used for easily adding other array properties, such as ProxyAddresses which would be useful, and is requested in #254.

Is there any official release schedule for DSC modules or is it ad hoc?

@X-Guardian X-Guardian restored the xADUser-ChangePasswordAtLogon-Add branch June 15, 2019 19:53
@X-Guardian
Copy link
Contributor Author

@johlju, I've discovered an issue with this PR. I've missed adding ADProperty = 'pwdLastSet' to the ChangePasswordAtLogon property of the adPropertyMap.

What's the process in this circumstance? I've restored my branch and pushed the update. Can you revert the Merge?

@johlju
Copy link
Member

johlju commented Jun 16, 2019

Ah, good catch! Easiest is to rebase against dev, then send in a new PR that adds this. You can just create a new branch too with that change. 🙂

It's more work reverting the PR, and in the end I think you need to send in a new PR anyway, let us only revert commits in dev when and if the entire PR must be reverted. 🙂

johlju pushed a commit that referenced this pull request Jun 16, 2019
- Changes to xADUser
  - This PR adds the missing pwdLastSet ADProperty to the ChangePasswordAtLogon adPropertyMap for PR #371
@johlju
Copy link
Member

johlju commented Jun 16, 2019

@X-Guardian Sorry, missed answering questions here

Looking at the code in the xADUser resource, there are issues with it processing parameters that are set to null or an empty string. (A common problem in DSC resources from what I've seen). I'll create an issue for this and raise a pull request to fix it.

Would this be solved (or helped to be solved) by using the helper function Compare-ResourcePropertyState and the pattern added to the resource xADObjectEnabledState

https://github.com/PowerShell/xActiveDirectory/blob/f26ddf5e4aa1b92453507c27ec8a225b6d0b54b0/DSCResources/MSFT_xADObjectEnabledState/MSFT_xADObjectEnabledState.psm1#L184

https://github.com/PowerShell/xActiveDirectory/blob/f26ddf5e4aa1b92453507c27ec8a225b6d0b54b0/DSCResources/MSFT_xADObjectEnabledState/MSFT_xADObjectEnabledState.psm1#L329

Is there any official release schedule for DSC modules or is it ad hoc?

Normally one week after each DSC Community Call which is ~6 weeks apart (next call is scheduled for June 19:th). You can add it to your calendar by downloading the zip (ICS) file here https://github.com/PowerShell/DscResources/tree/master/CommunityCalls.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

xADUser: Unable to add ChangePasswordAtLogon to the New-ADUser cmdlet
3 participants