From ce3a0f43380539d19b320da7de0d9ae105c8626f Mon Sep 17 00:00:00 2001 From: Cam Proudlock Date: Mon, 24 Aug 2026 08:55:45 -0400 Subject: [PATCH] Secure Send documented, and the machine form stops re-creating a controlling-PC link that already exists --- CHANGELOG.md | 42 ++++++++++ .../machines/frontend/views/MachineForm.vue | 60 +++++++++----- .../machines/frontend/views/controllingPc.js | 68 ++++++++++++++++ .../frontend/views/controllingPc.spec.js | 81 +++++++++++++++++++ 4 files changed, 232 insertions(+), 19 deletions(-) create mode 100644 plugins/machines/frontend/views/controllingPc.js create mode 100644 plugins/machines/frontend/views/controllingPc.spec.js diff --git a/CHANGELOG.md b/CHANGELOG.md index 4f8ca72f..e51b1789 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,30 @@ ADR-007 and ADR-002. ### Added +- **Secure Send: hand somebody a password without leaving it in a chat + window.** A link in Tech Tools that carries one credential, dies when it is + opened, and expires on its own if nobody opens it. The secret is encrypted in + the sender's browser and the key rides in the URL fragment, which browsers + never transmit, so the server stores ciphertext it cannot read and a + compromised database yields nothing. Nobody can recover a lost secret either, + including an administrator, which is the same property seen from the other + side. Redeeming is a POST because chat and mail clients prefetch links to + build previews, and a one-view secret consumed by a preview window is a secret + the recipient never sees; a GET reports availability only. Expiry defaults to + 24 hours (maximum a week) and views to one, whichever comes first, and a spent + row is deleted. Creating needs a login and `tools.secrets`; reading needs + neither, because the recipient usually has no account and the 128-bit token is + the authorisation. The 4096-character ciphertext limit is deliberately the + whole server-side enforcement: a zero-knowledge server cannot tell a password + from a document, so size is the only control it has, and raising it turns this + into the file transfer it was scoped not to be. Documented in + `docs/SECURE-SEND.md`. +- **A site can let people without an account create a Secure Send link.** + New `securesend_allow_guests` setting (Settings > Tools > Secure Send), off by + default because a guest secret has no creator and the audit trail can then say + when and from where but never who. Guest creation is rate limited per address + and the setting fails closed if it cannot be read, so an error can never be + what opens an unauthenticated write endpoint. - **The code generator can print a back side.** A badge card is two-sided and the generator only ever emitted one page per card. The Back side section turns on a second page per card carrying either the same code again, a fixed @@ -81,9 +105,27 @@ ADR-007 and ADR-002. the wrong code. The page index now travels with the page. - The back label's dotted preview outline outspecified the print reset and drew a dashed rule down the edge of every second card. +- **Saving a machine that has a controlling PC answered "this relationship + already exists" and refused the save.** Reported against a map position + change, but the map was incidental: the form re-POSTed the controlling-PC + link on every save, and the row was already there. It used "do I know the + existing link's id" as a stand-in for "is there an existing link", and read + that id from a key the API does not emit, so the two disagreed on any machine + that had a PC. The same conflation meant CLEARING a machine's controlling PC + silently did nothing, which was the more dangerous half and had not been + reported. The loaded PC is now what decides, and re-selecting the same PC is + a no-op rather than a delete-and-recreate. ### Changed +- **Tech Tools no longer asks people to sign in for tools that need no + account.** `/tools`, `/tools/codes` and `/tools/secure-send` were all behind + `requiresAuth`. The label generator runs entirely in the browser - no API, no + data, nothing to protect - so gating it only ever cost a guest the use of it, + and gating the Secure Send page contradicted the guest setting that lets + somebody with no account create a link. The server still decides who may + create a secret: making a page reachable is not the same as allowing the + action, and the two were conflated. - **Settings on the left, labels on the right.** The controls were a narrow column with the preview trailing underneath, so the labels being adjusted were off the bottom of the screen. The preview is now a column of its own that diff --git a/plugins/machines/frontend/views/MachineForm.vue b/plugins/machines/frontend/views/MachineForm.vue index efe0846c..cf8c28f2 100644 --- a/plugins/machines/frontend/views/MachineForm.vue +++ b/plugins/machines/frontend/views/MachineForm.vue @@ -379,6 +379,7 @@ import PrinterAssignmentPicker from '@/components/PrinterAssignmentPicker.vue' import { currentTheme } from '@/stores/theme' import { useIdentifierFlags } from '@/composables/identifierSettings' import { apiError } from '@/utils/apiError' +import { planControllingPcChange } from './controllingPc' const { isEnabled } = useIdentifierFlags() @@ -440,6 +441,9 @@ const relationshipTypes = ref([]) const controllingPcId = ref(null) const relationshipTypeId = ref(null) const existingRelationshipId = ref(null) +// Which PC the link named when the form loaded, so an unchanged one can be left +// alone. Editing a machine is mostly editing something else about it. +const loadedControllingPcId = ref(null) const currentMachine = ref(null) // Filter models by selected machine vendor @@ -575,7 +579,16 @@ onMounted(async () => { if (rel.sourceasset?.assettypename === 'computer' && (rel.relationshiptypename || '').toLowerCase() === 'controls') { controllingPcId.value = rel.sourceasset.assetid relationshipTypeId.value = rel.relationshiptypeid - existingRelationshipId.value = rel.assetrelationshipid + // relationshipid. `assetrelationshipid` is not a key the API has + // ever emitted - to_dict serialises the column names and the + // column is relationshipid - so this read undefined every time, + // and saveRelationship then took the "there is nothing to + // replace" path on a machine that had a controlling PC. Saving + // ANY change, a map position included, re-POSTed the link that + // was already there and the server correctly answered 409 "this + // relationship already exists". + existingRelationshipId.value = rel.relationshipid + loadedControllingPcId.value = rel.sourceasset.assetid break } } @@ -693,31 +706,40 @@ async function saveMachine() { } async function saveRelationship(assetId) { - // If no PC selected and no existing relationship, nothing to do - if (!controllingPcId.value && !existingRelationshipId.value) { + // The decision lives in controllingPc.js, where it is covered by tests. + // See that file for why "is there a link" and "do I know its id" are two + // different questions and what happened when they were treated as one. + const plan = planControllingPcChange({ + selected: controllingPcId.value, + loaded: loadedControllingPcId.value, + existingRelationshipId: existingRelationshipId.value, + }) + + if (plan.action === 'none') { return } - // If clearing the relationship - if (!controllingPcId.value && existingRelationshipId.value) { - await assetsApi.deleteRelationship(existingRelationshipId.value) + if (plan.deleteId !== null) { + await assetsApi.deleteRelationship(plan.deleteId) + existingRelationshipId.value = null + loadedControllingPcId.value = null + } + + if (plan.action === 'unlink') { return } - // If PC is selected - if (controllingPcId.value) { - // If there's an existing relationship, delete it first - if (existingRelationshipId.value) { - await assetsApi.deleteRelationship(existingRelationshipId.value) - } + // PC controls Machine, so the PC is the source and the machine the target. + const created = await assetsApi.createRelationship({ + sourceassetid: controllingPcId.value, + targetassetid: assetId, + relationshiptypeid: relationshipTypeId.value + }) - // Create new relationship (PC controls Machine, so PC is source, Machine is target) - await assetsApi.createRelationship({ - sourceassetid: controllingPcId.value, - targetassetid: assetId, - relationshiptypeid: relationshipTypeId.value - }) - } + // Keep the form honest about what is now stored, so a second save without a + // reload does not repeat the create it just did. + existingRelationshipId.value = created?.data?.data?.relationshipid ?? null + loadedControllingPcId.value = controllingPcId.value } diff --git a/plugins/machines/frontend/views/controllingPc.js b/plugins/machines/frontend/views/controllingPc.js new file mode 100644 index 00000000..1352a4e8 --- /dev/null +++ b/plugins/machines/frontend/views/controllingPc.js @@ -0,0 +1,68 @@ +// What saving the machine form should do about its controlling-PC link. +// +// Pulled out of MachineForm.vue because the component decided this inline and +// got it wrong in a way nothing could catch: it treated "I know the id of the +// existing link" as "there IS an existing link". The id was read from +// `rel.assetrelationshipid`, a key the API has never emitted - the column is +// `relationshipid` and to_dict serialises column names - so it was undefined on +// every load, and the two questions gave opposite answers on a machine that +// plainly had a controlling PC. +// +// Both failures followed from that single conflation: +// - Saving ANY change, a map position included, re-POSTed the link that was +// already there. The server answered 409 "this relationship already +// exists", correctly, and the form surfaced it as a failed save. +// - Clearing the controlling PC did nothing at all, because "no selection and +// no known id" reads as "nothing to do". +// +// So THE LOADED PC IS THE AUTHORITY on whether a link exists, and the +// relationship id is only ever used to delete a row. If the id goes missing +// again the worst case is a link that cannot be removed, never a duplicate +// create. + +/** + * Decide what to do with the controlling-PC link on save. + * + * @param {object} state + * @param {number|string|null} state.selected - PC id chosen in the form now. + * @param {number|string|null} state.loaded - PC id the link named on load. + * @param {number|null} state.existingRelationshipId - row id to delete, if known. + * @returns {{action: 'none'|'unlink'|'link'|'relink', deleteId: number|null}} + */ +export function planControllingPcChange({ selected, loaded, existingRelationshipId } = {}) { + const now = normalize(selected) + const before = normalize(loaded) + const deleteId = existingRelationshipId ?? null + + // Unchanged is not a change. Somebody moving a marker on the map is not + // asking to be re-linked to the same PC, and delete-then-create on an + // untouched link opens a window where the machine has no controlling PC - + // which resolve_asset_position walks across, so a failure between the two + // calls would lose the machine's inherited position along with the link. + if (now === before) { + return { action: 'none', deleteId: null } + } + + if (now === null) { + // Cleared. Nothing to delete means nothing to do, rather than an error: + // the link is already absent, which is the state being asked for. + return deleteId === null + ? { action: 'none', deleteId: null } + : { action: 'unlink', deleteId } + } + + if (before === null) { + return { action: 'link', deleteId: null } + } + + // Swapped to a different PC. The old row goes first: 'controls' is singular + // per source, not per target, so leaving it would leave the machine with two. + return { action: 'relink', deleteId } +} + +// Ids arrive as numbers from the API and strings from a hands back '42' while the relationship carried 42. Comparing + // those raw makes an untouched link look like a swap, which is the same + // duplicate POST by another route. + expect(planControllingPcChange({ + selected: '42', loaded: 42, existingRelationshipId: 7, + })).toEqual({ action: 'none', deleteId: null }) + }) + + it('treats an empty selection as cleared, not as a PC named ""', () => { + expect(planControllingPcChange({ + selected: '', loaded: 42, existingRelationshipId: 7, + })).toEqual({ action: 'unlink', deleteId: 7 }) + }) + + it('does nothing when there was no PC and none was chosen', () => { + expect(planControllingPcChange({ + selected: null, loaded: null, existingRelationshipId: null, + })).toEqual({ action: 'none', deleteId: null }) + }) + + it('does not attempt a delete it cannot perform', () => { + // Cleared, but the row id was never learned. Asking the API to delete + // `undefined` produced a request against /relationships/undefined. + expect(planControllingPcChange({ + selected: null, loaded: 42, existingRelationshipId: undefined, + })).toEqual({ action: 'none', deleteId: null }) + }) +})