diff --git a/CHANGELOG.md b/CHANGELOG.md index af07bb30..aeba1ac2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,213 @@ ADR-007 and ADR-002. ### Added +- **Reading anything now needs a permission.** A sweep of the URL map found + **165 GET routes that answered anybody with no token at all**: the whole + asset inventory, every machine, PC, printer and network device, the USB + checkout history and who holds which drive, the staff directory, the core + reports. Guest access had been argued about one feature-flag at a time while + the front door stood open. Every one of them is now behind the read + permission it belongs to - `assets.view`, `machines.view`, `printers.view` + and so on - which also means the guest role can open exactly the ones a site + chooses and nothing else. + + Thirty-three endpoints stay public, each named with its reason in + `tests/test_core/test_public_read_surface.py`: the app shell and its + bootstrap, the settings allowlist, images fetched by `` (which + carries no Authorization header), what a display kiosk renders before anybody + signs in, the parts and USB kiosks, the client scripts that run as SYSTEM + before a user session exists, and a secure-send link whose token is itself + the authorisation. That file is a standing gate: a new public read fails CI + until somebody adds it to the list, in front of a reviewer. Four more reads + need a login but no permission because the handler decides from who you are - + your own user record, your own API tokens, the role catalogue - and the same + test proves each still refuses an anonymous caller. + + The employees plugin gained `employees.view`: the directory is names, SSOs, + photographs and reporting lines, and it had no permission at all. + +- **A guide's sections can be ticked off while you read it.** Only checklist + items were tickable, which answered "what have I done" but not the question a + long procedure actually raises - where was I. Somebody looks away to do the + thing the guide asked for and looks back at a wall of text. Every step and + every heading now carries a checkbox in the reader; ticking one strikes it + through and fades it rather than hiding it, because scanning back up the page + has to still show what was done. Headings count as well as steps: a guide + converted from an HTML article often has fifty headings and no steps at all. + Progress is per browser and per version, as it already was for checklists - + shopfloor PCs share a login, so one technician's ticks must not greet the + next person on that bay. + +- **A guest is a role now, not a switch per feature.** Whether somebody with no + account may see something was answered one feature at a time: a + `securesend_allow_guests` setting, an enable flag per kiosk, and very nearly a + third for guides - each with its own settings page and its own hand-written + branch inside a route. It is a permissions question, and this app already + administers permissions per role, so anyone not signed in now holds the + seeded `guest` role. It starts with nothing, so an upgrade opens no door; a + site opens one by ticking a box in the same role grid it uses for everybody + else, and closes it the same way - taking effect on the next request, not the + next restart. `require_permission` consults the role when there is no token, + which is the whole mechanism. + + What a guest may be granted is bounded: any `.view` or `.read` permission, + plus writes a plugin has explicitly declared safe to perform without an + account (`get_guest_grantable_permissions`, contract 0.21.0). Secure Send + declares its create permission, because sending a password to a vendor is the + one write the product means to offer somebody who never signs in. Everything + else is refused by the API and greyed out in the grid rather than warned + about. A guest write records no creator - the audit trail can say when and + from where, never who - and that is the reason for the bar. + + GE-Enforce and Slides gained read permissions (`geenforce.view`, + `slides.view`) so they can be opened to guests without handing out the manage + right; their read routes accept either, so nothing an existing manager could + reach has moved. The navigation endpoint now returns only what the caller can + actually reach, so a signed-out visitor sees the pages that were opened to + them rather than a sidebar of links that bounce off a login page. + +- **USB check-in / check-out audit report.** CMMC removable-media tracking is an + audit obligation and there was no report for it - answering "who had this + drive, and when" meant paging through the check-in/out list by hand. The + window is named rather than a day count: today, last month, six months, a + year, or all time. Events rather than totals, because that is the question an + audit actually asks, with a CSV export for the people who want it in a + spreadsheet. Works against the external `cmmc_usb` log or the self-hosted + tables; in self-hosted mode one stored loan expands into a take and a return, + so a returned drive is distinguishable from one still out. + +- **`flask guides export` / `flask guides import` - a guide can reach another + site.** Sites are separate databases and the ones that matter are air-gapped + from wherever a guide gets written, so a procedure written once was stranded + where it was typed. Export writes one `.guide.json` per guide with its images + carried inside it - one file, because a bundle that needs its images copied + alongside it is a bundle that will eventually arrive without them - and + import reads them back. The slug travels, since that is the address somebody + pastes into a work order and the same guide should answer to the same name on + every site; ids, revision history and publication state do not, because those + belong to the database that issued them. Imports arrive as drafts; + `--replace` updates a guide already carrying that slug rather than creating a + second one, which is how a corrected guide is re-shipped. + +- **`flask guides import-html` - an existing HTML article becomes a guide.** + A site adopting this app arrives holding procedures already written: styled + HTML pages that get mailed around and opened off a share. Retyping one into + the editor is how a good procedure stays on a share forever. The importer + reads the page and produces blocks - steps, callouts, code, checklists, + tables, lists - and lifts embedded images into `guideimages` so a 1.6 MB + document stops carrying its screenshots as base64. Imports land as DRAFTS, + and `--dry-run` (the default) reports what it would produce without writing. + Conversion is deliberately lossy: colour, fonts and layout go, because that + is the whole reason a guide is stored as blocks rather than markup. The + blocks go through the same validator the editor uses, so an import cannot + store something the editor could not have produced. + +### Fixed + +- **Tech Tools was invisible to anyone without an account**, even though every + tool on the page is deliberately usable without one and Secure Send is the + single write this product offers a guest. The nav item named no permission, + because the plugin had none to name, so the sidebar filter dropped it. The + plugin gained `tools.view`, and the item now names either that or + `tools.secrets` - somebody granted only Secure Send still needs the entry + that leads to it. + +- **The sidebar offered pages the reader could not open, and hid ones they + could.** A nav item is hidden from anybody who does not hold its permission, + but only four items ever declared one - so the filter did nothing for the + rest, and a signed-out visitor granted `machines.view` still saw an empty + sidebar. Every item now names the read permission its page needs, in the + plugins, in the core navigation endpoint and in the frontend's fallback list. + A signed-in member with a sparse role will notice the same correction: a link + that could only answer 403 no longer appears. + +- **Every report offers to email itself, not just two of them.** `POST + /api/reports/email` has always taken any `{columns, rows}`, but the button was + on the toner and warranty pages only - because those two happen to be their + own routed pages, and the reports page had no button at all. It is now beside + Export CSV for every report the page can build a table from, which is all of + them: the plugin reports through their declared columns, the core reports + through the flat list they return, and the asset inventory summary by + flattening its three tables into one with the grouping named in a column. + Presentation-only fields - a status colour, a link id - are left out, since a + column of hex codes is worse than no column. The button is hidden without the + `reports.export` permission the endpoint enforces, on all three pages: offering + somebody a button that can only answer 403 is worse than not offering it. + +- **A long drive name pushed Take and Return out of the card.** On the USB kiosk + the name is a flex item, which defaults to `min-width: auto`, so it kept its + full intrinsic width and shoved the button through the side of the card - + leaving a drive nobody could take. Names now wrap, including the serial-style + ones that carry no spaces at all, and a description is clamped to two lines so + one long name cannot make every card in its row tall. The locker line is never + clipped: it is how somebody finds the thing. + +- **Muted text on the USB kiosk and in guides was not muted.** Both named a + `--textmuted` variable this app has never defined, so the browser dropped the + declaration and the hints, subtitles and locker lines rendered at full + strength against the text they were meant to sit behind. The variable is + `--text-light`. + +- **Plugin reports rendered a raw JSON dump.** The reports page ran a hardcoded + switch over five core report ids and fell through to + `console.error('Unknown report')`, and its results panel was a matching chain + of per-id templates ending in a `JSON.stringify`. So a plugin could register a + report, watch it appear in the catalogue, and get a wall of JSON when it was + opened - which is where the three printedparts reports have been since they + were added. Any report that declares an endpoint is now fetched from it, and + any report returning `{columns, rows}` renders as a table. A plugin needs no + core code to have a working report. CSV export follows the same endpoint + rather than assuming `/api/reports/`, which does not exist for a plugin + report, and a report that fails now says so instead of showing an empty panel. + Export CSV is fetched through the api client rather than `window.open`: a new + window carries no Authorization header, so every report behind a real + `jwt_required` answered the export button with a 401. The core reports are + jwt-optional, which is the only reason the old way ever appeared to work. + +- **Reports had no way to choose a time period.** Endpoints accepted a window; + the page offered no control for it, so `printedparts-consumption` was + permanently stuck on its 30-day default. + +- **USB kiosk: check a drive out and back in without a login.** The people who + actually take USB drives are standing at the cabinet, and most of them have no + ShopDB account - so check-out and check-in were gated behind a login that was + not doing much anyway, since both modes already attribute the action to a + scanned BADGE rather than to the signed-in user. `/usb-kiosk` is a full-screen + touch screen modelled on the parts kiosk, and works against the self-hosted + tables or an existing `cmmc_usb` database. + + It meets the same bar as the parts kiosk: it can move a device between + checked-out and checked-in and nothing else - no create, edit or retire - and + it does not exist at all until a site sets `usb_kiosk_enabled`. An + unauthenticated write is a decision about the door. + + The drives carry no barcode, so unlike the parts kiosk there is nothing to + present and the screen has to offer a LIST. That widens what a logged-out + screen exposes, so the list is not anonymous: an SSO comes first and has to + resolve to a real person before any drive is named - the same bar taking one + already had to clear. The main screen shows the caller's own drives, what is + free, and a COUNT of everything else; the holders of other people's drives + appear only behind a deliberate second tap, so a passer-by cannot read the + whole cabinet off a wall panel. + + A badge that resolves to no employee is REFUSED, unlike the signed-in route + which creates the user row: a checkout nobody can trace to a person defeats + the point, and at a kiosk there is no operator standing there to vouch for it. + + Returning somebody else's drive asks first, then records the badge that was + scanned - so the log says who actually handed it back, which is the honest + record and needs no change to a schema this app does not own. Refusing a + mismatch outright is available as a setting, but is not the default: a drive + left behind by somebody on leave would then be unreturnable at the kiosk, + which just moves the problem to a person with a login. + + The list of free drives is filtered from a search box, and lays itself out in + as many columns as the panel is wide - up to four on a wall display. A cabinet + of thirty drives was one long column somebody had to read through. A drive is + found by its id, by what is written on it, or by where it lives, in whatever + word order comes naturally: "locker engineer" finds the engineering locker, + because that is what people say. + - **Audit log retention.** The audit log is append-only and nothing ever removed a row, so it grew for as long as a site ran - here to the point where the audit page was tens of thousands of pages deep and therefore useless, because diff --git a/frontend/src/router/index.js b/frontend/src/router/index.js index 37562394..93dc2df6 100644 --- a/frontend/src/router/index.js +++ b/frontend/src/router/index.js @@ -131,6 +131,7 @@ const TITLE_SPELLINGS = { 'knowledgebase': 'Knowledge Base', 'printedparts': '3D Printed Parts', 'parts-kiosk': 'Parts Kiosk', + 'usb-kiosk': 'USB Kiosk', 'tv': 'TV Slideshow', 'shopfloor': 'Shopfloor Dashboard', 'measuringtools': 'Measuring Tools', diff --git a/frontend/src/stores/auth.js b/frontend/src/stores/auth.js index 2f0a96dd..9d8faaf5 100644 --- a/frontend/src/stores/auth.js +++ b/frontend/src/stores/auth.js @@ -75,13 +75,25 @@ export const useAuthStore = defineStore('auth', { } }, + // Re-read the signed-in user from the server, permissions included. + // + // The login response is the ONLY thing that used to populate this, and it + // is cached in localStorage - so granting somebody a permission did nothing + // visible until they happened to log out and back in. They were told they + // had rights, the button stayed hidden, and nothing explained why. Called + // on app boot so a permission change lands on the next page load. + // + // Only a 401 logs out. A transient failure - API restarting, network blip - + // must not throw somebody out of a session they still hold; the cached user + // is stale, not invalid. async fetchUser() { try { const response = await authApi.me() this.user = response.data.data await this.enrichFromDirectory() + localStorage.setItem('user', JSON.stringify(this.user)) } catch (error) { - this.logout() + if (error.response?.status === 401) this.logout() } }, diff --git a/frontend/src/utils/reportTable.js b/frontend/src/utils/reportTable.js new file mode 100644 index 00000000..41e207d3 --- /dev/null +++ b/frontend/src/utils/reportTable.js @@ -0,0 +1,69 @@ +// Turning a report payload into the one table shape everything else speaks. +// +// POST /api/reports/email has always taken {columns, rows} from any caller, but +// only two pages ever offered an Email report button - the two that happen to +// be their own routed pages. Every other report was reachable only from the +// reports page, which had no button at all. This builds that table from +// whatever shape the open report returned, so the offer is the same everywhere. + +export function titleCase (text) { + return String(text || '') + .split(/[\s_-]+/) + .map(word => word.charAt(0).toUpperCase() + word.slice(1)) + .join(' ') +} + +// Presentation-only keys. A colour swatch and a row id mean nothing in an +// email, and a column of hex codes is worse than no column at all. +const EMAIL_SKIP = ['color', 'linkid', 'linkurl', 'id'] + +function emailcolumns (keys) { + return keys.filter(key => !EMAIL_SKIP.includes(key)) + .map(key => ({ key, label: titleCase(key) })) +} + +export function emailTableFrom (data) { + if (!data) return null + + // Any report that speaks the {columns, rows} contract - every plugin report, + // and any core one that adopts it. Declared columns win; otherwise take them + // from the first row, the same way the on-screen table does. + if (Array.isArray(data.rows) && data.rows.length) { + const keys = (Array.isArray(data.columns) && data.columns.length) + ? data.columns + : Object.keys(data.rows[0]) + return { columns: emailcolumns(keys), rows: data.rows } + } + + // The core reports, which return a flat list under `data`. + if (Array.isArray(data.data) && data.data.length) { + return { columns: emailcolumns(Object.keys(data.data[0])), rows: data.data } + } + + // Asset inventory is three tables. One email, so they become one table with + // the grouping named in a column - sending a third of it would be worse. + if (data.data && typeof data.data === 'object') { + const rows = [] + for (const [section, entries] of Object.entries(data.data)) { + if (!Array.isArray(entries)) continue + for (const entry of entries) { + rows.push({ + section: titleCase(section), + name: entry.type || entry.status || entry.location || '', + count: entry.count, + }) + } + } + if (!rows.length) return null + return { + columns: [{ key: 'section', label: 'Grouping' }, + { key: 'name', label: 'Name' }, + { key: 'count', label: 'Count' }], + rows, + } + } + + // An empty report has nothing to send. The button hides rather than mailing + // somebody a table with no rows in it. + return null +} diff --git a/frontend/src/utils/reportTable.spec.js b/frontend/src/utils/reportTable.spec.js new file mode 100644 index 00000000..a63167f2 --- /dev/null +++ b/frontend/src/utils/reportTable.spec.js @@ -0,0 +1,84 @@ +// Every shape a report on this app actually returns, because the Email report +// button now has to build a table out of all of them rather than only the two +// that had their own page. + +import { describe, it, expect } from 'vitest' + +import { emailTableFrom, titleCase } from './reportTable' + +describe('emailTableFrom', () => { + it('has nothing to send for a report that has not run', () => { + expect(emailTableFrom(null)).toBe(null) + expect(emailTableFrom({})).toBe(null) + }) + + it('uses the declared columns of a {columns, rows} report', () => { + const table = emailTableFrom({ + columns: ['timestamp', 'action', 'device_id'], + rows: [{ timestamp: 't', action: 'check-out', device_id: 'USB-3' }], + }) + expect(table.columns).toEqual([ + { key: 'timestamp', label: 'Timestamp' }, + { key: 'action', label: 'Action' }, + { key: 'device_id', label: 'Device Id' }, + ]) + expect(table.rows).toHaveLength(1) + }) + + it('falls back to the first row when a report declares no columns', () => { + const table = emailTableFrom({ rows: [{ item: 'a', takes: 2 }] }) + expect(table.columns.map(c => c.key)).toEqual(['item', 'takes']) + }) + + it('builds a table from a core report that returns a flat list', () => { + const table = emailTableFrom({ + report: 'machines_by_type', + total: 231, + data: [{ machinetype: 'Lathe', description: 'd', count: 12 }], + }) + expect(table.columns.map(c => c.key)).toEqual(['machinetype', 'description', 'count']) + expect(table.rows[0].count).toBe(12) + }) + + it('drops the keys that only mean something on screen', () => { + const table = emailTableFrom({ + data: [{ status: 'In Use', color: '#28a745', count: 572 }], + }) + expect(table.columns.map(c => c.key)).toEqual(['status', 'count']) + + const kb = emailTableFrom({ + data: [{ linkid: 4, linkurl: 'http://x', shortdescription: 'How to', clicks: 9 }], + }) + expect(kb.columns.map(c => c.key)).toEqual(['shortdescription', 'clicks']) + }) + + it('flattens the three tables of asset inventory into one, named', () => { + const table = emailTableFrom({ + data: { + bytype: [{ type: 'computer', count: 289 }], + bystatus: [{ status: 'In Use', color: '#0f0', count: 572 }], + bylocation: [{ location: 'Cell A', count: 3 }], + }, + }) + expect(table.columns.map(c => c.key)).toEqual(['section', 'name', 'count']) + expect(table.rows).toEqual([ + { section: 'Bytype', name: 'computer', count: 289 }, + { section: 'Bystatus', name: 'In Use', count: 572 }, + { section: 'Bylocation', name: 'Cell A', count: 3 }, + ]) + }) + + it('has nothing to send for a report that came back empty', () => { + expect(emailTableFrom({ data: [], total: 0 })).toBe(null) + expect(emailTableFrom({ columns: ['a'], rows: [] })).toBe(null) + expect(emailTableFrom({ data: { bytype: [], bystatus: [] } })).toBe(null) + }) +}) + +describe('titleCase', () => { + it('turns a column key into a heading', () => { + expect(titleCase('locker_location')).toBe('Locker Location') + expect(titleCase('machines-by-type')).toBe('Machines By Type') + expect(titleCase('')).toBe('') + }) +}) diff --git a/frontend/src/views/AppLayout.vue b/frontend/src/views/AppLayout.vue index d110bf8f..84452a9c 100644 --- a/frontend/src/views/AppLayout.vue +++ b/frontend/src/views/AppLayout.vue @@ -11,7 +11,9 @@ -
+ +
TV Slideshow Parts Kiosk + USB Kiosk Settings @@ -136,6 +140,7 @@ const hasRoute = (path) => stagedRoutePaths.has(path) const showShopfloor = hasRoute('/shopfloor') && (hasRoute('/notifications') || hasRoute('/calendar')) const showDisplays = showShopfloor || hasRoute('/tv') || hasRoute('/parts-kiosk') + || hasRoute('/usb-kiosk') const routeViewKey = computed(() => route.path.startsWith('/settings') ? '/settings' : route.path ) @@ -183,50 +188,81 @@ const iconMap = { // Default navigation (used as fallback if API fails) const defaultNav = [ - { name: 'Dashboard', icon: 'layout-dashboard', route: '/', position: 0 }, - { name: 'Notifications', icon: 'bell', route: '/notifications', position: 5 }, - { name: 'Calendar', icon: 'calendar', route: '/calendar', position: 6 }, - { name: 'Map', icon: 'map', route: '/map', position: 4 }, - { name: 'Machines', icon: 'cog', route: '/machines', position: 10 }, - { name: 'PCs', icon: 'desktop', route: '/pcs', position: 15 }, - { name: 'Network', icon: 'network-wired', route: '/network', position: 18 }, - { name: 'Printers', icon: 'printer', route: '/printers', position: 20 }, - { name: 'USB Devices', icon: 'usb', route: '/usb', position: 45 }, - { name: 'Applications', icon: 'app-window', route: '/applications', position: 30, section: 'information' }, - { name: 'Knowledge Base', icon: 'book-open', route: '/knowledgebase', position: 35 }, - { name: 'Reports', icon: 'bar-chart-3', route: '/reports', position: 40 }, + { name: 'Dashboard', icon: 'layout-dashboard', route: '/', position: 0, permission: 'assets.view' }, + { name: 'Notifications', icon: 'bell', route: '/notifications', position: 5, permission: 'notifications.view' }, + { name: 'Calendar', icon: 'calendar', route: '/calendar', position: 6, permission: 'notifications.view' }, + { name: 'Map', icon: 'map', route: '/map', position: 4, permission: 'assets.view' }, + { name: 'Machines', icon: 'cog', route: '/machines', position: 10, section: 'assets', permission: 'machines.view' }, + { name: 'PCs', icon: 'desktop', route: '/pcs', position: 15, section: 'assets', permission: 'computers.view' }, + { name: 'Network', icon: 'network-wired', route: '/network', position: 18, section: 'assets', permission: 'network.view' }, + { name: 'Printers', icon: 'printer', route: '/printers', position: 20, section: 'assets', permission: 'printers.view' }, + { name: 'USB Devices', icon: 'usb', route: '/usb', position: 25, section: 'assets', permission: 'usb.view' }, + { name: 'Applications', icon: 'app-window', route: '/applications', position: 30, section: 'information', permission: 'applications.view' }, + { name: 'Knowledge Base', icon: 'book-open', route: '/knowledgebase', position: 35, section: 'information', permission: 'kb.view' }, + { name: 'Reports', icon: 'bar-chart-3', route: '/reports', position: 40, section: 'information', permission: 'reports.view' }, +] + +// The canonical sidebar sections, in the order they render. A nav item DECLARES +// which one it belongs to; nothing is inferred. +// +// Sections used to be derived from the position number - 10-29 became Assets, +// 30-49 became Information - so a plugin landed in whichever bucket its number +// happened to fall in. That is how USB Devices, an asset, came to sit under +// Information next to GE-Enforce and Tech Tools, and why two plugins picking +// the same number silently reordered each other. A number is for ORDER; it was +// never a statement about meaning. +// +// An item with no section (or an unknown one) stays in the unlabelled group at +// the top, which is the safe default: it appears, it is just not filed. +const NAV_SECTIONS = [ + { key: null, label: null }, + { key: 'assets', label: 'Assets' }, + { key: 'information', label: 'Information' }, + { key: 'fleet', label: 'Fleet' }, ] function buildNavItems(items) { // Drop items the user cannot reach: a nav item may carry a `permission` // (e.g. Slides -> slides.manage); hide it from anyone who lacks it so the // link does not dead-end at the router guard. Admins hold every permission. - const visible = items.filter(item => - !item.permission || authStore.hasPermission(item.permission)) + // + // A signed-out visitor sees only what a plugin has declared guest-reachable. + // Without this the sidebar offers a signed-out reader the whole app and every + // link bounces them to the login page, which reads as a broken site rather + // than as a closed door. + // A nav item may name one permission or several; holding ANY of them is + // enough. GE-Enforce and Slides need that: the page is reachable by a + // manager and by a plain viewer, and they hold different strings. + const holds = item => { + if (!item.permission) return true + const names = Array.isArray(item.permission) ? item.permission : [item.permission] + return names.some(name => authStore.hasPermission(name)) + } + // A signed-out visitor gets whatever the server sent, which is already + // filtered to what the guest role can reach - hasPermission knows nothing + // about guests, so asking it here would hide everything. + const visible = authStore.isAuthenticated ? items.filter(holds) : items - // Sort by position - const sorted = [...visible].sort((a, b) => (a.position || 99) - (b.position || 99)) + const known = new Set(NAV_SECTIONS.map(s => s.key).filter(Boolean)) + const bucket = item => { + const declared = (item.section || '').toLowerCase() + return known.has(declared) ? declared : null + } - // Assign section headers based on position ranges const result = [] - let currentSection = null - - for (const item of sorted) { - let section = null - if (item.position >= 10 && item.position < 30 && currentSection !== 'Assets') { - section = 'Assets' - currentSection = 'Assets' - } else if (item.section === 'information' || (item.position >= 30 && item.position < 50 && currentSection !== 'Information')) { - if (currentSection !== 'Information') { - section = 'Information' - currentSection = 'Information' - } - } - - result.push({ - ...item, - section, - iconComponent: iconMap[item.icon] || null + for (const { key, label } of NAV_SECTIONS) { + const inSection = visible + .filter(item => bucket(item) === key) + // ?? not ||: position 0 is a real position, and `0 || 99` is 99, which + // sent Dashboard - the only item that uses 0 - to the end of its group. + .sort((a, b) => (a.position ?? 99) - (b.position ?? 99)) + inSection.forEach((item, index) => { + result.push({ + ...item, + // Only the first item in a section carries its heading. + section: index === 0 ? label : null, + iconComponent: iconMap[item.icon] || null, + }) }) } @@ -234,6 +270,11 @@ function buildNavItems(items) { } onMounted(async () => { + // Refresh the signed-in user (and their permissions) on every app load. The + // login payload is cached in localStorage, so without this a permission + // granted mid-session stays invisible until the person logs out and back in. + if (authStore.isAuthenticated) authStore.fetchUser() + // Half-hourly: cheap, and it crosses midnight into the new week without a // reload on a screen nobody touches. fiscalWeekTimer = setInterval(() => { currentFiscalWeek.value = fiscalWeek() }, 30 * 60 * 1000) diff --git a/frontend/src/views/reports/ReportsIndex.vue b/frontend/src/views/reports/ReportsIndex.vue index 676a930b..8cef2891 100644 --- a/frontend/src/views/reports/ReportsIndex.vue +++ b/frontend/src/views/reports/ReportsIndex.vue @@ -43,12 +43,27 @@

{{ currentReport.name }}

+
+
{{ reportError }}
+
+ {{ p.description }} @@ -382,19 +397,33 @@ function formatCategory(cat) { return cat.charAt(0).toUpperCase() + cat.slice(1) } +// Which permissions this role is allowed to hold at all. Only the guest role +// is bounded; the API sends the list it will accept, so the grid and the server +// cannot disagree about it. +function mayGrant(name) { + if (!editingRole.value?.isguest) return true + const grantable = editingRole.value?.grantable + return Array.isArray(grantable) ? grantable.includes(name) : false +} + function isCategoryFullySelected(category) { - const catPerms = permissionsGrouped.value[category] || [] + const catPerms = (permissionsGrouped.value[category] || []) + .filter(p => mayGrant(p.name)) return catPerms.length > 0 && catPerms.every(p => roleForm.permissions.includes(p.name)) } function isCategoryPartiallySelected(category) { - const catPerms = permissionsGrouped.value[category] || [] + const catPerms = (permissionsGrouped.value[category] || []) + .filter(p => mayGrant(p.name)) const selected = catPerms.filter(p => roleForm.permissions.includes(p.name)) return selected.length > 0 && selected.length < catPerms.length } function toggleCategory(category, checked) { - const catPerms = permissionsGrouped.value[category] || [] + // Ticking a whole category must not try to grant a guest something the API + // will refuse - the header checkbox would then look broken. + const catPerms = (permissionsGrouped.value[category] || []) + .filter(p => mayGrant(p.name)) const permNames = catPerms.map(p => p.name) if (checked) { @@ -780,4 +809,13 @@ onMounted(loadData) font-size: 0.85rem; padding: 0.25rem 0; } + +.perm-blocked { opacity: 0.45; cursor: not-allowed; } +.guest-note { + font-size: 0.85rem; + color: var(--warning); + border-left: 3px solid var(--warning); + padding-left: 0.6rem; + margin: 0 0 0.75rem; +} diff --git a/plugins/backups/api/routes.py b/plugins/backups/api/routes.py index 86ac20ba..26953834 100644 --- a/plugins/backups/api/routes.py +++ b/plugins/backups/api/routes.py @@ -77,7 +77,6 @@ def _summary(revision, islatest=False): @backups_bp.route('/kinds', methods=['GET']) -@jwt_required() @require_permission('backups.view') def list_kinds(): """Registered backup kinds, for UI that needs to enumerate them.""" @@ -94,7 +93,6 @@ def list_kinds(): @backups_bp.route('/asset/', methods=['GET']) -@jwt_required() @require_permission('backups.view') def asset_revisions(assetid): """Revision history for one asset, newest first. @@ -130,7 +128,6 @@ def asset_revisions(assetid): @backups_bp.route('/asset//info', methods=['GET']) -@jwt_required() @require_permission('backups.view') def asset_info(assetid): """A kind's 'at a glance' card, built from its LATEST revision. @@ -190,7 +187,6 @@ def asset_info(assetid): @backups_bp.route('/revisions/', methods=['GET']) -@jwt_required() @require_permission('backups.view') def get_revision(backuprevisionid): """One revision, including its parsed payload where there is one.""" @@ -207,7 +203,6 @@ def get_revision(backuprevisionid): @backups_bp.route('/revisions//download', methods=['GET']) -@jwt_required() @require_permission('backups.download') def download_revision(backuprevisionid): """Render a revision back to its native file format. @@ -282,7 +277,6 @@ def download_revision(backuprevisionid): @backups_bp.route('/revisions//diff', methods=['GET']) -@jwt_required() @require_permission('backups.view') def diff_revision(backuprevisionid): """Diff two revisions of the same asset+kind. @@ -372,7 +366,6 @@ def _diffprojections(old, new): @backups_bp.route('/dashboard/stale', methods=['GET']) -@jwt_required() @require_permission('backups.view') def dashboard_stale(): """Chains whose backup has stopped running. diff --git a/plugins/backups/services/retention.py b/plugins/backups/services/retention.py index 69ffaecb..f9627ac8 100644 --- a/plugins/backups/services/retention.py +++ b/plugins/backups/services/retention.py @@ -5,10 +5,14 @@ changed - so retention exists for the pathological case, not the normal one: a value that flaps would otherwise append a revision every collection cycle forever. -Two independent limits, both off by default at 0: +Two independent limits. Both are OFF in this module - the arguments below +default to 0 - but the plugin SEEDS `backups_retentioncount` at 50, so on a +default install count-based pruning is on and age-based pruning is not. Read +the seeded settings in plugin.py, not these argument defaults, when answering +"is retention on here". - retentioncount keep at most N revisions per chain - retentiondays additionally drop anything older than N days + retentioncount keep at most N revisions per chain (setting default: 50) + retentiondays additionally drop anything older than N days (default: off) A CHAIN is one asset, one kind, one source hostname. Several PCs can share a machine number, and each keeps its own revision history against that machine; diff --git a/plugins/computers/api/routes.py b/plugins/computers/api/routes.py index 035198ad..9adb608a 100644 --- a/plugins/computers/api/routes.py +++ b/plugins/computers/api/routes.py @@ -17,7 +17,7 @@ computers_bp = Blueprint('computers', __name__) # ============================================================================= @computers_bp.route('/types', methods=['GET']) -@jwt_required(optional=True) +@require_permission('computers.view') def list_computer_types(): """List all computer types.""" page, per_page = get_pagination_params(request) @@ -39,7 +39,7 @@ def list_computer_types(): @computers_bp.route('/types/', methods=['GET']) -@jwt_required(optional=True) +@require_permission('computers.view') def get_computer_type(type_id: int): """Get a single computer type.""" t = db.session.get(ComputerType, type_id) @@ -55,7 +55,6 @@ def get_computer_type(type_id: int): @computers_bp.route('/types', methods=['POST']) -@jwt_required() @require_permission('computers.create') def create_computer_type(): """Create a new computer type.""" @@ -93,7 +92,6 @@ def create_computer_type(): @computers_bp.route('/types/', methods=['PUT']) -@jwt_required() @require_permission('computers.edit') def update_computer_type(type_id: int): """Update a computer type.""" @@ -127,7 +125,6 @@ def update_computer_type(type_id: int): @computers_bp.route('/types/', methods=['DELETE']) -@jwt_required() @require_permission('computers.delete') def delete_computer_type(type_id: int): """Delete a computer type. Refused if any PC still uses it.""" @@ -148,7 +145,7 @@ def delete_computer_type(type_id: int): # ============================================================================= @computers_bp.route('/protocols', methods=['GET']) -@jwt_required(optional=True) +@require_permission('computers.view') def list_protocols(): """List access protocols. ?active=false includes disabled ones.""" query = AccessProtocol.query @@ -159,7 +156,6 @@ def list_protocols(): @computers_bp.route('/protocols', methods=['POST']) -@jwt_required() @require_permission('computers.edit') def create_protocol(): data = request.get_json() or {} @@ -180,7 +176,6 @@ def create_protocol(): @computers_bp.route('/protocols/', methods=['PUT', 'PATCH']) -@jwt_required() @require_permission('computers.edit') def update_protocol(protocol_id): p = db.session.get(AccessProtocol, protocol_id) @@ -199,7 +194,6 @@ def update_protocol(protocol_id): @computers_bp.route('/protocols/', methods=['DELETE']) -@jwt_required() @require_permission('computers.edit') def delete_protocol(protocol_id): p = db.session.get(AccessProtocol, protocol_id) @@ -299,7 +293,7 @@ def _sync_access_methods(comp, data): # ============================================================================= @computers_bp.route('/display-kiosks', methods=['GET']) -@jwt_required(optional=True) +@require_permission('computers.view') def list_display_kiosks(): """Reporting display kiosks, for the Dashboard Defaults picker. @@ -335,7 +329,7 @@ def list_display_kiosks(): @computers_bp.route('', methods=['GET']) -@jwt_required(optional=True) +@require_permission('computers.view') def list_computers(): """ List all computers with filtering and pagination. @@ -443,7 +437,7 @@ def list_computers(): @computers_bp.route('/', methods=['GET']) -@jwt_required(optional=True) +@require_permission('computers.view') def get_computer(computer_id: int): """Get a single computer with full details.""" comp = db.session.get(Computer, computer_id) @@ -467,7 +461,7 @@ def get_computer(computer_id: int): @computers_bp.route('/by-asset/', methods=['GET']) -@jwt_required(optional=True) +@require_permission('computers.view') def get_computer_by_asset(asset_id: int): """Get computer data by asset ID.""" comp = Computer.query.filter_by(assetid=asset_id).first() @@ -486,7 +480,7 @@ def get_computer_by_asset(asset_id: int): @computers_bp.route('/by-hostname/', methods=['GET']) -@jwt_required(optional=True) +@require_permission('computers.view') def get_computer_by_hostname(hostname: str): """Get computer by hostname.""" comp = Computer.query.filter_by(hostname=hostname).first() @@ -505,7 +499,6 @@ def get_computer_by_hostname(hostname: str): @computers_bp.route('', methods=['POST']) -@jwt_required() @require_permission('computers.create') def create_computer(): """ @@ -623,7 +616,6 @@ def create_computer(): @computers_bp.route('/', methods=['PUT']) -@jwt_required() @require_permission('computers.edit') def update_computer(computer_id: int): """Update computer (both Asset and Computer records).""" @@ -736,7 +728,6 @@ def update_computer(computer_id: int): @computers_bp.route('/', methods=['DELETE']) -@jwt_required() @require_permission('computers.delete') def delete_computer(computer_id: int): """Delete (soft delete) computer.""" @@ -766,7 +757,7 @@ def delete_computer(computer_id: int): # ============================================================================= @computers_bp.route('//apps', methods=['GET']) -@jwt_required(optional=True) +@require_permission('computers.view') def get_installed_apps(computer_id: int): """Get all installed applications for a computer.""" comp = db.session.get(Computer, computer_id) @@ -789,7 +780,6 @@ def get_installed_apps(computer_id: int): @computers_bp.route('//apps', methods=['POST']) -@jwt_required() @require_permission('computers.create') def add_installed_app(computer_id: int): """Add an installed application to a computer.""" @@ -846,7 +836,6 @@ def add_installed_app(computer_id: int): @computers_bp.route('//apps/', methods=['DELETE']) -@jwt_required() @require_permission('computers.delete') def remove_installed_app(computer_id: int, app_id: int): """Remove an installed application from a computer.""" @@ -874,7 +863,6 @@ def remove_installed_app(computer_id: int, app_id: int): # ============================================================================= @computers_bp.route('//report', methods=['POST']) -@jwt_required() @require_permission('computers.create') def report_status(computer_id: int): """ @@ -913,7 +901,7 @@ def report_status(computer_id: int): # ============================================================================= @computers_bp.route('/dashboard/summary', methods=['GET']) -@jwt_required(optional=True) +@require_permission('computers.view') def dashboard_summary(): """Get computer dashboard summary data.""" # Total active computers @@ -958,7 +946,6 @@ def dashboard_summary(): @computers_bp.route('/dashboard/quiet', methods=['GET']) -@jwt_required() @require_permission('computers.view') def dashboard_quiet(): """PCs that WERE reporting, have stopped, and are still meant to be in use. @@ -1012,7 +999,6 @@ def dashboard_quiet(): @computers_bp.route('/dashboard/sharedmachines', methods=['GET']) -@jwt_required() @require_permission('computers.view') def dashboard_sharedmachines(): """Machine numbers claimed by more than one PC, with nothing filed under diff --git a/plugins/computers/plugin.py b/plugins/computers/plugin.py index b91e8cb8..c724fa54 100644 --- a/plugins/computers/plugin.py +++ b/plugins/computers/plugin.py @@ -1974,7 +1974,9 @@ class ComputersPlugin(BasePlugin): 'name': 'PCs', 'icon': 'desktop', 'route': '/pcs', + 'permission': 'computers.view', 'position': 15, + 'section': 'assets', }, ] diff --git a/plugins/employees/api/routes.py b/plugins/employees/api/routes.py index 92d1736a..4e9788c1 100644 --- a/plugins/employees/api/routes.py +++ b/plugins/employees/api/routes.py @@ -29,6 +29,7 @@ from shopdb.api import Setting from shopdb.api import send_upload from ..models import DirectoryEmployee +from shopdb.api import require_permission logger = logging.getLogger(__name__) @@ -198,6 +199,7 @@ def _with_photo_url(employee): @employees_bp.route('/resolve/', methods=['GET']) +@require_permission('employees.view') @jwt_required() def resolve_employee(sso): """Resolve one SSO to a name and photo, and say HOW. @@ -275,6 +277,7 @@ def resolve_employee(sso): @employees_bp.route('/search', methods=['GET']) +@require_permission('employees.view') def search_employees(): """ Search employees by name. @@ -327,6 +330,7 @@ def search_employees(): @employees_bp.route('/lookup/', methods=['GET']) +@require_permission('employees.view') def lookup_employee(sso): """Look up a single employee by SSO.""" if not sso.isdigit(): @@ -370,6 +374,7 @@ def lookup_employee(sso): @employees_bp.route('/lookup', methods=['GET']) +@require_permission('employees.view') def lookup_employees(): """ Look up multiple employees by SSO list. @@ -429,7 +434,7 @@ def lookup_employees(): # ============================================================================= @employees_bp.route('/directory', methods=['GET']) -@jwt_required(optional=True) +@require_permission('employees.view') def list_directory(): """Full self-hosted directory (for the management page).""" guard = _require_selfhosted() diff --git a/plugins/employees/plugin.py b/plugins/employees/plugin.py index dfdd1c3b..9c5a18f5 100644 --- a/plugins/employees/plugin.py +++ b/plugins/employees/plugin.py @@ -51,6 +51,18 @@ class EmployeesPlugin(BasePlugin): api_prefix=self._manifest.get('api_prefix', '/api/employees'), ) + def get_permissions(self) -> List: + """Reading the staff directory. + + The directory is names, SSOs, photographs and who reports to whom. It + was reachable with no account at all, which is how a permissions + question ends up answered by nobody. One read permission: the write + side of this plugin is an import, run from the CLI. + """ + return [ + ('employees.view', 'View the employee directory', 'employees'), + ] + def get_blueprint(self) -> Optional[Blueprint]: """Return Flask Blueprint with API routes.""" return employees_bp diff --git a/plugins/geenforce/api/routes.py b/plugins/geenforce/api/routes.py index 2470df98..f3d6217a 100644 --- a/plugins/geenforce/api/routes.py +++ b/plugins/geenforce/api/routes.py @@ -20,6 +20,7 @@ from sqlalchemy.exc import IntegrityError from shopdb.api import ( db, cache, success_response, error_response, ErrorCodes, require_permission, + require_any_permission, authorized_service_token, Setting, Application, ) @@ -338,8 +339,7 @@ def post_report(): # -- admin read surface (P2 adds full CRUD + publish) ------------------------- @geenforce_bp.route('/scopes', methods=['GET']) -@jwt_required() -@require_permission('geenforce.manage') +@require_any_permission('geenforce.view', 'geenforce.manage') def list_scopes(): """List imaging PC-type scopes with entry + published-version counts.""" scopes = ManifestScope.query.order_by( @@ -364,8 +364,7 @@ def list_scopes(): @geenforce_bp.route('/scopes//preview', methods=['GET']) -@jwt_required() -@require_permission('geenforce.manage') +@require_any_permission('geenforce.view', 'geenforce.manage') def preview_scope(scopeid): """Render the DRAFT manifest JSON a publish would freeze (review before ship).""" scope = db.session.get(ManifestScope, scopeid) @@ -377,8 +376,7 @@ def preview_scope(scopeid): @geenforce_bp.route('/applications', methods=['GET']) -@jwt_required() -@require_permission('geenforce.manage') +@require_any_permission('geenforce.view', 'geenforce.manage') def list_applications(): """The core Applications catalog (id + name) for the curated entry->app link picker in the entry editor.""" @@ -446,7 +444,6 @@ def _apply_app_link(entry, payload): @geenforce_bp.route('/scopes', methods=['POST']) -@jwt_required() @require_permission('geenforce.manage') def create_scope(): payload = request.get_json(silent=True) or {} @@ -475,8 +472,7 @@ def create_scope(): @geenforce_bp.route('/scopes/', methods=['GET']) -@jwt_required() -@require_permission('geenforce.manage') +@require_any_permission('geenforce.view', 'geenforce.manage') def get_scope(scopeid): scope = db.session.get(ManifestScope, scopeid) if not scope: @@ -487,7 +483,6 @@ def get_scope(scopeid): @geenforce_bp.route('/scopes/', methods=['PUT']) -@jwt_required() @require_permission('geenforce.manage') def update_scope(scopeid): scope = db.session.get(ManifestScope, scopeid) @@ -510,7 +505,6 @@ def update_scope(scopeid): @geenforce_bp.route('/scopes/', methods=['DELETE']) -@jwt_required() @require_permission('geenforce.manage') def delete_scope(scopeid): scope = db.session.get(ManifestScope, scopeid) @@ -533,7 +527,6 @@ def _validate_entry(payload): @geenforce_bp.route('/scopes//entries', methods=['POST']) -@jwt_required() @require_permission('geenforce.manage') def create_entry(scopeid): scope = db.session.get(ManifestScope, scopeid) @@ -558,7 +551,6 @@ def create_entry(scopeid): @geenforce_bp.route('/entries/', methods=['PUT']) -@jwt_required() @require_permission('geenforce.manage') def update_entry(entryid): entry = db.session.get(ManifestEntry, entryid) @@ -587,7 +579,6 @@ def update_entry(entryid): @geenforce_bp.route('/entries/', methods=['DELETE']) -@jwt_required() @require_permission('geenforce.manage') def delete_entry(entryid): entry = db.session.get(ManifestEntry, entryid) @@ -599,7 +590,6 @@ def delete_entry(entryid): @geenforce_bp.route('/scopes//entries/reorder', methods=['PUT']) -@jwt_required() @require_permission('geenforce.manage') def reorder_entries(scopeid): """Set entry order from a list of entryids (the ordering contract).""" @@ -621,8 +611,7 @@ def reorder_entries(scopeid): # -- simulate: "what would this PC get" --------------------------------------- @geenforce_bp.route('/scopes//simulate', methods=['GET']) -@jwt_required() -@require_permission('geenforce.manage') +@require_any_permission('geenforce.view', 'geenforce.manage') def simulate_scope(scopeid): """Which draft entries apply to a given machine profile, and why the rest are filtered out (reuses the engine-mirror filters).""" @@ -662,8 +651,7 @@ def simulate_scope(scopeid): # -- compliance: "how much of the fleet has this app" ------------------------- @geenforce_bp.route('/scopes//compliance', methods=['GET']) -@jwt_required() -@require_permission('geenforce.manage') +@require_any_permission('geenforce.view', 'geenforce.manage') def scope_compliance(scopeid): """Fleet-install coverage per app-linked entry (from collected PC data). @@ -684,7 +672,6 @@ PAYLOAD_MAX_BYTES = 1024 * 1024 @geenforce_bp.route('/entries//payload', methods=['POST']) -@jwt_required() @require_permission('geenforce.publish') def upload_entry_payload(entryid): """Store an inline payload (<= 1 MB) for an entry and point the entry at it.""" @@ -709,8 +696,7 @@ def upload_entry_payload(entryid): @geenforce_bp.route('/entries//payload', methods=['GET']) -@jwt_required() -@require_permission('geenforce.manage') +@require_any_permission('geenforce.view', 'geenforce.manage') def download_entry_payload(entryid): """Return the stored inline payload bytes for an entry (404 if none).""" entry = db.session.get(ManifestEntry, entryid) @@ -730,7 +716,6 @@ def download_entry_payload(entryid): # -- publish lifecycle (geenforce.publish) ------------------------------------ @geenforce_bp.route('/scopes//publish', methods=['POST']) -@jwt_required() @require_permission('geenforce.publish') def publish_scope_route(scopeid): scope = db.session.get(ManifestScope, scopeid) @@ -759,8 +744,7 @@ def publish_scope_route(scopeid): @geenforce_bp.route('/scopes//publish-preflight', methods=['GET']) -@jwt_required() -@require_permission('geenforce.manage') +@require_any_permission('geenforce.view', 'geenforce.manage') def publish_preflight(scopeid): """Would publishing this scope outrun the fleet's enforcer lib? @@ -781,8 +765,7 @@ def publish_preflight(scopeid): @geenforce_bp.route('/scopes//versions', methods=['GET']) -@jwt_required() -@require_permission('geenforce.manage') +@require_any_permission('geenforce.view', 'geenforce.manage') def list_versions(scopeid): scope = db.session.get(ManifestScope, scopeid) if not scope: @@ -801,8 +784,7 @@ def list_versions(scopeid): @geenforce_bp.route('/scopes//versions/', methods=['GET']) -@jwt_required() -@require_permission('geenforce.manage') +@require_any_permission('geenforce.view', 'geenforce.manage') def get_version(scopeid, versionnumber): version = ManifestPublishedVersion.query.filter_by( scopeid=scopeid, versionnumber=versionnumber).first() @@ -830,8 +812,7 @@ def _normalize_cidrs(raw): @geenforce_bp.route('/config', methods=['GET']) -@jwt_required() -@require_permission('geenforce.manage') +@require_any_permission('geenforce.view', 'geenforce.manage') def get_config(): """Plugin config: the on-share export root + the client IP allowlist.""" shareroot = Setting.query.filter_by(key=SHAREROOT_SETTING).first() @@ -843,7 +824,6 @@ def get_config(): @geenforce_bp.route('/config', methods=['PUT']) -@jwt_required() @require_permission('geenforce.publish') def put_config(): payload = request.get_json(silent=True) or {} @@ -870,7 +850,6 @@ def put_config(): @geenforce_bp.route('/scopes//export-share', methods=['POST']) -@jwt_required() @require_permission('geenforce.publish') def export_share_route(scopeid): """Write the scope's current published JSON to the configured share root @@ -892,7 +871,6 @@ def export_share_route(scopeid): @geenforce_bp.route('/scopes//rollback', methods=['POST']) -@jwt_required() @require_permission('geenforce.publish') def rollback_scope_route(scopeid): scope = db.session.get(ManifestScope, scopeid) @@ -1221,8 +1199,7 @@ def _report_stale_cutoff(): @geenforce_bp.route('/reports', methods=['GET']) -@jwt_required() -@require_permission('geenforce.manage') +@require_any_permission('geenforce.view', 'geenforce.manage') def list_reports(): """Latest enforcement report per PC (fleet compliance view). @@ -1328,8 +1305,7 @@ def list_reports(): @geenforce_bp.route('/reports/', methods=['GET']) -@jwt_required() -@require_permission('geenforce.manage') +@require_any_permission('geenforce.view', 'geenforce.manage') def get_report(reportid): """One report with per-entry outcomes (self-heal / failure detail).""" report = db.session.get(ManifestEnforcementReport, reportid) @@ -1358,8 +1334,7 @@ def get_report(reportid): @geenforce_bp.route('/dashboard/failures', methods=['GET']) -@jwt_required() -@require_permission('geenforce.manage') +@require_any_permission('geenforce.view', 'geenforce.manage') def dashboard_failures(): """Entries that FAILED on their PC's most recent enforcement cycle. diff --git a/plugins/geenforce/plugin.py b/plugins/geenforce/plugin.py index 1768805b..2e12b303 100644 --- a/plugins/geenforce/plugin.py +++ b/plugins/geenforce/plugin.py @@ -106,6 +106,8 @@ class GeEnforcePlugin(BasePlugin): def get_permissions(self) -> List: """RBAC permissions this plugin owns (edit vs ship are split).""" return [ + ('geenforce.view', 'View imaging PC types and manifests', + 'geenforce'), ('geenforce.manage', 'Edit imaging PC types and manifest drafts', 'geenforce'), ('geenforce.publish', 'Publish, roll back, and export manifests', @@ -125,6 +127,10 @@ class GeEnforcePlugin(BasePlugin): 'icon': 'shield', 'route': '/geenforce/manifests', 'position': 46, + # The READ permission: managing implies it, and it is the one a + # site can hand to guests. + 'permission': ['geenforce.view', 'geenforce.manage'], + 'section': 'fleet', }, ] diff --git a/plugins/guides/api/routes.py b/plugins/guides/api/routes.py index 2e858c84..04a64737 100644 --- a/plugins/guides/api/routes.py +++ b/plugins/guides/api/routes.py @@ -14,7 +14,7 @@ from datetime import datetime import json import re -from flask import Blueprint, request, Response +from flask import Blueprint, request, Response, current_app from flask_jwt_extended import jwt_required, get_jwt_identity, current_user from shopdb.api import ( @@ -42,9 +42,25 @@ ALLOWEDIMAGETYPES = { } +def _currentuser(): + """The signed-in user, or None. + + `current_user` is a LocalProxy, so `current_user is not None` is True even + when nobody is signed in - the proxy exists, the user behind it does not. + Under a required JWT that never showed; the read routes are jwt-optional + now, and the first guest request found it by way of + "'NoneType' object has no attribute 'haspermission'". + """ + try: + return current_user._get_current_object() + except Exception: + return None + + def _userid(): - if current_user is not None: - return current_user.userid + user = _currentuser() + if user is not None: + return user.userid try: return int(get_jwt_identity()) except (TypeError, ValueError): @@ -59,8 +75,12 @@ def _has(permission): the decorator makes, including the admin bypass that lives inside haspermission. """ - return bool(current_user is not None - and current_user.haspermission(permission)) + user = _currentuser() + return bool(user is not None and user.haspermission(permission)) + + +def _signedin(): + return _currentuser() is not None def _mayedit(guide): @@ -101,7 +121,6 @@ def _slugify(title, guideid=None): # ---------------------------------------------------------------- reading --- @guides_bp.route('', methods=['GET']) -@jwt_required() @require_permission('guides.view') def list_guides(): """Published guides, plus the caller's own drafts if they can author. @@ -109,9 +128,15 @@ def list_guides(): A half-written procedure on a shop floor is worse than no procedure, so drafts are invisible to readers - but an author must be able to find their own, or drafting is a trap door. + + A guest reaches this only if the guest role has been granted guides.view. + A guest is never an author, so the draft branches below cannot open for one. """ caneditall = _has('guides.edit') - mine = _userid() + # A guest has no user id, and `createdby == None` is a real match in SQL + # against every guide imported without an author. Their drafts are not + # theirs; nobody's are. + mine = _userid() if _signedin() else -1 # Match against the PUBLISHED text for anything the caller is only allowed # to read, and against the working copy only for guides they may edit. @@ -185,7 +210,6 @@ def list_guides(): @guides_bp.route('/tags', methods=['GET']) -@jwt_required() @require_permission('guides.view') def list_tags(): """Every tag in use, with a count, for the filter chips. @@ -215,7 +239,6 @@ def list_tags(): @guides_bp.route('/', methods=['GET']) -@jwt_required() @require_permission('guides.view') def read_guide(slug): """What a reader gets: the published revision. @@ -257,7 +280,6 @@ def read_guide(slug): @guides_bp.route('//working', methods=['GET']) -@jwt_required() @require_permission('guides.view') def read_working(guideid): """The working copy, for the editor.""" @@ -275,7 +297,6 @@ def read_working(guideid): @guides_bp.route('//revisions', methods=['GET']) -@jwt_required() @require_permission('guides.view') def list_revisions(guideid): guide = db.session.get(Guide, guideid) @@ -298,7 +319,6 @@ def list_revisions(guideid): @guides_bp.route('/revisions/', methods=['GET']) -@jwt_required() @require_permission('guides.view') def read_revision(revisionid): """One revision by id. @@ -327,8 +347,40 @@ def read_revision(revisionid): # ---------------------------------------------------------------- writing --- +def _attachimages(guide, blocks): + """Attach the images this guide's blocks reference but nothing owns yet. + + An image pasted into a guide that has not been saved is uploaded with no + guideid - there is no id to give it. The upload route says the editor + attaches it on save, and the editor never did: it only sends a guideid when + the guide already has one. So a screenshot pasted while writing a NEW guide + stayed unattached forever. + + An unattached image is visible to editors and to whoever uploaded it, and + to nobody else - so the guide looked right to the person who wrote it and + showed "Image unavailable" to every reader. Attaching on save is what makes + the image belong to the guide, and therefore readable by anyone allowed to + read the guide once it is published. + + Only images nothing else owns are claimed: an image already attached to + another guide is left alone rather than stolen by whoever references it. + """ + referenced = set() + for block in blocks or []: + if isinstance(block, dict) and block.get('imageid') is not None: + try: + referenced.add(int(block['imageid'])) + except (TypeError, ValueError): + continue + if not referenced: + return + (GuideImage.query + .filter(GuideImage.guideimageid.in_(referenced)) + .filter(GuideImage.guideid.is_(None)) + .update({'guideid': guide.guideid}, synchronize_session=False)) + + @guides_bp.route('', methods=['POST']) -@jwt_required() @require_permission('guides.create') def create_guide(): data = request.get_json(silent=True) or {} @@ -353,13 +405,15 @@ def create_guide(): db.session.rollback() return error_response(ErrorCodes.VALIDATION_ERROR, str(exc)) + # The first save is exactly when a pasted image has no owner yet. + _attachimages(guide, guide.blocks) + db.session.commit() return success_response(guide.to_dict(includeblocks=True), message='Guide created') @guides_bp.route('/', methods=['PUT']) -@jwt_required() @require_permission('guides.view') def update_guide(guideid): guide = db.session.get(Guide, guideid) @@ -387,6 +441,8 @@ def update_guide(guideid): db.session.rollback() return error_response(ErrorCodes.VALIDATION_ERROR, str(exc)) + _attachimages(guide, guide.blocks) + # The slug is set once, at create, and does NOT follow the title. # # _slugify's own docstring says the slug is stored so that renaming cannot @@ -409,7 +465,6 @@ def update_guide(guideid): @guides_bp.route('//publish', methods=['POST']) -@jwt_required() @require_permission('guides.publish') def publish_guide(guideid): guide = db.session.get(Guide, guideid) @@ -425,7 +480,6 @@ def publish_guide(guideid): @guides_bp.route('//unpublish', methods=['POST']) -@jwt_required() @require_permission('guides.publish') def unpublish_guide(guideid): guide = db.session.get(Guide, guideid) @@ -437,7 +491,6 @@ def unpublish_guide(guideid): @guides_bp.route('//restore/', methods=['POST']) -@jwt_required() @require_permission('guides.view') def restore_revision(guideid, revisionid): guide = db.session.get(Guide, guideid) @@ -458,7 +511,6 @@ def restore_revision(guideid, revisionid): @guides_bp.route('/', methods=['DELETE']) -@jwt_required() @require_permission('guides.delete') def delete_guide(guideid): guide = db.session.get(Guide, guideid) @@ -472,7 +524,6 @@ def delete_guide(guideid): # ----------------------------------------------------------------- images --- @guides_bp.route('/images', methods=['POST']) -@jwt_required() @require_permission('guides.create') def upload_image(): """Take a pasted or dropped image. @@ -553,9 +604,16 @@ def _mayseeimage(image): @guides_bp.route('/images/', methods=['GET']) -@jwt_required() @require_permission('guides.view') def serve_image(imageid): + """The image itself, under the same rule as the guide that holds it. + + A guest who may read a published guide must see its screenshots, or every + image renders as "Image unavailable" and the guide looks broken. + _mayseeimage already answers correctly for somebody holding nothing - it + requires the PUBLISHED revision to reference the image - so a guest needs no + separate branch here. + """ image = db.session.get(GuideImage, imageid) if image is None: return error_response(ErrorCodes.NOT_FOUND, 'Image not found', http_code=404) diff --git a/plugins/guides/cli.py b/plugins/guides/cli.py new file mode 100644 index 00000000..9b1aaf0e --- /dev/null +++ b/plugins/guides/cli.py @@ -0,0 +1,378 @@ +"""CLI: move guides in and out of a site. + + flask guides import-html PATH [PATH ...] an HTML article becomes a guide + flask guides export --out DIR guides become portable files + flask guides import PATH [PATH ...] those files become guides here + +A site that adopts this app usually arrives with a folder of procedures already +written - styled HTML pages that get mailed around and opened off a share. This +imports them so they live where they are used, become searchable, tickable and +printable, and stop being a file somebody has to find. + +Imports land as DRAFTS, always - both kinds. An HTML conversion is faithful but +it is not a render, and a bundle written on another site has not been read by +anybody here. Publishing is a decision somebody makes after reading. + +Sites are separate databases and the useful ones are air-gapped from where a +guide gets written, so export/import is how the same procedure reaches all of +them: one JSON file per guide, images carried inside it, moved by whatever +moves files to that site. +""" + +import click +from flask.cli import AppGroup, with_appcontext + +from shopdb.api import db + +from .models import Guide, GuideImage +from .services import authoring, bundle as bundleservice +from .services.blocks import BlockError +from .services.htmlimport import parse + + +# Mirrors what the upload route accepts. An import must not be able to store an +# image the editor would have refused, or the guide renders here and nowhere +# else. +ALLOWEDIMAGETYPES = ('image/png', 'image/jpeg', 'image/gif', 'image/webp') +MAXIMAGEBYTES = 8 * 1024 * 1024 + + +def _slugify(title): + """Same rule as the API's, kept here so the CLI does not import a route.""" + import re + base = re.sub(r'[^a-z0-9]+', '-', (title or '').lower()).strip('-') or 'guide' + base = base[:180] + candidate = base + suffix = 2 + while Guide.query.filter(Guide.slug == candidate).first() is not None: + candidate = '{}-{}'.format(base, suffix) + suffix += 1 + return candidate + + +def build_cli(): + guides_cli = AppGroup('guides') + + @guides_cli.command('import-html') + @click.argument('paths', nargs=-1, required=True, + type=click.Path(exists=True, dir_okay=False)) + @click.option('--title', default=None, + help='Override the title taken from the article.') + @click.option('--tags', default=None, + help='Comma-separated tags for every guide imported.') + @click.option('--userid', type=int, default=None, + help='Record this user as the author.') + @click.option('--replace', is_flag=True, + help='If a guide already has this slug, edit it in place ' + '(a new revision) instead of creating a second one.') + @click.option('--apply/--dry-run', default=False, + help='Write to the database. The default reports only.') + @with_appcontext + def import_html(paths, title, tags, userid, replace, apply): + """Convert HTML article(s) into draft guides.""" + if title and len(paths) > 1: + raise click.UsageError('--title names one guide; pass one file.') + + for path in paths: + with open(path, encoding='utf-8', errors='replace') as handle: + parsed = parse(handle.read()) + + name = title or parsed['title'] + counts = {} + for block in parsed['blocks']: + counts[block['type']] = counts.get(block['type'], 0) + 1 + + click.echo('{}'.format(path)) + click.echo(' title {}'.format(name)) + click.echo(' blocks {} ({})'.format( + len(parsed['blocks']), + ', '.join('{} {}'.format(v, k) for k, v in sorted(counts.items())))) + click.echo(' images {}'.format(len(parsed['images']))) + for note in parsed['skipped']: + click.echo(' skipped {}'.format(note)) + + rejected = [image for image in parsed['images'] + if image['mimetype'] not in ALLOWEDIMAGETYPES + or len(image['data']) > MAXIMAGEBYTES] + for image in rejected: + click.echo(' DROPPED {} ({}, {:.1f} MB) - not an accepted image' + .format(image['token'], image['mimetype'], + len(image['data']) / 1048576)) + + if not apply: + click.echo(' dry run, nothing written. Pass --apply to import.') + continue + + try: + guide = _import(parsed, name, tags, userid, replace, rejected) + except BlockError as error: + db.session.rollback() + click.echo(' FAILED {}'.format(error)) + continue + + db.session.commit() + click.echo(' imported as /guides/{} (draft, revision {})' + .format(guide.slug, guide.revision)) + + @guides_cli.command('fix-images') + @click.option('--apply/--dry-run', default=False, + help='Write to the database. The default reports only.') + @with_appcontext + def fix_images(apply): + """Attach images a guide references but nothing owns. + + An image pasted into a guide that had not been saved yet was uploaded + with no guideid, and nothing ever attached it. Editors and the uploader + could see it; every other reader got "Image unavailable" on a published + guide. Saving a guide attaches its images now, but a guide nobody edits + again would stay broken, so this repairs what is already stored. + """ + orphans = GuideImage.query.filter(GuideImage.guideid.is_(None)).all() + if not orphans: + click.echo('No unattached images.') + return + + wanted = {} + for guide in Guide.query.all(): + for block in guide.blocks or []: + if isinstance(block, dict) and block.get('imageid') is not None: + try: + wanted.setdefault(int(block['imageid']), guide) + except (TypeError, ValueError): + continue + + fixed = 0 + for image in orphans: + guide = wanted.get(image.guideimageid) + if guide is None: + click.echo(' image {} is referenced by no guide - left alone' + .format(image.guideimageid)) + continue + click.echo(' image {} -> guide {} ({})'.format( + image.guideimageid, guide.guideid, guide.slug)) + if apply: + image.guideid = guide.guideid + fixed += 1 + + if not apply: + click.echo('{} would be attached. Pass --apply to write.'.format(fixed)) + return + db.session.commit() + click.echo('{} image(s) attached.'.format(fixed)) + + @guides_cli.command('export') + @click.option('--out', 'outdir', required=True, + type=click.Path(file_okay=False), + help='Directory to write the bundles into. Created if needed.') + @click.option('--slug', 'slugs', multiple=True, + help='Export just this guide. Repeatable. Default: all.') + @click.option('--published-only', is_flag=True, + help='Skip guides that have never been published.') + @with_appcontext + def export_guides(outdir, slugs, published_only): + """Write guides out as portable .guide.json files.""" + import os + + query = Guide.query + if slugs: + query = query.filter(Guide.slug.in_(slugs)) + if published_only: + query = query.filter(Guide.publishedrevisionid.isnot(None)) + guides = query.order_by(Guide.slug).all() + + missing = set(slugs) - {guide.slug for guide in guides} + for slug in sorted(missing): + click.echo(' NOT FOUND {}'.format(slug)) + + if not guides: + click.echo('Nothing to export.') + return + + os.makedirs(outdir, exist_ok=True) + for guide in guides: + images = GuideImage.query.filter_by(guideid=guide.guideid).all() + payload = bundleservice.build(guide, images) + path = os.path.join(outdir, '{}.guide.json'.format(guide.slug)) + with open(path, 'w', encoding='utf-8') as handle: + handle.write(bundleservice.dumps(payload)) + click.echo(' {} {} blocks, {} image(s), {:.0f} KB{}'.format( + os.path.basename(path), len(payload['blocks']), + len(payload['images']), os.path.getsize(path) / 1024, + '' if guide.publishedrevisionid else ' [draft here]')) + click.echo('{} guide(s) -> {}'.format(len(guides), outdir)) + + @guides_cli.command('import') + @click.argument('paths', nargs=-1, required=True, + type=click.Path(exists=True)) + @click.option('--userid', type=int, default=None, + help='Record this user as the author on this site.') + @click.option('--replace', is_flag=True, + help='If a guide already has the bundle\'s slug, edit it in ' + 'place (a new revision) instead of creating a second ' + 'one. This is how an updated guide is re-shipped.') + @click.option('--apply/--dry-run', default=False, + help='Write to the database. The default reports only.') + @with_appcontext + def import_guides(paths, userid, replace, apply): + """Import .guide.json bundle(s) written by `guides export`.""" + import os + + files = [] + for path in paths: + if os.path.isdir(path): + files.extend(sorted(os.path.join(path, name) + for name in os.listdir(path) + if name.endswith('.guide.json'))) + else: + files.append(path) + if not files: + click.echo('No .guide.json files found.') + return + + for path in files: + click.echo(os.path.basename(path)) + try: + with open(path, encoding='utf-8') as handle: + payload = bundleservice.loads(handle.read()) + except bundleservice.BundleError as error: + click.echo(' SKIPPED {}'.format(error)) + continue + + existing = Guide.query.filter(Guide.slug == payload['slug']).first() + click.echo(' title {}'.format(payload.get('title'))) + click.echo(' blocks {}, images {}'.format( + len(payload['blocks']), len(payload['images']))) + if existing is not None: + click.echo(' {} already has this slug{}'.format( + 'guide {}'.format(existing.guideid), + ' - will be updated in place' if replace + else ' - a SECOND guide will be created; pass --replace to ' + 'update it instead')) + + rejected = [image for image in payload['images'] + if image['mimetype'] not in ALLOWEDIMAGETYPES + or len(image['data']) > MAXIMAGEBYTES] + for image in rejected: + click.echo(' DROPPED {} ({}) - not an accepted image' + .format(image['token'], image['mimetype'])) + + if not apply: + click.echo(' dry run, nothing written. Pass --apply to import.') + continue + + try: + guide = _importbundle(payload, userid, replace, rejected) + except BlockError as error: + db.session.rollback() + click.echo(' FAILED {}'.format(error)) + continue + + db.session.commit() + click.echo(' imported as /guides/{} (draft, revision {})' + .format(guide.slug, guide.revision)) + + return [guides_cli] + + +def _import(parsed, name, tags, userid, replace, rejected): + """Create or update the guide and its images. Caller commits.""" + slug = None + guide = None + if replace: + # Match on the slug the title WOULD produce, which is how somebody + # re-imports a corrected article over the one they imported yesterday. + import re + wanted = re.sub(r'[^a-z0-9]+', '-', (name or '').lower()).strip('-') + guide = Guide.query.filter(Guide.slug == wanted).first() + + if guide is None: + slug = _slugify(name) + guide = Guide(title=name, slug=slug, status='draft', createdby=userid) + db.session.add(guide) + db.session.flush() + + # Images first: the block needs the stored id, and an id only exists once + # the row does. + droppedtokens = {image['token'] for image in rejected} + ids = {} + for image in parsed['images']: + if image['token'] in droppedtokens: + continue + row = GuideImage(guideid=guide.guideid, filename=image['filename'], + mimetype=image['mimetype'], + bytesize=len(image['data']), data=image['data'], + uploadedby=userid) + db.session.add(row) + db.session.flush() + ids[image['token']] = row.guideimageid + + blocks = [] + for block in parsed['blocks']: + block = dict(block) + token = block.pop('imagetoken', None) + if token is not None: + if token not in ids: + continue # its image was dropped; the caption would lie + block['imageid'] = ids[token] + blocks.append(block) + + authoring.save(guide, title=name, summary=parsed['summary'] or None, + tags=tags, blocks=blocks, + note='Imported from HTML', userid=userid) + return guide + + +def _importbundle(payload, userid, replace, rejected): + """Create or update a guide from a bundle. Caller commits.""" + guide = None + if replace: + guide = Guide.query.filter(Guide.slug == payload['slug']).first() + + if guide is None: + # The bundle's own slug when it is free, because the slug is the name + # somebody pastes into a work order and the same guide should have the + # same address on every site. A clash only suffixes. + slug = payload['slug'] + if Guide.query.filter(Guide.slug == slug).first() is not None: + slug = _slugify(payload.get('title') or slug) + guide = Guide(title=payload.get('title') or slug, slug=slug, + status='draft', createdby=userid) + db.session.add(guide) + db.session.flush() + + # Replacing means the incoming bundle is the whole truth for this guide. + # Its blocks will reference the images arriving now, so the ones already + # stored become orphans - and re-shipping a guide monthly would otherwise + # grow a pile of them nothing points at. + if replace: + GuideImage.query.filter_by(guideid=guide.guideid).delete() + db.session.flush() + + droppedtokens = {image['token'] for image in rejected} + ids = {} + for image in payload['images']: + if image['token'] in droppedtokens: + continue + row = GuideImage(guideid=guide.guideid, filename=image['filename'], + mimetype=image['mimetype'], + bytesize=len(image['data']), data=image['data'], + uploadedby=userid) + db.session.add(row) + db.session.flush() + ids[image['token']] = row.guideimageid + + blocks = [] + for block in payload['blocks']: + block = dict(block) + token = block.pop('imagetoken', None) + if token is not None: + if token not in ids: + continue + block['imageid'] = ids[token] + blocks.append(block) + + authoring.save(guide, title=payload.get('title'), + summary=payload.get('summary'), tags=payload.get('tags'), + blocks=blocks, note='Imported from a guide bundle', + userid=userid) + return guide diff --git a/plugins/guides/frontend/components/GuideBlocks.vue b/plugins/guides/frontend/components/GuideBlocks.vue index d4459517..b446ca12 100644 --- a/plugins/guides/frontend/components/GuideBlocks.vue +++ b/plugins/guides/frontend/components/GuideBlocks.vue @@ -2,14 +2,36 @@