diff --git a/Rules/AvoidUserNameAndPasswordParams.cs b/Rules/AvoidUserNameAndPasswordParams.cs index 422b58080..4fd643d46 100644 --- a/Rules/AvoidUserNameAndPasswordParams.cs +++ b/Rules/AvoidUserNameAndPasswordParams.cs @@ -53,12 +53,16 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) // Iterrates all ParamAsts and check if their names are on the list. foreach (ParameterAst paramAst in paramAsts) { - TypeInfo paramType = (TypeInfo)paramAst.StaticType; + var psCredentialType = paramAst.Attributes.FirstOrDefault(paramAttribute => + (paramAttribute.TypeName.IsArray && (paramAttribute.TypeName as ArrayTypeName).ElementType.GetReflectionType() == typeof(PSCredential)) + || paramAttribute.TypeName.GetReflectionType() == typeof(PSCredential)); + + var credentialAttribute = paramAst.Attributes.FirstOrDefault(paramAttribute => paramAttribute.TypeName.GetReflectionType() == typeof(CredentialAttribute)); + String paramName = paramAst.Name.VariablePath.ToString(); - // if this is pscredential type with credential attribute, skip - if ((paramType == typeof(PSCredential) || (paramType.IsArray && paramType.GetElementType() == typeof (PSCredential))) - && paramAst.Attributes.Any(paramAttribute => paramAttribute.TypeName.GetReflectionType() == typeof(CredentialAttribute))) + // if this is pscredential type with credential attribute where pscredential type comes first + if (psCredentialType != null && credentialAttribute != null && psCredentialType.Extent.EndOffset < credentialAttribute.Extent.StartOffset) { continue; } diff --git a/Rules/Strings.Designer.cs b/Rules/Strings.Designer.cs index 22b8a9191..55a65fd0a 100644 --- a/Rules/Strings.Designer.cs +++ b/Rules/Strings.Designer.cs @@ -421,7 +421,7 @@ internal static string AvoidUsernameAndPasswordParamsCommonName { } /// - /// Looks up a localized string similar to Functions should only take in a credential parameter of type PSCredential with CredentialAttribute instead of username and password parameters.. + /// Looks up a localized string similar to Functions should only take in a credential parameter of type PSCredential with CredentialAttribute where PSCredential comes before CredentialAttribute instead of username and password parameters.. /// internal static string AvoidUsernameAndPasswordParamsDescription { get { @@ -430,7 +430,7 @@ internal static string AvoidUsernameAndPasswordParamsDescription { } /// - /// Looks up a localized string similar to Function '{0}' has both username and password parameters. A credential parameter of type PSCredential with a CredentialAttribute should be used.. + /// Looks up a localized string similar to Function '{0}' has both username and password parameters. A credential parameter of type PSCredential with a CredentialAttribute where PSCredential comes before CredentialAttribute should be used.. /// internal static string AvoidUsernameAndPasswordParamsError { get { @@ -1771,7 +1771,7 @@ internal static string UsePSCredentialTypeCommonName { } /// - /// Looks up a localized string similar to Checks that cmdlets that have a Credential parameter accept PSCredential with CredentialAttribute. This comes from the PowerShell teams best practices.. + /// Looks up a localized string similar to Checks that cmdlets that have a Credential parameter accept PSCredential with CredentialAttribute where PSCredential comes before CredentialAttribute.. This comes from the PowerShell teams best practices.. /// internal static string UsePSCredentialTypeDescription { get { @@ -1780,7 +1780,7 @@ internal static string UsePSCredentialTypeDescription { } /// - /// Looks up a localized string similar to The Credential parameter in '{0}' must be of the type PSCredential with CredentialAttribute.. + /// Looks up a localized string similar to The Credential parameter in '{0}' must be of the type PSCredential with CredentialAttribute where PSCredential comes before CredentialAttribute.. /// internal static string UsePSCredentialTypeError { get { @@ -1789,7 +1789,7 @@ internal static string UsePSCredentialTypeError { } /// - /// Looks up a localized string similar to The Credential parameter in a found script block must be of the type PSCredential with CredentialAttribute.. + /// Looks up a localized string similar to The Credential parameter in a found script block must be of the type PSCredential with CredentialAttribute where PSCredential comes before CredentialAttribute.. /// internal static string UsePSCredentialTypeErrorSB { get { diff --git a/Rules/Strings.resx b/Rules/Strings.resx index d27bbb8d7..319d72d27 100644 --- a/Rules/Strings.resx +++ b/Rules/Strings.resx @@ -217,13 +217,13 @@ One Char - Checks that cmdlets that have a Credential parameter accept PSCredential with CredentialAttribute. This comes from the PowerShell teams best practices. + Checks that cmdlets that have a Credential parameter accept PSCredential with CredentialAttribute where PSCredential comes before CredentialAttribute.. This comes from the PowerShell teams best practices. - The Credential parameter in '{0}' must be of the type PSCredential with CredentialAttribute. + The Credential parameter in '{0}' must be of the type PSCredential with CredentialAttribute where PSCredential comes before CredentialAttribute. - The Credential parameter in a found script block must be of the type PSCredential with CredentialAttribute. + The Credential parameter in a found script block must be of the type PSCredential with CredentialAttribute where PSCredential comes before CredentialAttribute. PSCredential @@ -511,10 +511,10 @@ Avoid Using Username and Password Parameters - Functions should only take in a credential parameter of type PSCredential with CredentialAttribute instead of username and password parameters. + Functions should only take in a credential parameter of type PSCredential with CredentialAttribute where PSCredential comes before CredentialAttribute instead of username and password parameters. - Function '{0}' has both username and password parameters. A credential parameter of type PSCredential with a CredentialAttribute should be used. + Function '{0}' has both username and password parameters. A credential parameter of type PSCredential with a CredentialAttribute where PSCredential comes before CredentialAttribute should be used. AvoidUsingUserNameAndPassWordParams diff --git a/Rules/UsePSCredentialType.cs b/Rules/UsePSCredentialType.cs index 57e4309a0..bddc3bf66 100644 --- a/Rules/UsePSCredentialType.cs +++ b/Rules/UsePSCredentialType.cs @@ -73,6 +73,12 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) foreach (ScriptBlockAst scriptBlockAst in scriptBlockAsts) { + // check for the case where it's parent is function, in that case we already processed above + if (scriptBlockAst.Parent != null && scriptBlockAst.Parent is FunctionDefinitionAst) + { + continue; + } + if (scriptBlockAst.ParamBlock != null && scriptBlockAst.ParamBlock.Parameters != null) { foreach (ParameterAst parameter in scriptBlockAst.ParamBlock.Parameters) @@ -90,10 +96,13 @@ private bool WrongCredentialUsage(ParameterAst parameter) { if (parameter.Name.VariablePath.UserPath.Equals("Credential", StringComparison.OrdinalIgnoreCase)) { - TypeInfo paramType = (TypeInfo)parameter.StaticType; + var psCredentialType = parameter.Attributes.FirstOrDefault(paramAttribute => (paramAttribute.TypeName.IsArray && (paramAttribute.TypeName as ArrayTypeName).ElementType.GetReflectionType() == typeof(PSCredential)) + || paramAttribute.TypeName.GetReflectionType() == typeof(PSCredential)); + + var credentialAttribute = parameter.Attributes.FirstOrDefault(paramAttribute => paramAttribute.TypeName.GetReflectionType() == typeof(CredentialAttribute)); - if ((paramType == typeof(PSCredential) || (paramType.IsArray && paramType.GetElementType() == typeof(PSCredential))) - && parameter.Attributes.Any(paramAttribute => paramAttribute.TypeName.GetReflectionType() == typeof(CredentialAttribute))) + // check that both exists and pscredentialtype comes before credential attribute + if (psCredentialType != null && credentialAttribute != null && psCredentialType.Extent.EndOffset < credentialAttribute.Extent.StartOffset) { return false; } diff --git a/Tests/Rules/AvoidUserNameAndPasswordParams.ps1 b/Tests/Rules/AvoidUserNameAndPasswordParams.ps1 index b764af199..cc89dcfe3 100644 --- a/Tests/Rules/AvoidUserNameAndPasswordParams.ps1 +++ b/Tests/Rules/AvoidUserNameAndPasswordParams.ps1 @@ -31,5 +31,29 @@ } } +function MyFunction3 +{ + [CmdletBinding()] + [Alias()] + [OutputType([int])] + Param + ( + # Param1 help description + [Parameter(Mandatory=$true, + ValueFromPipelineByPropertyName=$true, + Position=0)] + [System.Management.Automation.CredentialAttribute()] + [pscredential] + $UserName, + + # Param2 help description + [System.Management.Automation.CredentialAttribute()] + [pscredential] + $Password + ) +} + function TestFunction($password, [PSCredential[]]$passwords, $username){ -} \ No newline at end of file +} + + diff --git a/Tests/Rules/AvoidUserNameAndPasswordParams.tests.ps1 b/Tests/Rules/AvoidUserNameAndPasswordParams.tests.ps1 index 358997fe8..35895ebcd 100644 --- a/Tests/Rules/AvoidUserNameAndPasswordParams.tests.ps1 +++ b/Tests/Rules/AvoidUserNameAndPasswordParams.tests.ps1 @@ -1,6 +1,6 @@ Import-Module PSScriptAnalyzer -$violationMessage = "Function 'Verb-Noun' has both username and password parameters. A credential parameter of type PSCredential with a CredentialAttribute should be used." +$violationMessage = "Function 'Verb-Noun' has both username and password parameters. A credential parameter of type PSCredential with a CredentialAttribute where PSCredential comes before CredentialAttribute should be used." $violationName = "PSAvoidUsingUserNameAndPasswordParams" $directory = Split-Path -Parent $MyInvocation.MyCommand.Path $violations = Invoke-ScriptAnalyzer $directory\AvoidUserNameAndPasswordParams.ps1 | Where-Object {$_.RuleName -eq $violationName} @@ -8,12 +8,12 @@ $noViolations = Invoke-ScriptAnalyzer $directory\AvoidUserNameAndPasswordParamsN Describe "AvoidUserNameAndPasswordParams" { Context "When there are violations" { - It "has 2 avoid username and password parameter violations" { - $violations.Count | Should Be 2 + It "has 3 avoid username and password parameter violations" { + $violations.Count | Should Be 3 } It "has the correct violation message" { - $violations[1].Message | Should Match $violationMessage + $violations[2].Message | Should Match $violationMessage } } diff --git a/Tests/Rules/AvoidUserNameAndPasswordParamsNoViolations.ps1 b/Tests/Rules/AvoidUserNameAndPasswordParamsNoViolations.ps1 index b909d7b59..2ea0db9cc 100644 --- a/Tests/Rules/AvoidUserNameAndPasswordParamsNoViolations.ps1 +++ b/Tests/Rules/AvoidUserNameAndPasswordParamsNoViolations.ps1 @@ -19,8 +19,8 @@ function MyFunction3 [Parameter(Mandatory=$true, ValueFromPipelineByPropertyName=$true, Position=0)] - [System.Management.Automation.CredentialAttribute()] [pscredential] + [System.Management.Automation.CredentialAttribute()] $UserName, # Param2 help description diff --git a/Tests/Rules/PSCredentialType.ps1 b/Tests/Rules/PSCredentialType.ps1 index ddcae8231..3e2d9e730 100644 --- a/Tests/Rules/PSCredentialType.ps1 +++ b/Tests/Rules/PSCredentialType.ps1 @@ -1,3 +1,21 @@ function Credential([string]$credential) { +} + +# this one is wrong because pscredential should come first +function Credential2 +{ + [CmdletBinding()] + [Alias()] + [OutputType([int])] + Param + ( + # Param1 help description + [Parameter(Mandatory=$true, + ValueFromPipelineByPropertyName=$true, + Position=0)] + [System.Management.Automation.CredentialAttribute()] + [pscredential] + $Credential + ) } \ No newline at end of file diff --git a/Tests/Rules/PSCredentialType.tests.ps1 b/Tests/Rules/PSCredentialType.tests.ps1 index ccf13b6ef..ea642ce49 100644 --- a/Tests/Rules/PSCredentialType.tests.ps1 +++ b/Tests/Rules/PSCredentialType.tests.ps1 @@ -1,5 +1,5 @@ Import-Module PSScriptAnalyzer -$violationMessage = "The Credential parameter in 'Credential' must be of the type PSCredential with CredentialAttribute." +$violationMessage = "The Credential parameter in 'Credential' must be of the type PSCredential with CredentialAttribute where PSCredential comes before CredentialAttribute." $violationName = "PSUsePSCredentialType" $directory = Split-Path -Parent $MyInvocation.MyCommand.Path $violations = Invoke-ScriptAnalyzer $directory\PSCredentialType.ps1 | Where-Object {$_.RuleName -eq $violationName} @@ -7,12 +7,12 @@ $noViolations = Invoke-ScriptAnalyzer $directory\PSCredentialTypeNoViolations.ps Describe "PSCredentialType" { Context "When there are violations" { - It "has 1 PSCredential type violation" { - $violations.Count | Should Be 1 + It "has 2 PSCredential type violation" { + $violations.Count | Should Be 2 } It "has the correct description message" { - $violations.Message | Should Match $violationMessage + $violations[1].Message | Should Match $violationMessage } } diff --git a/Tests/Rules/PSCredentialTypeNoViolations.ps1 b/Tests/Rules/PSCredentialTypeNoViolations.ps1 index b907929b8..7b8f09bc0 100644 --- a/Tests/Rules/PSCredentialTypeNoViolations.ps1 +++ b/Tests/Rules/PSCredentialTypeNoViolations.ps1 @@ -9,8 +9,8 @@ [Parameter(Mandatory=$true, ValueFromPipelineByPropertyName=$true, Position=0)] - [System.Management.Automation.CredentialAttribute()] [pscredential] + [System.Management.Automation.CredentialAttribute()] $Credential ) } \ No newline at end of file