Add module AccountManagement #9926

iSazonov · 2019-06-18T14:34:51Z

PR Summary

Add new module Microsoft.PowerShell.AccountManagement
There is only single cmdlet New-User to keep the PR as small as possible.
Later we will add other cmdlets.

PR Context

Related #2996

Module Microsoft.PowerShell.LocalAccounts was removed from PowerShell Core because of using non-public APIs.
Microsoft.PowerShell.AccountManagement uses only public APIs and based on System.DirectoryServices.AccountManagement.

It is impossible to get full backward compatibility with Microsoft.PowerShell.LocalAccounts module due to differences of the underlying API.
So the main model was chosen Microsoft ActiveDirectory module.
It will be easier for users to migrate from Microsoft.PowerShell.LocalAccounts to a more functional Microsoft.PowerShell.LocalAccounts module if necessary.

PR Checklist

assets/files.wxs

TravisEz13 · 2019-06-18T17:36:21Z

is the syntax the same as the windows PowerShell cmdlet?

iSazonov · 2019-06-18T18:09:40Z

@TravisEz13 No, as mentioned in the PR description the used API is another and it impossible to keep full backward compatibility. The new design is closer to ActiveDirectory module. It make more sense because scripts can works (manage accounts) without the need to install AdminTools everyware.
I did not look at this in detail, but it is quite possible that we could get almost the same functionality using the cmdlet aliases and parameters. I suppose it should stay outside of this module.

iSazonov · 2019-06-18T18:11:44Z

Last days CI Macos frequently time out.

assets/files.wxs

iSazonov · 2019-06-19T11:25:36Z

@SteveL-MSFT Can your team review so that we can move on and add more cmdlets in follow PRs?

kilasuit · 2019-06-19T11:28:12Z

IMO this should be externalised, released to the Gallery and pulled into the output build of PowerShell (like PowerShellGet) instead of being added to the codebase

adityapatwardhan · 2019-06-19T16:15:49Z

I agree with @kilasuit. This should ideally go to PSGallery and be pulled down.

iSazonov · 2019-06-19T18:25:47Z

We have a base account management in Windows PowerShell many years. If plan is to replace Windows PowerShell with PowerShell 7.0 we need to get back most of common modules and features that we lost in porting time.

Please point me the community that is ready to take on the production of this module for all the necessary platforms. Over the past 3 years, I have repeatedly heard that we need to decouple modules. But where are the people who take it upon themselves? I am ready to make the module (and make other contributions) in the repo but I am not ready to take on the entire production cycle.
Currently I have 99.9% needed PowerShell features in-box on all my enterprise servers to manage the infrastructure. Today we (I say about enterprise) cannot replace Windows PowerShell with PowerShell Core because it is slow, consumes a lot of resources and, mostly, lost a lot of functionality. (I do not think that it is possible to fix it a few months before the 7.0 release.) We can not freely use third-party modules because strong enterprise policy - we can not trust whole Internet, validate every new version, we have isolated networks. PowerShell for us is an enterprise level management tool. If it will have not all we had many years we will have to spend time and money to implement/find workarounds (maybe write internal scripts to replace gaps or more likely to replace it with a better alternative management tool).

vexx32 · 2019-06-19T19:02:23Z

I don't think there's any necessary need to force these to be a completely separate module.

Maybe have a tracking issue or issues for such modules that we want to eventually have out of box? Separating them to a separately shipped module and/or pulling them into the PS build cycle isn't a huge undertaking in and of itself.

I don't see why we need reject this as it is at the present; we can always move to separate it to a separate module when someone willing to maintain and develop it further steps up, whether from the PS team or the community.

SteveL-MSFT · 2019-06-20T06:17:09Z

I think we would want to include this as part of the PS7 package, but also think it should be a separate module that is published to PSGallery and have its own repo. Eventually, I want to split up and move Utility and Management modules out of this repo (see #9960). Since the code is all self contained in a single folder, I think it's ok to just keep it in this repo for now and we can move it out when we do so for the others. By that time, hopefully we'll have templatized how to create the repos, contributing docs/etc.., enable CI, etc...

SteveL-MSFT · 2019-06-20T05:13:40Z

...s/Windows/Microsoft.PowerShell.AccountManagement/Microsoft.PowerShell.AccountManagement.psd1

+Author="PowerShell"
+CompanyName="Microsoft Corporation"
+Copyright="Copyright (c) Microsoft Corporation. All rights reserved."
+ModuleVersion="6.1.0.0"


I don't think we need to have module versions match the PowerShell version. This should really be 1.0.0

SteveL-MSFT · 2019-06-20T05:14:28Z

...s/Windows/Microsoft.PowerShell.AccountManagement/Microsoft.PowerShell.AccountManagement.psd1

+Copyright="Copyright (c) Microsoft Corporation. All rights reserved."
+ModuleVersion="6.1.0.0"
+CompatiblePSEditions = @("Core")
+PowerShellVersion="3.0"


This should be the minimum version that this works with which is probably 6.0. If it works with older than 6.0, then CompatiblePSEditions needs to include Desktop.

SteveL-MSFT · 2019-06-20T06:18:42Z

src/Microsoft.PowerShell.AccountManagement/commands/NewUserCommand.cs

+        /// Gets or sets a DateTime that specifies the date and time that the account expires.
+        /// </summary>
+        [Parameter(ValueFromPipelineByPropertyName = true)]
+        [AllowNull]


Null here means never expires? But you have -AccountNeverExpires below?

It doesn't work. Will remove.

SteveL-MSFT · 2019-06-20T06:19:27Z

src/Microsoft.PowerShell.AccountManagement/commands/NewUserCommand.cs

+        {
+            get;
+            set;
+        } = ContextType.Domain;


Does it make sense to be Domain by default? Does this work on non-Windows?

It is only Windows API. Although it is based on "providers" and we could get support Unix PAM or something like in future.

I choose the default as enterprise user to be close to New-ADUser cmdlet from ActiveDirectory module.

assets/files.wxs

PoshChan · 2019-12-27T15:08:03Z

assets/files.wxs

@@ -1964,6 +1964,11 @@
            <File Id="fil6B026F6D5210498B99E5F14386F4F90B" KeyPath="yes" Source="$(env.ProductSourcePath)\Modules\ThreadJob\Microsoft.PowerShell.ThreadJob.dll" />


@iSazonov, your last commit had 7 failures in PowerShell-CI-windows
(These are 5 of the failures)

Validate AccountManagement user cmdlets.Validate New-User.Can create New-User with only name

Expected exactly 'TestUserNew', but got $null. at <ScriptBlock>, D:\a\1\s\test\powershell\Modules\Microsoft.PowerShell.AccountManagement\AccountManagement.User.Tests.ps1: line 27 27: $result.Name | Should -BeExactly TestUserNew

Validate AccountManagement user cmdlets.Validate New-User.Can create New-User with password

Expected exactly 'TestUserNew', but got $null. at <ScriptBlock>, D:\a\1\s\test\powershell\Modules\Microsoft.PowerShell.AccountManagement\AccountManagement.User.Tests.ps1: line 38 38: $result.Name | Should -BeExactly TestUserNew

Validate AccountManagement user cmdlets.Validate New-User.Can create New-User with explicit properties in local machine

Expected exactly 'samq2', but got $null. at <ScriptBlock>, D:\a\1\s\test\powershell\Modules\Microsoft.PowerShell.AccountManagement\AccountManagement.User.Tests.ps1: line 54 54: $result.Name | Should -BeExactly "samq2"

Validate AccountManagement user cmdlets.Validate Remove-User.Can remove user account with pipeline

Expected an exception, with FullyQualifiedErrorId 'UserAlreadyRemoved,Microsoft.PowerShell.Commands.RemoveUserCommand' to be thrown, but no exception was thrown. at <ScriptBlock>, D:\a\1\s\test\powershell\Modules\Microsoft.PowerShell.AccountManagement\AccountManagement.User.Tests.ps1: line 137 137: { $userIdentity | Remove-User -ErrorAction Stop } | Should -Throw -ErrorId "UserAlreadyRemoved,Microsoft.PowerShell.Commands.RemoveUserCommand"

Validate AccountManagement user cmdlets.Validate Remove-User.Can remove user account with parameter

Expected no exception to be thrown, but an exception "Cannot bind argument to parameter 'Identity' because it is null." was thrown from D:\a\1\s\test\powershell\Modules\Microsoft.PowerShell.AccountManagement\AccountManagement.User.Tests.ps1:141 char:41 + { Remove-User -Identity $userIdentity -ErrorAction St ? + ~~~~~~~~~~~~~. at <ScriptBlock>, D:\a\1\s\test\powershell\Modules\Microsoft.PowerShell.AccountManagement\AccountManagement.User.Tests.ps1: line 141 141: { Remove-User -Identity $userIdentity -ErrorAction Stop } | Should -Not -Throw

assets/files.wxs

PoshChan · 2020-01-13T15:36:26Z

assets/files.wxs

@@ -1964,6 +1964,11 @@
            <File Id="fil6B026F6D5210498B99E5F14386F4F90B" KeyPath="yes" Source="$(env.ProductSourcePath)\Modules\ThreadJob\Microsoft.PowerShell.ThreadJob.dll" />


@iSazonov, your last commit had 7 failures in PowerShell-CI-windows
(These are 5 of the failures)

Validate AccountManagement user cmdlets.Validate New-User.Can create New-User with only name

Expected exactly 'TestUserNew', but got $null. at <ScriptBlock>, D:\a\1\s\test\powershell\Modules\Microsoft.PowerShell.AccountManagement\AccountManagement.User.Tests.ps1: line 27 27: $result.Name | Should -BeExactly TestUserNew

Validate AccountManagement user cmdlets.Validate New-User.Can create New-User with password

Expected exactly 'TestUserNew', but got $null. at <ScriptBlock>, D:\a\1\s\test\powershell\Modules\Microsoft.PowerShell.AccountManagement\AccountManagement.User.Tests.ps1: line 38 38: $result.Name | Should -BeExactly TestUserNew

Validate AccountManagement user cmdlets.Validate New-User.Can create New-User with explicit properties in local machine

Expected exactly 'samq2', but got $null. at <ScriptBlock>, D:\a\1\s\test\powershell\Modules\Microsoft.PowerShell.AccountManagement\AccountManagement.User.Tests.ps1: line 54 54: $result.Name | Should -BeExactly "samq2"

Validate AccountManagement user cmdlets.Validate Remove-User.Can remove user account with pipeline

Expected no exception to be thrown, but an exception "Cannot access a disposed object. Object name: 'PrincipalContext'." was thrown from D:\a\1\s\test\powershell\Modules\Microsoft.PowerShell.AccountManagement\AccountManagement.User.Tests.ps1:134 char:35 + ? { $userIdentity | Remove-User -ErrorAction Stop } | Sho ? + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~. at <ScriptBlock>, D:\a\1\s\test\powershell\Modules\Microsoft.PowerShell.AccountManagement\AccountManagement.User.Tests.ps1: line 134 134: { $userIdentity | Remove-User -ErrorAction Stop } | Should -Not -Throw

Validate AccountManagement user cmdlets.Validate Remove-User.Can remove user account with parameter

Expected no exception to be thrown, but an exception "Cannot bind argument to parameter 'Identity' because it is null." was thrown from D:\a\1\s\test\powershell\Modules\Microsoft.PowerShell.AccountManagement\AccountManagement.User.Tests.ps1:141 char:41 + { Remove-User -Identity $userIdentity -ErrorAction St ? + ~~~~~~~~~~~~~. at <ScriptBlock>, D:\a\1\s\test\powershell\Modules\Microsoft.PowerShell.AccountManagement\AccountManagement.User.Tests.ps1: line 141 141: { Remove-User -Identity $userIdentity -ErrorAction Stop } | Should -Not -Throw

ghost · 2020-05-27T02:03:44Z

This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days.
Mainainer, Please provide feedback and/or mark it as Waiting on Author

iSazonov · 2020-06-02T05:20:01Z

Closed with #12242 (comment)

iSazonov added the CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log label Jun 18, 2019

iSazonov added this to the 7.0.0-preview.2 milestone Jun 18, 2019

iSazonov requested review from daxian-dbw, rjmholt, TravisEz13, SteveL-MSFT and adityapatwardhan June 18, 2019 14:34

PoshChan reviewed Jun 18, 2019

View reviewed changes