gsadmin/projectsend
1
0
Fork 0
mirror of https://github.com/projectsend/projectsend.git synced 2026-10-04 13:33:22 +00:00
Code Issues Packages Projects Releases Wiki Activity

Keep your own credentials behind your profile, and end tokens on a reset

Browse Source 
Your own email address, password and second factor are changed from your
profile, which asks for your current password. The staff screen and the
API changed the first two with no password at all, and the API removed
the third on your own account without the confirmation the web asks
for. StaffAccounts::ownCredentialChanges is the one rule both now ask:
the staff screen refuses your own email or password with a validation
error and points to the profile, and the API answers 403, as it does for
removing your own second factor.

Changing somebody else's password is unchanged: that is what edit_users
and edit_clients mean, on the screen and over the API. It now also
revokes that account's API tokens. Browser sessions already ended with
the password hash; tokens did not.

GHSA-j5cp-r8pr-m5cr
This commit is contained in:
ignacionelson
2026-10-03 23:05:23 -03:00
parent 7fcfbb5c41
commit db65731c3a
5 changed files with 140 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files
app/Modules/Identity/Http/Controllers/Api/UsersController.php
+17 -1
View File
@@ -182,6 +182,15 @@ class UsersController extends Controller
'assigned_clients.*' => ['integer', Rule::in($this->accounts->assignableClientIds($actor))],
]);
// Your own email address and password are changed from your
// profile, which asks for your current password. A token cannot be
// asked for one, so here the answer is no.
abort_if(
$this->accounts->ownCredentialChanges($actor, $user, $validated['email'] ?? null, $validated['password'] ?? null) !== [],
403,
__('Change your own email address and password from your profile.'),
);
// Read through Request::boolean() rather than off the validated
// array, for the reason RolesController::guardScopeRemoval spells
// out: the `boolean` rule accepts 0 and "0" as well as false but
@@ -274,7 +283,14 @@ class UsersController extends Controller
{
abort_unless($user->isStaff(), 404);
$this->accounts->guardTarget($this->actor($request), $user);
$actor = $this->actor($request);
$this->accounts->guardTarget($actor, $user);
// The web asks for your password before this (password.confirm),
// and a token cannot give one. On your own account it would let a
// token clear the second factor standing between it and a browser
// session as you.
abort_if($user->is($actor), 403, __('Remove your own two-factor authentication from your profile.'));
$twoFactor->reset($user);
app/Modules/Identity/Http/Controllers/UsersController.php
+17
View File
@@ -226,6 +226,23 @@ class UsersController extends Controller
]);
}
// Your own email address and password are changed from your
// profile, which asks for your current password first; this screen
// does not, so it does not change them.
$ownCredentials = $this->accounts->ownCredentialChanges(
$this->actor(),
$user,
$validated['email'],
is_string($validated['password'] ?? null) ? $validated['password'] : null,
);
if ($ownCredentials !== []) {
throw ValidationException::withMessages(array_fill_keys(
$ownCredentials,
__('Change your own email address and password from your profile.'),
));
}
$this->accounts->update($user, [
'name' => $validated['name'],
'email' => $validated['email'],
app/Modules/Identity/StaffAccounts.php
+49 -1
View File
@@ -161,6 +161,42 @@ class StaffAccounts
abort_unless($role === null || $this->mayGrant($actor, $role), 403);
}
/**
* Which of your own credentials this change would replace: a different
* email address, or a new password. Empty when the target is somebody
* else, or when nothing that signs the account in is changing.
*
* Your own are changed from your profile, which asks for your current
* password first (GHSA-f32x-fgmp-q353). The staff screen and the API
* must not be a second door to them. Over the API that door was wider
* still: a token limited to manage_users and edit_users could give its
* own owner a password it chose, and then sign in as the owner with
* every ability the token had been denied.
*
* The email address counts because it is how a password is recovered:
* an address you control is a password you can set.
*
* @return list<'email'|'password'>
*/
public function ownCredentialChanges(User $actor, User $target, ?string $email, ?string $password): array
{
if (! $target->is($actor)) {
return [];
}
$fields = [];
if ($email !== null && mb_strtolower($email) !== mb_strtolower($target->email)) {
$fields[] = 'email';
}
if ($password !== null && $password !== '') {
$fields[] = 'password';
}
return $fields;
}
/**
* Refuse any change that would leave the installation without an
* active administrator.
@@ -297,12 +333,24 @@ class StaffAccounts
$user->fill(array_intersect_key($attributes, array_flip(['name', 'email', 'role_id', 'active'])));
if (is_string($attributes['password'] ?? null) && $attributes['password'] !== '') {
$passwordReplaced = is_string($attributes['password'] ?? null) && $attributes['password'] !== '';
if ($passwordReplaced) {
$user->password = $attributes['password'];
}
$user->save();
// Somebody else gave this account a new password: whatever had been
// holding it, a person or a stolen credential, is ended with it.
// Browser sessions end on their own (AuthenticateSession reads the
// password hash), but API tokens do not, and a reset that left the
// previous holder's token working would not be a reset. Never your
// own password: both callers refuse that (ownCredentialChanges).
if ($passwordReplaced) {
$user->tokens()->delete();
}
if ($assignedClients !== null) {
$this->syncAssignedClients($user, (int) $user->role_id, $assignedClients);
} elseif ($oldRoleId !== $user->role_id) {
resources/js/components/user-form.tsx
+56 -27
View File
@@ -1,10 +1,11 @@
import { Link } from '@inertiajs/react';
import { X } from 'lucide-react';
import InputError from '@/components/input-error';
import { PasswordRequirements } from '@/components/password-requirements';
import { Input } from '@/components/ui/input';
import { Label } from '@/components/ui/label';
import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from '@/components/ui/select';
import { PasswordRequirements } from '@/components/password-requirements';
import { useTranslation } from '@/hooks/use-translation';
export interface AssignableRole {
@@ -31,6 +32,12 @@ interface UserFormProps {
assignedClients: number[];
onAssignedClientsChange: (ids: number[]) => void;
passwordOptional: boolean;
/**
* Editing your own account: the email address and password are changed
* from your profile, which asks for your current password first, so
* this form shows the address and leaves both alone.
*/
ownAccount?: boolean;
errors: Partial<Record<string, string>>;
}
@@ -46,6 +53,7 @@ export function UserForm({
assignedClients,
onAssignedClientsChange,
passwordOptional,
ownAccount = false,
errors,
}: UserFormProps) {
const { t } = useTranslation();
@@ -62,7 +70,23 @@ export function UserForm({
<div className="grid gap-2">
<Label htmlFor="email">{t('Email address')}</Label>
<Input id="email" type="email" value={email} onChange={(e) => onChange('email', e.target.value)} required autoComplete="off" />
<Input
id="email"
type="email"
value={email}
onChange={(e) => onChange('email', e.target.value)}
required
readOnly={ownAccount}
autoComplete="off"
/>
{ownAccount && (
<p className="text-muted-foreground text-sm">
{t('Change your own email address and password from your profile.')}{' '}
<Link href={route('profile.edit')} className="underline underline-offset-4">
{t('Go to your profile')}
</Link>
</p>
)}
<InputError message={errors.email} />
</div>
@@ -92,32 +116,37 @@ export function UserForm({
/>
)}
<div className="grid gap-2">
<Label htmlFor="password">{passwordOptional ? t('New password (leave blank to keep current)') : t('Password')}</Label>
<Input
id="password"
type="password"
value={password}
onChange={(e) => onChange('password', e.target.value)}
required={!passwordOptional}
autoComplete="new-password"
/>
<PasswordRequirements />
<InputError message={errors.password} />
</div>
{!ownAccount && (
<>
<div className="grid gap-2">
<Label htmlFor="password">{passwordOptional ? t('New password (leave blank to keep current)') : t('Password')}</Label>
<Input
id="password"
type="password"
value={password}
onChange={(e) => onChange('password', e.target.value)}
required={!passwordOptional}
autoComplete="new-password"
/>
<PasswordRequirements />
<InputError message={errors.password} />
</div>
<div className="grid gap-2">
<Label htmlFor="password_confirmation">{t('Confirm password')}</Label>
<Input
id="password_confirmation"
type="password"
value={passwordConfirmation}
onChange={(e) => onChange('password_confirmation', e.target.value)}
required={!passwordOptional || password !== ''}
autoComplete="new-password"
/>
<InputError message={errors.password_confirmation} />
</div>
<div className="grid gap-2">
<Label htmlFor="password_confirmation">{t('Confirm password')}</Label>
<Input
id="password_confirmation"
type="password"
value={passwordConfirmation}
onChange={(e) => onChange('password_confirmation', e.target.value)}
required={!passwordOptional || password !== ''}
autoComplete="new-password"
/>
<InputError message={errors.password_confirmation} />
</div>
</>
)}
{ownAccount && <InputError message={errors.password} />}
</div>
);
}
resources/js/pages/users/edit.tsx
+1
View File
@@ -128,6 +128,7 @@ export default function UsersEdit({
assignedClients={data.assigned_clients}
onAssignedClientsChange={(ids) => setData('assigned_clients', ids)}
passwordOptional
ownAccount={is_self}
errors={errors}
/>