bitwarden/browser
1
0
Fork 0
mirror of https://github.com/bitwarden/browser synced 2026-01-21 20:03:43 +00:00
Code Issues Releases Wiki Activity

Desktop autotype remove SHIFT from valid modifier keys (#17347)

Browse Source 
Removal of SHIFT from valid modifier keys. As it stands, we allow [SHIFT + `<a-z>`] , which would prevent users from capitalizing letters. As a result, the default shortcut has to change (because it included SHIFT). Changed to CONTROL + ALT + b
This commit is contained in:
neuronull
2026-01-07 11:54:46 -07:00
committed by GitHub
parent 9ba9c89ee6
commit 196db093b2
11 changed files with 133 additions and 97 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
apps/desktop/desktop_native/autotype/src/lib.rs
View File

@@ -1,5 +1,11 @@
use anyhow::Result;
#[cfg(target_os = "windows")]
mod modifier_keys;
#[cfg(target_os = "windows")]
pub(crate) use modifier_keys::*;
#[cfg_attr(target_os = "linux", path = "linux.rs")]
#[cfg_attr(target_os = "macos", path = "macos.rs")]
#[cfg_attr(target_os = "windows", path = "windows/mod.rs")]

45
apps/desktop/desktop_native/autotype/src/modifier_keys.rs Normal file
View File

@@ -0,0 +1,45 @@
// Electron modifier keys
// <https://www.electronjs.org/docs/latest/tutorial/keyboard-shortcuts#cross-platform-modifiers>
pub(crate) const CONTROL_KEY_STR: &str = "Control";
pub(crate) const ALT_KEY_STR: &str = "Alt";
pub(crate) const SUPER_KEY_STR: &str = "Super";
// numeric values for modifier keys
pub(crate) const CONTROL_KEY: u16 = 0x11;
pub(crate) const ALT_KEY: u16 = 0x12;
pub(crate) const SUPER_KEY: u16 = 0x5B;
/// A mapping of <Electron modifier key string> to <numeric representation>
static MODIFIER_KEYS: [(&str, u16); 3] = [
(CONTROL_KEY_STR, CONTROL_KEY),
(ALT_KEY_STR, ALT_KEY),
(SUPER_KEY_STR, SUPER_KEY),
];
/// Provides a mapping of the valid modifier keys' electron
/// string representation to the numeric representation.
pub(crate) fn get_numeric_modifier_key(modifier: &str) -> Option<u16> {
for (modifier_str, modifier_num) in MODIFIER_KEYS {
if modifier_str == modifier {
return Some(modifier_num);
}
}
None
}
#[cfg(test)]
mod test {
use super::get_numeric_modifier_key;
#[test]
fn valid_modifier_keys() {
assert_eq!(get_numeric_modifier_key("Control").unwrap(), 0x11);
assert_eq!(get_numeric_modifier_key("Alt").unwrap(), 0x12);
assert_eq!(get_numeric_modifier_key("Super").unwrap(), 0x5B);
}
#[test]
fn does_not_contain_invalid_modifier_keys() {
assert!(get_numeric_modifier_key("Shift").is_none());
}
}

1
apps/desktop/desktop_native/autotype/src/windows/mod.rs
View File

@@ -44,7 +44,6 @@ pub fn get_foreground_window_title() -> Result<String> {
/// - Control
/// - Alt
/// - Super
/// - Shift
/// - \[a-z\]\[A-Z\]
struct KeyboardShortcutInput(INPUT);

47
apps/desktop/desktop_native/autotype/src/windows/type_input.rs
View File

@@ -6,11 +6,7 @@ use windows::Win32::UI::Input::KeyboardAndMouse::{
};
use super::{ErrorOperations, KeyboardShortcutInput, Win32ErrorOperations};
const SHIFT_KEY_STR: &str = "Shift";
const CONTROL_KEY_STR: &str = "Control";
const ALT_KEY_STR: &str = "Alt";
const LEFT_WINDOWS_KEY_STR: &str = "Super";
use crate::get_numeric_modifier_key;
const IS_VIRTUAL_KEY: bool = true;
const IS_REAL_KEY: bool = false;
@@ -88,22 +84,19 @@ impl TryFrom<&str> for KeyboardShortcutInput {
type Error = anyhow::Error;
fn try_from(key: &str) -> std::result::Result<Self, Self::Error> {
const SHIFT_KEY: u16 = 0x10;
const CONTROL_KEY: u16 = 0x11;
const ALT_KEY: u16 = 0x12;
const LEFT_WINDOWS_KEY: u16 = 0x5B;
// not modifier key
if key.len() == 1 {
let input = build_unicode_input(InputKeyPress::Up, get_alphabetic_hotkey(key)?);
return Ok(KeyboardShortcutInput(input));
}
// the modifier keys are using the Up keypress variant because the user has already
// pressed those keys in order to trigger the feature.
let input = match key {
SHIFT_KEY_STR => build_virtual_key_input(InputKeyPress::Up, SHIFT_KEY),
CONTROL_KEY_STR => build_virtual_key_input(InputKeyPress::Up, CONTROL_KEY),
ALT_KEY_STR => build_virtual_key_input(InputKeyPress::Up, ALT_KEY),
LEFT_WINDOWS_KEY_STR => build_virtual_key_input(InputKeyPress::Up, LEFT_WINDOWS_KEY),
_ => build_unicode_input(InputKeyPress::Up, get_alphabetic_hotkey(key)?),
};
Ok(KeyboardShortcutInput(input))
if let Some(numeric_modifier_key) = get_numeric_modifier_key(key) {
let input = build_virtual_key_input(InputKeyPress::Up, numeric_modifier_key);
Ok(KeyboardShortcutInput(input))
} else {
Err(anyhow!("Unsupported modifier key: {key}"))
}
}
}
@@ -278,7 +271,7 @@ mod tests {
#[test]
#[serial]
fn keyboard_shortcut_conversion_succeeds() {
let keyboard_shortcut = [CONTROL_KEY_STR, SHIFT_KEY_STR, "B"];
let keyboard_shortcut = ["Control", "Alt", "B"];
let _: Vec<KeyboardShortcutInput> = keyboard_shortcut
.iter()
.map(|s| KeyboardShortcutInput::try_from(*s))
@@ -290,7 +283,19 @@ mod tests {
#[serial]
#[should_panic = "Letter is not ASCII Alphabetic ([a-z][A-Z]): '1'"]
fn keyboard_shortcut_conversion_fails_invalid_key() {
let keyboard_shortcut = [CONTROL_KEY_STR, SHIFT_KEY_STR, "1"];
let keyboard_shortcut = ["Control", "Alt", "1"];
let _: Vec<KeyboardShortcutInput> = keyboard_shortcut
.iter()
.map(|s| KeyboardShortcutInput::try_from(*s))
.try_collect()
.unwrap();
}
#[test]
#[serial]
#[should_panic(expected = "Unsupported modifier key: Shift")]
fn keyboard_shortcut_conversion_fails_with_shift() {
let keyboard_shortcut = ["Control", "Shift", "B"];
let _: Vec<KeyboardShortcutInput> = keyboard_shortcut
.iter()
.map(|s| KeyboardShortcutInput::try_from(*s))

2
apps/desktop/src/app/accounts/settings.component.spec.ts
View File

@@ -188,7 +188,7 @@ describe("SettingsComponent", () => {
pinServiceAbstraction.isPinSet.mockResolvedValue(false);
policyService.policiesByType$.mockReturnValue(of([null]));
desktopAutotypeService.autotypeEnabledUserSetting$ = of(false);
desktopAutotypeService.autotypeKeyboardShortcut$ = of(["Control", "Shift", "B"]);
desktopAutotypeService.autotypeKeyboardShortcut$ = of(["Control", "Alt", "B"]);
billingAccountProfileStateService.hasPremiumFromAnySource$.mockReturnValue(of(false));
configService.getFeatureFlag$.mockReturnValue(of(false));
});

2
apps/desktop/src/autofill/components/autotype-shortcut.component.html
View File

@@ -5,7 +5,7 @@
</div>
<div bitDialogContent>
<p>
{{ "editAutotypeShortcutDescription" | i18n }}
{{ "editAutotypeKeyboardModifiersDescription" | i18n }}
</p>
<bit-form-field>
<bit-label>{{ "typeShortcut" | i18n }}</bit-label>

61
apps/desktop/src/autofill/components/autotype-shortcut.component.spec.ts
View File

@@ -30,11 +30,9 @@ describe("AutotypeShortcutComponent", () => {
const validShortcuts = [
"Control+A",
"Alt+B",
"Shift+C",
"Win+D",
"control+e", // case insensitive
"ALT+F",
"SHIFT+G",
"WIN+H",
];
@@ -46,14 +44,7 @@ describe("AutotypeShortcutComponent", () => {
});
it("should accept two modifiers with letter", () => {
const validShortcuts = [
"Control+Alt+A",
"Control+Shift+B",
"Control+Win+C",
"Alt+Shift+D",
"Alt+Win+E",
"Shift+Win+F",
];
const validShortcuts = ["Control+Alt+A", "Control+Win+C", "Alt+Win+D", "Alt+Win+E"];
validShortcuts.forEach((shortcut) => {
const control = createControl(shortcut);
@@ -63,7 +54,7 @@ describe("AutotypeShortcutComponent", () => {
});
it("should accept modifiers in different orders", () => {
const validShortcuts = ["Alt+Control+A", "Shift+Control+B", "Win+Alt+C"];
const validShortcuts = ["Alt+Control+A", "Win+Control+B", "Win+Alt+C"];
validShortcuts.forEach((shortcut) => {
const control = createControl(shortcut);
@@ -88,15 +79,14 @@ describe("AutotypeShortcutComponent", () => {
const invalidShortcuts = [
"Control+1",
"Alt+2",
"Shift+3",
"Win+4",
"Control+!",
"Alt+@",
"Shift+#",
"Alt+#",
"Win+$",
"Control+Space",
"Alt+Enter",
"Shift+Tab",
"Control+Tab",
"Win+Escape",
];
@@ -111,12 +101,10 @@ describe("AutotypeShortcutComponent", () => {
const invalidShortcuts = [
"Control",
"Alt",
"Shift",
"Win",
"Control+Alt",
"Control+Shift",
"Alt+Shift",
"Control+Alt+Shift",
"Control+Win",
"Control+Alt+Win",
];
invalidShortcuts.forEach((shortcut) => {
@@ -127,7 +115,7 @@ describe("AutotypeShortcutComponent", () => {
});
it("should reject shortcuts with invalid modifier names", () => {
const invalidShortcuts = ["Ctrl+A", "Command+A", "Super+A", "Meta+A", "Cmd+A", "Invalid+A"];
const invalidShortcuts = ["Ctrl+A", "Command+A", "Meta+A", "Cmd+A", "Invalid+A"];
invalidShortcuts.forEach((shortcut) => {
const control = createControl(shortcut);
@@ -137,7 +125,7 @@ describe("AutotypeShortcutComponent", () => {
});
it("should reject shortcuts with multiple base keys", () => {
const invalidShortcuts = ["Control+A+B", "Alt+Ctrl+Shift"];
const invalidShortcuts = ["Control+A+B", "Alt+Ctrl+Win"];
invalidShortcuts.forEach((shortcut) => {
const control = createControl(shortcut);
@@ -148,11 +136,10 @@ describe("AutotypeShortcutComponent", () => {
it("should reject shortcuts with more than two modifiers", () => {
const invalidShortcuts = [
"Control+Alt+Shift+A",
"Control+Alt+Win+A",
"Control+Alt+Win+B",
"Control+Shift+Win+C",
"Alt+Shift+Win+D",
"Control+Alt+Shift+Win+E",
"Control+Alt+Win+C",
"Alt+Control+Win+D",
];
invalidShortcuts.forEach((shortcut) => {
@@ -221,7 +208,7 @@ describe("AutotypeShortcutComponent", () => {
});
it("should handle very long strings", () => {
const longString = "Control+Alt+Shift+Win+A".repeat(100);
const longString = "Control+Alt+Win+A".repeat(100);
const control = createControl(longString);
const result = validator(control);
expect(result).toEqual({ invalidShortcut: { message: "Invalid shortcut" } });
@@ -230,7 +217,7 @@ describe("AutotypeShortcutComponent", () => {
describe("modifier combinations", () => {
it("should accept all possible single modifier combinations", () => {
const modifiers = ["Control", "Alt", "Shift", "Win"];
const modifiers = ["Control", "Alt", "Win"];
modifiers.forEach((modifier) => {
const control = createControl(`${modifier}+A`);
@@ -240,14 +227,7 @@ describe("AutotypeShortcutComponent", () => {
});
it("should accept all possible two-modifier combinations", () => {
const combinations = [
"Control+Alt+A",
"Control+Shift+A",
"Control+Win+A",
"Alt+Shift+A",
"Alt+Win+A",
"Shift+Win+A",
];
const combinations = ["Control+Alt+A", "Control+Win+A", "Alt+Win+A"];
combinations.forEach((shortcut) => {
const control = createControl(shortcut);
@@ -257,12 +237,7 @@ describe("AutotypeShortcutComponent", () => {
});
it("should reject all three-modifier combinations", () => {
const combinations = [
"Control+Alt+Shift+A",
"Control+Alt+Win+A",
"Control+Shift+Win+A",
"Alt+Shift+Win+A",
];
const combinations = ["Control+Alt+Win+A", "Alt+Control+Win+A", "Win+Alt+Control+A"];
combinations.forEach((shortcut) => {
const control = createControl(shortcut);
@@ -270,12 +245,6 @@ describe("AutotypeShortcutComponent", () => {
expect(result).toEqual({ invalidShortcut: { message: "Invalid shortcut" } });
});
});
it("should reject all four modifiers combination", () => {
const control = createControl("Control+Alt+Shift+Win+A");
const result = validator(control);
expect(result).toEqual({ invalidShortcut: { message: "Invalid shortcut" } });
});
});
});
});

28
apps/desktop/src/autofill/components/autotype-shortcut.component.ts
View File

@@ -77,25 +77,31 @@ export class AutotypeShortcutComponent {
}
}
// <https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent>
private buildShortcutFromEvent(event: KeyboardEvent): string | null {
const hasCtrl = event.ctrlKey;
const hasAlt = event.altKey;
const hasShift = event.shiftKey;
const hasMeta = event.metaKey; // Windows key on Windows, Command on macOS
const hasSuper = event.metaKey; // Windows key on Windows, Command on macOS
// Require at least one modifier (Control, Alt, Shift, or Super)
if (!hasCtrl && !hasAlt && !hasShift && !hasMeta) {
// Require at least one valid modifier (Control, Alt, Super)
if (!hasCtrl && !hasAlt && !hasSuper) {
return null;
}
const key = event.key;
// Ignore pure modifier keys themselves
if (key === "Control" || key === "Alt" || key === "Shift" || key === "Meta") {
// disallow pure modifier keys themselves
if (key === "Control" || key === "Alt" || key === "Meta") {
return null;
}
// Accept a single alphabetical letter as the base key
// disallow shift modifier
if (hasShift) {
return null;
}
// require a single alphabetical letter as the base key
const isAlphabetical = typeof key === "string" && /^[a-z]$/i.test(key);
if (!isAlphabetical) {
return null;
@@ -108,10 +114,7 @@ export class AutotypeShortcutComponent {
if (hasAlt) {
parts.push("Alt");
}
if (hasShift) {
parts.push("Shift");
}
if (hasMeta) {
if (hasSuper) {
parts.push("Super");
}
parts.push(key.toUpperCase());
@@ -129,10 +132,9 @@ export class AutotypeShortcutComponent {
}
// Must include exactly 1-2 modifiers and end with a single letter
// Valid examples: Ctrl+A, Shift+Z, Ctrl+Shift+X, Alt+Shift+Q
// Valid examples: Ctrl+A, Alt+B, Ctrl+Alt+X, Alt+Control+Q, Win+B, Ctrl+Win+A
// Allow modifiers in any order, but only 1-2 modifiers total
const pattern =
/^(?=.*\b(Control|Alt|Shift|Win)\b)(?:Control\+|Alt\+|Shift\+|Win\+){1,2}[A-Z]$/i;
const pattern = /^(?=.*\b(Control|Alt|Win)\b)(?:Control\+|Alt\+|Win\+){1,2}[A-Z]$/i;
return pattern.test(value)
? null
: { invalidShortcut: { message: this.i18nService.t("invalidShortcut") } };

24
apps/desktop/src/autofill/models/main-autotype-keyboard-shortcut.ts
View File

@@ -1,4 +1,14 @@
import { defaultWindowsAutotypeKeyboardShortcut } from "../services/desktop-autotype.service";
/**
Electron's representation of modifier keys
<https://www.electronjs.org/docs/latest/tutorial/keyboard-shortcuts#cross-platform-modifiers>
*/
export const CONTROL_KEY_STR = "Control";
export const ALT_KEY_STR = "Alt";
export const SUPER_KEY_STR = "Super";
export const VALID_SHORTCUT_MODIFIER_KEYS: string[] = [CONTROL_KEY_STR, ALT_KEY_STR, SUPER_KEY_STR];
export const DEFAULT_KEYBOARD_SHORTCUT: string[] = [CONTROL_KEY_STR, ALT_KEY_STR, "B"];
/*
This class provides the following:
@@ -13,7 +23,7 @@ export class AutotypeKeyboardShortcut {
private autotypeKeyboardShortcut: string[];
constructor() {
this.autotypeKeyboardShortcut = defaultWindowsAutotypeKeyboardShortcut;
this.autotypeKeyboardShortcut = DEFAULT_KEYBOARD_SHORTCUT;
}
/*
@@ -51,14 +61,16 @@ export class AutotypeKeyboardShortcut {
This private function validates the strArray input to make sure the array contains
valid, currently accepted shortcut keys for Windows.
Valid windows shortcut keys: Control, Alt, Super, Shift, letters A - Z
Valid macOS shortcut keys: Control, Alt, Command, Shift, letters A - Z (not yet supported)
Valid shortcut keys: Control, Alt, Super, letters A - Z
Platform specifics:
- On Windows, Super maps to the Windows key.
- On MacOS, Super maps to the Command key.
- On MacOS, Alt maps to the Option key.
See Electron keyboard shorcut docs for more info:
https://www.electronjs.org/docs/latest/tutorial/keyboard-shortcuts
*/
#keyboardShortcutIsValid(strArray: string[]) {
const VALID_SHORTCUT_CONTROL_KEYS: string[] = ["Control", "Alt", "Super", "Shift"];
const UNICODE_LOWER_BOUND = 65; // unicode 'A'
const UNICODE_UPPER_BOUND = 90; // unicode 'Z'
const MIN_LENGTH: number = 2;
@@ -77,7 +89,7 @@ export class AutotypeKeyboardShortcut {
// Ensure strArray is all modifier keys, and that the last key is a letter
for (let i = 0; i < strArray.length; i++) {
if (i < strArray.length - 1) {
if (!VALID_SHORTCUT_CONTROL_KEYS.includes(strArray[i])) {
if (!VALID_SHORTCUT_MODIFIER_KEYS.includes(strArray[i])) {
return false;
}
} else {

10
apps/desktop/src/autofill/services/desktop-autotype.service.ts
View File

@@ -33,11 +33,10 @@ import { UserId } from "@bitwarden/user-core";
import { AutotypeConfig } from "../models/autotype-config";
import { AutotypeVaultData } from "../models/autotype-vault-data";
import { DEFAULT_KEYBOARD_SHORTCUT } from "../models/main-autotype-keyboard-shortcut";
import { DesktopAutotypeDefaultSettingPolicy } from "./desktop-autotype-policy.service";
export const defaultWindowsAutotypeKeyboardShortcut: string[] = ["Control", "Shift", "B"];
export const AUTOTYPE_ENABLED = new KeyDefinition<boolean | null>(
AUTOTYPE_SETTINGS_DISK,
"autotypeEnabled",
@@ -72,10 +71,9 @@ export class DesktopAutotypeService implements OnDestroy {
private readonly isPremiumAccount$: Observable<boolean>;
// The enabled/disabled state from the user settings menu
autotypeEnabledUserSetting$: Observable<boolean>;
autotypeEnabledUserSetting$: Observable<boolean> = of(false);
// The keyboard shortcut from the user settings menu
autotypeKeyboardShortcut$: Observable<string[]> = of(defaultWindowsAutotypeKeyboardShortcut);
autotypeKeyboardShortcut$: Observable<string[]> = of(DEFAULT_KEYBOARD_SHORTCUT);
private destroy$ = new Subject<void>();
@@ -106,7 +104,7 @@ export class DesktopAutotypeService implements OnDestroy {
);
this.autotypeKeyboardShortcut$ = this.autotypeKeyboardShortcut.state$.pipe(
map((shortcut) => shortcut ?? defaultWindowsAutotypeKeyboardShortcut),
map((shortcut) => shortcut ?? DEFAULT_KEYBOARD_SHORTCUT),
takeUntil(this.destroy$),
);
}

4
apps/desktop/src/locales/en/messages.json
View File

@@ -4267,8 +4267,8 @@
"typeShortcut": {
"message": "Type shortcut"
},
"editAutotypeShortcutDescription": {
"message": "Include one or two of the following modifiers: Ctrl, Alt, Win, or Shift, and a letter."
"editAutotypeKeyboardModifiersDescription": {
"message": "Include one or two of the following modifiers: Ctrl, Alt, Win, and a letter."
},
"invalidShortcut": {
"message": "Invalid shortcut"