diff --git a/frontend/src/api/index.js b/frontend/src/api/index.js index 6100fe0f..29c39600 100644 --- a/frontend/src/api/index.js +++ b/frontend/src/api/index.js @@ -762,8 +762,16 @@ export const applicationsApi = { update(id, data) { return api.put(`/applications/${id}`, data) }, - delete(id) { - return api.delete(`/applications/${id}`) + // Hard delete. The backend requires the operator's typed confirmation in the + // body, so the caller must pass what the user actually typed - this is not + // defaulted here, or the guard would be meaningless. + delete(id, confirm) { + return api.delete(`/applications/${id}`, { data: { confirm } }) + }, + // Strips policies, versions and uploaded executables but keeps the catalogue + // row and its image. Same typed-confirmation contract as delete. + revertToRecord(id, confirm) { + return api.post(`/applications/${id}/revert`, { confirm }) }, // multipart image upload; backend sets image to the served URL uploadImage(id, file) { diff --git a/migrations/versions/7d45_purge_inactive_apps.py b/migrations/versions/7d45_purge_inactive_apps.py new file mode 100644 index 00000000..1cd933ad --- /dev/null +++ b/migrations/versions/7d45_purge_inactive_apps.py @@ -0,0 +1,86 @@ +"""Purge soft-deleted applications that still reserve their names. + +Delete used to set isactive=False and leave the row in place. The uniqueness +check on create looks at every row regardless of isactive, so each of those +rows kept its name reserved permanently: creating an application with the same +name failed with "already exists" against a row no screen would display, and +nothing in the UI could clear it. + +Delete is a hard delete now, but that only fixes rows deleted from here on. +This clears the ones already stranded, in every environment rather than just +the one where the bug was noticed. + +Dependent rows are cleared the same way the API does it, and for the same +reasons: appversions and computerinstalledapps are NOT NULL and meaningless +without their application, while knowledgebase.appid is nullable and the +article is somebody's writing, so it is unlinked rather than destroyed. + +Uploaded installers belonging to these rows are deliberately NOT touched. A +migration that deletes files cannot be rolled back and cannot be dry-run, and a +stray file is harmless next to that risk. Deletes made through the API do clean +them up. + +Revision ID: 7d45_purge_inactive_apps +Revises: 7d44_app_monitorable +""" + +from alembic import op +import sqlalchemy as sa + + +revision = '7d45_purge_inactive_apps' +down_revision = '7d44_app_monitorable' +branch_labels = None +depends_on = None + + +def _table_exists(bind, name): + """True when a table is present. + + The plugin tables are only there when the plugin is installed, so each is + checked before being touched. + """ + return sa.inspect(bind).has_table(name) + + +def upgrade(): + bind = op.get_bind() + + if not _table_exists(bind, 'applications'): + return + + doomed = [row[0] for row in bind.execute(sa.text( + 'SELECT appid FROM applications WHERE isactive = false' + ))] + + if not doomed: + return + + for appid in doomed: + params = {'appid': appid} + + if _table_exists(bind, 'knowledgebase'): + bind.execute(sa.text( + 'UPDATE knowledgebase SET appid = NULL WHERE appid = :appid' + ), params) + + if _table_exists(bind, 'computerinstalledapps'): + bind.execute(sa.text( + 'DELETE FROM computerinstalledapps WHERE appid = :appid' + ), params) + + if _table_exists(bind, 'appversions'): + bind.execute(sa.text( + 'DELETE FROM appversions WHERE appid = :appid' + ), params) + + bind.execute(sa.text( + 'DELETE FROM applications WHERE appid = :appid' + ), params) + + +def downgrade(): + # The rows are gone and their contents were not captured, so there is + # nothing to restore. Failing loudly is better than pretending otherwise. + raise NotImplementedError( + 'Purged applications cannot be restored; recover from a backup.') diff --git a/migrations/versions/7d46_nest_app_packages.py b/migrations/versions/7d46_nest_app_packages.py new file mode 100644 index 00000000..70e6a628 --- /dev/null +++ b/migrations/versions/7d46_nest_app_packages.py @@ -0,0 +1,136 @@ +"""Move uploaded installers into per-application directories. + +Installers used to be stored flat as "application-.", which threw away +the name the vendor shipped. That is what made a re-import look like new +software: the operator had no way to see that the file already in the catalogue +was the same one they were about to upload again, so duplicate applications and +versions accumulated. Installers now keep their filename, namespaced by +application id so a generic "setup.exe" from two vendors cannot collide. + +That only helps files uploaded from here on. This moves the ones already stored, +so existing download links keep working instead of 404ing against a path the +route no longer serves. + +The original filename cannot be recovered - the old scheme discarded it at +upload - so these land as "installer.". That is no worse than what they +were called before, and re-uploading the installer restores the real name. + +Files are MOVED, not copied or deleted: a move is reversible, which is what +lets downgrade() put them back. This is deliberately unlike 7d45, which refused +to touch files because it could only have deleted them. + +The database is the part that must not be left inconsistent, so installpath is +rewritten for every affected row whether or not its file was found on disk. A +row pointing at a missing file is a broken download; a row pointing at the old +scheme is a broken download that also cannot be repaired by re-uploading. + +Revision ID: 7d46_nest_app_packages +Revises: 7d45_purge_inactive_apps +""" + +import os +import re + +from alembic import op +import sqlalchemy as sa + +revision = '7d46_nest_app_packages' +down_revision = '7d45_purge_inactive_apps' +branch_labels = None +depends_on = None + + +OLD_PREFIX = '/api/applications/package/' + +# application-., the only shape the old uploader produced. +OLD_NAME = re.compile(r'^application-(\d+)\.([A-Za-z0-9]+)$') + + +def _package_dir(): + """The instance-relative directory installers are stored in. + + Resolved the same way the API resolves it. If the app context is not + available the migration still rewrites the database and simply leaves the + files alone, rather than failing the whole upgrade over a path lookup. + """ + try: + from flask import current_app + return os.path.join(current_app.instance_path, 'applicationpackages') + except Exception: + return None + + +def upgrade(): + bind = op.get_bind() + rows = bind.execute(sa.text( + "SELECT appid, installpath FROM applications " + "WHERE installpath LIKE :prefix" + ), {'prefix': OLD_PREFIX + '%'}).fetchall() + + directory = _package_dir() + + for appid, installpath in rows: + tail = installpath[len(OLD_PREFIX):] + match = OLD_NAME.match(tail) + if not match: + # Already nested (contains a slash) or otherwise not the old shape. + # Leaving it alone is correct: it is already being served. + continue + + ext = match.group(2).lower() + stored = f'installer.{ext}' + + if directory: + source = os.path.join(directory, tail) + target_dir = os.path.join(directory, str(appid)) + if os.path.isfile(source): + os.makedirs(target_dir, exist_ok=True) + try: + os.replace(source, os.path.join(target_dir, stored)) + except OSError: + # Best effort. The row is still repointed below so the + # schema is consistent; a missing file is a re-upload. + pass + + bind.execute(sa.text( + "UPDATE applications SET installpath = :path WHERE appid = :appid" + ), {'path': f'{OLD_PREFIX}{appid}/{stored}', 'appid': appid}) + + +def downgrade(): + bind = op.get_bind() + rows = bind.execute(sa.text( + "SELECT appid, installpath FROM applications " + "WHERE installpath LIKE :prefix" + ), {'prefix': OLD_PREFIX + '%'}).fetchall() + + directory = _package_dir() + + for appid, installpath in rows: + tail = installpath[len(OLD_PREFIX):] + if '/' not in tail: + continue + nested_id, _, stored = tail.partition('/') + if nested_id != str(appid): + continue + + ext = os.path.splitext(stored)[1].lstrip('.').lower() + if not ext: + continue + flat = f'application-{appid}.{ext}' + + if directory: + source = os.path.join(directory, str(appid), stored) + if os.path.isfile(source): + try: + os.replace(source, os.path.join(directory, flat)) + # Remove the now-empty directory so a re-upgrade does not + # find a stale one. Only if empty: a directory with other + # files in it is not ours to remove. + os.rmdir(os.path.join(directory, str(appid))) + except OSError: + pass + + bind.execute(sa.text( + "UPDATE applications SET installpath = :path WHERE appid = :appid" + ), {'path': OLD_PREFIX + flat, 'appid': appid}) diff --git a/plugins/applications/frontend/views/ApplicationDetail.vue b/plugins/applications/frontend/views/ApplicationDetail.vue index 2f31b286..5a558158 100644 --- a/plugins/applications/frontend/views/ApplicationDetail.vue +++ b/plugins/applications/frontend/views/ApplicationDetail.vue @@ -30,10 +30,7 @@ Printer App Hidden -
- + Up to 500MB. Anything larger belongs on the share - put its path in - Install Path instead. Downloads require a login. + Installer path instead. Downloads require a login. + +
+ +
+ + + + + Read from the installer. Change it if the vendor numbers releases + differently. + + + Read from the installer where one is present. MSI and archive + files do not carry a version, so type it in to keep a later + re-import from looking like new software. +
@@ -199,11 +260,26 @@
{{ error }}
+
{{ versionWarning }}
+
Cancel + +
@@ -233,12 +309,19 @@ const form = ref({ islicenced: false, isprinter: false, isrequired: false, + ismonitorable: false, + monitorlaunchpath: '', + monitorprocessname: '', ishidden: false, applicationlink: '', documentationpath: '', installpath: '', image: '', - applicationnotes: '' + applicationnotes: '', + // Not a column on applications - it creates/updates an AppVersion row on + // save. Held here so the field can sit next to the installer that produced + // it, which is where the operator is looking when it matters. + version: '' }) const supportTeams = ref([]) @@ -261,6 +344,12 @@ const uploadedPackage = ref('') const packageProgress = ref(null) const busyImage = ref(false) const busyPackage = ref(false) +const busyRevert = ref(false) +const detectedVersion = ref('') +const versionWarning = ref('') +// Versions already on the record, so saving an unchanged form does not try to +// re-create one and trip the duplicate check. +const existingVersions = ref([]) const PACKAGE_ACCEPT = '.exe,.msi,.msp,.zip,.7z,.cab,.iso,.appx,.msix,.ps1,.bat,.txt,.pdf' const MAX_PACKAGE_BYTES = 500 * 1024 * 1024 @@ -313,6 +402,60 @@ async function removePackage() { } } +// Strips the application back to a bare catalogue record. Distinct from Delete +// on the list: the row, name and knowledgebase links survive, so this is the +// action for "stop deploying this" rather than "this never existed". Still +// irreversible, so it uses the same typed-OK gate the backend enforces. +async function revertToRecord() { + const answer = prompt( + 'Remove all policy associations and uploaded executables? This action ' + + 'cannot be reversed. Type OK to proceed') + if ((answer || '').trim().toUpperCase() !== 'OK') return + + busyRevert.value = true + error.value = '' + try { + const response = await applicationsApi.revertToRecord( + route.params.id, (answer || '').trim()) + const app = response.data.data + // Re-seed from the server rather than guessing which fields it cleared. + // The image is deliberately untouched: revert removes executables and + // policy associations, not the record's icon. + form.value.installpath = app.installpath || '' + form.value.isrequired = app.isrequired + // Revert drops every AppVersion row, so the field must not keep showing a + // version that no longer exists. + form.value.version = '' + uploadedPackage.value = '' + pendingPackage.value = null + detectedVersion.value = '' + versionWarning.value = '' + } catch (err) { + error.value = apiError(err, 'Failed to revert the application') + } finally { + busyRevert.value = false + } +} + +// Records the typed version against the application, if it is new. Creating a +// version that already exists is not an error worth failing the save over - +// re-saving a form without touching the field is the normal case, and the +// backend answers 409, which would otherwise surface as "failed to save" on a +// save that in fact succeeded. +async function saveVersion(appid) { + const version = (form.value.version || '').trim() + if (!version) return + if (existingVersions.value.some(v => v.version === version)) return + + try { + await applicationsApi.createVersion(appid, { version }) + existingVersions.value = [{ version }, ...existingVersions.value] + } catch (err) { + if (err.response?.status === 409) return + throw err + } +} + // The image field holds either a served URL (new) or a bare filename from the // classic site, which is still rendered out of /images/applications/. function imageSrcFor(value) { @@ -341,13 +484,22 @@ onMounted(async () => { islicenced: app.islicenced || false, isprinter: app.isprinter || false, isrequired: app.isrequired || false, + ismonitorable: app.ismonitorable || false, + monitorlaunchpath: app.monitorlaunchpath || '', + monitorprocessname: app.monitorprocessname || '', ishidden: app.ishidden || false, applicationlink: app.applicationlink || '', documentationpath: app.documentationpath || '', installpath: app.installpath || '', image: app.image || '', - applicationnotes: app.applicationnotes || '' + applicationnotes: app.applicationnotes || '', + // The detail payload already carries versions newest-first, so the + // current one comes free rather than costing another request. + version: (app.versions && app.versions.length) + ? (app.versions[0].version || '') + : '' } + existingVersions.value = app.versions || [] imagePreview.value = imageSrcFor(app.image) wasRequired.value = app.isrequired || false if ((app.installpath || '').startsWith('/api/applications/package/')) { @@ -371,6 +523,20 @@ async function saveApplication() { return } + // Mirrors _validate_monitorable on the server. Checked here too so the + // operator is told which field is missing while they are still looking at it, + // rather than after a round trip. + if (form.value.ismonitorable) { + if (!(form.value.monitorprocessname || '').trim()) { + error.value = 'A monitorable application needs a process name.' + return + } + if (!(form.value.monitorlaunchpath || '').trim()) { + error.value = 'A monitorable application needs a launch path.' + return + } + } + saving.value = true try { @@ -382,6 +548,15 @@ async function saveApplication() { islicenced: form.value.islicenced, isprinter: form.value.isprinter, isrequired: form.value.isrequired, + ismonitorable: form.value.ismonitorable, + // Cleared when the box is unticked so a later re-tick cannot silently + // revive a stale process name the operator can no longer see. + monitorlaunchpath: form.value.ismonitorable + ? (form.value.monitorlaunchpath || null) + : null, + monitorprocessname: form.value.ismonitorable + ? (form.value.monitorprocessname || null) + : null, ishidden: form.value.ishidden, applicationlink: form.value.applicationlink || null, documentationpath: form.value.documentationpath || null, @@ -408,12 +583,33 @@ async function saveApplication() { } if (pendingPackage.value) { packageProgress.value = 0 - await applicationsApi.uploadPackage(appid, pendingPackage.value, - percent => { packageProgress.value = percent }) + const uploaded = await applicationsApi.uploadPackage( + appid, pendingPackage.value, + percent => { packageProgress.value = percent }) pendingPackage.value = null packageProgress.value = null + + // An installer with no readable version resource is how duplicate + // version rows got created: the operator saved, saw no version, and + // imported the same file again. MSIs and archives are not PE files and + // never carry one, so ask rather than guess. + detectedVersion.value = uploaded.data.data.detectedversion || '' + if (detectedVersion.value && !form.value.version.trim()) { + form.value.version = detectedVersion.value + } + if (!form.value.version.trim()) { + versionWarning.value = + `No version could be read from ${uploaded.data.data.uploadedfilename}. ` + + 'Enter one in the Version field above so a later import is not ' + + 'mistaken for new software.' + // The application and installer are already saved; only the version is + // outstanding, so stay on the form rather than losing that work. + return + } } + await saveVersion(appid) + router.push('/applications') } catch (err) { console.error('Error saving application:', err) @@ -489,11 +685,14 @@ async function saveApplication() { .policy-warning { margin-top: 1rem; padding: 0.75rem 1rem; - border: 1px solid var(--warning, #b45309); + border: 1px solid var(--warning); border-left-width: 4px; border-radius: 4px; - background: var(--warning-bg, #fffbeb); - color: var(--text, #333); + /* Derived from the real token rather than a fixed tint: a hardcoded light + value here survives the theme switch and renders light-on-light in dark + mode. --warning-bg does not exist in style.css. */ + background: color-mix(in srgb, var(--warning) 12%, var(--bg-card-solid)); + color: var(--text); } .policy-warning p { @@ -508,4 +707,13 @@ async function saveApplication() { cursor: pointer; font-weight: 600; } + +.monitor-fields { + margin-top: 1rem; + padding: 0.75rem 1rem; + border: 1px solid var(--border); + border-left: 4px solid var(--warning); + border-radius: 4px; + background: color-mix(in srgb, var(--warning) 6%, var(--bg-card-solid)); +} diff --git a/plugins/applications/frontend/views/ApplicationsList.vue b/plugins/applications/frontend/views/ApplicationsList.vue index 3e7a71f1..76bfaae9 100644 --- a/plugins/applications/frontend/views/ApplicationsList.vue +++ b/plugins/applications/frontend/views/ApplicationsList.vue @@ -44,9 +44,6 @@ ⬇ - - 🔗 - @@ -178,19 +175,23 @@ function debouncedSearch() { }, 300) } -// The server deactivates rather than drops the row, so this is reversible and -// the prompt does not claim otherwise. It does warn about the fleet-wide -// consequence, which is the part that is not obvious from a Delete button. +// This permanently drops the row, its versions and its uploaded installer, so +// the prompt says so plainly and requires the word OK to be typed. A plain +// confirm() is one misplaced click away from destroying licensed software. async function deleteApplication(app) { const warning = app.isrequired ? `\n\n"${app.appname}" is currently required on all PCs. Removing it also ` + 'withdraws the policy that installs it.' : '' - if (!confirm(`Remove "${app.appname}" from the catalogue?${warning}`)) return + const answer = prompt( + `Permanently delete "${app.appname}", its versions and its uploaded ` + + `executables? This action cannot be reversed.${warning}\n\n` + + 'Type OK to proceed') + if ((answer || '').trim().toUpperCase() !== 'OK') return try { - await applicationsApi.delete(app.appid) + await applicationsApi.delete(app.appid, (answer || '').trim()) loadApplications() } catch (error) { console.error('Error deleting application:', error) @@ -227,10 +228,6 @@ function changePerPage(newPerPage) { color: var(--success); } -.icon-link { - color: var(--link); -} - .icon-docs { color: var(--secondary); } diff --git a/plugins/endpoints/api/routes.py b/plugins/endpoints/api/routes.py index a2491484..cc1524fa 100644 --- a/plugins/endpoints/api/routes.py +++ b/plugins/endpoints/api/routes.py @@ -215,7 +215,8 @@ def list_endpoints(): query = (db.session.query(EndpointState, Computer.computerid, Computer.computertypeid, ComputerType.computertype, - Asset.assetnumber, statusrank.label('statusrank')) + Asset.assetnumber, Asset.name, + statusrank.label('statusrank')) .join(Asset, Asset.assetid == EndpointState.assetid) .join(Computer, Computer.assetid == EndpointState.assetid) .outerjoin(ComputerType, @@ -227,7 +228,8 @@ def list_endpoints(): if search := (request.args.get('search') or '').strip(): like = f'%{search}%' machinehits = _assetids_serving_machine_like(like) - clauses = [EndpointState.hostname.ilike(like), Asset.assetnumber.ilike(like)] + clauses = [EndpointState.hostname.ilike(like), Asset.assetnumber.ilike(like), + Asset.name.ilike(like)] if machinehits: clauses.append(EndpointState.assetid.in_(machinehits)) query = query.filter(db.or_(*clauses)) @@ -265,10 +267,14 @@ def list_endpoints(): health = _health_for_assets(states, computertypes) data = [] - for state, computerid, computertypeid, computertype, assetnumber, rank in items: + for (state, computerid, computertypeid, computertype, assetnumber, alias, + rank) in items: row = state.to_dict() row['computerid'] = computerid row['assetnumber'] = assetnumber + # Named 'alias' rather than 'name' because the fleet row already + # carries a hostname; 'name' next to 'hostname' reads as a duplicate. + row['alias'] = alias row['computertypeid'] = computertypeid row['computertype'] = computertype row['uptimedisplay'] = _format_uptime(state.uptimeseconds) diff --git a/plugins/endpoints/frontend/views/EndpointsList.vue b/plugins/endpoints/frontend/views/EndpointsList.vue index e640bb8b..57e7d265 100644 --- a/plugins/endpoints/frontend/views/EndpointsList.vue +++ b/plugins/endpoints/frontend/views/EndpointsList.vue @@ -87,6 +87,7 @@ Status + Alias Hostname Machine PC Type @@ -111,6 +112,7 @@ {{ statusLabel(item.status) }} + {{ item.alias || '-' }} {{ item.hostname || '-' }}