Skip to content

Commit 7f08193

Browse files
jaredhendrickson13jaredhendrickson13
committed
Revised firewall rule unit test to be more specific, fixed incorrect error codes returned in APIFirewallRuleCreate and APIFirewallRuleUpdate
1 parent 4579c5c commit 7f08193

5 files changed

Lines changed: 507 additions & 24 deletions

File tree

‎pfSense-pkg-API/files/etc/inc/api/models/APIFirewallRuleCreate.inc‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -249,9 +249,11 @@ class APIFirewallRuleCreate extends APIModel {
249249
$this->__validate_log();
250250
$this->__validate_top();
251251

252-
# Delay generating the tracker. Reduces the likelihood of two rules getting the same tracker in looped calls
252+
# Delay generating the tracker. Reduces the likelihood of two rules getting the same tracker in looped calls.
253253
# todo: this is a quick fix and still does not guarantee uniqueness, a better solution is needed
254-
sleep(1);
254+
if (!$this->errors) {
255+
sleep(1);
256+
}
255257

256258
# Add our static 'tracker', 'created' and 'updated' values
257259
$this->validated_data["tracker"] = (int)microtime(true);

‎pfSense-pkg-API/files/etc/inc/api/models/APIFirewallRuleUpdate.inc‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -161,14 +161,14 @@ class APIFirewallRuleUpdate extends APIModel {
161161
}
162162

163163
private function __validate_dst() {
164-
# Check for our optional 'src' payload value
164+
# Check for our optional 'dst' payload value
165165
if (isset($this->initial_data['dst'])) {
166166
# Check if our source and destination values are valid
167167
$dir_check = APITools\is_valid_rule_addr($this->initial_data['dst'], "destination");
168168
if ($dir_check["valid"] === true) {
169169
$this->validated_data = array_merge($this->validated_data, $dir_check["data"]);
170170
} else {
171-
$this->errors[] = APIResponse\get(4049);
171+
$this->errors[] = APIResponse\get(4045);
172172
}
173173
}
174174
}
@@ -195,7 +195,7 @@ class APIFirewallRuleUpdate extends APIModel {
195195
if (isset($this->initial_data['dstport'])) {
196196
$val = str_replace("-", ":", $this->initial_data['dstport']);
197197
if (!is_port_or_range_or_alias($val) and $val !== "any") {
198-
$this->errors[] = APIResponse\get(4048);
198+
$this->errors[] = APIResponse\get(4049);
199199
} elseif ($val !== "any") {
200200
$this->validated_data["destination"]["port"] = str_replace(":", "-", $val);;
201201
}

‎tests/test_api_v1_firewall_alias.py‎

Lines changed: 139 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ class APIUnitTestFirewallAlias(unit_test_framework.APIUnitTest):
3737
"name": "HTTP_PORTS",
3838
"type": "port",
3939
"descr": "Unit Test",
40-
"address": [80, 443, 8443],
40+
"address": [80, 443, 8443, "8080:8081"],
4141
"detail": ["HTTP", "HTTPS", "HTTPS-ALT"]
4242
},
4343
"resp_time": 3 # Allow a few seconds for the firewall filter to reload
@@ -52,6 +52,74 @@ class APIUnitTestFirewallAlias(unit_test_framework.APIUnitTest):
5252
"detail": ["Cloudflare DNS", "Google DNS", "Secondary Google DNS"]
5353
},
5454
"resp_time": 3 # Allow a few seconds for the firewall filter to reload
55+
},
56+
{
57+
"name": "Test name requirement",
58+
"status": 400,
59+
"return": 4050
60+
},
61+
{
62+
"name": "Test name unique constraint",
63+
"status": 400,
64+
"return": 4056,
65+
"payload": {
66+
"name": "DNS_SERVERS"
67+
}
68+
},
69+
{
70+
"name": "Test name validation",
71+
"status": 400,
72+
"return": 4053,
73+
"payload": {
74+
"name": "!@#"
75+
}
76+
},
77+
{
78+
"name": "Test type requirement",
79+
"status": 400,
80+
"return": 4061,
81+
"payload": {
82+
"name": "TEST"
83+
}
84+
},
85+
{
86+
"name": "Test type validation",
87+
"status": 400,
88+
"return": 4057,
89+
"payload": {
90+
"name": "TEST",
91+
"type": "INVALID"
92+
}
93+
},
94+
{
95+
"name": "Test network alias address validation",
96+
"status": 400,
97+
"return": 4059,
98+
"payload": {
99+
"name": "TEST",
100+
"type": "network",
101+
"address": ["!@#!@#!@#"]
102+
}
103+
},
104+
{
105+
"name": "Test host alias address validation",
106+
"status": 400,
107+
"return": 4058,
108+
"payload": {
109+
"name": "TEST",
110+
"type": "host",
111+
"address": ["!@#!@#!@#"]
112+
}
113+
},
114+
{
115+
"name": "Test port alias address validation",
116+
"status": 400,
117+
"return": 4060,
118+
"payload": {
119+
"name": "TEST",
120+
"type": "port",
121+
"address": ["!@#!@#!@#"]
122+
}
55123
}
56124
]
57125
put_tests = [
@@ -90,7 +158,66 @@ class APIUnitTestFirewallAlias(unit_test_framework.APIUnitTest):
90158
"detail": ["Google DNS"]
91159
},
92160
"resp_time": 3 # Allow a few seconds for the firewall filter to reload
93-
}
161+
},
162+
{
163+
"name": "Test ID requirement",
164+
"status": 400,
165+
"return": 4050
166+
},
167+
{
168+
"name": "Test name unique constraint",
169+
"status": 400,
170+
"return": 4056,
171+
"payload": {
172+
"id": "UPDATED_HTTP_PORTS",
173+
"name": "UPDATED_DNS_SERVERS"
174+
}
175+
},
176+
{
177+
"name": "Test name validation",
178+
"status": 400,
179+
"return": 4053,
180+
"payload": {
181+
"id": "UPDATED_HTTP_PORTS",
182+
"name": "!@#"
183+
}
184+
},
185+
{
186+
"name": "Test type validation",
187+
"status": 400,
188+
"return": 4057,
189+
"payload": {
190+
"id": "UPDATED_HTTP_PORTS",
191+
"type": "INVALID"
192+
}
193+
},
194+
{
195+
"name": "Test update host to network alias address validation",
196+
"status": 400,
197+
"return": 4059,
198+
"payload": {
199+
"id": "UPDATED_DNS_SERVERS",
200+
"type": "network"
201+
}
202+
},
203+
{
204+
"name": "Test update network to port alias address validation",
205+
"status": 400,
206+
"return": 4060,
207+
"payload": {
208+
"id": "UPDATED_RFC1918",
209+
"type": "port"
210+
}
211+
},
212+
{
213+
"name": "Test update port to host alias address validation",
214+
"status": 400,
215+
"return": 4058,
216+
"payload": {
217+
"id": "UPDATED_HTTP_PORTS",
218+
"type": "host"
219+
}
220+
},
94221
]
95222
delete_tests = [
96223
{
@@ -113,7 +240,16 @@ class APIUnitTestFirewallAlias(unit_test_framework.APIUnitTest):
113240
"id": "UPDATED_DNS_SERVERS"
114241
},
115242
"resp_time": 3 # Allow a few seconds for the firewall filter to reload
116-
}
243+
},
244+
{
245+
"name": "Test delete non-existing alias",
246+
"status": 400,
247+
"return": 4055,
248+
"payload": {
249+
"id": "INVALID"
250+
}
251+
},
252+
117253
]
118254

119255
APIUnitTestFirewallAlias()

0 commit comments

Comments
 (0)