8000 Add Support for Null Usernames in Web Cmdlet Basic Auth by markekraus · Pull Request #9536 · PowerShell/PowerShell · GitHub
[go: up one dir, main page]
Skip to content

Add Support for Null Usernames in Web Cmdlet Basic Auth#9536

Merged
adityapatwardhan merged 2 commits intoPowerShell:masterfrom
markekraus:9017-Fix-Basic-Auth-Null-Pasword
May 7, 2019
Merged

Add Support for Null Usernames in Web Cmdlet Basic Auth#9536
adityapatwardhan merged 2 commits intoPowerShell:masterfrom
markekraus:9017-Fix-Basic-Auth-Null-Pasword

Conversation

@markekraus
Copy link
Contributor
@markekraus markekraus commented May 5, 2019 •
edited
Loading

PR Summary

It is possible to construct PSCredential objects with null or empty usernames. However, doing so makes it impossible to retrieve the plain-text password from the PSCredential.GetNetworkCredential() API. There are some Web APIs which may require a null or empty username. This PR changes the way the password is parsed from the Credential parameter such that a null or empty username will not impact conversion to plain-text.

PR Context

Closes #9017

PR Checklist

@markekraus markekraus mentioned this pull request May 5, 2019
Closed
@markekraus markekraus changed the title Add support for null usernames in Web Cmdlet Basic Auth Add Support for Null Usernames in Web Cmdlet Basic Auth May 5, 2019
@iSazonov iSazonov added the CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log label May 5, 2019
@iSazonov iSazonov added this to the 7.0.0-preview.1 milestone May 5, 2019
@SteveL-MSFT @markekraus
Update src/Microsoft.PowerShell.Commands.Utility/commands/utility/Web…
4 8000 cdc771 
…Cmdlet/Common/WebRequestPSCmdlet.Common.cs


Style fix

Co-Authored-By: markekraus <github@markekraus.com>
@iSazonov
Copy link
Collaborator
iSazonov commented May 5, 2019

@SteveL-MSFT CodeFactor helped us with such formatting issues. Is it possible to bring it back?

@SteveL-MSFT
Copy link
Member
SteveL-MSFT commented May 6, 2019

@iSazonov seems odd that Codacy wouldn't also catch such an issue

@adityapatwardhan adityapatwardhan merged commit 935e164 into PowerShell:master May 7, 2019
@adityapatwardhan
Copy link
Member
adityapatwardhan commented May 7, 2019

@markekraus Thank you for your contribution.

@iSazonov
Copy link
Collaborator
iSazonov commented May 7, 2019 •
edited
Loading

@SteveL-MSFT > seems odd that Codacy wouldn't also catch such an issue

Sorry, I don't understand that you mean.
CodeFactor greatly motivated us. We fixed most of style issues. Now there are mainly documentation issues. Since we are generating XML documentation, we should fix such issues as well. We could create something like "continues documentations" so that the XML documentation transparently flows into Docs repo. Therefore, I never wanted to turn off doc issues in CodeFactor. (Although if we turn this off we will have very few reported issues)
On the other hand, Codacy also sometimes looks very strange. There are sometimes false positives.
Together they (CodeFActor and Codacy) would be good.

@iSazonov
Copy link
Collaborator
iSazonov commented May 8, 2019

Without CodeFactor, we will begin again to collect formatting errors and fall into discussions of this.
Each reviewer will make conflicting requests.
It will be sad this is a great job of correcting the formatting to be destroyed.
We already have some such PRs. I really don't understand why CodeFactor was removed if it motivated us to correct above 60000 (!) style issues and make all PRs style clean.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@SteveL-MSFT SteveL-MSFT SteveL-MSFT approved these changes

@adityapatwardhan adityapatwardhan adityapatwardhan approved these changes

@iSazonov iSazonov iSazonov approved these changes

@JamesWTruher JamesWTruher Awaiting requested review from JamesWTruher

+1 more reviewer

@PaulHigin PaulHigin PaulHigin approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@adityapatwardhan adityapatwardhan

Labels

CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log

Projects

None yet

Milestone

7.0.0-preview.1

Development

Successfully merging this pull request may close these issues.

Web Cmdlets don't allow $null username in a PSCredential for basic auth

5 participants

@markekraus @iSazonov @SteveL-MSFT @adityapatwardhan @PaulHigin
0