Skip to content

Restore backwards compatibility of SecureDesktopNVDAObject#14116

Merged
seanbudd merged 1 commit into
rcfrom
backwardsCompat-SecureDesktopNVDAObject
Sep 9, 2022
Merged

Restore backwards compatibility of SecureDesktopNVDAObject#14116
seanbudd merged 1 commit into
rcfrom
backwardsCompat-SecureDesktopNVDAObject

Conversation

@seanbudd

@seanbudd seanbudd commented Sep 9, 2022

Copy link
Copy Markdown
Member

After merging to beta, translations should be pushed and translators should be notified of the lines removed.

Link to issue number:

Follow up to #14105
Fixes issue described in #14111 (comment)

Summary of the issue:

SecureDesktopNVDAObject is an API end point used to indicate to the user and to API consumers (including NVDA remote), that the user has switched to a secure desktop.
This is triggered when Windows notification EVENT_SYSTEM_DESKTOPSWITCH notifies that the desktop has changed.
The switch is handled via a gainFocus event.
The gainFocus event causes the user instance of NVDA to enter sleep mode as the secure mode NVDA instance starts on the secure screen.

Information from SecureDesktopNVDAObject should not be accessible to the user, as it is backed by a valid MSAA desktop running on a secure profile, that NVDA can report information from.
This should generally be handled by NVDA entering sleep mode.
In #14105, SecureDesktopNVDAObject became based on NVDAObject to improve security for the object, by breaking it's connection to a valid window.
This was to decrease the theoretical risk of information leakage.

However, it was discovered that NVDA core event tracking and API consumers rely on SecureDesktopNVDAObject inheriting from Window (a parent class of Desktop).

As such, SecureDesktopNVDAObject must remain a Desktop subclass to retain backwards compatibility.

However we can prevent neighbouring objects from being accessed.

Description of user facing changes

Fixes bug in NVDA alpha with handling SecureDesktopNVDAObject.
Fixes API breakage.

Description of development approach

Reverts the change in #14105, making SecureDesktopNVDAObject inherit from Desktop.

Prevents neighbouring objects to SecureDesktopNVDAObject from being accessed by overriding relevant methods.

Testing strategy:

Checked diff between release-2022.2.2 and this branch.
Ensured "Secure Desktop" was still announced when switching to a secure desktop, i.e. #14094 was still fixed.

Known issues with pull request:

SecureDesktopNVDAObject is being used an API end point for handling a EVENT_SYSTEM_DESKTOPSWITCH to a secure desktop.
Using a gainFocus event on an NVDAObject to handle a Windows notification seems to be an obscure method of handling events.
NVDA has extensionPoints for that purpose.
In 2023.1 SecureDesktopNVDAObject should be removed, replaced by an extension point to notify add-ons that NVDA has entered a secure desktop.

Change log entries:

See PR diff

Code Review Checklist:

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

@seanbudd seanbudd requested a review from a team as a code owner September 9, 2022 00:35
@seanbudd seanbudd requested a review from feerrenrut September 9, 2022 00:35
@seanbudd seanbudd changed the base branch from master to rc September 9, 2022 00:49
@seanbudd seanbudd self-assigned this Sep 9, 2022
@seanbudd seanbudd added this to the 2022.2.3 milestone Sep 9, 2022
@seanbudd seanbudd merged commit 8eac659 into rc Sep 9, 2022
@seanbudd seanbudd deleted the backwardsCompat-SecureDesktopNVDAObject branch September 9, 2022 03:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants