Skip to content

Commit 46bf794

Browse files
committed
ui: refresh rules views when rules change in the db
- Notify the views whenever rules are added, deleted or disabled in the db. Rules created from pop-ups, temporary rules expiration and rules received when a node connects were not displayed until interacting with the views, because the periodic refresh is skipped while there're rows selected. - Emit the notification once per batch when adding several rules, to avoid a refresh storm when a node connects. - Misc: moved the views update of the rules editor to the central rules class, and added tests for these notifications.
1 parent ce20316 commit 46bf794

4 files changed

Lines changed: 107 additions & 5 deletions

File tree

ui/opensnitch/dialogs/ruleseditor/dialog.py

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -278,8 +278,6 @@ def cb_save_clicked(self):
278278
if constants.WORK_MODE == constants.ADD_RULE:
279279
constants.WORK_MODE = constants.EDIT_RULE
280280

281-
self._rules.updated.emit(0)
282-
283281
@QtCore.pyqtSlot(str, ui_pb2.NotificationReply)
284282
def cb_notification_callback(self, addr, reply):
285283
#print(self.LOG_TAG, "Rule notification received: ", reply.id, reply.code)

ui/opensnitch/rules.py

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ def __init__(self):
9494
QObject.__init__(self)
9595
self._db = Database.instance()
9696

97-
def add(self, time, node, name, description, enabled, precedence, nolog, action, duration, op_type, op_sensitive, op_operand, op_data, created):
97+
def add(self, time, node, name, description, enabled, precedence, nolog, action, duration, op_type, op_sensitive, op_operand, op_data, created, notify=True):
9898
# don't add rule if the user has selected to exclude temporary
9999
# rules
100100
if duration in Config.RULES_DURATION_FILTER:
@@ -104,6 +104,8 @@ def add(self, time, node, name, description, enabled, precedence, nolog, action,
104104
"(time, node, name, description, enabled, precedence, nolog, action, duration, operator_type, operator_sensitive, operator_operand, operator_data, created)",
105105
(time, node, name, description, enabled, precedence, nolog, action, duration, op_type, op_sensitive, op_operand, op_data, created),
106106
action_on_conflict="REPLACE")
107+
if notify:
108+
self.updated.emit(0)
107109

108110
def add_rules(self, addr, rules):
109111
try:
@@ -123,8 +125,12 @@ def add_rules(self, addr, rules):
123125
r.operator.type,
124126
str(r.operator.sensitive),
125127
r.operator.operand, r.operator.data,
126-
str(datetime.fromtimestamp(r.created).strftime(DBDateFieldFormat)))
128+
str(datetime.fromtimestamp(r.created).strftime(DBDateFieldFormat)),
129+
notify=False)
127130

131+
# notify once per batch, to avoid a refresh storm when a node
132+
# connects and sends all its rules.
133+
self.updated.emit(0)
128134
return True
129135
except Exception as e:
130136
log.warning("exception adding node rules to db: %s", repr(e))
@@ -142,6 +148,7 @@ def delete(self, name, addr, callback):
142148
if not self._db.delete_rule(rule.name, addr):
143149
return None
144150

151+
self.updated.emit(0)
145152
return rule
146153

147154
def delete_by_field(self, field, values):
@@ -184,6 +191,7 @@ def disable(self, addr, name):
184191
"name=? AND node=?",
185192
action_on_conflict="OR REPLACE"
186193
)
194+
self.updated.emit(0)
187195

188196
def update_time(self, time, name, addr):
189197
"""Updates the time of a rule, whenever a new connection matched a

ui/opensnitch/service.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
from opensnitch.notifications import DesktopNotifications
2424
from opensnitch.firewall import Rules as FwRules
2525
from opensnitch.nodes import Nodes
26+
from opensnitch.rules import Rules
2627
from opensnitch.config import Config
2728
from opensnitch.version import version
2829
from opensnitch.database import Database
@@ -129,6 +130,7 @@ def __init__(self, app, on_exit, start_in_bg=False):
129130

130131
self._nodes = Nodes.instance()
131132
self._nodes.reset_status()
133+
self._rules = Rules.instance()
132134

133135
self._last_stats = {}
134136
self._last_items = {
@@ -898,7 +900,9 @@ def _disable_temp_rule(args):
898900
ost.start()
899901

900902
elif kwargs['action'] == self.DELETE_RULE:
901-
self._db.delete_rule(kwargs['name'], kwargs['addr'])
903+
# route it through Rules, so the views are notified of the
904+
# change.
905+
self._rules.delete(kwargs['name'], kwargs['addr'], None)
902906

903907
elif kwargs['action'] == self.NODE_DELETE:
904908
self._delete_node(kwargs['peer'])

ui/tests/test_rules_signals.py

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
#
2+
# pytest -v tests/test_rules_signals.py
3+
#
4+
# Regression tests for the Rules.updated signal: rule changes written to
5+
# the db from any source (pop-up answers, temporary rules expiration,
6+
# rules received on node connection) must notify the views, so they
7+
# refresh what they display.
8+
#
9+
10+
from datetime import datetime
11+
12+
import opensnitch.proto as proto
13+
ui_pb2, ui_pb2_grpc = proto.import_()
14+
15+
# opensnitch.utils must be imported before opensnitch.database to resolve
16+
# their circular import
17+
import opensnitch.utils # noqa: F401
18+
from opensnitch.rules import Rules
19+
20+
TEST_NODE = "unix:/tmp/osui.sock"
21+
RULE_TIME = "2026-06-10 10:00:00"
22+
23+
24+
def new_proto_rule(name):
25+
rule = ui_pb2.Rule(name=name)
26+
rule.enabled = True
27+
rule.action = "allow"
28+
rule.duration = "always"
29+
rule.created = int(datetime.now().timestamp())
30+
rule.operator.type = "simple"
31+
rule.operator.operand = "process.path"
32+
rule.operator.data = "/bin/test-app"
33+
return rule
34+
35+
36+
def add_test_rule(rules, name):
37+
rules.add(
38+
RULE_TIME, TEST_NODE, name, "", "True", "False", "False",
39+
"allow", "always", "simple", "False", "process.path",
40+
"/bin/test-app", RULE_TIME
41+
)
42+
43+
44+
def count_emits(rules, operation):
45+
emitted = []
46+
47+
def _on_updated(what):
48+
emitted.append(what)
49+
50+
rules.updated.connect(_on_updated)
51+
try:
52+
operation()
53+
finally:
54+
rules.updated.disconnect(_on_updated)
55+
return len(emitted)
56+
57+
58+
def test_add_emits_updated(qtbot):
59+
rules = Rules.instance()
60+
assert count_emits(rules, lambda: add_test_rule(rules, "sig-add")) == 1
61+
62+
63+
def test_add_rules_emits_once_per_batch(qtbot):
64+
"""A node sends all its rules on connection: one refresh per batch,
65+
not one per rule."""
66+
rules = Rules.instance()
67+
batch = [new_proto_rule("sig-batch-0"), new_proto_rule("sig-batch-1")]
68+
results = []
69+
emits = count_emits(rules, lambda: results.append(rules.add_rules(TEST_NODE, batch)))
70+
assert results == [True]
71+
assert emits == 1
72+
73+
74+
def test_delete_emits_updated(qtbot):
75+
rules = Rules.instance()
76+
add_test_rule(rules, "sig-delete")
77+
assert count_emits(rules, lambda: rules.delete("sig-delete", TEST_NODE, None)) == 1
78+
79+
80+
def test_disable_emits_updated(qtbot):
81+
"""Temporary rules are marked as disabled in the db when they expire."""
82+
rules = Rules.instance()
83+
add_test_rule(rules, "sig-disable")
84+
assert count_emits(rules, lambda: rules.disable(TEST_NODE, "sig-disable")) == 1
85+
86+
87+
def test_update_time_does_not_emit(qtbot):
88+
"""update_time() runs for every connection matching a rule; emitting
89+
here would refresh the views non-stop."""
90+
rules = Rules.instance()
91+
add_test_rule(rules, "sig-time")
92+
assert count_emits(rules, lambda: rules.update_time(RULE_TIME, "sig-time", TEST_NODE)) == 0

0 commit comments

Comments
 (0)