search

dsccommunity / SecurityPolicyDsc

A wrapper around secedit.exe to configure local security policies
MIT License
177 stars 53 forks source link

Issue 126 add cim identity options #142

Closed ccunning closed 4 years ago

ccunning commented 4 years ago

Pull Request (PR) description

Added support for more SDDL SID constants [Issue #126]

This Pull Request (PR) fixes the following issues

Task list

This change is Reviewable

codecov[bot] commented 4 years ago

Codecov Report

Merging #142 into dev will increase coverage by 1%. The diff coverage is 98%.

Impacted file tree graph

@@        Coverage Diff         @@
##           dev   #142   +/-   ##
==================================
+ Coverage   89%    90%   +1%     
==================================
  Files        5      5           
  Lines      498    573   +75     
==================================
+ Hits       447    521   +74     
- Misses      51     52    +1
ccunning commented 4 years ago

Hey, I have not received anything about the license CLA. I'm not sure if the bot is stuck, or what is going on, but the check has been stuck for 12 hours. Is there any way I can get that passed, or have the check restarted?

Any help would be appreciated. Thanks!

gaelcolas commented 4 years ago

Hi @ccunning, Thanks for the contribution. Sorry for the delay, but if we take too long, feel free to come on the DSC channel of the PowerShell slack/discord and remind us: https://powershell.slack.com/ (register here http://slack.poshcode.org/)

Since the repositories have been transferred outside of Microsoft, the CLA bot should have been disabled and is not required anymore (I'll fix that shortly). Also, before those changes can be released, this repository should be moved to the new CI pipeline automation as described here: https://dsccommunity.org/blog/convert-a-module-for-continuous-delivery

This is only for releases, merging PRs is ok as long as the tests/quality work through AppVeyor.

ccunning commented 4 years ago

Hey @gaelcolas, Thanks for getting back to me on this. No worries on the delay, I know everyone is busy.

In order for me to be able to submit the pull request without the license/cla check, then, should I just close this PR and open up a new one? What's the best next step?

johlju commented 4 years ago

You don’t have to do anything. We will just remove the “required” (branch protection rule) for the CLA status check.

gaelcolas commented 4 years ago

Which I just did for the dev branch. Also, as I said:

Also, before those changes can be released, this repository should be moved to the new CI pipeline automation as described here: dsccommunity.org/blog/convert-a-module-for-continuous-delivery

ccunning commented 4 years ago

Modules/SecurityPolicyResourceHelper/SecurityPolicyResourceHelper.psm1, line 575 at r2 (raw file):

Previously, johlju (Johan Ljunggren) wrote…
> ``` > ConvertFrom-SDDLDescriptor > ``` Can you please add unit tests for these new functions? https://github.com/dsccommunity/SecurityPolicyDsc/blob/dev/Tests/Unit/SecurityPolicyResourceHelper.tests.ps1

Done. I added some unit tests for ConvertFrom-SDDLDescriptor and ConvertTo-SDDLDescriptor with various possibilities.

johlju commented 4 years ago

I have merged this but I cannot release this since the CI pipeline has not yet been converted, see issue https://github.com/dsccommunity/SecurityPolicyDsc/issues/143

johlju commented 4 years ago

@ccunning Thanks for this. Great work! 😃

ccunning commented 4 years ago

Thanks! Glad I could help. Sorry it took me so long... things have been a little crazy.

johlju commented 4 years ago

No worries, I know how it is! We get this released as soon as possible.