Skip to content

Commit dba1aca

Browse files
Redesign site-wide user management page (#2830)
This PR builds on top of the work started in #2778, completely rethinking the design of the previous `manageUsers.php` page. Since the global invitation workflow is substantially different from the project invitation workflow, I created completely separate tables and models. Accordingly, I made the project invitations names more specific to better differentiate between the two invitation types. A few key changes: - Instead of giving administrators the ability to add users directly, users must now be invited via email. - User invitation is only allowed if username+password is allowed (according to the `USERNAME_PASSWORD_AUTHENTICATION_ENABLED` configuration option). - The list of users is now available to users of all access levels, with email addresses limited to administrators thanks to #2818. - Administrators can no longer change their own role. - Everything is now controlled via the GraphQL API, meaning that users can now execute these actions programmatically if desired. This may be useful for CDash administrators who wish to use a custom user management workflow not otherwise support by our LDAP, OAuth, or SAML integrations. The new look: ![image](https://github.com/user-attachments/assets/6043da57-1d9c-4e95-ae6a-a415f3ed395d)
1 parent 708a21b commit dba1aca

54 files changed

Lines changed: 2826 additions & 570 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.eslintignore

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,6 @@ resources/js/angular/cdashUploadFilesSorter.js
4747
resources/js/angular/cdashSortable.js
4848
resources/js/angular/cdashSiteSorter.js
4949
resources/js/angular/cdashProjectRole.js
50-
resources/js/angular/cdashManageUsers.js
5150
resources/js/angular/cdashFilters.js
5251
resources/js/angular/cdashCoverageGraph.js
5352
resources/js/angular/bulletchart.js

app/Enums/GlobalRole.php

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace App\Enums;
6+
7+
enum GlobalRole: string
8+
{
9+
case USER = 'USER';
10+
case ADMINISTRATOR = 'ADMINISTRATOR';
11+
}
Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace App\GraphQL\Mutations;
6+
7+
use App\Enums\GlobalRole;
8+
use App\Models\User;
9+
use Exception;
10+
use Illuminate\Support\Facades\Validator;
11+
use Illuminate\Validation\Rule;
12+
use Illuminate\Validation\ValidationException;
13+
14+
final class ChangeGlobalRole extends AbstractMutation
15+
{
16+
public ?User $user = null;
17+
18+
/**
19+
* @param array{
20+
* userId: int,
21+
* role: GlobalRole,
22+
* } $args
23+
*
24+
* @throws ValidationException
25+
*/
26+
protected function mutate(array $args): void
27+
{
28+
Validator::make($args, [
29+
'userId' => [
30+
'required',
31+
],
32+
'role' => [
33+
'required',
34+
Rule::enum(GlobalRole::class),
35+
],
36+
])->validate();
37+
38+
/** @var ?User $user */
39+
$user = auth()->user();
40+
if ($user === null) {
41+
// This should never happen, but we handle the case anyway to make PHPStan happy.
42+
throw new Exception('Attempt to invite user when not signed in.');
43+
}
44+
45+
$userToChange = User::find((int) $args['userId']);
46+
if ($userToChange === null) {
47+
$this->message = 'Cannot change role for user which does not exist.';
48+
return;
49+
}
50+
51+
if ($user->cannot('changeRole', $userToChange)) {
52+
$this->message = 'Insufficient permissions.';
53+
return;
54+
}
55+
56+
$userToChange->admin = $args['role'] === GlobalRole::ADMINISTRATOR;
57+
$userToChange->save();
58+
59+
$this->user = $userToChange;
60+
}
61+
}
Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace App\GraphQL\Mutations;
6+
7+
use App\Enums\GlobalRole;
8+
use App\Mail\InvitedToCdash;
9+
use App\Models\GlobalInvitation;
10+
use App\Models\User;
11+
use Exception;
12+
use Illuminate\Support\Carbon;
13+
use Illuminate\Support\Facades\Hash;
14+
use Illuminate\Support\Facades\Mail;
15+
use Illuminate\Support\Facades\Validator;
16+
use Illuminate\Support\Str;
17+
use Illuminate\Validation\Rule;
18+
19+
final class CreateGlobalInvitation extends AbstractMutation
20+
{
21+
public ?GlobalInvitation $invitedUser = null;
22+
23+
/**
24+
* @param array{
25+
* email: string,
26+
* role: GlobalRole,
27+
* } $args
28+
*
29+
* @throws Exception
30+
*/
31+
protected function mutate(array $args): void
32+
{
33+
// This field might not reset when testing since the same mocked request is reused.
34+
$this->invitedUser = null;
35+
36+
Validator::make($args, [
37+
'email' => [
38+
'required',
39+
'email:strict',
40+
],
41+
'role' => [
42+
'required',
43+
Rule::enum(GlobalRole::class),
44+
],
45+
])->validate();
46+
47+
/** @var ?User $user */
48+
$user = auth()->user();
49+
if ($user === null) {
50+
// This should never happen, but we handle the case anyway to make PHPStan happy.
51+
throw new Exception('Attempt to invite user when not signed in.');
52+
}
53+
54+
if ($user->cannot('createInvitation', GlobalInvitation::class)) {
55+
abort(401, 'This action is unauthorized.');
56+
}
57+
58+
if (GlobalInvitation::where('email', $args['email'])->exists()) {
59+
abort(400, 'Duplicate invitations are not allowed.');
60+
}
61+
62+
if (User::where('email', $args['email'])->exists()) {
63+
abort(401, 'User is already a member of this instance.');
64+
}
65+
66+
$password = Str::password();
67+
68+
$this->invitedUser = GlobalInvitation::create([
69+
'email' => $args['email'],
70+
'invited_by_id' => $user->id,
71+
'role' => $args['role'], // Note: we assume that anyone who can invite users can assign them any role.
72+
'invitation_timestamp' => Carbon::now(),
73+
'password' => Hash::make($password),
74+
]);
75+
76+
// The email gets sent to the queue, so we have no way to know immediately whether it was sent or not.
77+
// TODO: We should eventually track whether the email was actually sent.
78+
Mail::to($args['email'])->send(new InvitedToCdash($this->invitedUser, $password));
79+
}
80+
}

app/GraphQL/Mutations/InviteToProject.php

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@
77
use App\Enums\ProjectRole;
88
use App\Mail\InvitedToProject;
99
use App\Models\Project;
10+
use App\Models\ProjectInvitation;
1011
use App\Models\User;
11-
use App\Models\UserInvitation;
1212
use Exception;
1313
use Illuminate\Support\Carbon;
1414
use Illuminate\Support\Facades\Mail;
@@ -17,7 +17,7 @@
1717

1818
final class InviteToProject extends AbstractMutation
1919
{
20-
public ?UserInvitation $invitedUser = null;
20+
public ?ProjectInvitation $invitedUser = null;
2121

2222
/**
2323
* @param array{
@@ -55,12 +55,12 @@ protected function mutate(array $args): void
5555
}
5656

5757
$project = isset($args['projectId']) ? Project::find((int) $args['projectId']) : null;
58-
if ($project === null || $user->cannot('inviteUsers', $project)) {
58+
if ($project === null || $user->cannot('inviteUser', $project)) {
5959
$this->message = 'This action is unauthorized.';
6060
return;
6161
}
6262

63-
if (UserInvitation::where(['email' => $args['email'], 'project_id' => $args['projectId']])->exists()) {
63+
if (ProjectInvitation::where(['email' => $args['email'], 'project_id' => $args['projectId']])->exists()) {
6464
$this->message = 'Duplicate invitations are not allowed.';
6565
return;
6666
}
@@ -70,7 +70,7 @@ protected function mutate(array $args): void
7070
return;
7171
}
7272

73-
$this->invitedUser = UserInvitation::create([
73+
$this->invitedUser = ProjectInvitation::create([
7474
'email' => $args['email'],
7575
'invited_by_id' => $user->id,
7676
'project_id' => $args['projectId'],
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace App\GraphQL\Mutations;
6+
7+
use App\Models\User;
8+
use Illuminate\Support\Facades\Gate;
9+
10+
final class RemoveUser extends AbstractMutation
11+
{
12+
/**
13+
* @param array{
14+
* userId: int,
15+
* } $args
16+
*/
17+
protected function mutate(array $args): void
18+
{
19+
$user = User::find((int) $args['userId']);
20+
21+
if ($user === null) {
22+
abort(404, 'User does not exist.');
23+
}
24+
25+
Gate::authorize('delete', $user);
26+
27+
$user->delete();
28+
}
29+
}
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace App\GraphQL\Mutations;
6+
7+
use App\Models\GlobalInvitation;
8+
use Illuminate\Support\Facades\Gate;
9+
10+
final class RevokeGlobalInvitation extends AbstractMutation
11+
{
12+
/**
13+
* @param array{
14+
* invitationId: int,
15+
* } $args
16+
*/
17+
protected function mutate(array $args): void
18+
{
19+
$invitation = GlobalInvitation::find((int) $args['invitationId']);
20+
21+
if ($invitation === null) {
22+
abort(400, 'Invitation does not exist.');
23+
}
24+
25+
Gate::authorize('revokeInvitation', $invitation);
26+
27+
$invitation->delete();
28+
}
29+
}

app/GraphQL/Mutations/RevokeInvitation.php renamed to app/GraphQL/Mutations/RevokeProjectInvitation.php

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,10 @@
44

55
namespace App\GraphQL\Mutations;
66

7-
use App\Models\UserInvitation;
7+
use App\Models\ProjectInvitation;
88
use Illuminate\Support\Facades\Gate;
99

10-
final class RevokeInvitation extends AbstractMutation
10+
final class RevokeProjectInvitation extends AbstractMutation
1111
{
1212
/**
1313
* @param array{
@@ -16,14 +16,14 @@ final class RevokeInvitation extends AbstractMutation
1616
*/
1717
protected function mutate(array $args): void
1818
{
19-
$invitation = UserInvitation::find((int) $args['invitationId']);
19+
$invitation = ProjectInvitation::find((int) $args['invitationId']);
2020

2121
if ($invitation === null) {
2222
$this->message = 'Invitation does not exist.';
2323
return;
2424
}
2525

26-
Gate::authorize('revokeInvitations', $invitation->project);
26+
Gate::authorize('revokeInvitation', $invitation->project);
2727

2828
$invitation->delete();
2929
}

app/Http/Controllers/Auth/LoginController.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ public function __construct()
5656
*/
5757
public function login(Request $request)
5858
{
59-
if (config('auth.username_password_authentication_enabled') === false) {
59+
if (config('cdash.username_password_authentication_enabled') === false) {
6060
return $this->sendFailedLoginResponse($request);
6161
}
6262
return $this->traitLogin($request);
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
<?php
2+
3+
namespace App\Http\Controllers;
4+
5+
use App\Enums\GlobalRole;
6+
use App\Models\GlobalInvitation;
7+
use App\Models\User;
8+
use Illuminate\Http\RedirectResponse;
9+
use Illuminate\Http\Request;
10+
use Illuminate\Support\Carbon;
11+
use Illuminate\Support\Facades\Auth;
12+
13+
final class GlobalInvitationController extends AbstractController
14+
{
15+
// We assume that this signed route has been verified already...
16+
public function __invoke(Request $request, int $invitationId): RedirectResponse
17+
{
18+
if (config('cdash.username_password_authentication_enabled') === false) {
19+
abort(401, 'Username and password registration is disabled.');
20+
}
21+
22+
$user_invite = GlobalInvitation::find($invitationId);
23+
if ($user_invite === null) {
24+
abort(404, 'Invitation does not exist.');
25+
}
26+
27+
if (User::where('email', $user_invite->email)->exists()) {
28+
$user_invite->deleteOrFail();
29+
abort(401, 'You are already registered.');
30+
}
31+
32+
Auth::login(User::forceCreate([
33+
'admin' => $user_invite->role === GlobalRole::ADMINISTRATOR,
34+
'email' => $user_invite->email,
35+
'password' => $user_invite->password,
36+
'password_updated_at' => Carbon::now(),
37+
]));
38+
39+
$user_invite->deleteOrFail();
40+
41+
return response()->redirectTo('/profile?password_expired=1');
42+
}
43+
}

0 commit comments

Comments
 (0)