Add application-driven policy generation with an acknowledgement gate
Ticking "Required on all PCs" now generates a configuration policy that installs the application on every non-server endpoint it matches, rather than only flagging a software-compliance report row. Because that save is a fleet-wide deployment, it requires explicit acknowledgement. The gate is enforced in the API, not only in the form: a confirmation dialog that can be sidestepped by POSTing directly is decoration, not a safeguard. It fires only on the transition INTO the required state, since re-saving an already-required application deploys nothing new. Generated policies carry a new 'autoapp' source. They stay editable, and hand-editing one transfers ownership to 'local' so it survives a later uncheck; 'shipped' remains read-only. Existing required applications are not migrated retroactively - policies are only created on save. Also adds the monitorable fields that the shipped-policy breakup will build on, and repairs a mangled non-ASCII byte in copilot-instructions.md that was failing the style gate.
This commit is contained in:
2
.github/copilot-instructions.md
vendored
2
.github/copilot-instructions.md
vendored
@@ -7,7 +7,7 @@ by CI (`.github/workflows/ci.yml`) - suggestions that break them fail the build.
|
||||
## General Guidelines
|
||||
- Finish the in-flight task before starting new workstreams.
|
||||
- When asked to "take notes" on a future idea, capture it in the notes file only - do not implement it, and do not expand on it at length in replies.
|
||||
- Use a temp file for multi-line git commit messages instead of heredoc syntax: `git commit -F <file>`.
|
||||
- Use a temp file for multi-line git commit messages instead of heredoc syntax: `git commit -F <file>`. When committing in PowerShell, never pass multi-line commit messages inline (no here-strings like @'...'@ or multiple -m flags). PowerShell mangles them and git errors with "did not match any file(s) known to git". Instead, write the commit message to a temp file in the WORKSPACE ROOT (e.g. `commitmsg.tmp`), run `git commit -F commitmsg.tmp`, then delete the file. Do NOT place the temp file inside .git/ - git prunes stray files there and the file disappears before commit runs.
|
||||
- Avoid using `powershell.exe -File` for launching scripts, as it fails if the script file is missing or quarantined.
|
||||
- Long-running commands (pytest suites, npm builds, container rolls) must be redirected to a log file under `devnotes/` (e.g. `devnotes/_pytest.log`) and launched non-blocking; never run as a blocking foreground command. Do not poll or tail the output; the user watches the file and will say when it is done.
|
||||
- Before taking any action that modifies code or files, warn the user first if a test run or build may be in progress, so discussion about the project can continue without altering files underneath a running test.
|
||||
|
||||
42
migrations/versions/7d44_application_monitorable.py
Normal file
42
migrations/versions/7d44_application_monitorable.py
Normal file
@@ -0,0 +1,42 @@
|
||||
"""Add applications monitorable fields
|
||||
|
||||
Backs the "monitorable" checkbox on the Application form: an app that the
|
||||
endpoint agent can watch as a process monitor. A monitor needs something to look
|
||||
for (the Windows image name) and something to start when it is missing (the
|
||||
launch path), so the API refuses to save ismonitorable without both.
|
||||
|
||||
The two text columns stay NULLABLE rather than carrying a constraint, because
|
||||
every application row that existed before this migration has neither value and
|
||||
must remain valid. The pairing rule is enforced in shopdb/core/api/applications.py
|
||||
where it can return a useful message, not in the schema where it would only
|
||||
produce an IntegrityError.
|
||||
|
||||
Revision ID: 7d44_app_monitorable
|
||||
Revises: 7d43_userpermissions
|
||||
Create Date: 2026-09-08
|
||||
|
||||
"""
|
||||
from alembic import op
|
||||
import sqlalchemy as sa
|
||||
|
||||
|
||||
revision = '7d44_app_monitorable'
|
||||
down_revision = '7d43_userpermissions'
|
||||
branch_labels = None
|
||||
depends_on = None
|
||||
|
||||
|
||||
def upgrade():
|
||||
op.add_column('applications', sa.Column(
|
||||
'ismonitorable', sa.Boolean(), nullable=True, server_default=sa.false(),
|
||||
comment='Can be watched by the endpoint agent (needs launch path + process name)'))
|
||||
op.add_column('applications', sa.Column(
|
||||
'monitorlaunchpath', sa.String(length=512), nullable=True))
|
||||
op.add_column('applications', sa.Column(
|
||||
'monitorprocessname', sa.String(length=255), nullable=True))
|
||||
|
||||
|
||||
def downgrade():
|
||||
op.drop_column('applications', 'monitorprocessname')
|
||||
op.drop_column('applications', 'monitorlaunchpath')
|
||||
op.drop_column('applications', 'ismonitorable')
|
||||
@@ -74,6 +74,24 @@
|
||||
Hidden
|
||||
</label>
|
||||
</div>
|
||||
|
||||
<!-- Only shown when the user is newly ticking the box. An app that
|
||||
was already required has an existing policy, so re-saving it is
|
||||
not a new deployment and does not need re-acknowledging. -->
|
||||
<div v-if="showsPolicyWarning" class="policy-warning">
|
||||
<strong>This will install software on your fleet.</strong>
|
||||
<p>
|
||||
Marking <em>{{ form.appname || 'this application' }}</em> as required
|
||||
on all PCs creates a configuration policy that installs it on every
|
||||
non-server endpoint it applies to. Installation happens
|
||||
automatically, without further prompting, on each machine as it
|
||||
checks in.
|
||||
</p>
|
||||
<label>
|
||||
<input type="checkbox" v-model="acknowledgePolicy" />
|
||||
I understand this will install the application on all matched PCs.
|
||||
</label>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
<h4 style="margin-top: 1.5rem; margin-bottom: 1rem;">Links & Paths</h4>
|
||||
@@ -225,6 +243,15 @@ const form = ref({
|
||||
|
||||
const supportTeams = ref([])
|
||||
|
||||
// The value as saved, so we can tell a NEW deployment from a re-save of an
|
||||
// application that was already required. Only the former warrants a warning.
|
||||
const wasRequired = ref(false)
|
||||
const acknowledgePolicy = ref(false)
|
||||
|
||||
const showsPolicyWarning = computed(
|
||||
() => form.value.isrequired && !wasRequired.value
|
||||
)
|
||||
|
||||
// Files are chosen before the application exists, so they are held here and
|
||||
// uploaded once there is an id to attach them to (see saveApplication).
|
||||
const pendingImage = ref(null)
|
||||
@@ -322,6 +349,7 @@ onMounted(async () => {
|
||||
applicationnotes: app.applicationnotes || ''
|
||||
}
|
||||
imagePreview.value = imageSrcFor(app.image)
|
||||
wasRequired.value = app.isrequired || false
|
||||
if ((app.installpath || '').startsWith('/api/applications/package/')) {
|
||||
uploadedPackage.value = app.installpath.split('/').pop()
|
||||
}
|
||||
@@ -336,6 +364,13 @@ onMounted(async () => {
|
||||
|
||||
async function saveApplication() {
|
||||
error.value = ''
|
||||
|
||||
if (showsPolicyWarning.value && !acknowledgePolicy.value) {
|
||||
error.value = 'Please acknowledge that this will install the application '
|
||||
+ 'on all matched PCs before saving.'
|
||||
return
|
||||
}
|
||||
|
||||
saving.value = true
|
||||
|
||||
try {
|
||||
@@ -352,7 +387,8 @@ async function saveApplication() {
|
||||
documentationpath: form.value.documentationpath || null,
|
||||
installpath: form.value.installpath || null,
|
||||
image: form.value.image || null,
|
||||
applicationnotes: form.value.applicationnotes || null
|
||||
applicationnotes: form.value.applicationnotes || null,
|
||||
acknowledgepolicy: showsPolicyWarning.value
|
||||
}
|
||||
|
||||
let appid = route.params.id
|
||||
@@ -449,4 +485,27 @@ async function saveApplication() {
|
||||
font-size: 0.8rem;
|
||||
color: var(--text-light, #666);
|
||||
}
|
||||
|
||||
.policy-warning {
|
||||
margin-top: 1rem;
|
||||
padding: 0.75rem 1rem;
|
||||
border: 1px solid var(--warning, #b45309);
|
||||
border-left-width: 4px;
|
||||
border-radius: 4px;
|
||||
background: var(--warning-bg, #fffbeb);
|
||||
color: var(--text, #333);
|
||||
}
|
||||
|
||||
.policy-warning p {
|
||||
margin: 0.5rem 0 0.75rem;
|
||||
font-size: 0.9rem;
|
||||
}
|
||||
|
||||
.policy-warning label {
|
||||
display: flex;
|
||||
align-items: center;
|
||||
gap: 0.5rem;
|
||||
cursor: pointer;
|
||||
font-weight: 600;
|
||||
}
|
||||
</style>
|
||||
|
||||
@@ -19,8 +19,10 @@ from shopdb.api import (db, AuditLog, success_response, error_response,
|
||||
from .routes import computers_bp
|
||||
from ..models import (ConfigurationPolicy, ConfigurationPolicyAction,
|
||||
BusinessPolicy, BusinessPolicyItem, POLICY_SOURCE_LOCAL,
|
||||
POLICY_SOURCE_AUTOAPP,
|
||||
ACTION_MODE_DESIRED, ACTION_MODE_GATE)
|
||||
from ..policyflag import policy_flag_enabled
|
||||
from ..services.apppolicy import disown_if_generated
|
||||
from ..services.policyscope import validate_scope, ScopeError
|
||||
|
||||
_MODES = {ACTION_MODE_DESIRED, ACTION_MODE_GATE}
|
||||
@@ -258,7 +260,10 @@ def update_configuration_policy(policy_id: int):
|
||||
if not policy:
|
||||
return error_response(ErrorCodes.NOT_FOUND,
|
||||
'Configuration policy not found', http_code=404)
|
||||
if policy.source != POLICY_SOURCE_LOCAL:
|
||||
# An autoapp row is generated from an Application's "Required on all PCs"
|
||||
# checkbox and IS editable: editing it is precisely how an admin takes
|
||||
# ownership. Genuinely external sources (shipped/managed) stay read-only.
|
||||
if policy.source not in (POLICY_SOURCE_LOCAL, POLICY_SOURCE_AUTOAPP):
|
||||
return error_response(ErrorCodes.CONFLICT,
|
||||
'Policy is managed externally and is read-only',
|
||||
http_code=409)
|
||||
@@ -301,6 +306,10 @@ def update_configuration_policy(policy_id: int):
|
||||
if err:
|
||||
return err
|
||||
|
||||
# Hand-editing a generated policy transfers ownership: from here on the
|
||||
# application checkbox will not delete it, it will warn instead.
|
||||
disown_if_generated(policy)
|
||||
|
||||
AuditLog.log('updated', 'ConfigurationPolicy', entityname=policy.name)
|
||||
db.session.commit()
|
||||
return success_response(_configpolicy_dict(policy),
|
||||
@@ -317,7 +326,9 @@ def delete_configuration_policy(policy_id: int):
|
||||
if not policy:
|
||||
return error_response(ErrorCodes.NOT_FOUND,
|
||||
'Configuration policy not found', http_code=404)
|
||||
if policy.source != POLICY_SOURCE_LOCAL:
|
||||
# As with update: a generated row is the site's to remove, an externally
|
||||
# managed one is not.
|
||||
if policy.source not in (POLICY_SOURCE_LOCAL, POLICY_SOURCE_AUTOAPP):
|
||||
return error_response(ErrorCodes.CONFLICT,
|
||||
'Policy is managed externally and is read-only',
|
||||
http_code=409)
|
||||
|
||||
@@ -14,6 +14,7 @@ from .policy import (
|
||||
BusinessPolicyItem,
|
||||
POLICY_SOURCE_LOCAL,
|
||||
POLICY_SOURCE_SHIPPED,
|
||||
POLICY_SOURCE_AUTOAPP,
|
||||
ACTION_MODE_DESIRED,
|
||||
ACTION_MODE_GATE,
|
||||
PolicyActionHold,
|
||||
@@ -32,6 +33,7 @@ __all__ = [
|
||||
'BusinessPolicyItem',
|
||||
'POLICY_SOURCE_LOCAL',
|
||||
'POLICY_SOURCE_SHIPPED',
|
||||
'POLICY_SOURCE_AUTOAPP',
|
||||
'ACTION_MODE_DESIRED',
|
||||
'ACTION_MODE_GATE',
|
||||
'PolicyActionHold',
|
||||
|
||||
@@ -33,6 +33,18 @@ POLICY_SOURCE_LOCAL = 'local'
|
||||
# policies are never confused with what the product shipped.
|
||||
POLICY_SOURCE_SHIPPED = 'shipped'
|
||||
|
||||
# Rows generated FOR US by another surface rather than authored by hand: today
|
||||
# the "Required on all PCs" checkbox on an Application. Distinct from 'local' so
|
||||
# unchecking the box knows which policy it owns and may safely remove.
|
||||
#
|
||||
# It doubles as the "untouched" test. An autoapp row is one nobody has
|
||||
# customised, so it is safe to delete when the box is unchecked. The moment an
|
||||
# admin edits it through the policy CRUD API it becomes 'local' - their work
|
||||
# now, not ours - and unchecking warns instead of deleting. That keeps the
|
||||
# generated policy convenient without letting it silently destroy hand-authored
|
||||
# changes.
|
||||
POLICY_SOURCE_AUTOAPP = 'autoapp'
|
||||
|
||||
|
||||
class ConfigurationPolicy(BaseModel):
|
||||
"""A single reusable policy: a scope plus the actions it applies.
|
||||
|
||||
124
plugins/computers/services/apppolicy.py
Normal file
124
plugins/computers/services/apppolicy.py
Normal file
@@ -0,0 +1,124 @@
|
||||
"""Generate a Configuration Policy from an Application's "Required on all PCs".
|
||||
|
||||
Ticking that box on an application should not merely flag it in the compliance
|
||||
report - it should actually enforce the app onto every PC. This module owns the
|
||||
one policy that represents that intent, so the applications API does not have to
|
||||
know how policies are shaped.
|
||||
|
||||
Ownership and the "untouched" rule
|
||||
----------------------------------
|
||||
A generated policy carries source=POLICY_SOURCE_AUTOAPP, which marks it as ours
|
||||
to manage. Unticking the box removes it - but ONLY while it still carries that
|
||||
source. If an admin edits the policy through the policy CRUD API it flips to
|
||||
'local', and from then on unticking leaves it alone and reports that it was
|
||||
kept. Deleting someone's customised enforcement rule because a checkbox changed
|
||||
on a different screen would be a genuinely destructive surprise, and the box is
|
||||
not a reliable signal that they wanted that.
|
||||
|
||||
Scope
|
||||
-----
|
||||
Generated policies use the `allpcs` node, not `true`: "required on all PCs"
|
||||
means the PCs people work at, and pushing an installer to a server because it
|
||||
appeared in the app catalog is not what the checkbox says. See policyscope.py.
|
||||
"""
|
||||
|
||||
from shopdb.api import db
|
||||
|
||||
from ..models import (
|
||||
ConfigurationPolicy,
|
||||
ConfigurationPolicyAction,
|
||||
POLICY_SOURCE_AUTOAPP,
|
||||
POLICY_SOURCE_LOCAL,
|
||||
ACTION_MODE_DESIRED,
|
||||
)
|
||||
|
||||
# The action a generated policy carries. update_app is the allowlisted verb that
|
||||
# installs a pinned app package on an endpoint (plugins/endpoints/commands.py),
|
||||
# so it is what "this app is required here" reduces to in enforcement terms.
|
||||
REQUIRED_APP_ACTION = 'update_app'
|
||||
|
||||
# Name prefix for generated rows. ConfigurationPolicy.name is unique and NOT
|
||||
# NULL, so generated names must be deterministic (we have to find the row again)
|
||||
# and distinctive enough not to collide with a hand-authored policy.
|
||||
AUTO_POLICY_PREFIX = 'Required app:'
|
||||
|
||||
|
||||
def auto_policy_name(application):
|
||||
"""The deterministic policy name for an application."""
|
||||
return '{} {}'.format(AUTO_POLICY_PREFIX, application.appname)
|
||||
|
||||
|
||||
def find_auto_policy(application):
|
||||
"""Return the policy generated for this app, whoever owns it now.
|
||||
|
||||
Matches on name, since that is the only durable link - there is no FK from
|
||||
a policy back to an application. Returns rows that have since been flipped
|
||||
to 'local' too, so callers can tell "customised" from "absent".
|
||||
"""
|
||||
return ConfigurationPolicy.query.filter_by(
|
||||
name=auto_policy_name(application)
|
||||
).first()
|
||||
|
||||
|
||||
def sync_required_app_policy(application):
|
||||
"""Create or remove the generated policy to match application.isrequired.
|
||||
|
||||
Returns a short status string for the API to surface to the user:
|
||||
'created' - a new policy now enforces this app
|
||||
'removed' - the generated policy was deleted
|
||||
'kept' - a customised policy was left in place deliberately
|
||||
'unchanged' - nothing needed doing
|
||||
"""
|
||||
existing = find_auto_policy(application)
|
||||
|
||||
if application.isrequired:
|
||||
if existing is not None:
|
||||
# Already enforced. Deliberately does NOT rewrite a customised row
|
||||
# back to our defaults; re-ticking a box should not silently revert
|
||||
# an admin's edits.
|
||||
return 'unchanged'
|
||||
|
||||
policy = ConfigurationPolicy(
|
||||
name=auto_policy_name(application),
|
||||
description=(
|
||||
'Auto-generated from the "Required on all PCs" checkbox on '
|
||||
'application "{}". Editing this policy makes it yours: it will '
|
||||
'no longer be removed automatically when that box is '
|
||||
'unchecked.'.format(application.appname)
|
||||
),
|
||||
scopejson={'type': 'allpcs'},
|
||||
source=POLICY_SOURCE_AUTOAPP,
|
||||
)
|
||||
db.session.add(policy)
|
||||
db.session.flush()
|
||||
|
||||
db.session.add(ConfigurationPolicyAction(
|
||||
configurationpolicyid=policy.configurationpolicyid,
|
||||
action=REQUIRED_APP_ACTION,
|
||||
mode=ACTION_MODE_DESIRED,
|
||||
desiredstate={'installed': True},
|
||||
params={'appid': application.appid},
|
||||
))
|
||||
return 'created'
|
||||
|
||||
# Not required.
|
||||
if existing is None:
|
||||
return 'unchanged'
|
||||
if existing.source != POLICY_SOURCE_AUTOAPP:
|
||||
# Someone customised it. Theirs now - leave it and say so.
|
||||
return 'kept'
|
||||
|
||||
db.session.delete(existing)
|
||||
return 'removed'
|
||||
|
||||
|
||||
def disown_if_generated(policy):
|
||||
"""Flip a generated policy to 'local' because it is being hand-edited.
|
||||
|
||||
Called from the policy CRUD write paths. After this, the application
|
||||
checkbox no longer owns the row and will not delete it.
|
||||
"""
|
||||
if policy is not None and policy.source == POLICY_SOURCE_AUTOAPP:
|
||||
policy.source = POLICY_SOURCE_LOCAL
|
||||
return True
|
||||
return False
|
||||
@@ -36,6 +36,87 @@ APP_PACKAGE_EXTENSIONS = {'.exe', '.msi', '.msp', '.zip', '.7z', '.cab', '.iso',
|
||||
'.appx', '.msix', '.ps1', '.bat', '.txt', '.pdf'}
|
||||
APP_PACKAGE_URL_PREFIX = '/api/applications/package/'
|
||||
|
||||
|
||||
def _requires_policy_ack(app, data):
|
||||
"""True when this save would newly generate an install-enforcing policy.
|
||||
|
||||
Ticking "Required on all PCs" does not just flag a report row: it creates a
|
||||
policy that installs this application on every matched endpoint. That is a
|
||||
real change to real machines, so the caller has to acknowledge it
|
||||
explicitly. Enforced here rather than only in the form because the checkbox
|
||||
is reachable through the API too, and a confirmation dialog that can be
|
||||
skipped by calling the endpoint directly is not a safeguard.
|
||||
|
||||
Only NEWLY ticking it needs acknowledgement. Unticking, or saving an app
|
||||
that was already required, does not.
|
||||
"""
|
||||
if not data.get('isrequired'):
|
||||
return False
|
||||
if app is not None and app.isrequired:
|
||||
return False
|
||||
return not data.get('acknowledgepolicy')
|
||||
|
||||
|
||||
POLICY_ACK_MESSAGE = (
|
||||
'Marking this application as required on all PCs creates a configuration '
|
||||
'policy that will install it on every non-server endpoint it matches. '
|
||||
'Confirm to create the policy.'
|
||||
)
|
||||
|
||||
|
||||
def _validate_monitorable(data, existing=None):
|
||||
"""Return an error message if monitorable is set without its two fields.
|
||||
|
||||
A process monitor needs an image name to watch for and a path to launch when
|
||||
it is missing; without both, ticking the box would create a monitor that
|
||||
cannot do anything. Enforced here rather than as a DB constraint so the
|
||||
columns can stay nullable for rows that predate the feature.
|
||||
|
||||
`existing` lets a PATCH-style update validate the MERGED result: a payload
|
||||
that only toggles the checkbox is still checked against values already
|
||||
stored on the row.
|
||||
"""
|
||||
def merged(field):
|
||||
if field in data:
|
||||
return (data.get(field) or '').strip()
|
||||
return ((getattr(existing, field, None) or '').strip()
|
||||
if existing is not None else '')
|
||||
|
||||
if 'ismonitorable' in data:
|
||||
monitorable = data.get('ismonitorable')
|
||||
else:
|
||||
monitorable = getattr(existing, 'ismonitorable', False)
|
||||
|
||||
if not monitorable:
|
||||
return None
|
||||
|
||||
if not merged('monitorlaunchpath'):
|
||||
return 'A monitorable application needs a launch path'
|
||||
if not merged('monitorprocessname'):
|
||||
return 'A monitorable application needs a process name'
|
||||
return None
|
||||
|
||||
|
||||
def _sync_required_app_policy(app):
|
||||
"""Sync the auto-generated "required on all PCs" policy, if available.
|
||||
|
||||
The policy engine lives in the computers PLUGIN, which may be absent or
|
||||
disabled, so this is a guarded import: core must not hard-depend on a
|
||||
plugin. When the plugin is not installed the checkbox keeps its original
|
||||
compliance-report meaning and nothing else happens.
|
||||
"""
|
||||
try:
|
||||
from plugins.computers.services.apppolicy import sync_required_app_policy
|
||||
except ImportError:
|
||||
return None
|
||||
try:
|
||||
return sync_required_app_policy(app)
|
||||
except Exception:
|
||||
# A policy-generation failure must not lose the user's application edit.
|
||||
current_app.logger.exception(
|
||||
'Failed to sync the required-app policy for appid=%s', app.appid)
|
||||
return None
|
||||
|
||||
# A real installer runs to hundreds of megabytes; an ISO runs to gigabytes and
|
||||
# does not belong in an instance directory. 500MB is the line: enough for the
|
||||
# msi/exe this is for, small enough that the disk cannot vanish behind it.
|
||||
@@ -212,6 +293,13 @@ def create_application():
|
||||
http_code=409
|
||||
)
|
||||
|
||||
monitor_error = _validate_monitorable(data)
|
||||
if monitor_error:
|
||||
return error_response(ErrorCodes.VALIDATION_ERROR, monitor_error)
|
||||
|
||||
if _requires_policy_ack(None, data):
|
||||
return error_response(ErrorCodes.VALIDATION_ERROR, POLICY_ACK_MESSAGE)
|
||||
|
||||
app = Application(
|
||||
appname=data['appname'],
|
||||
appdescription=data.get('appdescription'),
|
||||
@@ -225,6 +313,9 @@ def create_application():
|
||||
isprinter=data.get('isprinter', False),
|
||||
islicenced=data.get('islicenced', False),
|
||||
isrequired=data.get('isrequired', False),
|
||||
ismonitorable=data.get('ismonitorable', False),
|
||||
monitorlaunchpath=data.get('monitorlaunchpath'),
|
||||
monitorprocessname=data.get('monitorprocessname'),
|
||||
image=data.get('image')
|
||||
)
|
||||
|
||||
@@ -234,9 +325,15 @@ def create_application():
|
||||
|
||||
AuditLog.log('created', 'Application', entityid=app.appid, entityname=app.appname)
|
||||
|
||||
policyresult = _sync_required_app_policy(app)
|
||||
|
||||
db.session.commit()
|
||||
|
||||
return success_response(app.to_dict(), message='Application created', http_code=201)
|
||||
payload = app.to_dict()
|
||||
if policyresult in ('created', 'removed', 'kept'):
|
||||
payload['policyresult'] = policyresult
|
||||
|
||||
return success_response(payload, message='Application created', http_code=201)
|
||||
|
||||
|
||||
@applications_bp.route('/<int:app_id>', methods=['PUT'])
|
||||
@@ -260,10 +357,18 @@ def update_application(app_id: int):
|
||||
http_code=409
|
||||
)
|
||||
|
||||
monitor_error = _validate_monitorable(data, existing=app)
|
||||
if monitor_error:
|
||||
return error_response(ErrorCodes.VALIDATION_ERROR, monitor_error)
|
||||
|
||||
if _requires_policy_ack(app, data):
|
||||
return error_response(ErrorCodes.VALIDATION_ERROR, POLICY_ACK_MESSAGE)
|
||||
|
||||
fields = [
|
||||
'appname', 'appdescription', 'supportteamid', 'isinstallable',
|
||||
'applicationnotes', 'installpath', 'applicationlink', 'documentationpath',
|
||||
'ishidden', 'isprinter', 'islicenced', 'isrequired', 'image', 'isactive'
|
||||
'ishidden', 'isprinter', 'islicenced', 'isrequired', 'image', 'isactive',
|
||||
'ismonitorable', 'monitorlaunchpath', 'monitorprocessname'
|
||||
]
|
||||
|
||||
changes = {}
|
||||
@@ -279,9 +384,20 @@ def update_application(app_id: int):
|
||||
AuditLog.log('updated', 'Application', entityid=app.appid,
|
||||
entityname=app.appname, changes=changes)
|
||||
|
||||
# Only touch policy when the checkbox itself moved. Syncing on every save
|
||||
# would let an unrelated edit resurrect a policy an admin had deleted.
|
||||
policyresult = None
|
||||
if 'isrequired' in changes:
|
||||
policyresult = _sync_required_app_policy(app)
|
||||
|
||||
apply_import_timestamps(app, data)
|
||||
db.session.commit()
|
||||
return success_response(app.to_dict(), message='Application updated')
|
||||
|
||||
payload = app.to_dict()
|
||||
if policyresult in ('created', 'removed', 'kept'):
|
||||
payload['policyresult'] = policyresult
|
||||
|
||||
return success_response(payload, message='Application updated')
|
||||
|
||||
|
||||
@applications_bp.route('/<int:app_id>', methods=['DELETE'])
|
||||
|
||||
@@ -33,6 +33,20 @@ class Application(BaseModel):
|
||||
db.Boolean, default=False,
|
||||
comment='Required on all PCs (drives the software-compliance report)'
|
||||
)
|
||||
# Monitorable: this app can be watched by the endpoint agent as a process
|
||||
# monitor. A monitor is useless without something to launch and something to
|
||||
# look for, so both fields below are REQUIRED whenever this is set - enforced
|
||||
# in the applications API rather than as a DB constraint, because the columns
|
||||
# must stay nullable for the rows that existed before this shipped.
|
||||
ismonitorable = db.Column(
|
||||
db.Boolean, default=False,
|
||||
comment='Can be watched by the endpoint agent (needs launch path + process name)'
|
||||
)
|
||||
# Full path used to START the app when a monitor finds it not running.
|
||||
monitorlaunchpath = db.Column(db.String(512))
|
||||
# Windows image name to watch for, e.g. "notepad.exe". This is what the agent
|
||||
# matches against the running process list.
|
||||
monitorprocessname = db.Column(db.String(255))
|
||||
image = db.Column(db.String(255))
|
||||
# Digest of the app-level uploaded installer (installpath). Lets the
|
||||
# endpoint update_app push resolve an install source by app alone, with no
|
||||
|
||||
110
tests/test_core/test_application_required_policy.py
Normal file
110
tests/test_core/test_application_required_policy.py
Normal file
@@ -0,0 +1,110 @@
|
||||
"""Tests for the acknowledgement gate on "Required on all PCs".
|
||||
|
||||
Ticking that box is not a reporting flag. It generates a configuration policy
|
||||
that installs the application on every non-server endpoint it matches, so the
|
||||
save is a fleet-wide deployment. The gate exists so nobody triggers one by
|
||||
tabbing through a form.
|
||||
|
||||
The gate lives in the API rather than only in the form on purpose, and that is
|
||||
most of what is pinned here: a confirmation dialog that can be sidestepped by
|
||||
POSTing directly is decoration, not a safeguard. The other half is that it
|
||||
fires only on the transition INTO the required state, since re-saving an app
|
||||
that already has its policy deploys nothing new.
|
||||
"""
|
||||
|
||||
from shopdb.core.models import Application
|
||||
|
||||
|
||||
def _app(db, name='Test App', isrequired=False):
|
||||
app = Application(appname=name, isrequired=isrequired)
|
||||
db.session.add(app)
|
||||
db.session.commit()
|
||||
return app
|
||||
|
||||
|
||||
def test_create_marked_required_is_refused_without_acknowledgement(
|
||||
client, db, auth_headers):
|
||||
response = client.post('/api/applications',
|
||||
json={'appname': 'Fleet App', 'isrequired': True},
|
||||
headers=auth_headers)
|
||||
|
||||
assert response.status_code == 400, response.get_json()
|
||||
message = response.get_json()['data']['error']['message']
|
||||
assert 'install' in message.lower()
|
||||
assert Application.query.filter_by(appname='Fleet App').first() is None
|
||||
|
||||
|
||||
def test_create_marked_required_succeeds_once_acknowledged(
|
||||
client, db, auth_headers):
|
||||
response = client.post('/api/applications',
|
||||
json={'appname': 'Fleet App', 'isrequired': True,
|
||||
'acknowledgepolicy': True},
|
||||
headers=auth_headers)
|
||||
|
||||
assert response.status_code == 201, response.get_json()
|
||||
assert Application.query.filter_by(appname='Fleet App').first().isrequired
|
||||
|
||||
|
||||
def test_update_into_required_is_refused_without_acknowledgement(
|
||||
client, db, auth_headers):
|
||||
app = _app(db)
|
||||
|
||||
response = client.put(f'/api/applications/{app.appid}',
|
||||
json={'isrequired': True}, headers=auth_headers)
|
||||
|
||||
assert response.status_code == 400, response.get_json()
|
||||
# The refusal must leave the flag alone, not half-apply the change.
|
||||
assert db.session.get(Application, app.appid).isrequired is False
|
||||
|
||||
|
||||
def test_update_into_required_succeeds_once_acknowledged(
|
||||
client, db, auth_headers):
|
||||
app = _app(db)
|
||||
|
||||
response = client.put(f'/api/applications/{app.appid}',
|
||||
json={'isrequired': True, 'acknowledgepolicy': True},
|
||||
headers=auth_headers)
|
||||
|
||||
assert response.status_code == 200, response.get_json()
|
||||
assert db.session.get(Application, app.appid).isrequired is True
|
||||
|
||||
|
||||
def test_resaving_an_already_required_app_needs_no_acknowledgement(
|
||||
client, db, auth_headers):
|
||||
# The policy already exists, so this save deploys nothing new. Prompting
|
||||
# again would train people to click through the warning that matters.
|
||||
app = _app(db, isrequired=True)
|
||||
|
||||
response = client.put(f'/api/applications/{app.appid}',
|
||||
json={'isrequired': True,
|
||||
'appdescription': 'Edited'},
|
||||
headers=auth_headers)
|
||||
|
||||
assert response.status_code == 200, response.get_json()
|
||||
|
||||
|
||||
def test_clearing_required_needs_no_acknowledgement(client, db, auth_headers):
|
||||
# Unticking withdraws the policy. It never installs anything, so there is
|
||||
# nothing to warn about.
|
||||
app = _app(db, isrequired=True)
|
||||
|
||||
response = client.put(f'/api/applications/{app.appid}',
|
||||
json={'isrequired': False}, headers=auth_headers)
|
||||
|
||||
assert response.status_code == 200, response.get_json()
|
||||
assert db.session.get(Application, app.appid).isrequired is False
|
||||
|
||||
|
||||
def test_acknowledgement_flag_is_not_persisted_to_the_application(
|
||||
client, db, auth_headers):
|
||||
# It is a control flag for one request, not a column. If it ever reached
|
||||
# the model it would be a stored "yes" that suppresses future warnings.
|
||||
response = client.post('/api/applications',
|
||||
json={'appname': 'Fleet App', 'isrequired': True,
|
||||
'acknowledgepolicy': True},
|
||||
headers=auth_headers)
|
||||
|
||||
assert response.status_code == 201, response.get_json()
|
||||
app = Application.query.filter_by(appname='Fleet App').first()
|
||||
assert not hasattr(app, 'acknowledgepolicy')
|
||||
assert 'acknowledgepolicy' not in response.get_json()['data']
|
||||
Reference in New Issue
Block a user