Add an All PCs policy scope that excludes servers
The policy builder only offered "Matches everything" (the `true` scope node), which also targets servers. Authoring an estate-wide policy almost always means "every PC an operator sits at", and a reboot or script push against a server is a materially different event. Adds an `allpcs` scope node that matches every endpoint except those whose computer type is server-designated. Server designation reads the computer type's ICON slug, which is nominally a UI hint and is now load-bearing; it was chosen over matching the type NAME (a site can rename "Server") and over a new isserver column (needs a migration). It is isolated behind SERVER_ICON so promoting it to a real flag later is a one-place change. The `true` node is retained and still matches servers, so existing policies keep their exact meaning rather than silently re-scoping. It stays visible in the dropdown only when editing a policy that already uses it; new policies default to `allpcs`. Covered by tests/test_plugins/test_computers_policy_allpcs.py, verified against a planted canary so the server-exclusion assertions are known to bite.
This commit is contained in:
@@ -140,7 +140,7 @@ const error = ref('')
|
|||||||
const form = ref(emptyForm())
|
const form = ref(emptyForm())
|
||||||
|
|
||||||
function emptyForm() {
|
function emptyForm() {
|
||||||
return { name: '', description: '', scopejson: { type: 'true' }, actions: [] }
|
return { name: '', description: '', scopejson: { type: 'allpcs' }, actions: [] }
|
||||||
}
|
}
|
||||||
|
|
||||||
onMounted(() => loadData())
|
onMounted(() => loadData())
|
||||||
@@ -221,7 +221,7 @@ function openModal(item = null) {
|
|||||||
form.value = {
|
form.value = {
|
||||||
name: item.name || '',
|
name: item.name || '',
|
||||||
description: item.description || '',
|
description: item.description || '',
|
||||||
scopejson: item.scopejson ? JSON.parse(JSON.stringify(item.scopejson)) : { type: 'true' },
|
scopejson: item.scopejson ? JSON.parse(JSON.stringify(item.scopejson)) : { type: 'allpcs' },
|
||||||
actions: (item.actions || []).map(a => ({
|
actions: (item.actions || []).map(a => ({
|
||||||
action: a.action, mode: a.mode || 'gate',
|
action: a.action, mode: a.mode || 'gate',
|
||||||
desiredstate: a.desiredstate, params: a.params, intervalminutes: a.intervalminutes
|
desiredstate: a.desiredstate, params: a.params, intervalminutes: a.intervalminutes
|
||||||
|
|||||||
@@ -2,7 +2,8 @@
|
|||||||
<div class="scope-node">
|
<div class="scope-node">
|
||||||
<div class="scope-node-row">
|
<div class="scope-node-row">
|
||||||
<select v-model="node.type" class="form-control scope-type" @change="onTypeChange">
|
<select v-model="node.type" class="form-control scope-type" @change="onTypeChange">
|
||||||
<option value="true">Matches everything</option>
|
<option value="allpcs">All PCs (excludes servers)</option>
|
||||||
|
<option v-if="node.type === 'true'" value="true">Matches everything (incl. servers)</option>
|
||||||
<option value="pctype">PC type is</option>
|
<option value="pctype">PC type is</option>
|
||||||
<option value="app">App is installed</option>
|
<option value="app">App is installed</option>
|
||||||
<option value="and">ALL of (AND)</option>
|
<option value="and">ALL of (AND)</option>
|
||||||
@@ -71,20 +72,20 @@ function onTypeChange() {
|
|||||||
delete node.nodes
|
delete node.nodes
|
||||||
delete node.node
|
delete node.node
|
||||||
if (node.type === 'and' || node.type === 'or') {
|
if (node.type === 'and' || node.type === 'or') {
|
||||||
node.nodes = [{ type: 'true' }]
|
node.nodes = [{ type: 'allpcs' }]
|
||||||
} else if (node.type === 'not') {
|
} else if (node.type === 'not') {
|
||||||
node.node = { type: 'true' }
|
node.node = { type: 'allpcs' }
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
function addChild() {
|
function addChild() {
|
||||||
props.node.nodes.push({ type: 'true' })
|
props.node.nodes.push({ type: 'allpcs' })
|
||||||
}
|
}
|
||||||
|
|
||||||
function removeChild(index) {
|
function removeChild(index) {
|
||||||
props.node.nodes.splice(index, 1)
|
props.node.nodes.splice(index, 1)
|
||||||
if (props.node.nodes.length === 0) {
|
if (props.node.nodes.length === 0) {
|
||||||
props.node.nodes.push({ type: 'true' })
|
props.node.nodes.push({ type: 'allpcs' })
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
</script>
|
</script>
|
||||||
|
|||||||
@@ -20,7 +20,7 @@ The result is an EffectivePolicy: a flat, resolved map of action name -> decisio
|
|||||||
and the allowlist need.
|
and the allowlist need.
|
||||||
"""
|
"""
|
||||||
|
|
||||||
from .policyscope import evaluate_scope
|
from .policyscope import SERVER_ICON, evaluate_scope
|
||||||
|
|
||||||
|
|
||||||
class EffectiveAction:
|
class EffectiveAction:
|
||||||
@@ -100,15 +100,21 @@ class EffectivePolicy:
|
|||||||
def build_endpoint_context(computer):
|
def build_endpoint_context(computer):
|
||||||
"""Resolve a Computer row into the context the scope evaluator needs.
|
"""Resolve a Computer row into the context the scope evaluator needs.
|
||||||
|
|
||||||
Returns {'computertypeid': int|None, 'appids': set[int]}.
|
Returns {'computertypeid': int|None, 'appids': set[int], 'isserver': bool}.
|
||||||
"""
|
"""
|
||||||
appids = set()
|
appids = set()
|
||||||
for link in getattr(computer, 'installedapps', None) or ():
|
for link in getattr(computer, 'installedapps', None) or ():
|
||||||
if getattr(link, 'isactive', True) and link.appid is not None:
|
if getattr(link, 'isactive', True) and link.appid is not None:
|
||||||
appids.add(link.appid)
|
appids.add(link.appid)
|
||||||
|
# Server designation rides on the computer type's icon slug; policyscope
|
||||||
|
# owns that rule, we just resolve it for the endpoint. An unset type is not
|
||||||
|
# a server.
|
||||||
|
computertype = getattr(computer, 'computertype', None)
|
||||||
|
isserver = getattr(computertype, 'icon', None) == SERVER_ICON
|
||||||
return {
|
return {
|
||||||
'computertypeid': getattr(computer, 'computertypeid', None),
|
'computertypeid': getattr(computer, 'computertypeid', None),
|
||||||
'appids': appids,
|
'appids': appids,
|
||||||
|
'isserver': isserver,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -11,7 +11,21 @@ Node grammar (all dict nodes carry a "type"):
|
|||||||
{"type": "not", "node": <node>}
|
{"type": "not", "node": <node>}
|
||||||
{"type": "pctype", "id": <computertypeid:int>}
|
{"type": "pctype", "id": <computertypeid:int>}
|
||||||
{"type": "app", "id": <appid:int>}
|
{"type": "app", "id": <appid:int>}
|
||||||
{"type": "true"} # matches everything (an unscoped/global policy)
|
{"type": "allpcs"} # every PC EXCEPT those whose type is designated a server
|
||||||
|
{"type": "true"} # matches everything, servers included
|
||||||
|
|
||||||
|
"allpcs" vs "true": authoring an estate-wide policy almost always means "every
|
||||||
|
PC an operator sits at", not "every computer we have a row for". Those differ by
|
||||||
|
the servers, where a reboot or a script push is a materially different event.
|
||||||
|
"true" is retained because a stored scope of None is equivalent to it and older
|
||||||
|
policies were written with it, but the builder offers "All PCs" instead.
|
||||||
|
|
||||||
|
Server designation is read from the computer type's ICON slug ('server'). That
|
||||||
|
field is nominally a UI hint, which is a real wart - it is load-bearing here. It
|
||||||
|
was chosen over matching the type NAME because a site can rename "Server" but a
|
||||||
|
renamed type keeps its icon, and over a new isserver column because that needs a
|
||||||
|
migration this did not warrant. If server designation ever becomes a first-class
|
||||||
|
flag, this is the single place that reads it.
|
||||||
|
|
||||||
Kept pure and dependency-free (no db, no Flask) so it is trivially unit-tested
|
Kept pure and dependency-free (no db, no Flask) so it is trivially unit-tested
|
||||||
and so a future GE-Enforce assembly engine can reuse the exact same evaluator on
|
and so a future GE-Enforce assembly engine can reuse the exact same evaluator on
|
||||||
@@ -22,8 +36,12 @@ the exact same AST when it composes policies.
|
|||||||
# recursion. Ten levels is far past any policy a human would author.
|
# recursion. Ten levels is far past any policy a human would author.
|
||||||
MAX_SCOPE_DEPTH = 10
|
MAX_SCOPE_DEPTH = 10
|
||||||
|
|
||||||
|
# The computer-type icon slug that designates a type as a server. See the module
|
||||||
|
# docstring for why the icon carries this.
|
||||||
|
SERVER_ICON = 'server'
|
||||||
|
|
||||||
_LOGICAL = {'and', 'or', 'not'}
|
_LOGICAL = {'and', 'or', 'not'}
|
||||||
_LEAF = {'pctype', 'app', 'true'}
|
_LEAF = {'pctype', 'app', 'true', 'allpcs'}
|
||||||
|
|
||||||
|
|
||||||
class ScopeError(ValueError):
|
class ScopeError(ValueError):
|
||||||
@@ -60,7 +78,7 @@ def validate_scope(node, _depth=0):
|
|||||||
ident = node.get('id')
|
ident = node.get('id')
|
||||||
if not isinstance(ident, int) or isinstance(ident, bool):
|
if not isinstance(ident, int) or isinstance(ident, bool):
|
||||||
raise ScopeError(f'{ntype} requires an integer "id"')
|
raise ScopeError(f'{ntype} requires an integer "id"')
|
||||||
# 'true' carries no payload.
|
# 'true' and 'allpcs' carry no payload.
|
||||||
|
|
||||||
|
|
||||||
def evaluate_scope(node, context):
|
def evaluate_scope(node, context):
|
||||||
@@ -69,6 +87,7 @@ def evaluate_scope(node, context):
|
|||||||
context is expected to expose:
|
context is expected to expose:
|
||||||
context['computertypeid'] -> int or None
|
context['computertypeid'] -> int or None
|
||||||
context['appids'] -> set/collection of installed appids
|
context['appids'] -> set/collection of installed appids
|
||||||
|
context['isserver'] -> bool, whether the type is server-designated
|
||||||
|
|
||||||
Returns True when the endpoint falls in scope. An empty/None scope means the
|
Returns True when the endpoint falls in scope. An empty/None scope means the
|
||||||
policy is unscoped and matches everything (same as {"type": "true"}), which
|
policy is unscoped and matches everything (same as {"type": "true"}), which
|
||||||
@@ -80,6 +99,10 @@ def evaluate_scope(node, context):
|
|||||||
ntype = node['type']
|
ntype = node['type']
|
||||||
if ntype == 'true':
|
if ntype == 'true':
|
||||||
return True
|
return True
|
||||||
|
if ntype == 'allpcs':
|
||||||
|
# Absent designation means "not a server": an endpoint with no type set
|
||||||
|
# is an ordinary PC until something says otherwise.
|
||||||
|
return not context.get('isserver', False)
|
||||||
if ntype == 'and':
|
if ntype == 'and':
|
||||||
return all(evaluate_scope(child, context) for child in node['nodes'])
|
return all(evaluate_scope(child, context) for child in node['nodes'])
|
||||||
if ntype == 'or':
|
if ntype == 'or':
|
||||||
|
|||||||
87
tests/test_plugins/test_computers_policy_allpcs.py
Normal file
87
tests/test_plugins/test_computers_policy_allpcs.py
Normal file
@@ -0,0 +1,87 @@
|
|||||||
|
"""Scope evaluation for the All PCs policy target.
|
||||||
|
|
||||||
|
These are pure unit tests: policyscope has no db or Flask dependency, so the
|
||||||
|
whole grammar can be exercised with plain dicts. build_endpoint_context needs
|
||||||
|
only duck-typed stand-ins for a Computer row.
|
||||||
|
"""
|
||||||
|
import pytest
|
||||||
|
|
||||||
|
from plugins.computers.services.policyresolver import build_endpoint_context
|
||||||
|
from plugins.computers.services.policyscope import (
|
||||||
|
ScopeError,
|
||||||
|
evaluate_scope,
|
||||||
|
validate_scope,
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
class FakeComputerType:
|
||||||
|
def __init__(self, icon):
|
||||||
|
self.icon = icon
|
||||||
|
|
||||||
|
|
||||||
|
class FakeComputer:
|
||||||
|
"""Minimal stand-in for a Computer row, no db required."""
|
||||||
|
|
||||||
|
def __init__(self, computertypeid=None, icon=None, installedapps=()):
|
||||||
|
self.computertypeid = computertypeid
|
||||||
|
self.computertype = FakeComputerType(icon) if icon is not None else None
|
||||||
|
self.installedapps = installedapps
|
||||||
|
|
||||||
|
|
||||||
|
def test_allpcs_is_a_valid_scope_node():
|
||||||
|
validate_scope({'type': 'allpcs'})
|
||||||
|
|
||||||
|
|
||||||
|
def test_allpcs_matches_an_ordinary_pc():
|
||||||
|
context = {'computertypeid': 1, 'appids': set(), 'isserver': False}
|
||||||
|
assert evaluate_scope({'type': 'allpcs'}, context) is True
|
||||||
|
|
||||||
|
|
||||||
|
def test_allpcs_does_not_match_a_server():
|
||||||
|
"""The whole point of the option: servers are excluded."""
|
||||||
|
context = {'computertypeid': 4, 'appids': set(), 'isserver': True}
|
||||||
|
assert evaluate_scope({'type': 'allpcs'}, context) is False
|
||||||
|
|
||||||
|
|
||||||
|
def test_allpcs_matches_an_endpoint_with_no_type_designation():
|
||||||
|
"""An untyped endpoint is an ordinary PC until something says otherwise."""
|
||||||
|
context = {'computertypeid': None, 'appids': set()}
|
||||||
|
assert evaluate_scope({'type': 'allpcs'}, context) is True
|
||||||
|
|
||||||
|
|
||||||
|
def test_true_still_matches_a_server():
|
||||||
|
"""'true' keeps its old meaning so existing policies do not silently shift."""
|
||||||
|
context = {'computertypeid': 4, 'appids': set(), 'isserver': True}
|
||||||
|
assert evaluate_scope({'type': 'true'}, context) is True
|
||||||
|
|
||||||
|
|
||||||
|
def test_allpcs_composes_with_other_nodes():
|
||||||
|
"""AND of allpcs and an app still excludes a server running that app."""
|
||||||
|
scope = {
|
||||||
|
'type': 'and',
|
||||||
|
'nodes': [{'type': 'allpcs'}, {'type': 'app', 'id': 7}],
|
||||||
|
}
|
||||||
|
pc = {'computertypeid': 1, 'appids': {7}, 'isserver': False}
|
||||||
|
server = {'computertypeid': 4, 'appids': {7}, 'isserver': True}
|
||||||
|
assert evaluate_scope(scope, pc) is True
|
||||||
|
assert evaluate_scope(scope, server) is False
|
||||||
|
|
||||||
|
|
||||||
|
def test_context_marks_a_server_icon_type_as_a_server():
|
||||||
|
computer = FakeComputer(computertypeid=4, icon='server')
|
||||||
|
assert build_endpoint_context(computer)['isserver'] is True
|
||||||
|
|
||||||
|
|
||||||
|
def test_context_does_not_mark_a_desktop_type_as_a_server():
|
||||||
|
computer = FakeComputer(computertypeid=1, icon='desktop')
|
||||||
|
assert build_endpoint_context(computer)['isserver'] is False
|
||||||
|
|
||||||
|
|
||||||
|
def test_context_handles_a_computer_with_no_type():
|
||||||
|
computer = FakeComputer()
|
||||||
|
assert build_endpoint_context(computer)['isserver'] is False
|
||||||
|
|
||||||
|
|
||||||
|
def test_an_unknown_node_type_is_still_rejected():
|
||||||
|
with pytest.raises(ScopeError):
|
||||||
|
validate_scope({'type': 'allpc'})
|
||||||
Reference in New Issue
Block a user