Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 0 additions & 45 deletions src/Microsoft.WSMan.Management/CredSSP.cs
Original file line number Diff line number Diff line change
Expand Up @@ -55,29 +55,6 @@ public string Role
set { role = value; }
}
private string role;

/*/// <summary>
/// Role can either "Client" or "Server".
/// </summary>
[Parameter(ParameterSetName = Client, Mandatory = true, Position = 0)]
public SwitchParameter ClientRole
{
get { return isClient; }
set { isClient = value; }
}
private bool isClient;

/// <summary>
///
/// </summary>
[Parameter(ParameterSetName = Server, Mandatory = true, Position = 0)]
public SwitchParameter ServerRole
{
get { return isServer; }
set { isServer = value; }
}
private bool isServer;*/

#endregion

#region Utilities
Expand Down Expand Up @@ -165,7 +142,6 @@ private void DisableClientSideSettings()
}
m_SessionObj.Put(helper.CredSSP_RUri, inputXml, 0);

#if !CORECLR
if (Thread.CurrentThread.GetApartmentState() == ApartmentState.STA)
{
this.DeleteUserDelegateSettings();
Expand All @@ -178,14 +154,6 @@ private void DisableClientSideSettings()
thread.Start();
thread.Join();
}
#else
{
ThreadStart start = new ThreadStart(this.DeleteUserDelegateSettings);
Thread thread = new Thread(start);
thread.Start();
thread.Join();
}
#endif

if (!helper.ValidateCreadSSPRegistryRetry(false, null, applicationname))
{
Expand Down Expand Up @@ -493,8 +461,6 @@ protected override void BeginProcessing()
throw new InvalidOperationException(message);
}
#endif
//If not running elevated, then throw an "elevation required" error message.
WSManHelper.ThrowIfNotAdministrator();
Copy link
Member Author

Choose a reason for hiding this comment

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

this is redundant from line 486


// DelegateComputer cannot be specified when Role is other than client
if ((delegatecomputer != null) && !Role.Equals(Client, StringComparison.OrdinalIgnoreCase))
Expand Down Expand Up @@ -613,7 +579,6 @@ private void EnableClientSideSettings()
//push the xml string with credssp enabled
xmldoc.LoadXml(m_SessionObj.Put(helper.CredSSP_RUri, newxmlcontent, 0));

#if !CORECLR // No ApartmentState In CoreCLR
// set the Registry using GroupPolicyObject
if (Thread.CurrentThread.GetApartmentState() == ApartmentState.STA)
{
Expand All @@ -627,14 +592,6 @@ private void EnableClientSideSettings()
thread.Start();
thread.Join();
}
#else
{
ThreadStart start = new ThreadStart(this.UpdateCurrentUserRegistrySettings);
Thread thread = new Thread(start);
thread.Start();
thread.Join();
}
#endif

if (helper.ValidateCreadSSPRegistryRetry(true, delegatecomputer, applicationname))
{
Expand Down Expand Up @@ -941,8 +898,6 @@ protected override void BeginProcessing()
throw new InvalidOperationException(message);
}
#endif
//If not running elevated, then throw an "elevation required" error message.
WSManHelper.ThrowIfNotAdministrator();
Copy link
Member

Choose a reason for hiding this comment

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

Why were these two checks removed?

Copy link
Member Author

@SteveL-MSFT SteveL-MSFT Jul 26, 2017

Choose a reason for hiding this comment

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

line 486 and 890 already does the check

Copy link
Member

Choose a reason for hiding this comment

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

Why were these two checks removed?

Copy link
Member Author

Choose a reason for hiding this comment

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

check happens twice


IWSManSession m_SessionObj = null;
try
Expand Down
11 changes: 5 additions & 6 deletions src/Microsoft.WSMan.Management/WsManHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -80,14 +80,13 @@ internal class WSManHelper
//string for operation
internal string WSManOp = null;

private ResourceManager _resourceMgr = null;
private PSCmdlet cmdletname;
private NavigationCmdletProvider _provider;

private FileStream _fs;
private StreamReader _sr;

private static ResourceManager g_resourceMgr = new ResourceManager("Microsoft.WSMan.Management.resources.WsManResources", typeof(WSManHelper).GetTypeInfo().Assembly);
private static ResourceManager _resourceMgr = new ResourceManager("Microsoft.WSMan.Management.resources.WsManResources", typeof(WSManHelper).GetTypeInfo().Assembly);


//
Expand Down Expand Up @@ -153,26 +152,26 @@ internal static void ThrowIfNotAdministrator()
System.Security.Principal.WindowsPrincipal principal = new System.Security.Principal.WindowsPrincipal(currentIdentity);
if (!principal.IsInRole(System.Security.Principal.WindowsBuiltInRole.Administrator))
{
string message = g_resourceMgr.GetString("ErrorElevationNeeded");
string message = _resourceMgr.GetString("ErrorElevationNeeded");
throw new InvalidOperationException(message);
}
}

internal string GetResourceMsgFromResourcetext(string rscname)
{
return g_resourceMgr.GetString(rscname);
return _resourceMgr.GetString(rscname);
}

static internal string FormatResourceMsgFromResourcetextS(string rscname,
params object[] args)
{
return FormatResourceMsgFromResourcetextS(g_resourceMgr, rscname, args);
return FormatResourceMsgFromResourcetextS(_resourceMgr, rscname, args);
}

internal string FormatResourceMsgFromResourcetext(string resourceName,
params object[] args)
{
return FormatResourceMsgFromResourcetextS(this._resourceMgr, resourceName, args);
return FormatResourceMsgFromResourcetextS(_resourceMgr, resourceName, args);
}

static private string FormatResourceMsgFromResourcetextS(
Expand Down
112 changes: 112 additions & 0 deletions test/powershell/Modules/Microsoft.WSMan.Management/CredSSP.Tests.ps1
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
Describe "CredSSP cmdlet tests" -Tags 'Feature','RequireAdminOnWindows' {
Copy link
Member

Choose a reason for hiding this comment

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

Why are these tests separated into separate files? We have been grouping tests for each cmdlet into a single file.

Copy link
Member Author

Choose a reason for hiding this comment

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

I suppose I can combine them. I thought to separate them because at least one test case requires non-admin.

Copy link
Member Author

Choose a reason for hiding this comment

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

Just tried it combined and start-pspester only ran the tests as elevated so that one test gets skipped. I'll have to have the non-admin tests as separate.

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually it appears you have explicitly run start-pspester with -unelevated. I'll have to take care of this myself in the script then.


BeforeAll {
$powershell = Join-Path $PSHOME "powershell"
$notEnglish = $false
$IsToBeSkipped = !$IsWindows;

$originalDefaultParameterValues = $PSDefaultParameterValues.Clone()
if ( $IsToBeSkipped )
{
$PSDefaultParameterValues["it:skip"] = $true
}
else
{
if ([System.Globalization.CultureInfo]::CurrentCulture.Name -ne "en-US")
{
$notEnglish = $true
}
}
}

AfterAll {
$global:PSDefaultParameterValues = $originalDefaultParameterValues
}

BeforeEach {
if ( ! $IsToBeSkipped )
{
$errtxt = "$testdrive/error.txt"
Remove-Item $errtxt -Force -ErrorAction SilentlyContinue
$donefile = "$testdrive/done"
Remove-Item $donefile -Force -ErrorAction SilentlyContinue
}
}

It "Error returned if invalid parameters: <description>" -TestCases @(
@{params=@{Role="Client"};Description="Client role, no DelegateComputer"},
@{params=@{Role="Server";DelegateComputer="."};Description="Server role w/ DelegateComputer"}
) {
param ($params)
{ Enable-WSManCredSSP @params } | ShouldBeErrorId "System.InvalidOperationException,Microsoft.WSMan.Management.EnableWSManCredSSPCommand"
}

It "Enable-WSManCredSSP works: <description>" -Skip:($NotEnglish -or $IsToBeSkipped) -TestCases @(
@{params=@{Role="Client";DelegateComputer="*"};description="client"},
@{params=@{Role="Server"};description="server"}
) {
param ($params)
$c = Enable-WSManCredSSP @params -Force
$c.CredSSP | Should Be $true

$c = Get-WSManCredSSP
if ($params.Role -eq "Client")
{
$c[0] | Should Match "The machine is configured to allow delegating fresh credentials to the following target\(s\):wsman/\*"
Copy link
Member

Choose a reason for hiding this comment

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

Please check for en-US before comparing.

Copy link
Member Author

Choose a reason for hiding this comment

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

will fix

}
else
{
$c[1] | Should Match "This computer is configured to receive credentials from a remote client computer"
Copy link
Member

Choose a reason for hiding this comment

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

Same as above

Copy link
Member Author

Choose a reason for hiding this comment

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

will fix

}
}

It "Disable-WSManCredSSP works: <role>" -Skip:($NotEnglish -or $IsToBeSkipped) -TestCases @(
@{Role="Client"},
@{Role="Server"}
) {
param ($role)
Disable-WSManCredSSP -Role $role | Should BeNullOrEmpty

$c = Get-WSManCredSSP
if ($role -eq "Client")
{
$c[0] | Should Match "The machine is not configured to allow delegating fresh credentials."
Copy link
Member

Choose a reason for hiding this comment

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

Same as above.

Copy link
Member Author

Choose a reason for hiding this comment

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

will fix

}
else
{
$c[1] | Should Match "This computer is not configured to receive credentials from a remote client computer"
Copy link
Member

Choose a reason for hiding this comment

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

Same as above

Copy link
Member Author

Choose a reason for hiding this comment

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

will fix

}
}

It "Call cmdlet as API" {
$credssp = [Microsoft.WSMan.Management.EnableWSManCredSSPCommand]::new()
$credssp.Role = "Client"
$credssp.Role | Should BeExactly "Client"
$credssp.DelegateComputer = "foo", "bar"
$credssp.DelegateComputer -join ',' | Should Be "foo,bar"
$credssp.Force = $true
$credssp.Force | Should Be $true

$credssp = [Microsoft.WSMan.Management.DisableWSManCredSSPCommand]::new()
$credssp.Role = "Server"
$credssp.Role | Should BeExactly "Server"
}

It "Error returned if runas non-admin: <cmdline>" -TestCases @(
@{cmdline = "Enable-WSManCredSSP -Role Server -Force"; cmd = "EnableWSManCredSSPCommand"},
@{cmdline = "Disable-WSManCredSSP -Role Server"; cmd = "DisableWSManCredSSPCommand"},
@{cmdline = "Get-WSManCredSSP"; cmd = "GetWSmanCredSSPCommand"}
) {
param ($cmdline, $cmd)

runas.exe /trustlevel:0x20000 "$powershell -nop -c try { $cmdline } catch { `$_.FullyQualifiedErrorId | Out-File $errtxt }; New-Item -Type File -Path $donefile"
$startTime = Get-Date
while (((Get-Date) - $startTime).TotalSeconds -lt 5 -and -not (Test-Path "$donefile"))
{
Start-Sleep -Milliseconds 100
}
$errtxt | Should Exist
$err = Get-Content $errtxt
$err | Should Be "System.InvalidOperationException,Microsoft.WSMan.Management.$cmd"
}
}