Skip to content
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 “Visibility Eye Icon” Option to Read-Host #19726

Open
aksarben opened this issue Jun 1, 2023 · 21 comments
Open

Add “Visibility Eye Icon” Option to Read-Host #19726

aksarben opened this issue Jun 1, 2023 · 21 comments
Labels
Issue-Enhancement the issue is more of a feature request than a bug Needs-Triage The issue is new and needs to be triaged by a work group. WG-Cmdlets-Utility cmdlets in the Microsoft.PowerShell.Utility module

Comments

@aksarben
Copy link

aksarben commented Jun 1, 2023

Summary of the new feature / enhancement

Can an optional parameter be added to PowerShell’s Read-Host command to show a “visibility eye icon” (aka the “Password Reveal button,” looks like Unicode character “👁”, U+1F441)?

Many secure sites and applications use this technique to let users temporarily display otherwise-masked input in plain text.

This would be a great aid in helping the user verify that he’s entering data correctly. Many times I’ve gotten an “incorrect password” error because I didn’t realize I’d made a typo when entering the password.

For security purposes, the “revealed” input should probably revert automatically to “masked” after a short time interval, or when focus moves to another input field.

Proposed technical implementation details (optional)

No response

@aksarben aksarben added Issue-Enhancement the issue is more of a feature request than a bug Needs-Triage The issue is new and needs to be triaged by a work group. labels Jun 1, 2023
@237dmitry
Copy link

You can write function to implement red eye:

function Prompt-User {

    param ( [Parameter()] [switch] $Unsecure )

    if ($Unsecure) { $data = Read-Host "`e[31m`u{1f441}`e[0m  Enter" }
    else { $data = Read-Host "Enter" -MaskInput }

    return $data
}

PS > Prompt-User -Unsecure
PS > Prompt-User

@mklement0
Copy link
Contributor

@237dmitry, your function won't help, because the intent is to have masked input by default, and then - while the masked prompt is already being displayed - have the option to temporarily unmask the input.

A few thoughts:

  • The user-interface aspect of implementing such a feature would be tricky, given that Read-Host doesn't currently support mouse interaction (and providing a keyboard shortcut, while feasible, would be obscure unless visually advertised somehow).

  • Such a feature would either become an enhancement to -MaskInput, or would require opt-in via a new switch to be combined with -MaskInput (though its use alone could be made to imply -MaskInput

@aksarben
Copy link
Author

aksarben commented Jun 1, 2023 via email

@jhoneill
Copy link

jhoneill commented Jun 1, 2023

The functionality is in the host part rather the read-host cmdlet, $Host.ui.PromptForSecureString and .PromptForCredential would support it and then it could be controlled in the cmdlets
It is not in the GUI prompt for a credential in windows PowerShell 5 and I've never seen it in a command line interface, and I can hear the voice of corporate security people saying "we need a method to disable this for all users via group policy". But it would improve the the user experience. (Although anything well written goes to great lengths to avoid users needing to type their password at all.)

It has the potential to decrease account lockouts due to (accidentally) repeatedly entering wrong passwords.

I'd settle for the these printing a warning if caps lock is on, which would probably halve the problem. :-)

@237dmitry
Copy link

237dmitry commented Jun 1, 2023

your function won't help, because the intent is to have masked input by default

This is one-minute-written script example. Not the solution nor the suggestion.
The idea is bad itself, not secure feature.

@kilasuit kilasuit added the WG-Cmdlets-Utility cmdlets in the Microsoft.PowerShell.Utility module label Jun 1, 2023
@mklement0
Copy link
Contributor

@237dmitry, if you had stated your fundamental opposition to this feature request up front - on the grounds of security, as you state - you wouldn't have had to suggest an - even less secure - (non-)workaround, and there would have been no need for this back-and-forth.

@jhoneill
Copy link

jhoneill commented Jun 3, 2023

The idea is bad itself, not secure feature.

Agree. At the very least it would need an option so corporate security could turn it off.
With a gui or web browser the visible password immediately goes away. With a command line interface someone can scroll back up the terminal buffer and see it, so it would need to auto hide when the user hits enter. I think there are good reasons why we don't see this in CLIs

@dwtaber
Copy link
Contributor

dwtaber commented Jun 5, 2023

With a gui or web browser the visible password immediately goes away. With a command line interface someone can scroll back up the terminal buffer and see it, so it would need to auto hide when the user hits enter.

How about "reveal while [key] is held" to temporarily show masked input while the input is still in progress?

@mklement0
Copy link
Contributor

@dwtaber, while that sounds like the best option for a console-based UI, the challenge is how to advertise this feature to the user (documenting it wouldn't be enough, as the target audience for such a feature may not even know PowerShell).

Of course, leaving it up to the script author to include something like "hold down Alt for 2 seconds to temporary reveal what you've typed" in the -Prompt message is one way to solve this problem.

If such a feature does get implemented, it should probably also be included (possibly by opt-in in all cases) in Get-Credential.

@ThomasNieto
Copy link
Contributor

While you can't do this today in Read-Host, what you could do is use Terminal.Gui to create a form that has a text field for the secret and a button to toggle the Secret property.

This is created from the example on the readme and shows the text box with text visible and hidden.

image

@mklement0
Copy link
Contributor

mklement0 commented Jun 5, 2023

Thanks, @ThomasNieto - Terminal.Gui is a really powerful way to build sophisticated, open-ended TUIs.

The bigger question with respect to this feature request - and others like #19680, #19664 and #16660 - is where to draw the line between what makes sense to build into PowerShell's standard cmdlets vs. what to leave to speciality third-party libraries.

(Personally, I think accommodating very common use cases as part of the standard cmdlets makes sense.)

@SteveL-MSFT
Copy link
Member

SteveL-MSFT commented Jun 5, 2023

I think a TUI based cmdlet as @ThomasNieto suggested (perhaps as part of https://github.com/powershell/graphicaltools would be best as that module already leverages Terminal.Gui.

GitHub
Modules that mix PowerShell and GUIs/CUIs! - built on Avalonia and gui.cs - GitHub - PowerShell/GraphicalTools: Modules that mix PowerShell and GUIs/CUIs! - built on Avalonia and gui.cs

@aksarben
Copy link
Author

aksarben commented Jun 5, 2023

Not sure I understand. Are you saying most people paste passwords from the clipboard? Don’t think I could agree, given that the visibility icon is used on thousands of Web sites.

@jhoneill
Copy link

jhoneill commented Jun 6, 2023

Not sure I understand. Are you saying most people paste passwords from the clipboard? Don’t think I could agree, given that the visibility icon is used on thousands of Web sites.

(a) The request is for a clickable icon. I can't think of case where something in the CLI happens as a result of clicking something. In a browser (or anything GUI based) click control A and to change the display parameters of control B is pretty standard.
(b) Anything where the user types their password should offer to save it and re-use it the next time. It's what browsers have done for 10 or 15 years.
(c) Have a look at one way scripts can avoid asking users to type a password time after time at https://github.com/jhoneill/OctopusTools/blob/main/Octopus.psm1 - lines 2 and 3 explain how to put the secret into a secure file. The end of script uses that file (but also works with the vendors method of putting the values in a environment variables). It's not difficult. Some use of get-credential (or read-host, or $host.ui.ReadLineAsSecureString) or even param ([Parameter(Mandatory)] [pscredential]$cred ) is unavoidable, but minimizing it is a sign of good practice.

GitHub
Work in progress: PowerShell bits for Octopus deploy - OctopusTools/Octopus.psm1 at main · jhoneill/OctopusTools

@mklement0
Copy link
Contributor

Quick preface: When I use the terms specious and fallacy, they are shorthands for - alleged -
logical errors. They are not meant to antagonize, and they only carry weight to the extent they
are backed up by arguments proffered alongside. As such, these arguments can be debated.


This thread has been hijacked with specious arguments similar to what happened in #19680:

  • The feature request is being dismissed not on its own merits, but on the grounds of
    opposing the functionality it is meant to enhance itself - which is an entirely different debate (but see below).

Additionally:

  • Opposing the request based on the presumed particulars of its implementation (e.g., requiring clicking)
    rather than the spirit of what it asks for is unhelpful.

  • Pointing out that the desired functionality can be achieved through a custom implementation may be helpful
    as an aside, but has no bearing on whether the feature request has merit (unless the custom implementation
    is so trivial that it renders the feature request unnecessary).


If the argument is: "The underlying functionality should never have been implemented / will go away soon, so there's no point in enhancing it."

  • A simple note to that effect will do - no point in conflating this fundamental opposition to the functionality with the particulars of the efforts of those trying to enhance it.

  • Instead, open separate issues to argue for the deprecation of the functionality and/or for amending the documentation to provide guidance as to why the functionality should be avoided.


On a meta note:

  • Using phrases such as the following only serves to antagonize, without meaningfully contributing to the discussion:

    • "you're clearly doing something wrong"
    • "I don't see the point" (at least without specific arguments)
    • "Nothing's stoping you" (quoted from the linked issue).

@jhoneill
Copy link

jhoneill commented Jun 6, 2023

Much has been said here and #19680 and its spin offs.
When looking at requests for new or changed functionality, we need to ask how great is the need, which covers a lot of things - including both the number of people asking, and the effort they need to put in to achieve what they want to with things as they are. The bar is set high for changes to commands which have been in PowerShell from the early versions, because (a) the probability of something major not coming to light in all the usage-hours that PowerShell has clocked up so far is pretty small (b) Adding new switches creates scripts which fail in old versions.
#19680 went from "As someone who uses Read-Host, I don't want to trim the string it returns as a separate operation" to a philosophical debate about where commands that return strings should have additional options to validate and/or transform them. The general principle is "they shouldn't", but exceptions can be made. That particular issue couldn't show a lot of demand, or a great saving in effort. It was also targeting a specific command that some who are new to PowerShell lean on quite heavily but the proficient generally avoid.
If people feel the need for a better wrapper for $Host.ui.ReadLine and $Host.ui.ReadLineAsSecureString the barriers to creating it are low. Some useful functionality is not delivered with PowerShell itself. From time to time the question of YAML raises its head, there is a good YAML module available, but it needs to be downloaded, is YAML such a standard that YAML support should be in the box? Some people have strong views for others are strongly against, for now it remains outside. Anyone can write a "Read-host++" share it, and if well received make a case for the functionality to be 'in the box'.

This issue is different - GUIs now commonly feature a reveal password button and CLIs don't. I've asked people to give a CLI example but haven't seen one yet. Here it's not possible to say "Good idea, write it and share it", because it needs functionality not present in $host.ui - i.e. if done at all it needs resource from the PowerShell team - although someone could create a Winforms app to do it, a single platform solution won't get into the product. It would, presumably work for anything which called $Host.ui.ReadLineAsSecureString. Not being able to have a clickable icon in a terminal / remote session etc, means the reveal would need to be triggered by some magic key - there would be nothing visible to tell the user the functionality is present and it would need to autohide after enter is pressed. Whilst Read-Host "Enter your password" might indicate a writer early in their PowerShell career, even using a secret store or my simplistic Get-Credential | Export-Clixml to enter it only once would benefit.
So one needs to ask how great is the need ? Users don't expect it from experience with other CLIs - as far as I know , if there is evidence to the contrary I'm willing to change my mind. How great is the effort ? from what I know I think it is substantial (as it is for, .e.g. adding a timeout to the two readline operations) - but again willing to be persuaded. Is it a good idea ? It has multiple security concerns and we should be encouraging better coding practices which ask for passwords less often. It looks to me like the need doesn't justify the resource needed before we get into debates about whether it is a good idea.

@dwtaber
Copy link
Contributor

dwtaber commented Jun 7, 2023

@dwtaber, while that sounds like the best option for a console-based UI, the challenge is how to advertise this feature to the user (documenting it wouldn't be enough, as the target audience for such a feature may not even know PowerShell).

This is enough scope creep that it really should be its own feature request, but what I'm thinking of is a general mechanism for temporarily displaying hints while waiting for user input, similar to how progress information is only shown while the activity is still in progress. A $HintPreference or similar could be used to toggle whether these hints are shown.

If such a feature does get implemented, it should probably also be included (possibly by opt-in in all cases) in Get-Credential.

Personally, I'd lean toward making it a feature of how prompting for masked input works in general.

@mklement0
Copy link
Contributor

When looking at requests for new or changed functionality, we need to ask how great is the need,

Sometimes, it takes someone giving voice to a previously unexpressed need. If you voice it, they may join in the chors, if you will - obviously, that may take time.

Based on the the pre-open-source days, PowerShell users have been trained to quietly put up with and work around PowerShell's warts, due to lack of official responses.

It seems to me that spirit is, regrettably, still alive - to everyone's detriment.

the effort they need to put in to achieve what they want to with things as they are.

That many people put up with the inconvenience of having to implement themselves what the built-in cmdlets should support is no argument against adding the necessary features to the latter.

(b) Adding new switches creates scripts which fail in old versions.

This mindset is a fundamental barrier to the progress of the language.

to a philosophical debate about where commands that return strings should have additional options to validate and/or transform them.

There was no reason for it to turn into that, as that debates was not worth having: there's ample precedent for such "convenience duplication"

That particular issue couldn't show a lot of demand, or a great saving in effort

Expecting a freshly hatched feature request to show "a lot of demand" is disingenous.

but the proficient generally avoid.

A false dichotomy, as previously explained: Read-Host is for building user interfaces, primarily, but not exclusively for end user who may not even know or care what PowerShell is.

Anyone can write a "Read-host++" share it

The only question that matter is: does the feature request fit in with the core mandate of the cmdlet at hand?
Is it something that users reasonably expect?

Waiting for potential community implementations that may or may not gain any traction - not least to due to lack of exposure - is a guaranteed way not to see the built-in functionality improved, but not necessarily due to lack of demand.

and CLIs don't.

If the functionality is useful, let's close that gap.

but haven't seen one yet.

@ThomasNieto'e example shows you that it's possible.

it needs resource from the PowerShell team

Not necessarily: if an issue is labeled "up-for-grabs", the bulk of the effort is on whatever community member decides to take this on.
For that reason, the "effort is better spent elsewhere" strike me as fundamentally specious.

@ThomasNieto
Copy link
Contributor

ThomasNieto commented Jun 7, 2023

Here is a proof of concept using Terminal.Gui:

  • Supports mouse input
  • Supports keyboard navigation with tab stops and space/enter button actions
  • Supports ALT keyboard shortcuts
  • Initiated via cmdlet
  • Outputs a PSCredential

image

image

I think this specific proof of concept / feature would be a good addition to the ConsoleGuiTools module as @SteveL-MSFT has mentioned.

@mklement0
Copy link
Contributor

mklement0 commented Jun 7, 2023

To be clear: My arguments were of a general nature, as I was getting the impression that feature requests are being opposed on grounds unrelated to their intrinsic merits.

Yes, there can ultimately be pragmatic reasons not to implement a request, but I feel that every request deserves "steel-manning", even if it turns out not to be viable (whether at all or as a core feature).

@ThomasNieto's contributions here and @SteveL-MSFT's suggestion exemplify this spirit, and for the request at hand I personally agree that https://github.com/powershell/graphicaltools is the better place for such a feature (whereas I do think #19680 deserves to be implemented as part of Read-Host).

@dwtaber

a general mechanism for temporarily displaying hints while waiting for user input,

This reminds me of the suggestion to generalize PSConsoleHostReadLine for open-ended prompting: PowerShell/PSReadLine#881

GitHub
Modules that mix PowerShell and GUIs/CUIs! - built on Avalonia and gui.cs - GitHub - PowerShell/GraphicalTools: Modules that mix PowerShell and GUIs/CUIs! - built on Avalonia and gui.cs

@jhoneill
Copy link

jhoneill commented Jun 7, 2023

Sticking to
(a) User expectations at the commandline
(b) The feasibility of a changing the functionality in System.Management.Automation.Internal.Host.InternalHostUserInterface to do it at the command line
(c) Whether the graphical tools should ship with PowerShell - making a form of out grid view everywhere, and enabling a dialog box for credential entry
Helps. Especially if one is concise.
Adding 800 words to argue about behaviour of the product group, the amount of resource it has, how priorities are chosen, the way other people explain how decisions come to be made and so on - and in terms which are likely to offend - does not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue-Enhancement the issue is more of a feature request than a bug Needs-Triage The issue is new and needs to be triaged by a work group. WG-Cmdlets-Utility cmdlets in the Microsoft.PowerShell.Utility module
Projects
None yet
Development

No branches or pull requests

8 participants