Skip to content

Commit 387655e

Browse files
c15k0c15k0
committed
feat(admin): support v2 user updates
1 parent 14c6598 commit 387655e

7 files changed

Lines changed: 213 additions & 13 deletions

File tree

‎docs/superpowers/plans/2026-09-09-admin-v2-phase-2-users.md‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
# Admin 2.0: fase 2 — edición segura de usuarios
2+
3+
## Objetivo
4+
5+
Completar la paridad de la pantalla nativa de usuarios sin crear lógica de
6+
identidad nueva: listar, crear, editar la contraseña/rol y eliminar usando los
7+
servicios PSFS existentes.
8+
9+
## Contrato
10+
11+
- `GET /admin/api/v2/users` entrega alias, rol visible, clase y el identificador
12+
de perfil necesario para preseleccionar el formulario; nunca hash ni
13+
contraseña.
14+
- `PUT /admin/api/v2/users/{username}` conserva el alias de la ruta, exige CSRF
15+
y superadmin, valida con `AdminForm` y persiste mediante `Security::save()`.
16+
- La contraseña permanece write-only y es obligatoria en cada actualización.
17+
- Un alias inexistente recibe 404; un alias distinto en el body recibe 422.
18+
19+
## UI
20+
21+
La tabla ofrece una edición explícita. El alias se precarga y no se permite
22+
renombrarlo; el formulario reinicia sus controles sólo al seleccionar o
23+
cancelar una edición, nunca durante la escritura del usuario.
24+
25+
## Verificación
26+
27+
1. PHPUnit con doubles de controlador para no tocar `admins.json`.
28+
2. Vitest de la página para comprobar la llamada `PUT` y la invariabilidad del
29+
alias.
30+
3. Playwright contra Vite y el mock API aislado: editar un usuario, comprobar
31+
`PUT /admin/api/v2/users/admin` y el mensaje de éxito.

‎src/controller/AdminFrontendUsersController.php‎

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,56 @@ public function create(): string
6868
return $this->json(AdminApiResponse::success([], t('User created successfully')));
6969
}
7070

71+
#[HttpMethod('PUT')]
72+
#[Route('/admin/api/v2/users/{username}')]
73+
#[Visible(false)]
74+
public function update(string $username): string
75+
{
76+
AdminFrontendCsrf::assertValid();
77+
$this->assertSuperAdminWriteAccess();
78+
if (!$this->isValidUsername($username) || !array_key_exists($username, $this->admins())) {
79+
return $this->json(AdminApiResponse::failure(
80+
t('User not found'),
81+
['username' => [t('User not found')]]
82+
), 404);
83+
}
84+
85+
$payload = $this->requestPayload();
86+
$values = $payload['values'] ?? null;
87+
if (!$this->isRecord($values) || $values === []) {
88+
return $this->json(AdminApiResponse::failure(t('Invalid user payload'), [
89+
'payload' => [t('Expected a values object')],
90+
]), 422);
91+
}
92+
93+
$submittedUsername = $values['username'] ?? $username;
94+
if (!is_string($submittedUsername) || $submittedUsername !== $username) {
95+
return $this->json(AdminApiResponse::failure(t('Invalid user payload'), [
96+
'username' => [t('Username cannot be changed')],
97+
]), 422);
98+
}
99+
$values['username'] = $username;
100+
101+
$form = $this->adminForm();
102+
$form->setMethod('POST')->build();
103+
$form->setData($values);
104+
$errors = $this->requiredFieldErrors($values);
105+
if ($errors !== [] || !$form->isValid()) {
106+
return $this->json(AdminApiResponse::failure(
107+
t('Invalid user'),
108+
$errors + $this->fieldErrors($form)
109+
), 422);
110+
}
111+
112+
if (!$this->saveUser($form->getData())) {
113+
return $this->json(AdminApiResponse::failure(
114+
t('Error while saving administrators, please verify filesystem permissions')
115+
), 500);
116+
}
117+
118+
return $this->json(AdminApiResponse::success([], t('User updated successfully')));
119+
}
120+
71121
#[HttpMethod('DELETE')]
72122
#[Route('/admin/api/v2/users')]
73123
#[Visible(false)]
@@ -144,7 +194,7 @@ private function isValidUsername(string $username): bool
144194
/**
145195
* @param array<string,array<string,mixed>> $admins
146196
* @param array<string,string> $profiles
147-
* @return array<int,array{username:string,role:string,class:string}>
197+
* @return array<int,array{username:string,role:string,class:string,profile:string}>
148198
*/
149199
private function sanitizeUsers(array $admins, array $profiles): array
150200
{
@@ -154,6 +204,7 @@ private function sanitizeUsers(array $admins, array $profiles): array
154204
'username' => (string) $username,
155205
'role' => (string) ($profiles[(string) ($admin['profile'] ?? '')] ?? t('User')),
156206
'class' => (string) ($admin['class'] ?? ''),
207+
'profile' => (string) ($admin['profile'] ?? ''),
157208
];
158209
}
159210

‎tests/controller/AdminFrontendUsersControllerTest.php‎

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ public function testIndexReturnsSanitizedUsersAndCreationSchema(): void
2727
self::assertTrue($response['ok'], json_encode($response));
2828
self::assertSame('alice', $response['data']['users'][0]['username']);
2929
self::assertArrayNotHasKey('password', $response['data']['users'][0]);
30-
self::assertArrayNotHasKey('profile', $response['data']['users'][0]);
30+
self::assertSame('889a3a791b3875cfae413574b53da4bb8a90d53e', $response['data']['users'][0]['profile']);
3131
self::assertSame('Administrator', $response['data']['users'][0]['role']);
3232
self::assertArrayHasKey('username', $response['data']['form']['fields']);
3333
self::assertSame(['889a3a791b3875cfae413574b53da4bb8a90d53e' => 'Administrator'], $response['data']['profiles']);
@@ -62,13 +62,48 @@ public function testDeleteAcceptsTheUnderscoreAliasesThatUserCreationAlreadyAllo
6262
self::assertTrue($response['ok'], json_encode($response));
6363
self::assertTrue($controller->deleted, json_encode($response));
6464
}
65+
66+
public function testUpdateRejectsAnAliasMismatchBeforeSaving(): void
67+
{
68+
$controller = new AdminFrontendUsersControllerProbe([
69+
'values' => [
70+
'username' => 'other-admin',
71+
'password' => 'new-password',
72+
'profile' => '889a3a791b3875cfae413574b53da4bb8a90d53e',
73+
],
74+
]);
75+
76+
$response = json_decode($controller->update('alice'), true, 512, JSON_THROW_ON_ERROR);
77+
78+
self::assertSame(422, $controller->statusCode);
79+
self::assertFalse($response['ok']);
80+
self::assertFalse($controller->saved);
81+
}
82+
83+
public function testUpdateReusesTheExistingUserPersistenceForTheRouteAlias(): void
84+
{
85+
$controller = new AdminFrontendUsersControllerProbe([
86+
'values' => [
87+
'password' => 'new-password',
88+
'profile' => '889a3a791b3875cfae413574b53da4bb8a90d53e',
89+
],
90+
]);
91+
92+
$response = json_decode($controller->update('alice'), true, 512, JSON_THROW_ON_ERROR);
93+
94+
self::assertTrue($response['ok'], json_encode($response));
95+
self::assertTrue($controller->saved);
96+
self::assertSame('alice', $controller->savedValues['username']);
97+
}
6598
}
6699

67100
class AdminFrontendUsersControllerProbe extends AdminFrontendUsersController
68101
{
69102
public int $statusCode = 200;
70103
public bool $saved = false;
71104
public bool $deleted = false;
105+
/** @var array<string,mixed> */
106+
public array $savedValues = [];
72107

73108
/** @param array<string,mixed> $payload */
74109
public function __construct(private readonly array $payload = [])
@@ -107,6 +142,7 @@ protected function profiles(): array
107142
protected function saveUser(array $data): bool
108143
{
109144
$this->saved = true;
145+
$this->savedValues = $data;
110146
return true;
111147
}
112148

‎ui/e2e/admin-v2-users.spec.mjs‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,3 +55,21 @@ test('crea y elimina una cuenta temporal mediante el diálogo de confirmación',
5555
await expect((await deleted).status()).toBe(200);
5656
await expect(page.locator('table tbody tr')).toHaveCount(0);
5757
});
58+
59+
test('actualiza una cuenta existente sin permitir que el formulario cambie su alias', async ({ page }) => {
60+
await page.goto('/admin-v2/setup');
61+
await page.getByRole('button', { name: 'Editar' }).click();
62+
63+
await expect(page.getByRole('heading', { name: 'Editar usuario' })).toBeVisible();
64+
await expect(page.locator('input#username')).toHaveValue('admin');
65+
await page.locator('input#password').fill('Replacement-e2e-password-2026');
66+
const updated = page.waitForResponse((response) => {
67+
const url = new URL(response.url());
68+
return url.pathname === '/admin/api/v2/users/admin' && response.request().method() === 'PUT';
69+
});
70+
await page.getByRole('button', { name: 'Actualizar usuario' }).click();
71+
72+
const updateResponse = await updated;
73+
await expect(updateResponse.status(), await updateResponse.text()).toBe(200);
74+
await expect(page.locator('.notice--success')).toContainText('Usuario actualizado correctamente.');
75+
});

‎ui/e2e/mock-api.mjs‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ async function bodyOf(request) {
6262
}
6363

6464
export function createMockApiServer() {
65-
const state = { users: [{ username: 'admin', role: 'Administrator', class: 'admin' }] };
65+
const state = { users: [{ username: 'admin', role: 'Administrator', class: 'admin', profile: 'admin' }] };
6666
const server = createServer(async (request, reply) => {
6767
const url = new URL(request.url ?? '/', 'http://ui-e2e.local');
6868
const path = url.pathname;
@@ -131,11 +131,27 @@ export function createMockApiServer() {
131131
if (!username || !payload.values?.password) {
132132
response(reply, 422, error('Invalid user.', { username: !username ? ['Required'] : [], password: !payload.values?.password ? ['Required'] : [] }));
133133
} else {
134-
state.users.push({ username, role: 'Administrator', class: 'admin' });
134+
state.users.push({ username, role: 'Administrator', class: 'admin', profile: payload.values.profile ?? 'admin' });
135135
response(reply, 200, envelope({}, 'Usuario creado correctamente.'));
136136
}
137137
return;
138138
}
139+
if (method === 'PUT' && /^\/admin\/api\/v2\/users\/[^/]+$/.test(path)) {
140+
const username = decodeURIComponent(path.split('/').at(-1));
141+
const payload = await bodyOf(request);
142+
const user = state.users.find((candidate) => candidate.username === username);
143+
if (!user) {
144+
response(reply, 404, error('User not found.', { username: ['User not found.'] }));
145+
} else if (payload.values?.username && payload.values.username !== username) {
146+
response(reply, 422, error('Invalid user payload.', { username: ['Username cannot be changed.'] }));
147+
} else if (!payload.values?.password) {
148+
response(reply, 422, error('Invalid user.', { password: ['Required'] }));
149+
} else {
150+
user.profile = payload.values.profile ?? user.profile;
151+
response(reply, 200, envelope({}, 'Usuario actualizado correctamente.'));
152+
}
153+
return;
154+
}
139155
if (method === 'DELETE' && path === '/admin/api/v2/users') {
140156
const payload = await bodyOf(request);
141157
state.users = state.users.filter((user) => user.username !== payload.user);

‎ui/projects/admin/src/app/users-page.component.spec.ts‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { TestBed } from '@angular/core/testing';
22
import { of, throwError } from 'rxjs';
3-
import { describe, expect, it } from 'vitest';
3+
import { describe, expect, it, vi } from 'vitest';
44
import { AdminApiService } from './admin-api.service';
55
import { UsersPageComponent } from './users-page.component';
66

@@ -34,10 +34,27 @@ describe('UsersPageComponent', () => {
3434

3535
const fixture = TestBed.createComponent(UsersPageComponent);
3636
fixture.detectChanges();
37-
fixture.componentInstance.create({ values: { username: '', password: '', profile: 'manager' }, extra: {} });
37+
fixture.componentInstance.save({ values: { username: '', password: '', profile: 'manager' }, extra: {} });
3838
fixture.detectChanges();
3939

4040
expect(fixture.nativeElement.textContent).toContain('El alias es obligatorio');
4141
expect(fixture.componentInstance.fieldErrors()['username']).toEqual(['El alias es obligatorio']);
4242
});
43+
44+
it('updates an existing alias through the v2 update contract without allowing a rename', () => {
45+
const put = vi.fn(() => of({ ok: true, message: 'Usuario actualizado', data: {}, errors: {} }));
46+
const api = {
47+
get: () => of({ ok: true, message: null, data: { users: [{ username: 'alice', role: 'Manager', class: 'warning', profile: 'manager' }], form, profiles: { manager: 'Manager' } }, errors: {} }),
48+
post: () => of({ ok: true, message: null, data: {}, errors: {} }),
49+
put
50+
};
51+
TestBed.configureTestingModule({ providers: [{ provide: AdminApiService, useValue: api }] });
52+
53+
const fixture = TestBed.createComponent(UsersPageComponent);
54+
fixture.detectChanges();
55+
fixture.componentInstance.edit({ username: 'alice', role: 'Manager', class: 'warning', profile: 'manager' });
56+
fixture.componentInstance.save({ values: { username: 'alice', password: 'new-password', profile: 'manager' }, extra: {} });
57+
58+
expect(put).toHaveBeenCalledWith('users/alice', { values: { username: 'alice', password: 'new-password', profile: 'manager' }, extra: {} });
59+
});
4360
});

‎ui/projects/admin/src/app/users-page.component.ts‎

Lines changed: 38 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ interface AdminUser {
99
username: string;
1010
role: string;
1111
class: string;
12+
profile: string;
1213
}
1314

1415
interface UsersContract {
@@ -48,7 +49,7 @@ interface UsersContract {
4849
<tr *ngFor="let user of visibleUsers()">
4950
<td><strong>{{ user.username }}</strong></td>
5051
<td><span class="role-badge" [class]="'role-badge role-badge--' + user.class">{{ user.role }}</span></td>
51-
<td class="table-actions"><button type="button" class="button button--danger button--small" [disabled]="saving()" (click)="remove(user.username)">Eliminar</button></td>
52+
<td class="table-actions"><button type="button" class="button button--secondary button--small" [disabled]="saving()" (click)="edit(user)">Editar</button><button type="button" class="button button--danger button--small" [disabled]="saving()" (click)="remove(user.username)">Eliminar</button></td>
5253
</tr>
5354
</tbody>
5455
</table>
@@ -61,9 +62,9 @@ interface UsersContract {
6162
<ng-template #emptyUsers><p class="empty-state">No hay usuarios configurados todavía.</p></ng-template>
6263
</div>
6364
64-
<section class="panel users-create" *ngIf="schema() as form">
65-
<div class="panel-heading"><div><h2>Nuevo usuario</h2><p>Define alias, contraseña y rol.</p></div></div>
66-
<psfs-dynamic-form [schema]="form" [errors]="fieldErrors()" [disabled]="saving()" submitLabel="Crear usuario" (submitted)="create($event)" />
65+
<section class="panel users-create" *ngIf="activeSchema() as form">
66+
<div class="panel-heading"><div><h2>{{ editing() ? 'Editar usuario' : 'Nuevo usuario' }}</h2><p>{{ editing() ? 'La contraseña se sustituirá al guardar.' : 'Define alias, contraseña y rol.' }}</p></div><button *ngIf="editing()" class="button button--secondary" type="button" (click)="cancelEdit()">Cancelar</button></div>
67+
<psfs-dynamic-form [schema]="form" [errors]="fieldErrors()" [disabled]="saving()" [submitLabel]="editing() ? 'Actualizar usuario' : 'Crear usuario'" (submitted)="save($event)" />
6768
</section>
6869
</section>
6970
</article>
@@ -81,6 +82,21 @@ export class UsersPageComponent {
8182
readonly failure = signal<AdminEnvelope<null> | null>(null);
8283
readonly fieldErrors = signal<Record<string, string[]>>({});
8384
readonly pendingDelete = signal('');
85+
readonly editing = signal<AdminUser | null>(null);
86+
readonly activeSchema = computed<AdminFormSchema | null>(() => {
87+
const form = this.schema();
88+
const user = this.editing();
89+
if (!form || !user) return form;
90+
return {
91+
...form,
92+
fields: {
93+
...form.fields,
94+
username: { ...form.fields['username'], value: user.username },
95+
password: { ...form.fields['password'], value: '' },
96+
profile: { ...form.fields['profile'], value: user.profile },
97+
}
98+
};
99+
});
84100
readonly filter = signal('');
85101
readonly page = signal(0);
86102
readonly filteredUsers = computed(() => {
@@ -98,13 +114,18 @@ export class UsersPageComponent {
98114
this.load();
99115
}
100116

101-
create(submission: DynamicFormSubmission): void {
117+
save(submission: DynamicFormSubmission): void {
102118
this.saving.set(true);
103119
this.resetFeedback();
104-
this.api.post<Record<string, never>>('users', submission).subscribe({
120+
const current = this.editing();
121+
const request = current
122+
? this.api.put<Record<string, never>>(`users/${encodeURIComponent(current.username)}`, submission)
123+
: this.api.post<Record<string, never>>('users', submission);
124+
request.subscribe({
105125
next: (response) => {
106126
this.saving.set(false);
107-
this.message.set(response.message ?? 'Usuario creado correctamente.');
127+
this.message.set(response.message ?? (current ? 'Usuario actualizado correctamente.' : 'Usuario creado correctamente.'));
128+
this.editing.set(null);
108129
this.load();
109130
},
110131
error: (failure: unknown) => {
@@ -118,6 +139,16 @@ export class UsersPageComponent {
118139
this.pendingDelete.set(username);
119140
}
120141

142+
edit(user: AdminUser): void {
143+
this.editing.set(user);
144+
this.resetFeedback();
145+
}
146+
147+
cancelEdit(): void {
148+
this.editing.set(null);
149+
this.resetFeedback();
150+
}
151+
121152
setFilter(value: string): void {
122153
this.filter.set(value);
123154
this.page.set(0);

0 commit comments

Comments
 (0)