Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 11 additions & 10 deletions src/components/Advanced.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import React, { useState } from 'react';
import { useDeviceStore } from '../store/useDeviceStore';
import { KEY_SLOTS } from '../api/device/keyParser';
import { hexStringToByteArray } from '../api/device/utils';
import { CONFIG_MODE_REQUIRED, configModeTooltipText } from '../data/configMode';
import { configModeTooltipText } from '../data/configMode';
import { CautionButton, CriticalText, SetButton } from './ui/forms';
import { Tooltip } from './ui/Tooltip';

Expand All @@ -22,7 +22,7 @@ const ECC_SLOTS = [
const KEY_MODIFIERS = { Backup: 128, Signature: 64, Decryption: 32 };

const Advanced: React.FC = () => {
const { device, deviceType, isConfigMode, setWorking } = useDeviceStore();
const { device, deviceType, setWorking } = useDeviceStore();
const [yubiForm, setYubiForm] = useState({ publicId: '', privateId: '', secretKey: '' });
const [eccType, setEccType] = useState(1);
const [eccSlot, setEccSlot] = useState(101);
Expand All @@ -33,12 +33,15 @@ const Advanced: React.FC = () => {

if (!device) return null;

const requireConfigMode = (): boolean => {
if (isConfigMode) return true;
setStatus(null);
setError(CONFIG_MODE_REQUIRED);
return false;
};
// No client-side config-mode gate. The firmware reports the same UNLOCKED
// status in config mode as out of it (okcore.cpp set_time), so the store's
// isConfigMode is an inference that reads false whenever the app missed the
// transition that sets it - a reconnect, or starting up with the key already
// in config mode. Gating writes on that guess refused them on a key that WAS
// in config mode, with no way to recover. The device is the authority: send,
// and let it answer. Its refusal already routes through the catch blocks
// below, and OnlyKeyDevice.formatDeviceLockedError turns "Error not in config
// mode" into the instructions. The legacy app never gated on this either.

return (
<div className="page-shell">
Expand Down Expand Up @@ -203,7 +206,6 @@ const Advanced: React.FC = () => {
onClick={async () => {
setError(null);
setStatus(null);
if (!requireConfigMode()) return;
const maxLen = eccType === 9 ? 40 : 64;
const key = eccKey.replace(/\s/g, '').slice(0, maxLen);
if (!key || key.length !== maxLen) {
Expand Down Expand Up @@ -233,7 +235,6 @@ const Advanced: React.FC = () => {
if (!window.confirm(`Wipe private key from slot ${eccSlot}?`)) return;
setError(null);
setStatus(null);
if (!requireConfigMode()) return;
setWorking(true, `Wiping private key from slot ${eccSlot}…`);
try {
await device.wipePrivateKey(eccSlot);
Expand Down
53 changes: 50 additions & 3 deletions src/components/__tests__/Advanced.ui.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,23 @@ describe('Advanced page', () => {
expect(screen.getByText(/yubikey security info saved/i)).toBeInTheDocument();
});

it('blocks private-key save outside config mode', async () => {
/*
* THE APP MUST NOT PRE-JUDGE CONFIG MODE.
*
* A user reported "unable to wipe key, says to put into config mode even when
* it is in config mode". The cause was a client-side gate here that refused to
* send when the store's isConfigMode read false - and it reads false whenever
* the app missed the one transition that sets it, such as starting up with the
* key already in config mode. The device was willing the whole time;
* onlykey-testing/test/01-protocol/27-config-mode-observability.test.js
* measures the firmware accepting OKWIPEPRIV in exactly that state.
*
* The legacy app never gated on this - it sent and let the device answer - so
* the gate was a regression introduced by the rewrite. These pin the fix: the
* command goes out regardless of the flag, and the device's own refusal is
* what the user sees.
*/
it('sends a private-key save even when isConfigMode reads false', async () => {
const user = userEvent.setup();
const device = createMockDeviceClient();
seedDeviceStore({ device, isConfigMode: false });
Expand All @@ -50,8 +66,39 @@ describe('Advanced page', () => {
);
await user.click(screen.getAllByRole('button', { name: /save to onlykey/i })[1]);

expect(screen.getByText(/flashing red led/i)).toBeInTheDocument();
expect(device.setPrivateKey).not.toHaveBeenCalled();
expect(device.setPrivateKey).toHaveBeenCalledWith(101, 1, expect.any(Array));
expect(screen.queryByText(/flashing red led/i)).not.toBeInTheDocument();
});

it('sends a private-key wipe even when isConfigMode reads false', async () => {
const user = userEvent.setup();
vi.spyOn(window, 'confirm').mockReturnValue(true);
const device = createMockDeviceClient();
seedDeviceStore({ device, isConfigMode: false });
renderWithProviders(<Advanced />);

await user.click(screen.getAllByRole('button', { name: /wipe from onlykey/i })[1]);

expect(device.wipePrivateKey).toHaveBeenCalledWith(101);
expect(screen.queryByText(/flashing red led/i)).not.toBeInTheDocument();
});

it("surfaces the device's own config-mode refusal instead of guessing", async () => {
const user = userEvent.setup();
vi.spyOn(window, 'confirm').mockReturnValue(true);
const device = createMockDeviceClient();
/* What OnlyKeyDevice.formatDeviceLockedError already makes of the firmware's
* "Error not in config mode" - which OKWIPEPRIV now returns for this state. */
device.wipePrivateKey = vi
.fn()
.mockRejectedValue(new Error('OnlyKey must be in config mode (flashing red LED) for this operation.'));
seedDeviceStore({ device, isConfigMode: false });
renderWithProviders(<Advanced />);

await user.click(screen.getAllByRole('button', { name: /wipe from onlykey/i })[1]);

expect(device.wipePrivateKey).toHaveBeenCalledWith(101);
expect(await screen.findByText(/flashing red led/i)).toBeInTheDocument();
});

it('saves an ECC key in config mode', async () => {
Expand Down