Skip to content

Commit 5755860

Browse files
Restructured APIUserDelete model to fix bug that allowed system users to be deleted
1 parent bf8e9f7 commit 5755860

2 files changed

Lines changed: 25 additions & 14 deletions

File tree

pfSense-pkg-API/files/etc/inc/api/framework/APIResponse.inc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1598,7 +1598,7 @@ function get($id, $data=[], $all=false) {
15981598
"status" => "bad request",
15991599
"code" => 400,
16001600
"return" => $id,
1601-
"message" => "User privilege must be type array or string"
1601+
"message" => "System users cannot be deleted"
16021602
],
16031603
5006 => [
16041604
"status" => "bad request",

pfSense-pkg-API/files/etc/inc/api/models/APIUserDelete.inc

Lines changed: 24 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -25,26 +25,37 @@ class APIUserDelete extends APIModel {
2525
}
2626

2727
public function action() {
28-
$index_id = index_users()[$this->validated_data["username"]]; // Save our user's index ID number
29-
$del_user = $this->config["system"]["user"][$index_id];
30-
local_user_del($this->config["system"]["user"][$index_id]); // Delete our user on the backend
31-
unset($this->config['system']['user'][$index_id]); // Unset our user from config
32-
$this->config['system']['user'] = array_values($this->config['system']['user']); // Reindex our users
33-
$this->write_config(); // Write our new config
34-
return APIResponse\get(0, $del_user);
28+
# Remove user from backend and remove from config
29+
local_user_del($this->config["system"]["user"][$this->id]);
30+
unset($this->config["system"]["user"][$this->id]);
31+
$this->write_config();
32+
return APIResponse\get(0, $this->validated_data);
3533
}
3634

37-
public function validate_payload() {
38-
if (isset($this->initial_data["username"])) {
39-
if (!array_key_exists($this->initial_data["username"], index_users())) {
35+
private function __validate_username() {
36+
# Check for our required `username` payload value
37+
if (isset($this->initial_data['username'])) {
38+
# Loop through each configured user and check if this user exists
39+
foreach ($this->config["system"]["user"] as $id=>$user) {
40+
if ($this->initial_data["username"] === $user["name"]) {
41+
$this->validated_data = $user;
42+
$this->id = intval($id);
43+
}
44+
}
45+
# Set an error if no user was found
46+
if (!isset($this->validated_data["uid"])) {
4047
$this->errors[] = APIResponse\get(5001);
41-
} else {
42-
$this->validated_data["username"] = $this->initial_data['username'];
43-
$this->validated_data["username"] = trim($this->validated_data["username"]);
48+
}
49+
# Set an error if this is a system user
50+
if ($this->validated_data["scope"] !== "user") {
51+
$this->errors[] = APIResponse\get(5005);
4452
}
4553
} else {
4654
$this->errors[] = APIResponse\get(5000);
4755
}
56+
}
4857

58+
public function validate_payload() {
59+
$this->__validate_username();
4960
}
5061
}

0 commit comments

Comments
 (0)