ADGroup: Updating means to retrieve group members #617

jeremyciak · 2020-06-24T01:48:10Z

Pull Request (PR) description

This Pull Request is intended to offer a viable mechanism for the Get-TargetResource aspect of the ADGroup resource when dealing with group members through a one-way trust.

This Pull Request (PR) fixes the following issues

Fixes ADGroup: Group Members From One-Way Trusted Domain Break Functionality #616

Task list

Added an entry to the change log under the Unreleased section of the
file CHANGELOG.md. Entry should say what was changed and how that
affects users (if applicable), and reference the issue being resolved
(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 Community Testing Guidelines.
Integration tests added/updated (where possible). See DSC Community Testing Guidelines.
New/changed code adheres to DSC Community Style Guidelines.

This change is

jeremyciak · 2020-06-24T01:49:58Z

#616

codecov · 2020-06-24T20:11:39Z

Codecov Report

Merging #617 into master will increase coverage by 0%.
The diff coverage is 100%.

@@          Coverage Diff          @@
##           master   #617   +/-   ##
=====================================
  Coverage      98%    98%           
=====================================
  Files          24     24           
  Lines        3110   3154   +44     
=====================================
+ Hits         3049   3093   +44     
  Misses         61     61

X-Guardian · 2020-06-25T21:16:39Z

@jeremyciak, please continue the discussion on the issue thread, before you make any further additions to this PR.

jeremyciak · 2020-06-25T21:22:23Z

@X-Guardian, I apologize! I had a reply ready to comment on the issue thread and never actually hit the button to send it through. I have added that comment now.

X-Guardian · 2020-07-08T15:45:32Z

@jeremyciak, can you edit the PR description and add some detail and link to the issue.

jeremyciak · 2020-07-08T15:53:35Z

@X-Guardian I have added a description and link to the issue in the initial PR comment.

X-Guardian

Reviewable status: 0 of 3 files reviewed, 6 unresolved discussions (waiting on @jeremyciak)

CHANGELOG.md, line 29 at r6 (raw file):

### Fixed

- ADGroup

Can you move this section to be alphabetically ordered in the list, and add detail stating from another domain over an external one-way trust. Also, I don't think the second sentence starting 'Switched...' is now relevant?

source/DSCResources/MSFT_ADGroup/MSFT_ADGroup.psm1, line 220 at r6 (raw file):

Quoted 7 lines of code…

                    # This creates a filter which multiple -or statements matching the various DistinguishedName values of the Members
                    # This allows for a single Get-ADObject call instead of multiple calls in a loop
                    $adObjectFilter = @(
                        "(DistinguishedName -eq '",
                        ($adGroup.Members -join "') -or (DistinguishedName -eq '"),
                        "')"
                    ) -join ''

I'm concerned that if the group has a large number of members, this code will blow the maximum size of a filter (whatever that may be). How about just piping $adGroup.Members into Get-ADObject?

source/DSCResources/MSFT_ADGroup/MSFT_ADGroup.psm1, line 237 at r6 (raw file):

                else
                {
                    Write-Error -Exception $_.Exception

There are common functions imported from DscResource.Common to handle exceptions. I would suggest using New-InvalidOperationException here. A standard pattern is to add an error message string to the localization file en-US\MSFT_ADGroup.strings.psd1 and reference it here like so:

$errorMessage = $script:localizedData.RetreivingGroupMembersError
New-InvalidOperationException -Message $errorMessage -ErrorRecord $_

source/DSCResources/MSFT_ADGroup/MSFT_ADGroup.psm1, line 779 at r6 (raw file):

                        # This creates a filter which multiple -or statements matching the various DistinguishedName values of the Members
                        # This allows for a single Get-ADObject call instead of multiple calls in a loop
                        $adObjectFilter = @(

Same comments as above.

tests/Unit/MSFT_ADGroup.Tests.ps1, line 73 at r6 (raw file):

            SamAccountName    = 'USER1'
            SID               = 'S-1-5-21-1131554080-2861379300-292325817-1106'
            ObjectSID         = 'S-1-5-21-1131554080-2861379300-292325817-1106'

Rather than adding ObjectSID to the current ADUser variables which are used as mocks for Get-ADGroupMember output, can you create new variables with just ObjectSID and not SID which can then be used to mock the Get-ADObjectoutput.

tests/Unit/MSFT_ADGroup.Tests.ps1, line 664 at r6 (raw file):

        #region Function Set-TargetResource
        Describe 'ADGroup\Set-TargetResource' {

Can we look at improving the test code coverage for the Set-TargetResource function. See CodeCov.io for a diff report showing lines not covered.

X-Guardian · 2020-07-08T16:13:22Z

Hi @jeremyciak, don't know if you have used Reviewable before, but if you haven't, press the Reviewable button in the PR description to see all the review comments in the context of the code. You can then respond to each one individually there.

jeremyciak

Reviewable status: 0 of 3 files reviewed, 6 unresolved discussions (waiting on @jeremyciak and @X-Guardian)

CHANGELOG.md, line 29 at r6 (raw file):

Previously, X-Guardian (Simon Heather) wrote…

Can you move this section to be alphabetically ordered in the list, and add detail stating from another domain over an external one-way trust. Also, I don't think the second sentence starting 'Switched...' is now relevant?

I have moved the section alphabetically now underneath the ADDomainController item. I think the line starting with 'Switched...' is still relevant because that is how I am pulling in the DistinguishedName property for the group members and then I am sending that to the Get-ADObject call to resolve the desired MembershipAttribute.

jeremyciak

Reviewable status: 0 of 3 files reviewed, 6 unresolved discussions (waiting on @jeremyciak and @X-Guardian)

source/DSCResources/MSFT_ADGroup/MSFT_ADGroup.psm1, line 220 at r6 (raw file):

Previously, X-Guardian (Simon Heather) wrote…

                    # This creates a filter which multiple -or statements matching the various DistinguishedName values of the Members
                    # This allows for a single Get-ADObject call instead of multiple calls in a loop
                    $adObjectFilter = @(
                        "(DistinguishedName -eq '",
                        ($adGroup.Members -join "') -or (DistinguishedName -eq '"),
                        "')"
                    ) -join ''

I'm concerned that if the group has a large number of members, this code will blow the maximum size of a filter (whatever that may be). How about just piping $adGroup.Members into Get-ADObject?

I have updated the code to pipe the Members into Get-ADObject instead as requested.

source/DSCResources/MSFT_ADGroup/MSFT_ADGroup.psm1, line 237 at r6 (raw file):

Previously, X-Guardian (Simon Heather) wrote…

There are common functions imported from DscResource.Common to handle exceptions. I would suggest using New-InvalidOperationException here. A standard pattern is to add an error message string to the localization file en-US\MSFT_ADGroup.strings.psd1 and reference it here like so:
$errorMessage = $script:localizedData.RetreivingGroupMembersError
New-InvalidOperationException -Message $errorMessage -ErrorRecord $_

I can switch to the New-InvalidOperationException to standardize on that, however I don't know what to put for the $errorMessage here since this is an else statement where I want to surface unknown errors other than the one I am handling.

jeremyciak

Reviewable status: 1 of 7 files reviewed, 9 unresolved discussions (waiting on @X-Guardian)

CHANGELOG.md, line 29 at r6 (raw file):

Previously, X-Guardian (Simon Heather) wrote…

OK, but this needs to be a single sentence, and needs to include the fact that the issue being fixed is when the other domain is accessed over an external one-way trust.

I have updated the code to reflect your requested adjustments

source/DSCResources/MSFT_ADGroup/MSFT_ADGroup.psm1, line 197 at r7 (raw file):

Previously, X-Guardian (Simon Heather) wrote…

Can we keep all the lines to under 120 chars? You can split with backticks if there is not a natural PowerShell split point in the line.

I have updated the code with something I believe will be suitable for this request

source/DSCResources/MSFT_ADGroup/MSFT_ADGroup.psm1, line 212 at r7 (raw file):

getADObjectCommonParameters
I have updated the code with these requests and some simple modifications

source/DSCResources/MSFT_ADGroup/MSFT_ADGroup.psm1, line 227 at r7 (raw file):

Previously, X-Guardian (Simon Heather) wrote…

Is there a scenario where you may not get a unique object list?

I did this for the testing and mocking. If i mock Get-ADObject to return the same two objects every time it is called then I need to consolidate those mocked objects at the end. This was one of the reasons I had initially built a giant Filter statement for Get-ADObject instead of using the loop: so that when it was mocked it would be the single call result set.

source/DSCResources/MSFT_ADGroup/MSFT_ADGroup.psm1, line 779 at r7 (raw file):

Previously, X-Guardian (Simon Heather) wrote…

We need to deal with the scenario where the ForeignSecurityPrincipal is orphaned and therefore this function call is going to fail with a System.Security.Principal.IdentityNotMappedException. To prevent the DscResource from failing we need to catch this exception. In this scenario I suggest we just return the untranslated DN.

I handled this request within the Resolve-SamAccountName function.

source/Modules/ActiveDirectoryDsc.Common/ActiveDirectoryDsc.Common.psm1, line 2494 at r7 (raw file):

Previously, X-Guardian (Simon Heather) wrote…

You need to add this function name to the ActiveDirectoryDsc.Common.psd1 file, in the FunctionsToExport property.

Done.

source/Modules/ActiveDirectoryDsc.Common/ActiveDirectoryDsc.Common.psm1, line 2495 at r7 (raw file):

Previously, X-Guardian (Simon Heather) wrote…

Can you also run the Build-Readme.ps1 script in this folder to update the docs with your new function.

I tried performing this task but I cannot get it to behave appropriately. Are there prerequisites needed for this that are not packaged with the repository? I found that New-MarkdownHelp is a function from the platyPS module so I tried installing/importing that module and it seemed to make things worse.

tests/Unit/MSFT_ADGroup.Tests.ps1, line 304 at r7 (raw file):

Previously, X-Guardian (Simon Heather) wrote…

Can you change this to 'Throws the correct error...' and add a check for the correct error message in the Should -Throw command. You can reference the localized data. There are examples of this in the ADManagedServiceAccount tests.

Done.

tests/Unit/MSFT_ADGroup.Tests.ps1, line 949 at r7 (raw file):

Previously, X-Guardian (Simon Heather) wrote…

Same comment as above.

Done.

jeremyciak

Reviewable status: 1 of 8 files reviewed, 9 unresolved discussions (waiting on @X-Guardian)

source/DSCResources/MSFT_ADGroup/MSFT_ADGroup.psm1, line 227 at r7 (raw file):

Previously, jeremyciak wrote…

I did this for the testing and mocking. If i mock Get-ADObject to return the same two objects every time it is called then I need to consolidate those mocked objects at the end. This was one of the reasons I had initially built a giant Filter statement for Get-ADObject instead of using the loop: so that when it was mocked it would be the single call result set.

I removed this and simply kept the Members to a single object for testing so that there would be no duplication in multiple looped calls. I'm sure it's better practice to code the tests around the code instead of vice versa anyway!

jeremyciak · 2020-07-12T03:58:00Z

Woohoo! Successful code coverage!

@X-Guardian, thank you so much for all of your time and effort spent guiding and educating me in all of this. I have learned a ton and I feel like a much more worthy contributor now.

X-Guardian

Reviewed 2 of 7 files at r8.
Reviewable status: 3 of 8 files reviewed, 5 unresolved discussions (waiting on @jeremyciak and @X-Guardian)

source/DSCResources/MSFT_ADGroup/MSFT_ADGroup.psm1, line 197 at r7 (raw file):

Previously, jeremyciak wrote…

I have updated the code with something I believe will be suitable for this request

Still a few lines over 120 chars. Are you using Visual Studio Code as your IDE? If so, you can add "editor.rulers": [ 120 ] to your settings.json which will give you a helpful ruler when you are editing the file.

source/DSCResources/MSFT_ADGroup/MSFT_ADGroup.psm1, line 212 at r7 (raw file):

Previously, jeremyciak wrote…

getADObjectCommonParameters
I have updated the code with these requests and some simple modifications

I now get an Exception calling "Add" with "2" argument(s): "Item has already been added. Key in dictionary: 'Filter' Key being added: 'Filter'" after this change if I have a group with more than one member and at least one of them is a ForeignSecurityPrinicipal do you want to take another look?

source/DSCResources/MSFT_ADGroup/MSFT_ADGroup.psm1, line 227 at r7 (raw file):

Previously, jeremyciak wrote…

I removed this and simply kept the Members to a single object for testing so that there would be no duplication in multiple looped calls. I'm sure it's better practice to code the tests around the code instead of vice versa anyway!

There is a pattern for mocking a function and returning a different value on each call. See Change a mocks return value based on number of times called for details.

source/Modules/ActiveDirectoryDsc.Common/ActiveDirectoryDsc.Common.psm1, line 2495 at r7 (raw file):

Previously, jeremyciak wrote…

I tried performing this task but I cannot get it to behave appropriately. Are there prerequisites needed for this that are not packaged with the repository? I found that New-MarkdownHelp is a function from the platyPS module so I tried installing/importing that module and it seemed to make things worse.

OK, I'll take a look.

tests/Unit/MSFT_ADGroup.Tests.ps1, line 331 at r8 (raw file):

                }

                { Get-TargetResource @testPresentParams -ErrorAction Stop } |

Do you need the -ErrorAction Stop?

jeremyciak

Reviewable status: 3 of 8 files reviewed, 5 unresolved discussions (waiting on @X-Guardian)

source/DSCResources/MSFT_ADGroup/MSFT_ADGroup.psm1, line 197 at r7 (raw file):

Previously, X-Guardian (Simon Heather) wrote…

Still a few lines over 120 chars. Are you using Visual Studio Code as your IDE? If so, you can add "editor.rulers": [ 120 ] to your settings.json which will give you a helpful ruler when you are editing the file.

Thanks for that tip. I have added that setting specific to `"[powershell]". I have made sure all of my lines of code added to this file are below that threshold.

source/DSCResources/MSFT_ADGroup/MSFT_ADGroup.psm1, line 212 at r7 (raw file):

Previously, X-Guardian (Simon Heather) wrote…

I now get an Exception calling "Add" with "2" argument(s): "Item has already been added. Key in dictionary: 'Filter' Key being added: 'Filter'" after this change if I have a group with more than one member and at least one of them is a ForeignSecurityPrinicipal do you want to take another look?

I was definitely trying to be too fancy using the Add method. I switched to just a standard "key lookup equals value" syntax like so:

$object['key'] = 'value'

source/DSCResources/MSFT_ADGroup/MSFT_ADGroup.psm1, line 227 at r7 (raw file):

Previously, X-Guardian (Simon Heather) wrote…

There is a pattern for mocking a function and returning a different value on each call. See Change a mocks return value based on number of times called for details.

Implementing this was pretty difficult, but I think I ended up with a functional solution. I also tried to be mindful for what I am hoping will lead to the means by which to mock and test any eventual efforts to support added functionality for editing the group members and not just retrieving them. I gladly welcome feedback on my implementation here.

source/DSCResources/MSFT_ADGroup/MSFT_ADGroup.psm1, line 779 at r7 (raw file):

Previously, jeremyciak wrote…

I handled this request within the Resolve-SamAccountName function.

Done.

tests/Unit/MSFT_ADGroup.Tests.ps1, line 331 at r8 (raw file):

Previously, X-Guardian (Simon Heather) wrote…

Do you need the -ErrorAction Stop?

No, apparently I do not. I have removed it.

jeremyciak

Reviewable status: 3 of 8 files reviewed, 5 unresolved discussions (waiting on @X-Guardian)

source/DSCResources/MSFT_ADGroup/MSFT_ADGroup.psm1, line 197 at r7 (raw file):

Previously, jeremyciak wrote…

Thanks for that tip. I have added that setting specific to `"[powershell]". I have made sure all of my lines of code added to this file are below that threshold.

Done.

source/DSCResources/MSFT_ADGroup/MSFT_ADGroup.psm1, line 212 at r7 (raw file):

Previously, jeremyciak wrote…

I was definitely trying to be too fancy using the Add method. I switched to just a standard "key lookup equals value" syntax like so:
$object['key'] = 'value'

Done.

source/DSCResources/MSFT_ADGroup/MSFT_ADGroup.psm1, line 227 at r7 (raw file):

Previously, jeremyciak wrote…

Implementing this was pretty difficult, but I think I ended up with a functional solution. I also tried to be mindful for what I am hoping will lead to the means by which to mock and test any eventual efforts to support added functionality for editing the group members and not just retrieving them. I gladly welcome feedback on my implementation here.

Done.

tests/Unit/MSFT_ADGroup.Tests.ps1, line 331 at r8 (raw file):

Previously, jeremyciak wrote…

No, apparently I do not. I have removed it.

Done.

X-Guardian

Reviewed 1 of 7 files at r8.
Reviewable status: 4 of 8 files reviewed, 5 unresolved discussions (waiting on @jeremyciak)

source/DSCResources/MSFT_ADGroup/MSFT_ADGroup.psm1, line 779 at r7 (raw file):

Previously, jeremyciak wrote…

Done.

Its returning the SID instead of the DN, but that's fine.

tests/Unit/MSFT_ADGroup.Tests.ps1, line 116 at r9 (raw file):

            }
            # This entry is used to represent a group member from a one-way trusted domain
            # Removing this entry or changing its ObjectClass will reduce code coverage

First line of this comment is good to explain what this object is for, but no need for the second line. Stating the obvious!

tests/Unit/MSFT_ADGroup.Tests.ps1, line 320 at r9 (raw file):

            }

            if ($fakeADGroupMembersAsADObjects.ObjectClass -contains 'foreignSecurityPrincipal')

It is not a normal pattern we use in the DscResources to have the tests modify themselves based on the test data. Can you simplify this and just have the It Calls test.

tests/Unit/MSFT_ADGroup.Tests.ps1, line 510 at r9 (raw file):

            } #end foreach attribute

            It "Passes when 'MembershipAttribute' is 'SID' and 'Get-ADGroupMember' fails due to one-way trust" {

Can we cover the MembershipAttribute = 'SID test in Get-TargetResource rather than here? We try to Mock only one function deep.

tests/Unit/MSFT_ADGroup.Tests.ps1, line 1197 at r9 (raw file):

            }

            Context "Mocking 'Get-TargetResource' and 'Get-ADGroupMember' fails due to one-way trust" {

We normally use the text Context "When..." rather than Context "Mocking..."

tests/Unit/MSFT_ADGroup.Tests.ps1, line 1265 at r9 (raw file):

                    }

                    if ($fakeADGroupMembersAsADObjects.ObjectClass -contains 'foreignSecurityPrincipal')

Same comment as above.

X-Guardian · 2020-07-13T17:49:35Z

Regarding updating the ActiveDirectoryDsc.Common docs, you just need install the platyps module, change your path to the source\Modules\ActiveDirectoryDsc.Common folder and then run the Build-Readme.ps1 script. If you get any errors, let me know what they are.

jeremyciak

Reviewable status: 4 of 8 files reviewed, 5 unresolved discussions (waiting on @X-Guardian)

source/DSCResources/MSFT_ADGroup/MSFT_ADGroup.psm1, line 779 at r7 (raw file):

Previously, X-Guardian (Simon Heather) wrote…

Its returning the SID instead of the DN, but that's fine.

Yes, I was trying to figure out how to return the DN when the function requests the SID and decided it was probably better to see if an actual use case popped up to drive that effort.

tests/Unit/MSFT_ADGroup.Tests.ps1, line 116 at r9 (raw file):

Previously, X-Guardian (Simon Heather) wrote…

First line of this comment is good to explain what this object is for, but no need for the second line. Stating the obvious!

Done.

tests/Unit/MSFT_ADGroup.Tests.ps1, line 320 at r9 (raw file):

Previously, X-Guardian (Simon Heather) wrote…

It is not a normal pattern we use in the DscResources to have the tests modify themselves based on the test data. Can you simplify this and just have the It Calls test.

Yes, this was me trying to reduce repeated code. I have reworked some of the mocking and scoping to retain some of my efforts to reduce repeated code and fulfill your request. I also addressed issues along the way.

tests/Unit/MSFT_ADGroup.Tests.ps1, line 510 at r9 (raw file):

Previously, X-Guardian (Simon Heather) wrote…

Can we cover the MembershipAttribute = 'SID test in Get-TargetResource rather than here? We try to Mock only one function deep.

I moved this into the Describe 'ADGroup\Get-TargetResource' block and tested accurate Members were returned instead.

tests/Unit/MSFT_ADGroup.Tests.ps1, line 1197 at r9 (raw file):

Previously, X-Guardian (Simon Heather) wrote…

We normally use the text Context "When..." rather than Context "Mocking..."

Done.

tests/Unit/MSFT_ADGroup.Tests.ps1, line 1265 at r9 (raw file):

Previously, X-Guardian (Simon Heather) wrote…

Same comment as above.

Done.

jeremyciak

Reviewable status: 4 of 8 files reviewed, 5 unresolved discussions (waiting on @X-Guardian)

tests/Unit/MSFT_ADGroup.Tests.ps1, line 320 at r9 (raw file):

Previously, jeremyciak wrote…

Yes, this was me trying to reduce repeated code. I have reworked some of the mocking and scoping to retain some of my efforts to reduce repeated code and fulfill your request. I also addressed issues along the way.

Done.

tests/Unit/MSFT_ADGroup.Tests.ps1, line 510 at r9 (raw file):

Previously, jeremyciak wrote…

I moved this into the Describe 'ADGroup\Get-TargetResource' block and tested accurate Members were returned instead.

Done.

jeremyciak

When I try to Import platyPS it complains that the YamlDotNet.dll conflicts with the one already loaded from the powershell-yaml module, which is one of the RequiredModules with the ActiveDirectoryDsc module. So I destroyed my session and then tried again to load the platyPS module and that worked, but when I tried to run the Build-Readme.ps1 script I get the following error:

Error loading Config C:\Users\<redacted_username>\Documents\Git\GitHub\ActiveDirectoryDsc\build.yaml.
 Are you missing dependencies?
Make sure you run './build.ps1 -ResolveDependency -tasks noop' to restore the Required modules the first time
ERROR: (7,36): error CS0012: The type 'Object' is defined in an assembly that is not referenced. You must add a reference to assembly 'mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089'.
public class StringQuotingEmitter: ChainedEventEmitter {
                                   ^

Build ABORTED C:\Users\<redacted_username>\Documents\Git\GitHub\ActiveDirectoryDsc\build.ps1. 0 tasks, 1 errors, 0 warnings 00:00:00.9484432

When I run the Build-Readme.ps1 script without messing with the platyPS module it gets much further but then I get this error instead:

Build-Readme.ps1: The 'New-MarkdownHelp' command was found in the module 'platyPS', but the module could not be loaded. For more information, run 'Import-Module platyPS'.

I don't know if this will be helpful, but just in case here is the output from $PSVersionTable:

Name                           Value
----                           -----
PSVersion                      7.0.2
PSEdition                      Core
GitCommitId                    7.0.2
OS                             Microsoft Windows 10.0.18363
Platform                       Win32NT
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0…}
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1
WSManStackVersion              3.0

Reviewable status: 4 of 8 files reviewed, 5 unresolved discussions (waiting on @X-Guardian)

X-Guardian · 2020-07-13T19:26:47Z

@jeremyciak, can you try Build-Readme.ps1 with PowerShell 5.x rather than 7?

X-Guardian · 2020-07-13T20:08:28Z

OK @jeremyciak, looking good. My one-way trust tests work and I've just successfully run the ADGroup integration tests, so just the ActiveDirectoryDsc.Common docs update and we should be good to go!

jeremyciak · 2020-07-13T20:12:50Z

@X-Guardian, I just got the docs to successfully create and those changes are in the pipeline now. Seems like if you import the platyPS module first and then let the build mechanism do its thing then the powershell-yaml module does not have issue on Powershell 5.1. Probably something that warrants investigation at some point!

X-Guardian

Yep, lets worry about that another day.

Reviewed 2 of 7 files at r8, 1 of 2 files at r9, 1 of 1 files at r10, 2 of 2 files at r11.
Reviewable status: complete! all files reviewed, all discussions resolved

X-Guardian

Excellent work @jeremyciak. LGTM

Reviewable status: complete! all files reviewed, all discussions resolved

briantist · 2020-09-04T20:11:35Z

I'ms o happy to see this fixed! #235

Updating means to obtain group members

e0fe7ee

jeremyciak mentioned this pull request Jun 24, 2020

ADGroup: Group Members From One-Way Trusted Domain Break Functionality #616

Closed

Jeremy Ciak added 8 commits June 24, 2020 09:59

Fixing formatting and adding CHANGELOG entry

a355f1b

Adding and updating format, comments, and tests

bc33fad

Removing unit test Get-ADGroupMember references

8f92154

Adding back some mocking where needed

881a403

Adding ObjectSID attribute and mock scenario

ae106a5

Removing all references of Get-ADGroupMember

dc293c8

Adding SID property back to fakeADUsers

f856c6d

Restructuring and organizing the updated logic

7627352

johlju added the needs review The pull request needs a code review. label Jun 25, 2020

Switching to try/catch logic

ce0abc8

X-Guardian added on hold The issue or pull request has been put on hold by a maintainer. and removed needs review The pull request needs a code review. labels Jun 25, 2020

Jeremy Ciak and others added 4 commits June 26, 2020 15:54

Moving to a new resource property IgnoreMembership

912a51e

Switching back to targeted try/catch scenario

3714b51

Merge branch 'master' into master

df817b8

Fixing Mocked command

53f7c41

X-Guardian added needs review The pull request needs a code review. and removed on hold The issue or pull request has been put on hold by a maintainer. labels Jul 8, 2020

X-Guardian suggested changes Jul 8, 2020

View reviewed changes

jeremyciak commented Jul 8, 2020

View reviewed changes

Jeremy Ciak added 3 commits July 11, 2020 22:13

Editing description and adding error handling

0b9ba0d

Keeping line length lower and changing some format

27f7f00

Multiple adds and fixes to increase code coverage

a7524ed

jeremyciak commented Jul 12, 2020

View reviewed changes

Adding code coverage for new common function

bba5869

jeremyciak commented Jul 12, 2020

View reviewed changes

X-Guardian suggested changes Jul 12, 2020

View reviewed changes

Jeremy Ciak added 3 commits July 12, 2020 21:14

Reducing multiple lines of code to below 120 chars

c45ac9b

Adding extensible group member mocking

e1dc570

Removing extraneous ErrorAction flag

d71c654

jeremyciak commented Jul 13, 2020

View reviewed changes

Fixing code coverage versus test passing situation

cba054e

X-Guardian suggested changes Jul 13, 2020

View reviewed changes

Implementing various fixes based on review

1a31d89

jeremyciak commented Jul 13, 2020

View reviewed changes

X-Guardian mentioned this pull request Jul 13, 2020

ADGroup: Proposal to Support Group Members Across One-Way External Trusts #619

Closed

Updating common README information

d01c2dc

X-Guardian approved these changes Jul 13, 2020

View reviewed changes

X-Guardian merged commit 1400a18 into dsccommunity:master Jul 13, 2020

X-Guardian removed the needs review The pull request needs a code review. label Aug 1, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

ADGroup: Updating means to retrieve group members #617

ADGroup: Updating means to retrieve group members #617

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ADGroup: Updating means to retrieve group members #617

ADGroup: Updating means to retrieve group members #617

Uh oh!

Conversation

Uh oh!

Pull Request (PR) description

This Pull Request (PR) fixes the following issues

Task list

Uh oh!

Uh oh!

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!