Skip to content

Commit 03beec1

Browse files
author
Quoc Truong
committed
Enforce that pscredential attribute must come before credentialattribute
1 parent c279303 commit 03beec1

10 files changed

+83
-28
lines changed

Rules/AvoidUserNameAndPasswordParams.cs

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -53,12 +53,16 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
5353
// Iterrates all ParamAsts and check if their names are on the list.
5454
foreach (ParameterAst paramAst in paramAsts)
5555
{
56-
TypeInfo paramType = (TypeInfo)paramAst.StaticType;
56+
var psCredentialType = paramAst.Attributes.FirstOrDefault(paramAttribute =>
57+
(paramAttribute.TypeName.IsArray && (paramAttribute.TypeName as ArrayTypeName).ElementType.GetReflectionType() == typeof(PSCredential))
58+
|| paramAttribute.TypeName.GetReflectionType() == typeof(PSCredential));
59+
60+
var credentialAttribute = paramAst.Attributes.FirstOrDefault(paramAttribute => paramAttribute.TypeName.GetReflectionType() == typeof(CredentialAttribute));
61+
5762
String paramName = paramAst.Name.VariablePath.ToString();
5863

59-
// if this is pscredential type with credential attribute, skip
60-
if ((paramType == typeof(PSCredential) || (paramType.IsArray && paramType.GetElementType() == typeof (PSCredential)))
61-
&& paramAst.Attributes.Any(paramAttribute => paramAttribute.TypeName.GetReflectionType() == typeof(CredentialAttribute)))
64+
// if this is pscredential type with credential attribute where pscredential type comes first
65+
if (psCredentialType != null && credentialAttribute != null && psCredentialType.Extent.EndOffset < credentialAttribute.Extent.StartOffset)
6266
{
6367
continue;
6468
}

Rules/Strings.Designer.cs

Lines changed: 5 additions & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Rules/Strings.resx

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -217,13 +217,13 @@
217217
<value>One Char</value>
218218
</data>
219219
<data name="UsePSCredentialTypeDescription" xml:space="preserve">
220-
<value>Checks that cmdlets that have a Credential parameter accept PSCredential with CredentialAttribute. This comes from the PowerShell teams best practices.</value>
220+
<value>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.</value>
221221
</data>
222222
<data name="UsePSCredentialTypeError" xml:space="preserve">
223-
<value>The Credential parameter in '{0}' must be of the type PSCredential with CredentialAttribute.</value>
223+
<value>The Credential parameter in '{0}' must be of the type PSCredential with CredentialAttribute where PSCredential comes before CredentialAttribute.</value>
224224
</data>
225225
<data name="UsePSCredentialTypeErrorSB" xml:space="preserve">
226-
<value>The Credential parameter in a found script block must be of the type PSCredential with CredentialAttribute.</value>
226+
<value>The Credential parameter in a found script block must be of the type PSCredential with CredentialAttribute where PSCredential comes before CredentialAttribute.</value>
227227
</data>
228228
<data name="UsePSCredentialTypeCommonName" xml:space="preserve">
229229
<value>PSCredential</value>
@@ -511,10 +511,10 @@
511511
<value>Avoid Using Username and Password Parameters</value>
512512
</data>
513513
<data name="AvoidUsernameAndPasswordParamsDescription" xml:space="preserve">
514-
<value>Functions should only take in a credential parameter of type PSCredential with CredentialAttribute instead of username and password parameters.</value>
514+
<value>Functions should only take in a credential parameter of type PSCredential with CredentialAttribute where PSCredential comes before CredentialAttribute instead of username and password parameters.</value>
515515
</data>
516516
<data name="AvoidUsernameAndPasswordParamsError" xml:space="preserve">
517-
<value>Function '{0}' has both username and password parameters. A credential parameter of type PSCredential with a CredentialAttribute should be used.</value>
517+
<value>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.</value>
518518
</data>
519519
<data name="AvoidUsernameAndPasswordParamsName" xml:space="preserve">
520520
<value>AvoidUsingUserNameAndPassWordParams</value>

Rules/UsePSCredentialType.cs

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,12 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
7373

7474
foreach (ScriptBlockAst scriptBlockAst in scriptBlockAsts)
7575
{
76+
// check for the case where it's parent is function, in that case we already processed above
77+
if (scriptBlockAst.Parent != null && scriptBlockAst.Parent is FunctionDefinitionAst)
78+
{
79+
continue;
80+
}
81+
7682
if (scriptBlockAst.ParamBlock != null && scriptBlockAst.ParamBlock.Parameters != null)
7783
{
7884
foreach (ParameterAst parameter in scriptBlockAst.ParamBlock.Parameters)
@@ -90,10 +96,13 @@ private bool WrongCredentialUsage(ParameterAst parameter)
9096
{
9197
if (parameter.Name.VariablePath.UserPath.Equals("Credential", StringComparison.OrdinalIgnoreCase))
9298
{
93-
TypeInfo paramType = (TypeInfo)parameter.StaticType;
99+
var psCredentialType = parameter.Attributes.FirstOrDefault(paramAttribute => (paramAttribute.TypeName.IsArray && (paramAttribute.TypeName as ArrayTypeName).ElementType.GetReflectionType() == typeof(PSCredential))
100+
|| paramAttribute.TypeName.GetReflectionType() == typeof(PSCredential));
101+
102+
var credentialAttribute = parameter.Attributes.FirstOrDefault(paramAttribute => paramAttribute.TypeName.GetReflectionType() == typeof(CredentialAttribute));
94103

95-
if ((paramType == typeof(PSCredential) || (paramType.IsArray && paramType.GetElementType() == typeof(PSCredential)))
96-
&& parameter.Attributes.Any(paramAttribute => paramAttribute.TypeName.GetReflectionType() == typeof(CredentialAttribute)))
104+
// check that both exists and pscredentialtype comes before credential attribute
105+
if (psCredentialType != null && credentialAttribute != null && psCredentialType.Extent.EndOffset < credentialAttribute.Extent.StartOffset)
97106
{
98107
return false;
99108
}

Tests/Rules/AvoidUserNameAndPasswordParams.ps1

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,5 +31,29 @@
3131
}
3232
}
3333

34+
function MyFunction3
35+
{
36+
[CmdletBinding()]
37+
[Alias()]
38+
[OutputType([int])]
39+
Param
40+
(
41+
# Param1 help description
42+
[Parameter(Mandatory=$true,
43+
ValueFromPipelineByPropertyName=$true,
44+
Position=0)]
45+
[System.Management.Automation.CredentialAttribute()]
46+
[pscredential]
47+
$UserName,
48+
49+
# Param2 help description
50+
[System.Management.Automation.CredentialAttribute()]
51+
[pscredential]
52+
$Password
53+
)
54+
}
55+
3456
function TestFunction($password, [PSCredential[]]$passwords, $username){
35-
}
57+
}
58+
59+

Tests/Rules/AvoidUserNameAndPasswordParams.tests.ps1

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,19 @@
11
Import-Module PSScriptAnalyzer
22

3-
$violationMessage = "Function 'Verb-Noun' has both username and password parameters. A credential parameter of type PSCredential with a CredentialAttribute should be used."
3+
$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."
44
$violationName = "PSAvoidUsingUserNameAndPasswordParams"
55
$directory = Split-Path -Parent $MyInvocation.MyCommand.Path
66
$violations = Invoke-ScriptAnalyzer $directory\AvoidUserNameAndPasswordParams.ps1 | Where-Object {$_.RuleName -eq $violationName}
77
$noViolations = Invoke-ScriptAnalyzer $directory\AvoidUserNameAndPasswordParamsNoViolations.ps1 | Where-Object {$_.RuleName -eq $violationName}
88

99
Describe "AvoidUserNameAndPasswordParams" {
1010
Context "When there are violations" {
11-
It "has 2 avoid username and password parameter violations" {
12-
$violations.Count | Should Be 2
11+
It "has 3 avoid username and password parameter violations" {
12+
$violations.Count | Should Be 3
1313
}
1414

1515
It "has the correct violation message" {
16-
$violations[1].Message | Should Match $violationMessage
16+
$violations[2].Message | Should Match $violationMessage
1717
}
1818
}
1919

Tests/Rules/AvoidUserNameAndPasswordParamsNoViolations.ps1

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@ function MyFunction3
1919
[Parameter(Mandatory=$true,
2020
ValueFromPipelineByPropertyName=$true,
2121
Position=0)]
22-
[System.Management.Automation.CredentialAttribute()]
2322
[pscredential]
23+
[System.Management.Automation.CredentialAttribute()]
2424
$UserName,
2525

2626
# Param2 help description

Tests/Rules/PSCredentialType.ps1

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,21 @@
11
function Credential([string]$credential) {
22

3+
}
4+
5+
# this one is wrong because pscredential should come first
6+
function Credential2
7+
{
8+
[CmdletBinding()]
9+
[Alias()]
10+
[OutputType([int])]
11+
Param
12+
(
13+
# Param1 help description
14+
[Parameter(Mandatory=$true,
15+
ValueFromPipelineByPropertyName=$true,
16+
Position=0)]
17+
[System.Management.Automation.CredentialAttribute()]
18+
[pscredential]
19+
$Credential
20+
)
321
}

Tests/Rules/PSCredentialType.tests.ps1

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,18 @@
11
Import-Module PSScriptAnalyzer
2-
$violationMessage = "The Credential parameter in 'Credential' must be of the type PSCredential with CredentialAttribute."
2+
$violationMessage = "The Credential parameter in 'Credential' must be of the type PSCredential with CredentialAttribute where PSCredential comes before CredentialAttribute."
33
$violationName = "PSUsePSCredentialType"
44
$directory = Split-Path -Parent $MyInvocation.MyCommand.Path
55
$violations = Invoke-ScriptAnalyzer $directory\PSCredentialType.ps1 | Where-Object {$_.RuleName -eq $violationName}
66
$noViolations = Invoke-ScriptAnalyzer $directory\PSCredentialTypeNoViolations.ps1 | Where-Object {$_.RuleName -eq $violationName}
77

88
Describe "PSCredentialType" {
99
Context "When there are violations" {
10-
It "has 1 PSCredential type violation" {
11-
$violations.Count | Should Be 1
10+
It "has 2 PSCredential type violation" {
11+
$violations.Count | Should Be 2
1212
}
1313

1414
It "has the correct description message" {
15-
$violations.Message | Should Match $violationMessage
15+
$violations[1].Message | Should Match $violationMessage
1616
}
1717
}
1818

Tests/Rules/PSCredentialTypeNoViolations.ps1

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@
99
[Parameter(Mandatory=$true,
1010
ValueFromPipelineByPropertyName=$true,
1111
Position=0)]
12-
[System.Management.Automation.CredentialAttribute()]
1312
[pscredential]
13+
[System.Management.Automation.CredentialAttribute()]
1414
$Credential
1515
)
1616
}

0 commit comments

Comments
 (0)