GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — Add AvoidUsingAllowUnencryptedAuthentication by MJVL · Pull Request #1857 · PowerShell/PSScriptAnalyzer

Conversation

Michael Van Leeuwen (MJVL) commented Nov 6, 2022

Copy link

Copy Markdown

Contributor

PR Summary

This adds a new rule: AvoidUsingAllowUnencryptedAuthentication.

This rule searches for use of the -AllowUnencryptedAuthentication switch. This mainly serves to flag use with Invoke-WebRequest and Invoke-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

PR has a meaningful title
- Use the present tense and imperative mood when describing your changes
Summarized changes
Change is not breaking
Make sure all .cs, .ps1 and .psm1 files have the correct copyright header
Make sure you've added a new test if existing tests do not effectively test the code changed and/or updated documentation
This PR is ready to merge and is not Work in Progress.
- If the PR is work in progress, please add the prefix WIP: to the beginning of the title and remove the prefix when the PR is ready.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Michael Van Leeuwen (MJVL) added 2 commits

November 6, 2022 12:13


          Add AvoidUsingAllowUnencryptedAuthentication rule

4ae2407


          Add AvoidUsingAllowUnencryptedAuthentication docs and tests

827c2cf

Christoph Bergmeister (bergmeister) approved these changes

Nov 7, 2022

View reviewed changes

Christoph Bergmeister (bergmeister) left a comment

Copy link

Copy Markdown

Collaborator

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

docs/Rules/AvoidUsingAllowUnencryptedAuthentication.md Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

docs/Rules/AvoidUsingAllowUnencryptedAuthentication.md Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

Tests/Rules/AvoidUsingAllowUnencryptedAuthentication.tests.ps1 Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

Rules/AvoidUsingAllowUnencryptedAuthentication.cs

		#if !CORECLR
		[Export(typeof(IScriptRule))]
		#endif
		public class AvoidUsingAllowUnencryptedAuthentication : AvoidParameterGeneric

Christoph Bergmeister (bergmeister) Nov 7, 2022

Copy link

Copy Markdown

Collaborator

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

MJVL reacted with heart emoji

All reactions

❤️ 1 reaction

Michael Van Leeuwen (MJVL) and others added 3 commits

November 7, 2022 08:02


          Update docs/Rules/AvoidUsingAllowUnencryptedAuthentication.md

4ebdcb9

Co-authored-by: Christoph Bergmeister <[email protected]>


          Fix code review suggestions

c489ba4


          Fix md code styling

8ee0d15

Christoph Bergmeister (bergmeister) reviewed

Feb 1, 2024

View reviewed changes

Comment thread

docs/Rules/AvoidUsingAllowUnencryptedAuthentication.md Resolved

Uh oh!

There was an error while loading. Please reload this page.


          Merge branch 'master' of https://github.com/PowerShell/PSScriptAnalyzer…

e1eca74

… into authRule

# Conflicts:
#	Rules/Strings.Designer.cs
#	Rules/Strings.resx

Christoph Bergmeister (bergmeister) mentioned this pull request

Feb 1, 2024

Rule request: Get-Content x | ConvertFrom-Json should suggest adding -raw for Get-content #1951

Open

Christoph Bergmeister and others added 2 commits

February 1, 2024 12:42


          bump rule count in tests again

da58e92


          Update docs/Rules/AvoidUsingAllowUnencryptedAuthentication.md

4e7449a

Christoph Bergmeister (bergmeister) requested a review from James Truher (JamesWTruher)

February 12, 2024 18:11

James Truher (JamesWTruher) reviewed

Feb 13, 2024

View reviewed changes

Comment thread

Tests/Rules/AvoidUsingAllowUnencryptedAuthentication.tests.ps1

		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

James Truher (JamesWTruher) Feb 13, 2024

Copy link

Copy Markdown

Contributor

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

James Truher (JamesWTruher) approved these changes

Feb 13, 2024

View reviewed changes

James Truher (JamesWTruher) merged commit c085ee3 into PowerShell:master

Feb 13, 2024

This was referenced Mar 16, 2024

Update PSScriptAnalyzer to v1.22.0 ForNeVeR/intellij-powershell-fork#1

Closed

Update PSScriptAnalyzer to v1.22.0 intellij-powershell/intellij-powershell#244

Merged

Update PowerShellEditorServices to v3.18.1 and PSScriptAnalyzer to v1.22.0 ForNeVeR/intellij-powershell-fork#2

Closed

Reviewers

Christoph Bergmeister (bergmeister)

bergmeister approved these changes

+1 more reviewer

James Truher (JamesWTruher)

JamesWTruher approved these changes

Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.