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 ability to speak typed text in password fields #17481

Closed
wants to merge 6 commits into from

Conversation

nvdaes
Copy link
Collaborator

@nvdaes nvdaes commented Dec 3, 2024

Link to issue number:

Fixes issue #17451

Summary of the issue:

Some users require to listen the real typed characters in password edit controls.
Different add-ons were created independently by various authors to bring this feature to NVDA, patching the isTypingProtected function in the apimodule.

Description of user facing changes

In keyboard settings, a checkbox without an Accelerator has been added to allow NVDA speaking typed text in password fields. It's disabled by default.
A script has been added to globalCommands to toggle this setting. It doesn't have an assigned gesture to prevent accidental activation.

Description of development approach

If the speakPasswords configuration option of keyboard section is set to True, api/isTypingProtected returns False. Otherwise, the function check if focus object is protected.

Testing strategy:

Tested manually with QWERTY keyboard and braille input.

Known issues with pull request:

No major issues than using an add-on, not managed by NV Access.

Code Review Checklist:

  • Documentation:
    • Change log entry
    • User Documentation
    • Developer / Technical Documentation
    • Context sensitive help for GUI changes
  • Testing:
    • Unit tests
    • System (end to end) tests
    • Manual testing
  • UX of all users considered:
    • Speech
    • Braille
    • Low Vision
    • Different web browsers
    • Localization in other languages / culture than English
  • API is compatible with existing add-ons.
  • Security precautions taken.

@coderabbitai summary

@AppVeyorBot
Copy link

  • PASS: Lint check.
  • PASS: System tests (tags: installer NVDA).
  • PASS: Unit tests.
  • Build (for testing PR): https://ci.appveyor.com/api/buildjobs/so664yl239a1iv4o/artifacts/output/l10nUtil.exe nvda_snapshot_pr17481-34682,4812ff7e.exe
  • FAIL: Translation comments check. Translation comments missing or unexpectedly included. See build log for more information.
  • PASS: License check.
  • CI timing (mins):
    INIT 0.0,
    INSTALL_START 1.4,
    INSTALL_END 1.1,
    BUILD_START 0.0,
    BUILD_END 27.6,
    TESTSETUP_START 0.0,
    TESTSETUP_END 0.4,
    TEST_START 0.0,
    TEST_END 19.2,
    FINISH_END 0.2

See test results for failed build of commit 4812ff7e2b

@CyrilleB79
Copy link
Collaborator

I will not provide a feedback regarding the security of the feature here since it is already being discussed on NVDA mailing list, on NVDA AG list and in the related issue.

Regarding the UX:
The option is called "Mask passwords while typing". I feel it not very user friendly because:

  1. Disabling the "Mask password" option leads to a double negation which forces the user to think twice what NVDA is really doing or preventing.
  2. In the GUI, the password is visually masked, no matter the state of the option. I guess it is also masked in braille. The option only determines if NVDA reports (almost) what is actually displayed in the field, that is, stars; or if NVDA should report the typed text despite the fact that it does not actually appears on the screen.

Thus I'd recommend to rename the option. It could be renamed as follows:
"Allow speaking typed text in protected fields"
or "Allow speaking typed text in password fields"

Once we have agreed on the name of the option, all the variable names and other related content should be updated accordingly for clarity.

@nvdaes
Copy link
Collaborator Author

nvdaes commented Dec 4, 2024

Cyrille wrote:

In the GUI, the password is visually masked, no matter the state of the option. I guess it is also masked in braille.

In fact, passwords are masked in braille.

Thanks for your review. I think that I've addressed your comments.

source/globalCommands.py Outdated Show resolved Hide resolved
source/globalCommands.py Outdated Show resolved Hide resolved
source/globalCommands.py Outdated Show resolved Hide resolved
@nvdaes
Copy link
Collaborator Author

nvdaes commented Dec 4, 2024

@CyrilleB79 , I've applied your suggestions. Thanks.

@nvdaes nvdaes marked this pull request as ready for review December 4, 2024 18:24
@nvdaes nvdaes requested review from a team as code owners December 4, 2024 18:24
@nvdaes nvdaes changed the title Add ability to disable the masking of passwords while typing Add ability to speak typed text in password fields Dec 4, 2024
@gerald-hartig gerald-hartig added the conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review. label Dec 4, 2024
Copy link
Member

@seanbudd seanbudd left a comment

Choose a reason for hiding this comment

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

Thanks @nvdaes

source/globalCommands.py Outdated Show resolved Hide resolved
@nvdaes
Copy link
Collaborator Author

nvdaes commented Dec 5, 2024

@seanbudd , thanks for your review. I was addressing your comment but I see that you added a type hint.
If this is included in NVDA, I'll stop maintaining the reportPasswords add-on.

@Adriani90
Copy link
Collaborator

I would agree with this feature if it was introduced only for touch screen devices, maybe as part of the touch handler.

Just to mention it would be really nice to hear some arguments from NV Access in this whole discussion why it makes sense that blind people can hear passwords in any use case while sighted people cannot see them. Does this really tackle accessibility problems?
I agree with Jamie that this is not the task of the screen reader to demask password fields. If the password field really provides this feature, than it is the decision taken by the developers of that interface. But then it is a decision that is valid for everyone and not only for screen reader users.

I didn't hear any logical argument yet why speaking passwords in screen reading is needed at all. The only argument I heared is that NVDA should do the same like Voice Over. But then we compare apples with pairs. Wit Voice over you can decide when you lift your finger from the screen, so no one can tell that you really typed the password.

I would really like to hear opinions from @michaelDCurran and @LeonarddeR.
@jcsteh already replied in the advisory group and in my view it would really make sense to prevent even add-ons to do something like this.
We should concentrate on how to handle the problem in the touch screen scenario, and not introduce this as a general feature for every password typing scenario.

I am commenting here because it seems this PR is approved already.

@michaelDCurran michaelDCurran removed the conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review. label Dec 6, 2024
@michaelDCurran
Copy link
Member

My feelings are as follows:

  • I do think there is a valid use case here. There are many many beginner users who find it difficult to type in general. Sighted people can see the keys, and can use their sight to accurately press the right key. this is more than glancing at the keyboard and then pressing the key from memory, this is directly moving their finger to the key whilst looking at it.
  • Assuming this feature does exist, it should be in the core, not an add-on. We cannot right now necessarily stop what goes into a add-on, but we should in no way make it easier for an author to put it in an add-on, just because then that makes us feel more comfortable that it is not in the core. that is just passing the buck and hiding security concerns.
  • the scenario where a sighted user connects remotely, mutes, and types their password without knowing NvDA is reading it is the most valid security risk I've heard, and may be enough to kill this entirely.

In conclusion, I think this is a valid feature. But, if there are valid security concerns from the community here, then we should always be conservative and not merge this PR. But, we would also have to think about removing the add-on as well. In other words, I either support no add-on and nothing in core, or no add-on and a clearly visible and documented feature in core.

Based on the further discussion over the last day, I have removed the concept approved label. But I am not going as far as closing the PR yet. NV Access shall talk about this further after the weekend.

@ruifontes
Copy link
Contributor

It is not possible to mask the typing of the controlled machine to the controller and other way around?

@Adriani90
Copy link
Collaborator

@michaelDCurran you wrote:

There are many many beginner users who find it difficult to type in general. Sighted people can see the keys, and can use their sight to accurately press the right key. this is more than glancing at the keyboard and then pressing the key from memory, this is directly moving their finger to the key whilst looking at it.

In general I agree, but sighted people do not see all the symbols on the keyboard either. e.g. percent, dollar, question mark etc. It is even more signifikant when they use multiple input languages on a keyboard.
I think there are enough workarounds to learn the keyboard layout precisely as a blind person even if you are a beginner. And a beginner in this case would probably not use all the features on a PC either if they don't learn the keyboard layout first. In every school I did consultancy projects sofar, people learned the keyboard layout first before they had to enter passwords and other things. There are also lots of tactile orientations on a keyboard, and there are lots of tutorials and courses that help in this regard.
In NVDA there are following workarounds already:

  • Use speak typed characters in non password fields to learn the keyboard layout
  • Use input help mode before entering passwords to learn the keyboard layout

Is it really worth it to take this risk given the workarounds we have already?

@Adriani90
Copy link
Collaborator

@ruifontes

It is not possible to mask the typing of the controlled machine to the controller and other way around?

No because NVDA is not working via the NVDA remote addon in this case. Basically the software through wich the person remotely controls a keyboard wouuld have to mask that. NVDA cannot do anything about it.

@ruifontes
Copy link
Contributor

@Adriani90 wrote:
In general I agree, but sighted people do not see all the symbols on the keyboard either. e.g. percent, dollar, question mark etc.

Sorry, but the signs are also printed in the keyboard.
Only if you change the language keyboard to a one different fron the printing you have problems...

@Adriani90
Copy link
Collaborator

Sorry, but the signs are also printed in the keyboard.

That's true for some of them, but definitely not for all of them.

By the way NVDA would be the only screen reader on Windows that would provide such a risky and controversial feature.

@nvdaes
Copy link
Collaborator Author

nvdaes commented Dec 7, 2024

Mick wrote:

I either support no add-on and nothing in core, or no add-on and a clearly visible and documented feature in core.

I agree with this.
Anyway, sometimes passwords, for example in workplaces, are created by admins, not by employees, and they include symbols that may be difficult to type while reading the document which contains the password. Hearing the typed characters in the password field may be more efficient than using a workaround like input help, which needs to be activated and deactivated, since pasting the password may not be possible, and sighted people may not need to memorize the symbols during seconds.
I think that some kind of info may be useful for people who control a remote system, advicing them to deactivate NVDA to prevent risks, which aren't exclusively due to NVDA. Other software in the controlled or controlling computer may produce risks.
Anyway, if this is not included in the core, let me know if the add-on should be removed from the store and I'll create a PR to remove it.

@CyrilleB79
Copy link
Collaborator

Sorry, but the signs are also printed in the keyboard.

That's true for some of them, but definitely not for all of them.

@Adriani90 as already written by @ruifontes symbols are usually printed on the keyboard. Sighted people do not need to memorize how to write a symbol, they just look at it.
If you have in mind some symbols that are not printed on the keyboard, please be more specific, indicating both the symbol and the keyboard layout.
But from my memory, percent, dollar, question mark are printed on usual US keyboard.

@Adriani90
Copy link
Collaborator

On a german keyboard for example, the keys 8, 9 and 0 contain basically the symbols that you can trigger when pressing shift+8, shift+9 and shift+0, but you can also press ctrl+alt+8, 9 or 0 to e.g. insert different types of brackets. These are not printed on a keyboard but still are very commonly used. Also ctrl+alt+plus sign inserts a tilde, but this is not printed on a keyboard usually.
You are right that percent, dollar sign etc are printed, at least if you use the factory keyboard layout.

Moreover, there are laptops (e.g. like Asus or Lenovo), which habe a dedicated key for < and > symbols, but there are many keyboards out there which do not have these symbols and still they are commonly used.

@Adriani90
Copy link
Collaborator

Adriani90 commented Dec 8, 2024

Before we loose ourselves in such a discussion, why not trying to figure out how to combine both worlds?
The feature as it is designed by this PR is not compliant e.g. with European guidelines for data protection due to the reasons I outlined already and which Mick already pointed out above, this affects not only system administrators but also private sighted people who provide remote support and they don't know that a screen reader user might be able to hear when they enter a password.
I agree there is a valid use case here, but this solution design is not satisfactory yet.
NV Access states on its website that NVDA fully complies with General Data Protection Regulation (GDPR) and I don't think anyone here has the interest to officially become non compliant.
https://www.nvaccess.org/corporate-government/

Note that the GDPR does not only apply for organizations within EU, but for every private person living ini EU. So having this statement only on the corporate environment page might be misleading.
https://gdpr.eu/what-is-gdpr/

@Adriani90
Copy link
Collaborator

Adriani90 commented Dec 8, 2024

My proposal after all this discussion is as follows, which would make this feature at least GDPR compliant:

  • Redesign the feature so that the password becomes not only hearable but also visible, in that case at least a sighted person could directly see that the password field is not masked for a screen reader user when they start typing, and would at least have a chance to stop typing or ask to turn off this behavior on the controlled machine.
    • If this is not possible on the password field itself, NVDA could display a custom field (like the speech viewer) on top of the password field with the letters which are being heared, when the setting of speaking passwords is enabled.
    • Create a dedicated settings category for "data privacy and security", and put this and other settings such as speak typed character, screen curtain etc. that are related to data privacy under a disclaimer, same as we do already with the "advanced settings category".

@nvdaes I understand that you might interpret my way of challenging this feature as harsh, but I really don't want to offend you or hurt your feelings in any kind. I just gave you my honnest feedback because I really respect your work for NVDA and I really appreciate your initiatives. And I know that you support security and data privacy at least as much as me, you contributed to the codeQL integration and Addon security checks etc. I don't think we are as far away apart as it seems.

@CyrilleB79
Copy link
Collaborator

On a german keyboard for example, the keys 8, 9 and 0 contain basically the symbols that you can trigger when pressing shift+8, shift+9 and shift+0, but you can also press ctrl+alt+8, 9 or 0 to e.g. insert different types of brackets. These are not printed on a keyboard but still are very commonly used. Also ctrl+alt+plus sign inserts a tilde, but this is not printed on a keyboard usually. You are right that percent, dollar sign etc are printed, at least if you use the factory keyboard layout.

Could you double check please with a sighted person? This seems very strange to me: many sighted people do not know the keyboard by heart; how do they know how to input such brackets?

On this image of a German keyboard (source wikipedia), on the key "0" we can see:

  • "0" is written at the bottom left
  • "=" is written at the top left
  • "}" is written at the bottom right
  • Nothing is written at the top right, probably because

@amirmahdifard
Copy link

hi, everyone. i've came to give my opinion for this feature. In my own laptop, I used to Know how is my keyboard completely, so i had no problem to enter my passwords. but, when I was acsepted to my new job and I started my work with their computer and their keyboard which was a bit difrent, I mean places of the keys wor difrent, and they gave me my account and my password in their automation, and to prevent abuce and problems, they had a 15 seconds timer on their password request section, so I was needed to use this to be able to enter my password correctly and quickly, so I was needed to install the report passwords addon, and any other above solootions wasn't good option for me. I also agree to add this feature but let sited users know about this using the ways suggested in above comments. any, if we want to talk about this, we have some other unfare problems too. for example, we stil cannot solv captchers in websites or apps which doesn't have audio captcher or is not using h captcher service and only have a grafic contanes the text of the captcher, nvda doesn't help us in this case. we can only use ocr in this case, sometimes it reads the captcher correctly, sometimes not, And some others.

@nvdaes
Copy link
Collaborator Author

nvdaes commented Dec 8, 2024

amirmahdifard wrote:

I was needed to install the report passwords addon.

I'm happy knowing that the add-on worked for you. I'm the maintainer of this add-on available on the store.
Imo, showing the pasword to sighted people may not be good since they may prefer to hide it visually. They can show it if the speech viewer is active, and the fact that a password is shown using a feature different than the documented speech viewer doesn't imply that it's been listened.
For me the best solution would be to document this feature, warning sighted people that they should check if this is active. And maybe adding other documentation for them. For example, sometimes I need to explain how to lock capital characters, or that enter must be press to use focus mode, or how to exit NVDA.
This may be very important and a different issue or discussion may be opened. For example one day a sighted person muted my speakers and he forgot to activate them due to lack of knowledge about NVDA, and I spend some time trying to find out what was happening on the computer.
Thanks for your feedback.

@gerald-hartig
Copy link
Collaborator

Closed as per #17451

@davidacm
Copy link
Contributor

davidacm commented Dec 9, 2024

Hi, I just had an idea for echoing when typing passwords. I don't know if it has been discussed previously, I don't want to read the entire message thread...

Initially, it could be implemented in an add-on for testing.

A custom dialog could be implemented where the user can type the password, and when they press OK, NVDA will type the password in the focused field.
The flow would be like this:

  1. The user finds a password field.
  2. The user presses a command, which opens an NVDA dialog with an edit field, an OK and cancel button.
  3. The user writes the password, it is visible and audible.
  4. The user presses enter or presses OK.
  5. The dialog closes, and the password is typed in the password field.
    Optionally, the user can press escape and cancel the dialog.

This way braille users will also be able to know what they type in the password field, so it's even better.

The only security risk I can think of is someone viewing the password. Perhaps could make the edit box very small, or a custom implementation of an edit field, but the latter would be much more complex. Especially if it's intended to be compatible with braille displays.
I've made some personal plugins that write text strings into edit boxes without going to the clipboard. I find no technical limitations to this description of functionality.

I think I could make a add-on that does something like described above.
But I don't feel like it's as essential as other features, so I'm leaving it as an idea in case someone else would like to implement it.
If there are enough requests, I will reconsider my decision to developt it.
I know that typing passwords can be tedious, especially in some languages and keyboards that we don't know very well. But security is also important.
This would involve an extra step, but allows the user to decide when to use the feature or not.

@seanbudd
Copy link
Member

seanbudd commented Dec 9, 2024

@davidacm - can you comment this on #17451? this PR is closed and discussion is easily missed

@nvaccess nvaccess locked as resolved and limited conversation to collaborators Dec 9, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants