-
Notifications
You must be signed in to change notification settings - Fork 383
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add AvoidUsingAllowUnencryptedAuthentication #1857
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, just some minor suggestions but looks good otherwise.
#if !CORECLR | ||
[Export(typeof(IScriptRule))] | ||
#endif | ||
public class AvoidUsingAllowUnencryptedAuthentication : AvoidParameterGeneric |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice, didn't know there was this base class to make implementation so much simpler
Co-authored-by: Christoph Bergmeister <c.bergmeister@gmail.com>
… into authRule # Conflicts: # Rules/Strings.Designer.cs # Rules/Strings.resx
Describe "AvoidUsingAllowUnencryptedAuthentication" { | ||
Context "When there are violations" { | ||
It "detects unencrypted authentication violations" { | ||
(Invoke-ScriptAnalyzer -ScriptDefinition 'Invoke-WebRequest foo -AllowUnencryptedAuthentication' -Settings $settings).Count | Should -Be 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i'd be happier if these were separate test cases, but this is ok
PR Summary
This adds a new rule:
AvoidUsingAllowUnencryptedAuthentication
.This rule searches for use of the
-AllowUnencryptedAuthentication
switch. This mainly serves to flag use withInvoke-WebRequest
andInvoke-RestMethod
, but also works for other cmdlets which may use the same parameter scheme.This parameter is directly warned against in MSDN as it isn't secure. It is risky and should only be used for compatibility with legacy systems. A
Warning
here should be sufficient to dissuade against this.PR Checklist
.cs
,.ps1
and.psm1
files have the correct copyright headerWIP:
to the beginning of the title and remove the prefix when the PR is ready.