Change `Read-Host -MaskInput` to use existing SecureString path, but return as plain text by SteveL-MSFT · Pull Request #13256 · PowerShell/PowerShell · GitHub
Skip to content

Change Read-Host -MaskInput to use existing SecureString path, but return as plain text#13256

Merged
daxian-dbw merged 4 commits into
PowerShell:masterfrom
SteveL-MSFT:mask-prompt
Jul 31, 2020
Merged

Change Read-Host -MaskInput to use existing SecureString path, but return as plain text#13256
daxian-dbw merged 4 commits into
PowerShell:masterfrom
SteveL-MSFT:mask-prompt

Conversation

@SteveL-MSFT

@SteveL-MSFT SteveL-MSFT commented Jul 23, 2020

Copy link
Copy Markdown
Member

PR Summary

#10908 introduced a new -MaskInput switch to enable hiding the input while still returning a string. It did this by introducing a new method ReadLineMaskedAsString(), however, this was not necessary and would not be supported by remoting as it required hosts to implement it. It also did not support use of -MaskInput with -Prompt.

The change here is to revert most of the changes in that PR to instead use existing ReadLineAsSecureString() method which would already be implemented by different hosts and convert to a string if -MaskInput is used.

PR Context

Fix #13011

PR Checklist

@SteveL-MSFT SteveL-MSFT changed the title Change Read-Host -MaskInput to use existig SecureString path, but return as plain text Change Read-Host -MaskInput to use existing SecureString path, but return as plain text Jul 29, 2020

@rjmholt rjmholt left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The host change makes me think we should have more SDK tests, particularly a test host implementation, so we have a good litmus test for breaking changes in the API surface

Comment thread src/Microsoft.PowerShell.Commands.Utility/commands/utility/ReadConsoleCmdlet.cs Outdated
Comment thread src/Microsoft.PowerShell.Commands.Utility/commands/utility/ReadConsoleCmdlet.cs Outdated
@SteveL-MSFT

Copy link
Copy Markdown
Member Author

@PoshChan

Copy link
Copy Markdown
Collaborator

@SteveL-MSFT, did not find matching build context: PowerShell-CI-macOS; allowed contexts: PowerShell-CI-SSH

@SteveL-MSFT SteveL-MSFT reopened this Jul 29, 2020

@daxian-dbw daxian-dbw left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM except for one minor comment.

Comment thread src/Microsoft.PowerShell.Commands.Utility/commands/utility/ReadConsoleCmdlet.cs Outdated
@daxian-dbw

Copy link
Copy Markdown
Member

@SteveL-MSFT Can you please resolve the conflict? mac CI failing tests are marked as pending for now, so CI should pass after you rebase.

@daxian-dbw

Copy link
Copy Markdown
Member

@PoshChan Please remind me in 1 hour

@PoshChan

Copy link
Copy Markdown
Collaborator

@daxian-dbw, this is the reminder you requested 1 hour ago

@daxian-dbw daxian-dbw merged commit 99b3bfa into PowerShell:master Jul 31, 2020
@daxian-dbw daxian-dbw added the CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log label Jul 31, 2020
@daxian-dbw daxian-dbw added this to the 7.1.0-preview.7 milestone Jul 31, 2020
@TravisEz13 TravisEz13 modified the milestones: 7.1.0-preview.7, 7.1.0-preview.6 Aug 5, 2020
@ghost

ghost commented Aug 17, 2020

Copy link
Copy Markdown

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

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Read-Host -MaskInput -Prompt 'foo' doesn't mask the input

5 participants