Skip to content

Commit d69e841

Browse files
LukeTowersLukeTowers
committed
Simplify FormController context identification logic
1 parent e539b71 commit d69e841

4 files changed

Lines changed: 262 additions & 7 deletions

File tree

‎modules/backend/behaviors/FormController.php‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -122,11 +122,7 @@ public function __construct($controller)
122122
*/
123123
public function initForm($model, $context = null)
124124
{
125-
if ($context !== null) {
126-
$this->context = $context;
127-
}
128-
129-
$context = $this->formGetContext();
125+
$context = $this->context = $context ?? $this->formGetContext();
130126

131127
/*
132128
* Each page can supply a unique form definition, if desired
@@ -442,7 +438,7 @@ public function formGetModel()
442438
*/
443439
public function formGetContext()
444440
{
445-
return post('form_context', $this->context);
441+
return $this->context;
446442
}
447443

448444
/**

‎modules/backend/lang/en/lang.php‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,9 @@
167167
'updated_at' => 'Updated at',
168168
'deleted_at' => 'Deleted at',
169169
'show_deleted' => 'Show deleted',
170+
'self_escalation_denied' => 'You cannot modify your own role, permissions, or superuser status.',
171+
'superuser_grant_denied' => 'Only superusers can grant superuser status or modify other superuser accounts.',
172+
'manage_users_denied' => 'You do not have permission to manage other administrators.',
170173
'throttle_tab' => 'Failed Logins',
171174
'throttle_tab_label' => 'Failed Login Records',
172175
'throttle_comment' => 'View failed login attempts for this user. These records are automatically generated when login attempts fail. Users are suspended after exceeding the attempt limit.',

‎modules/backend/models/User.php‎

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
use Backend\Facades\Backend;
44
use Backend\Facades\BackendAuth;
55
use Illuminate\Support\Facades\Event;
6+
use Illuminate\Support\Facades\Lang;
7+
use Winter\Storm\Auth\AuthorizationException;
68
use Winter\Storm\Auth\Models\User as UserBase;
79
use Winter\Storm\Support\Facades\Mail;
810

@@ -126,6 +128,36 @@ public function getAvatarThumb($size = 25, $options = null)
126128
'&d='. urlencode($default);
127129
}
128130

131+
/**
132+
* Before save event — enforce authorization rules to prevent privilege escalation.
133+
* @return void
134+
*/
135+
public function beforeSave()
136+
{
137+
$actor = BackendAuth::getUser();
138+
$isCurrentUser = $this->exists && $actor && $actor->getKey() === $this->getKey();
139+
140+
// No authenticated user (CLI, artisan, queue, seeders) — allow everything
141+
if (!$actor) {
142+
return;
143+
}
144+
145+
// Rule 1: Self-escalation — users cannot modify their own role, superuser status, or permissions
146+
if ($isCurrentUser && $this->isDirty(['role_id', 'is_superuser', 'permissions'])) {
147+
throw new AuthorizationException(Lang::get('backend::lang.user.self_escalation_denied'));
148+
}
149+
150+
// Rule 2: Must have backend.manage_users to manage other users
151+
if (!$isCurrentUser && !$actor->hasAccess('backend.manage_users')) {
152+
throw new AuthorizationException(Lang::get('backend::lang.user.manage_users_denied'));
153+
}
154+
155+
// Rule 3: Only superusers can grant superuser status or edit existing superusers
156+
if (!$actor->isSuperUser() && ($this->is_superuser || $this->getOriginal('is_superuser'))) {
157+
throw new AuthorizationException(Lang::get('backend::lang.user.superuser_grant_denied'));
158+
}
159+
}
160+
129161
/**
130162
* After create event
131163
* @return void
@@ -225,7 +257,7 @@ public function unsuspend()
225257

226258
/**
227259
* Returns an array of merged permissions based on the user's individual permissions
228-
* and their group permissions filtering out any permissions the impersonator doesn't
260+
* and their role permissions filtering out any permissions the impersonator doesn't
229261
* have access to (if the current user is being impersonated)
230262
*
231263
* @return array

‎modules/backend/tests/models/UserTest.php‎

Lines changed: 224 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,224 @@
1+
<?php
2+
3+
namespace Backend\Tests\Models;
4+
5+
use Backend\Models\User;
6+
use Backend\Models\UserRole;
7+
use System\Tests\Bootstrap\PluginTestCase;
8+
use Winter\Storm\Auth\AuthorizationException;
9+
use Winter\Storm\Database\Model as Eloquent;
10+
11+
class UserTest extends PluginTestCase
12+
{
13+
protected User $superuser;
14+
protected User $admin;
15+
protected User $lowPriv;
16+
17+
public function setUp(): void
18+
{
19+
parent::setUp();
20+
21+
Eloquent::unguarded(function () {
22+
$developerRole = UserRole::where('code', UserRole::CODE_DEVELOPER)->first();
23+
$publisherRole = UserRole::where('code', UserRole::CODE_PUBLISHER)->first();
24+
25+
$this->superuser = User::create([
26+
'email' => 'superuser@test.com',
27+
'login' => 'superuser',
28+
'password' => 'Testing123!',
29+
'password_confirmation' => 'Testing123!',
30+
'first_name' => 'Super',
31+
'last_name' => 'User',
32+
'is_superuser' => true,
33+
'is_activated' => true,
34+
'role_id' => $developerRole->id,
35+
'permissions' => [],
36+
]);
37+
38+
$this->admin = User::create([
39+
'email' => 'admin@test.com',
40+
'login' => 'admin_user',
41+
'password' => 'Testing123!',
42+
'password_confirmation' => 'Testing123!',
43+
'first_name' => 'Admin',
44+
'last_name' => 'User',
45+
'is_superuser' => false,
46+
'is_activated' => true,
47+
'role_id' => $publisherRole->id,
48+
'permissions' => ['backend.manage_users' => 1],
49+
]);
50+
51+
$this->lowPriv = User::create([
52+
'email' => 'lowpriv@test.com',
53+
'login' => 'lowpriv',
54+
'password' => 'Testing123!',
55+
'password_confirmation' => 'Testing123!',
56+
'first_name' => 'Low',
57+
'last_name' => 'Priv',
58+
'is_superuser' => false,
59+
'is_activated' => true,
60+
'role_id' => null,
61+
'permissions' => [],
62+
]);
63+
});
64+
}
65+
66+
// ---- Denied operations ----
67+
68+
public function testSelfEditRoleThrows()
69+
{
70+
$this->actingAs($this->admin);
71+
72+
$this->admin->role_id = $this->admin->role_id + 1;
73+
74+
$this->expectException(AuthorizationException::class);
75+
$this->admin->save();
76+
}
77+
78+
public function testSelfEditSuperuserThrows()
79+
{
80+
$this->actingAs($this->admin);
81+
82+
$this->admin->is_superuser = true;
83+
84+
$this->expectException(AuthorizationException::class);
85+
$this->admin->save();
86+
}
87+
88+
public function testSelfEditPermissionsThrows()
89+
{
90+
$this->actingAs($this->admin);
91+
92+
$this->admin->permissions = ['backend.manage_users' => 1, 'backend.access_dashboard' => 1];
93+
94+
$this->expectException(AuthorizationException::class);
95+
$this->admin->save();
96+
}
97+
98+
public function testNonSuperuserGrantSuperuserThrows()
99+
{
100+
$this->actingAs($this->admin);
101+
102+
$this->lowPriv->is_superuser = true;
103+
104+
$this->expectException(AuthorizationException::class);
105+
$this->lowPriv->save();
106+
}
107+
108+
public function testNonSuperuserRevokeSuperuserThrows()
109+
{
110+
$this->actingAs($this->admin);
111+
112+
$this->superuser->is_superuser = false;
113+
114+
$this->expectException(AuthorizationException::class);
115+
$this->superuser->save();
116+
}
117+
118+
public function testNonSuperuserEditingSuperuserThrows()
119+
{
120+
$this->actingAs($this->admin);
121+
122+
$this->superuser->first_name = 'Changed';
123+
124+
$this->expectException(AuthorizationException::class);
125+
$this->superuser->save();
126+
}
127+
128+
public function testNoManageUsersEditingOtherThrows()
129+
{
130+
$this->actingAs($this->lowPriv);
131+
132+
$this->admin->first_name = 'Changed';
133+
134+
$this->expectException(AuthorizationException::class);
135+
$this->admin->save();
136+
}
137+
138+
public function testNoManageUsersCreatingUserThrows()
139+
{
140+
$this->actingAs($this->lowPriv);
141+
142+
$this->expectException(AuthorizationException::class);
143+
Eloquent::unguarded(function () {
144+
User::create([
145+
'email' => 'newuser@test.com',
146+
'login' => 'newuser',
147+
'password' => 'Testing123!',
148+
'password_confirmation' => 'Testing123!',
149+
'first_name' => 'New',
150+
'last_name' => 'User',
151+
'is_superuser' => false,
152+
'is_activated' => true,
153+
'permissions' => [],
154+
]);
155+
});
156+
}
157+
158+
// ---- Allowed operations ----
159+
160+
public function testSelfEditNonProtectedFieldsAllowed()
161+
{
162+
$this->actingAs($this->admin);
163+
164+
$this->admin->first_name = 'NewFirst';
165+
$this->admin->last_name = 'NewLast';
166+
$this->admin->email = 'newemail@test.com';
167+
$this->admin->save();
168+
169+
$this->admin->refresh();
170+
$this->assertEquals('NewFirst', $this->admin->first_name);
171+
$this->assertEquals('NewLast', $this->admin->last_name);
172+
$this->assertEquals('newemail@test.com', $this->admin->email);
173+
}
174+
175+
public function testAdminCanChangeOtherUserRole()
176+
{
177+
$this->actingAs($this->admin);
178+
179+
$publisherRole = UserRole::where('code', UserRole::CODE_PUBLISHER)->first();
180+
$this->lowPriv->role_id = $publisherRole->id;
181+
$this->lowPriv->save();
182+
183+
$this->lowPriv->refresh();
184+
$this->assertEquals($publisherRole->id, $this->lowPriv->role_id);
185+
}
186+
187+
public function testSuperuserCanGrantSuperuser()
188+
{
189+
$this->actingAs($this->superuser);
190+
191+
$this->lowPriv->is_superuser = true;
192+
$this->lowPriv->save();
193+
194+
$this->lowPriv->refresh();
195+
$this->assertTrue((bool) $this->lowPriv->is_superuser);
196+
}
197+
198+
public function testAdminCanChangeOtherUserPermissions()
199+
{
200+
$this->actingAs($this->admin);
201+
202+
$this->lowPriv->permissions = ['backend.access_dashboard' => 1];
203+
$this->lowPriv->save();
204+
205+
$this->lowPriv->refresh();
206+
$this->assertEquals(['backend.access_dashboard' => 1], $this->lowPriv->permissions);
207+
}
208+
209+
public function testNoAuthUserCanModifyAnyField()
210+
{
211+
// No actingAs — simulates CLI/artisan/queue context
212+
$developerRole = UserRole::where('code', UserRole::CODE_DEVELOPER)->first();
213+
214+
$this->lowPriv->role_id = $developerRole->id;
215+
$this->lowPriv->is_superuser = true;
216+
$this->lowPriv->permissions = ['backend.manage_users' => 1];
217+
$this->lowPriv->save();
218+
219+
$this->lowPriv->refresh();
220+
$this->assertEquals($developerRole->id, $this->lowPriv->role_id);
221+
$this->assertTrue((bool) $this->lowPriv->is_superuser);
222+
$this->assertEquals(['backend.manage_users' => 1], $this->lowPriv->permissions);
223+
}
224+
}

0 commit comments

Comments
 (0)