diff --git a/src-tauri/src/commands/license_commands.rs b/src-tauri/src/commands/license_commands.rs index d44bce2..8977430 100644 --- a/src-tauri/src/commands/license_commands.rs +++ b/src-tauri/src/commands/license_commands.rs @@ -1979,6 +1979,22 @@ mod tests { assert!(dir.0.join(ACTIVATION_FILE).exists(), "the same key keeps its activation"); } + #[test] + fn store_refuses_another_key_while_the_previous_token_cannot_be_dropped() { + // Kept next to the new key, that token would name the previous license to the price + // server (#347 review, the folder of #367): the key change stops on `Storage` instead, + // and the previous key stays (#387 review). + let (enc, dec) = default_keys(); + let dir = TestDir::new("store_token_not_deletable"); + let old_key = base_license_with_features(&enc, vec!["balance".to_string()]); + dir.write(LICENSE_FILE, &old_key); + // A directory in place of the token: removing it as a file fails. + fs::create_dir(dir.0.join(ACTIVATION_FILE)).unwrap(); + let new_key = base_license_with_features(&enc, vec![]); + assert_eq!(store_license_in(&dir.0, &new_key, &dec).unwrap_err(), LicenseError::Storage); + assert_eq!(fs::read_to_string(dir.0.join(LICENSE_FILE)).unwrap(), old_key); + } + #[test] fn store_drops_a_token_left_without_a_key() { let (enc, dec) = default_keys(); @@ -3575,6 +3591,16 @@ mod tests { assert_eq!(remove_activation_token_if_unchanged_in(&dir.0, " token-a"), Ok(true)); assert!(!dir.0.join(ACTIVATION_FILE).exists()); + // A token that does not read as text: nothing says it is the one the answer is about, + // so it stays (#388 review; the directory below cannot tell, its deletion fails anyway). + dir.write(ACTIVATION_FILE, NOT_UTF8); + assert_eq!( + remove_activation_token_if_unchanged_in(&dir.0, "token-a"), + Err(LicenseError::Storage) + ); + assert!(dir.0.join(ACTIVATION_FILE).exists(), "an unreadable token stays"); + fs::remove_file(dir.0.join(ACTIVATION_FILE)).unwrap(); + // A directory in place of the token: it cannot be read as a file. fs::create_dir(dir.0.join(ACTIVATION_FILE)).unwrap(); assert_eq!( diff --git a/src/components/settings/LicenseCard.render.test.tsx b/src/components/settings/LicenseCard.render.test.tsx index defd88d..3edc275 100644 --- a/src/components/settings/LicenseCard.render.test.tsx +++ b/src/components/settings/LicenseCard.render.test.tsx @@ -10,10 +10,13 @@ vi.mock("@tauri-apps/api/core", () => ({ invoke: vi.fn() })); vi.mock("@tauri-apps/plugin-opener", () => ({ openUrl: vi.fn(() => Promise.resolve()) })); let language = "fr"; +// The fake `t` marks what it translates, `t(key[options])`: the assertions written in that form +// fail on a string that skipped `t` (#387 review). Substring assertions on a bare key still read +// the key inside the mark. vi.mock("react-i18next", () => ({ useTranslation: () => ({ t: (k: string, o?: Record) => - o ? `${k}[${Object.entries(o).map(([a, b]) => `${a}=${String(b)}`).join(",")}]` : k, + o ? `t(${k}[${Object.entries(o).map(([a, b]) => `${a}=${String(b)}`).join(",")}])` : `t(${k})`, i18n: { language }, }), })); @@ -300,8 +303,8 @@ describe("LicenseCard machines", () => { it("locks key entry while an activation runs, and Activate while a key is stored", () => { // Either order would leave the previous key's token next to the new key (#364 review). - const enterKeyDisabled = /]*disabled=""[^>]*>license\.enterKey<\/button>/; - const activateDisabled = /]*disabled=""[^>]*>license\.activate<\/button>/; + const enterKeyDisabled = /]*disabled=""[^>]*>t\(license\.enterKey\)<\/button>/; + const activateDisabled = /]*disabled=""[^>]*>t\(license\.activate\)<\/button>/; machines = hookView({ activation: notActivated, activating: true }); expect(render()).toMatch(enterKeyDisabled); @@ -328,11 +331,11 @@ describe("LicenseCard machines", () => { // "Remove the key" shows for any stored key, valid or revoked (#370): `hasLicense` (a paid // edition) would hide it for a revoked key, whose daily check is what the removal stops. describe("LicenseCard key removal (#370)", () => { - const REMOVE = "license.removeKey.button"; - const removeDisabled = /]*disabled=""[^>]*>license\.removeKey\.button<\/button>/; - const enterKeyDisabled = /]*disabled=""[^>]*>license\.enterKey<\/button>/; - const activateDisabled = /]*disabled=""[^>]*>license\.activate<\/button>/; - const deactivateDisabled = /]*disabled=""[^>]*>license\.machines\.deactivate<\/button>/; + const REMOVE = "t(license.removeKey.button)"; + const removeDisabled = /]*disabled=""[^>]*>t\(license\.removeKey\.button\)<\/button>/; + const enterKeyDisabled = /]*disabled=""[^>]*>t\(license\.enterKey\)<\/button>/; + const activateDisabled = /]*disabled=""[^>]*>t\(license\.activate\)<\/button>/; + const deactivateDisabled = /]*disabled=""[^>]*>t\(license\.machines\.deactivate\)<\/button>/; it("is offered for a valid key", () => { const html = render(); @@ -376,7 +379,7 @@ describe("LicenseCard key removal (#370)", () => { it("while it runs, says so, and key entry, Activate and Deactivate wait", () => { machines = hookView({ activation: notActivated, removing: true }); let html = render(); - expect(html).toContain("license.removeKey.removing"); + expect(html).toContain("t(license.removeKey.removing)"); expect(html).not.toContain(REMOVE); expect(html).toMatch(enterKeyDisabled); expect(html).toMatch(activateDisabled); @@ -396,7 +399,7 @@ describe("LicenseCard key removal (#370)", () => { it("names a failed removal with its reason", () => { machines = hookView({ removeError: parseLicenseError('{"code":"storage"}') }); const html = render(); - expect(html).toContain("license.removeKey.failed[reason=license.errors.storage]"); + expect(html).toContain("t(license.removeKey.failed[reason=t(license.errors.storage)])"); expect(html.split("license.removeKey.failed")).toHaveLength(2); }); }); diff --git a/src/contexts/LicenseContext.test.ts b/src/contexts/LicenseContext.test.ts index 0b13455..be55532 100644 --- a/src/contexts/LicenseContext.test.ts +++ b/src/contexts/LicenseContext.test.ts @@ -601,3 +601,33 @@ describe("performRevocationCheck", () => { expect(deps.refresh).not.toHaveBeenCalled(); }); }); + +// The launcher relays the revoked state the provider read: a reload follows a change only. +// Every other test of the launcher starts from a key that is not revoked (#386 review). +describe("revocationCheckLauncher", () => { + const settle = () => new Promise((resolve) => setTimeout(resolve, 0)); + function launcherHarness(result: RevocationCheck) { + const deps: RevocationCheckDeps = { + checkLicenseRevocation: vi.fn(async (): Promise => result), + refresh: vi.fn(async () => {}), + logInfo: vi.fn(), + logWarn: vi.fn(), + }; + return deps; + } + + it("reloads when a key known as revoked is confirmed valid again", async () => { + const deps = launcherHarness({ outcome: "valid", revoked: false }); + revocationCheckLauncher(deps)(true); + await settle(); + expect(deps.refresh).toHaveBeenCalledTimes(1); + }); + + it("does not reload a key known as revoked that is still revoked", async () => { + const deps = launcherHarness({ outcome: "revoked", revoked: true }); + revocationCheckLauncher(deps)(true); + await settle(); + expect(deps.checkLicenseRevocation).toHaveBeenCalledTimes(1); + expect(deps.refresh).not.toHaveBeenCalled(); + }); +}); diff --git a/src/hooks/useLicenseMachines.test.ts b/src/hooks/useLicenseMachines.test.ts index c5dc556..892d3a8 100644 --- a/src/hooks/useLicenseMachines.test.ts +++ b/src/hooks/useLicenseMachines.test.ts @@ -642,7 +642,7 @@ describe("useLicenseMachines wiring to the LicenseContext", () => { await view.removeKey({ confirm, t: echo }); - expect(confirm).toHaveBeenCalledWith("license.removeKey.confirmRevoked"); + expect(confirm).toHaveBeenCalledWith("t(license.removeKey.confirmRevoked)"); expect(invoke).toHaveBeenCalledWith("remove_license", { releaseSlot: false }); expect(context.refresh).toHaveBeenCalledTimes(1); expect(log.logInfo).toHaveBeenCalledWith("License: the stored key was removed"); @@ -653,17 +653,19 @@ describe("useLicenseMachines wiring to the LicenseContext", () => { const confirm = vi.fn(() => false); const view = renderHook(); await view.removeKey({ confirm, t: echo }); - expect(confirm).toHaveBeenCalledWith("license.removeKey.confirmValid"); + expect(confirm).toHaveBeenCalledWith("t(license.removeKey.confirmValid)"); expect(invoke).not.toHaveBeenCalled(); }); }); -// The removal of the stored key (#370). The fake `t` echoes the key and its options, so each -// assertion names the message the user would read. +// The removal of the stored key (#370). The fake `t` marks what it translates, `t(key[options])`, +// so each assertion names the message the user would read, and a string that skipped `t` (a raw +// key) fails it (#387 review). function echo(key: string, options?: Record): string { - return options - ? `${key}[${Object.entries(options).map(([name, value]) => `${name}=${String(value)}`).join(",")}]` - : key; + const params = options + ? `[${Object.entries(options).map(([name, value]) => `${name}=${String(value)}`).join(",")}]` + : ""; + return `t(${key}${params})`; } describe("licenseMachinesReducer — removal of the key", () => { @@ -776,7 +778,7 @@ describe("performRemoveLicense", () => { it("calls nothing when the confirmation is refused", async () => { const { deps, events, getState } = removalHarness([false], []); await performRemoveLicense(deps, activated); - expect(events).toEqual(["confirm license.removeKey.confirmActivated"]); + expect(events).toEqual(["confirm t(license.removeKey.confirmActivated)"]); expect(deps.removeLicense).not.toHaveBeenCalled(); expect(deps.refresh).not.toHaveBeenCalled(); expect(deps.logInfo).not.toHaveBeenCalled(); @@ -788,7 +790,7 @@ describe("performRemoveLicense", () => { const { deps, events, getState } = removalHarness([true], [removed]); await performRemoveLicense(deps, valid); expect(events).toEqual([ - "confirm license.removeKey.confirmValid", + "confirm t(license.removeKey.confirmValid)", "REMOVE_START", "remove(false)", "refresh", @@ -805,7 +807,7 @@ describe("performRemoveLicense", () => { const { deps, events } = removalHarness([true], [removed]); await performRemoveLicense(deps, activated); expect(events).toEqual([ - "confirm license.removeKey.confirmActivated", + "confirm t(license.removeKey.confirmActivated)", "REMOVE_START", "remove(true)", "refresh", @@ -820,7 +822,7 @@ describe("performRemoveLicense", () => { const { deps, events } = removalHarness([true], [removed]); await performRemoveLicense(deps, revoked); expect(events.slice(0, 3)).toEqual([ - "confirm license.removeKey.confirmRevoked", + "confirm t(license.removeKey.confirmRevoked)", "REMOVE_START", "remove(false)", ]); @@ -829,10 +831,10 @@ describe("performRemoveLicense", () => { // Error table, second row: the server could not be reached or answered badly. Rust removed // nothing; the second confirmation names the reason and warns that the slot stays occupied. const retryable: [LicenseErrorCode, Record, string][] = [ - ["network", {}, "license.errors.network"], - ["server", { status: 502 }, "license.errors.server"], - ["rate_limit", { retry_after_s: 42 }, "license.errors.rateLimit[count=42]"], - ["invalid_response", {}, "license.errors.invalidResponse"], + ["network", {}, "t(license.errors.network)"], + ["server", { status: 502 }, "t(license.errors.server)"], + ["rate_limit", { retry_after_s: 42 }, "t(license.errors.rateLimit[count=42])"], + ["invalid_response", {}, "t(license.errors.invalidResponse)"], ]; it.each(retryable)( @@ -841,10 +843,10 @@ describe("performRemoveLicense", () => { const { deps, events, getState } = removalHarness([true, true], [failure(code, extra), removed]); await performRemoveLicense(deps, activated); expect(events).toEqual([ - "confirm license.removeKey.confirmActivated", + "confirm t(license.removeKey.confirmActivated)", "REMOVE_START", "remove(true)", - `confirm license.removeKey.confirmWithoutRelease[reason=${reason}]`, + `confirm t(license.removeKey.confirmWithoutRelease[reason=${reason}])`, "remove(false)", "refresh", "REMOVE_END", @@ -888,14 +890,15 @@ describe("performRemoveLicense", () => { const { deps, events, getState } = removalHarness([true], [failure(code)]); await performRemoveLicense(deps, activated); expect(events).toEqual([ - "confirm license.removeKey.confirmActivated", + "confirm t(license.removeKey.confirmActivated)", "REMOVE_START", "remove(true)", + "refresh", "REMOVE_ERROR", ]); expect(getState().removeError?.code).toBe(code); expect(getState().removing).toBe(false); - expect(deps.refresh).not.toHaveBeenCalled(); + expect(deps.refresh).toHaveBeenCalledTimes(1); expect(deps.logInfo).not.toHaveBeenCalled(); expect(deps.logWarn).toHaveBeenCalledWith("License: the stored key could not be removed", code); }); @@ -905,9 +908,10 @@ describe("performRemoveLicense", () => { const { deps, events, getState } = removalHarness([true, true], [failure("network")]); await performRemoveLicense(deps, valid); expect(events).toEqual([ - "confirm license.removeKey.confirmValid", + "confirm t(license.removeKey.confirmValid)", "REMOVE_START", "remove(false)", + "refresh", "REMOVE_ERROR", ]); expect(getState().removeError?.code).toBe("network"); @@ -916,9 +920,19 @@ describe("performRemoveLicense", () => { it("names a removal that fails after the slot was given up", async () => { const { deps, events, getState } = removalHarness([true, true], [failure("network"), failure("storage")]); await performRemoveLicense(deps, activated); - expect(events.slice(-2)).toEqual(["remove(false)", "REMOVE_ERROR"]); + expect(events.slice(-3)).toEqual(["remove(false)", "refresh", "REMOVE_ERROR"]); expect(getState().removeError?.code).toBe("storage"); - expect(deps.refresh).not.toHaveBeenCalled(); + expect(deps.refresh).toHaveBeenCalledTimes(1); expect(deps.logWarn).toHaveBeenLastCalledWith("License: the stored key could not be removed", "storage"); }); + + it("reloads the license context after a failure, before naming it", async () => { + // Rust deletes the key first: a token that resisted leaves the key gone, and the card must + // follow the disk rather than keep the key under "not removed" (#387 review). + const { deps, events, getState } = removalHarness([true], [failure("storage")]); + await performRemoveLicense(deps, valid); + expect(events.slice(-2)).toEqual(["refresh", "REMOVE_ERROR"]); + expect(deps.refresh).toHaveBeenCalledTimes(1); + expect(getState().removeError?.code).toBe("storage"); + }); }); diff --git a/src/hooks/useLicenseMachines.ts b/src/hooks/useLicenseMachines.ts index 85597d8..67038bf 100644 --- a/src/hooks/useLicenseMachines.ts +++ b/src/hooks/useLicenseMachines.ts @@ -29,9 +29,11 @@ import { useLicenseContext } from "../contexts/LicenseContext"; // A `license_revoked` answer, from any of the three calls, is persisted by the Rust // command that received it (#365): this hook only reloads the LicenseContext, so the // edition (Free) and the card follow. The revocation check, made at app start and after -// a key is entered (#373), shares that budget: with it, the machine-limit path reaches -// the 5 requests of the first minute. Removing the key of an activated machine spends one -// deactivate call (#370). +// a key is entered (#373), shares that budget: one check brings the machine-limit path to +// the 5 requests of the first minute, and two checks in that minute (one at start for a +// previous key, one when a new key is entered) bring it to 6, so its last call can get a +// 429, which the card names as a rate-limit error. Removing the key of an activated +// machine spends one deactivate call (#370). // // Loading the list may remove this machine's activation token, when the list names no // activation of this machine (#367): the activation status is read again after every list @@ -370,6 +372,9 @@ export interface RemoveLicenseDeps extends RemoveLicenseUi { * staying occupied. Any other failure is named on the card. Each removal, and each release or * removal that failed, is written to the in-app log. After a removal the license context is * reloaded: the card remounts without a key, and the revocation check has nothing to send. + * It is reloaded after a failure too (local reads only): Rust deletes the key first, so a + * token that could not be deleted leaves the key gone, and the card must not keep showing it + * under "the key was not removed". */ export async function performRemoveLicense( deps: RemoveLicenseDeps, @@ -392,6 +397,7 @@ export async function performRemoveLicense( } if (!result.ok) { deps.logWarn(REMOVE_LOG.failed, result.error.code); + await deps.refresh(); deps.dispatch({ type: "REMOVE_ERROR", error: result.error }); return; }