Until now every account could do the same, and managing accounts meant running make-user.php on the server. Accounts now carry a role, and admins get a tab in the editor to create, rename, re-role, reset and delete accounts. Both issues share the same server files, so they land in one commit. #180 Roles: - Every account is admin or user. A missing or unknown role counts as user, on the server and in the client. - auth/session and auth/login return {user, role}. - Auth::requireAdmin() answers everyone else with 403. - make-user.php takes the role as a third argument. Without it an existing account keeps its role, so a password reset from the CLI never demotes an admin. - Admins see and save other users' projects like their own. Saving keeps the original owner. #179 User management: - The account menu gets "User management" (admins only) and "Plugins" (everyone). Both open the same closable tab, accounts on top and plugins below. The fixed Plugins tab is gone. - Per account: Rename, Role, Password (no old password needed) and Delete with a confirmation. "New user..." creates one. - Each account lists its projects with an Open button. - Delete is refused while the account still owns projects. - Renaming moves the projects along and ends every session of the account. transfer takes the same lock as save. - On their own row an admin can only set the password. The last admin cannot be demoted or deleted. The server enforces both. - The account rules live in AccountRules.php and accountRules.ts as pure functions, so they are testable without a server. Findings: - At 1280 px the Delete button reached past the column edge and was not clickable. The name column is flexible now, and module 49 checks the layout. - The server lets an admin delete other users' projects, but the editor has no button for it yet. Left open in the issue. Tests: - ./check.sh: 148 files, 2214 tests, exit 0 (was 147 / 2204). - New tests/php/accounts.test.php, 51 checks, plain PHP. CI runs it as its own step in the gate job, the host has no PHP. - New e2e module 49 with 18 checks. CI creates the e2e-admin account in serve.sh. Modules 14, 38, 46 and the scaffold follow the new tab. - e2e 1-16,37,38,45,46,49: all passed, 581 checks. - Counter-checks: owner on save, admin view, default role and self delete in PHP fail 7, 5, 5 and 1 checks. Unchecked requireAdmin, a wider column and the caller as owner each fail exactly their e2e check. - Check sum 1314 -> 1333 (+1 in 38, +18 in 49), computed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
30fb3a2316
commit
ceaa44e5b8
38 files changed
+1803
-138
No files matched your search
+26
-7
@@ -23,17 +23,36 @@ afterEach(() => {
|
||||
});
|
||||
|
||||
describe('HttpAuthStore', () => {
|
||||
it('reads who is signed in, null included', async () => {
|
||||
stubFetch(JSON.stringify({ user: 'e2e' }), { status: 200 });
|
||||
await expect(new HttpAuthStore('/api').session()).resolves.toBe('e2e');
|
||||
it('reads who is signed in and their role, null included', async () => {
|
||||
stubFetch(JSON.stringify({ user: 'e2e', role: 'user' }), { status: 200 });
|
||||
await expect(new HttpAuthStore('/api').session()).resolves.toEqual({ user: 'e2e', role: 'user' });
|
||||
|
||||
stubFetch(JSON.stringify({ user: null }), { status: 200 });
|
||||
stubFetch(JSON.stringify({ user: 'admin', role: 'admin' }), { status: 200 });
|
||||
await expect(new HttpAuthStore('/api').session()).resolves.toEqual({ user: 'admin', role: 'admin' });
|
||||
|
||||
stubFetch(JSON.stringify({ user: null, role: null }), { status: 200 });
|
||||
await expect(new HttpAuthStore('/api').session()).resolves.toBeNull();
|
||||
});
|
||||
|
||||
it('sends the pair as JSON and answers with the name the server confirms', async () => {
|
||||
const mock = stubFetch(JSON.stringify({ user: 'user' }), { status: 200 });
|
||||
await expect(new HttpAuthStore('/api').logIn('user', 'secret')).resolves.toBe('user');
|
||||
// #180: the weaker reading is the safe one – a server from before roles, or a role this editor does
|
||||
// not know, must not make anyone an administrator.
|
||||
it('takes a missing or unknown role for user', async () => {
|
||||
stubFetch(JSON.stringify({ user: 'old' }), { status: 200 });
|
||||
await expect(new HttpAuthStore('/api').session()).resolves.toEqual({ user: 'old', role: 'user' });
|
||||
|
||||
stubFetch(JSON.stringify({ user: 'odd', role: 'root' }), { status: 200 });
|
||||
await expect(new HttpAuthStore('/api').logIn('odd', 'secret')).resolves.toEqual({
|
||||
user: 'odd',
|
||||
role: 'user',
|
||||
});
|
||||
});
|
||||
|
||||
it('sends the pair as JSON and answers with the name and role the server confirms', async () => {
|
||||
const mock = stubFetch(JSON.stringify({ user: 'user', role: 'user' }), { status: 200 });
|
||||
await expect(new HttpAuthStore('/api').logIn('user', 'secret')).resolves.toEqual({
|
||||
user: 'user',
|
||||
role: 'user',
|
||||
});
|
||||
const [url, init] = mock.mock.calls[0]!;
|
||||
expect(url as string).toContain('r=auth%2Flogin');
|
||||
expect(init?.method).toBe('POST');
|
||||
|
||||
@@ -0,0 +1,97 @@
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest';
|
||||
import { HttpUsersStore } from '../../src/core/store/HttpUsersStore';
|
||||
import { accountActions, nameProblem, newPasswordProblem } from '../../src/editor/users/accountRules';
|
||||
import { MIN_PASSWORD_LENGTH } from '../../src/editor/auth/loginRules';
|
||||
|
||||
function stubFetch(body: BodyInit | null, init: ResponseInit) {
|
||||
const mock = vi.fn<typeof fetch>(() => Promise.resolve(new Response(body, init)));
|
||||
vi.stubGlobal('fetch', mock);
|
||||
return mock;
|
||||
}
|
||||
|
||||
afterEach(() => {
|
||||
vi.unstubAllGlobals();
|
||||
});
|
||||
|
||||
describe('HttpUsersStore (#179)', () => {
|
||||
it('lists the accounts with their projects, an unknown role read as user', async () => {
|
||||
const mock = stubFetch(
|
||||
JSON.stringify([
|
||||
{ username: 'admin', role: 'admin', createdAt: '', projects: [] },
|
||||
{ username: 'odd', role: 'root', createdAt: '2026-09-23T10:00:00+00:00', projects: [{ id: 'p1' }] },
|
||||
]),
|
||||
{ status: 200 },
|
||||
);
|
||||
const accounts = await new HttpUsersStore('/api').list();
|
||||
expect(accounts.map((a) => [a.username, a.role])).toEqual([
|
||||
['admin', 'admin'],
|
||||
['odd', 'user'],
|
||||
]);
|
||||
expect(accounts[1]?.projects).toHaveLength(1);
|
||||
expect(mock.mock.calls[0]?.[0] as string).toContain('r=users');
|
||||
});
|
||||
|
||||
it('creates, changes and deletes through their own routes and methods', async () => {
|
||||
const mock = stubFetch(JSON.stringify({}), { status: 200 });
|
||||
const store = new HttpUsersStore('/api');
|
||||
await store.create('anna', 'a-password', 'admin');
|
||||
await store.update('anna', { username: 'annabel' });
|
||||
stubFetch(null, { status: 204 });
|
||||
await store.remove('annabel');
|
||||
|
||||
const [createUrl, create] = mock.mock.calls[0]!;
|
||||
expect(createUrl as string).toMatch(/r=users$/);
|
||||
expect(create?.method).toBe('POST');
|
||||
expect(JSON.parse(create?.body as string)).toEqual({
|
||||
username: 'anna',
|
||||
password: 'a-password',
|
||||
role: 'admin',
|
||||
});
|
||||
const [patchUrl, patch] = mock.mock.calls[1]!;
|
||||
expect(patchUrl as string).toContain('r=users%2Fanna');
|
||||
expect(patch?.method).toBe('PATCH');
|
||||
// Only what changes travels: a field left out is one the server leaves alone.
|
||||
expect(JSON.parse(patch?.body as string)).toEqual({ username: 'annabel' });
|
||||
});
|
||||
|
||||
// The dialogs show the error as it stands, so it has to be the server's sentence and nothing else.
|
||||
it('turns a refusal into the server’s own sentence', async () => {
|
||||
stubFetch(JSON.stringify({ error: 'anna still owns 2 projects' }), { status: 409 });
|
||||
await expect(new HttpUsersStore('/api').remove('anna')).rejects.toThrow(/^anna still owns 2 projects$/);
|
||||
|
||||
stubFetch(JSON.stringify({ error: 'Administrators only' }), { status: 403 });
|
||||
await expect(new HttpUsersStore('/api').list()).rejects.toThrow(/^Administrators only$/);
|
||||
});
|
||||
|
||||
it('keeps route and status for a failure that is no refusal', async () => {
|
||||
stubFetch(JSON.stringify({ error: 'Internal server error' }), { status: 500 });
|
||||
await expect(new HttpUsersStore('/api').list()).rejects.toThrow(/Internal server error \(500\)/);
|
||||
});
|
||||
});
|
||||
|
||||
describe('the rules of the user management forms (#179)', () => {
|
||||
it('asks for a name the server takes', () => {
|
||||
expect(nameProblem('')).toBe('Enter a user name');
|
||||
expect(nameProblem('a')).not.toBeNull();
|
||||
expect(nameProblem('a b')).not.toBeNull();
|
||||
expect(nameProblem('-dash-first')).not.toBeNull();
|
||||
expect(nameProblem('x'.repeat(33))).not.toBeNull();
|
||||
expect(nameProblem('anna.b_2-x')).toBeNull();
|
||||
expect(nameProblem('x'.repeat(32))).toBeNull();
|
||||
});
|
||||
|
||||
it('asks for a password long enough, typed twice the same', () => {
|
||||
const long = 'a'.repeat(MIN_PASSWORD_LENGTH);
|
||||
expect(newPasswordProblem('', '')).toBe('Enter a password');
|
||||
expect(newPasswordProblem(long.slice(1), long.slice(1))).toBe(
|
||||
`The password needs at least ${MIN_PASSWORD_LENGTH} characters`,
|
||||
);
|
||||
expect(newPasswordProblem(long, `${long}x`)).toBe('The two passwords are different');
|
||||
expect(newPasswordProblem(long, long)).toBeNull();
|
||||
});
|
||||
|
||||
it('offers nothing but a new password on the administrator’s own row', () => {
|
||||
expect(accountActions('admin', 'Admin')).toEqual({ rename: false, role: false, remove: false });
|
||||
expect(accountActions('admin', 'anna')).toEqual({ rename: true, role: true, remove: true });
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,133 @@
|
||||
<?php
|
||||
declare(strict_types=1);
|
||||
|
||||
/**
|
||||
* Accounts, roles and who may touch which project (#180, #179), without a web server: the stores
|
||||
* work on a scratch directory. Plain PHP, no framework – CI runs it with `php tests/php/accounts.test.php`
|
||||
* and reads the last line, as it does for the other suites.
|
||||
*/
|
||||
|
||||
require __DIR__ . '/../../api/lib/AccountRules.php';
|
||||
require __DIR__ . '/../../api/lib/Store.php';
|
||||
require __DIR__ . '/../../api/lib/UserStore.php';
|
||||
|
||||
$failed = 0;
|
||||
$passed = 0;
|
||||
|
||||
function check(string $name, bool $ok, mixed $got = null): void
|
||||
{
|
||||
global $failed, $passed;
|
||||
if ($ok) {
|
||||
$passed++;
|
||||
echo "PASS {$name}\n";
|
||||
return;
|
||||
}
|
||||
$failed++;
|
||||
echo "FAIL {$name}", $got === null ? '' : ' — got ' . json_encode($got), "\n";
|
||||
}
|
||||
|
||||
function throws(callable $work): bool
|
||||
{
|
||||
try {
|
||||
$work();
|
||||
} catch (InvalidArgumentException) {
|
||||
return true;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
$dir = sys_get_temp_dir() . '/game-maker-php-test-' . bin2hex(random_bytes(4));
|
||||
mkdir($dir);
|
||||
$file = "{$dir}/users.json";
|
||||
|
||||
// --- UserStore ---------------------------------------------------------------------------------
|
||||
|
||||
file_put_contents($file, json_encode(['users' => [
|
||||
['username' => 'old', 'password' => password_hash('secret12', PASSWORD_DEFAULT)],
|
||||
['username' => 'odd', 'password' => password_hash('secret12', PASSWORD_DEFAULT), 'role' => 'root'],
|
||||
]]));
|
||||
$users = new UserStore($file);
|
||||
check('an account without a role is a user', $users->role('old') === 'user', $users->role('old'));
|
||||
check('an unknown role counts as user', $users->role('odd') === 'user', $users->role('odd'));
|
||||
check('an account that is not there is a user', $users->role('nobody') === 'user');
|
||||
check('the old accounts have no creation date', $users->accounts()[1]['createdAt'] === '', $users->accounts());
|
||||
|
||||
$users->put('boss', 'secret12', 'admin');
|
||||
check('a role given on creation is kept', $users->role('boss') === 'admin');
|
||||
check('a new account gets a creation date', $users->accounts()[0]['createdAt'] !== '', $users->accounts()[0]);
|
||||
$created = $users->accounts()[0]['createdAt'];
|
||||
$users->put('boss', 'another12');
|
||||
check('a new password without a role leaves the role alone', $users->role('boss') === 'admin');
|
||||
check('a new password leaves the creation date alone', $users->accounts()[0]['createdAt'] === $created);
|
||||
check('the new password is the one that works', $users->verify('boss', 'another12') === 'boss');
|
||||
$users->put('fresh', 'secret12');
|
||||
check('a new account without a role is a user', $users->role('fresh') === 'user');
|
||||
check('the role is written to the file', str_contains((string) file_get_contents($file), '"role": "admin"'));
|
||||
check('an unknown role is refused', throws(fn() => $users->put('x1', 'secret12', 'root')));
|
||||
check(
|
||||
'the accounts carry no hash',
|
||||
!str_contains(json_encode($users->accounts()), 'password') && !str_contains(json_encode($users->accounts()), '$2y$'),
|
||||
);
|
||||
check('one administrator counted', $users->adminCount() === 1, $users->adminCount());
|
||||
$users->setRole('fresh', 'admin');
|
||||
check('setRole makes an administrator', $users->role('fresh') === 'admin' && $users->adminCount() === 2);
|
||||
check('setRole on nobody answers false', $users->setRole('nobody', 'user') === false);
|
||||
|
||||
check('rename moves the account', $users->rename('fresh', 'renamed') && $users->exists('renamed') && !$users->exists('fresh'));
|
||||
check('rename keeps password and role', $users->verify('renamed', 'secret12') === 'renamed' && $users->role('renamed') === 'admin');
|
||||
check('rename onto a taken name is refused', throws(fn() => $users->rename('renamed', 'OLD')));
|
||||
check('the refused rename changed nothing', $users->exists('renamed') && $users->role('old') === 'user');
|
||||
check('rename to an invalid name is refused', throws(fn() => $users->rename('renamed', 'a b')));
|
||||
check('a change of case is the same account', $users->rename('renamed', 'Renamed') && $users->accounts()[0]['username'] === 'Renamed', $users->accounts());
|
||||
check('rename of nobody answers false', $users->rename('nobody', 'somebody') === false);
|
||||
check('remove takes the account away', $users->remove('renamed') && !$users->exists('renamed'));
|
||||
check('remove of nobody answers false', $users->remove('renamed') === false);
|
||||
|
||||
// --- Store: who reaches a project --------------------------------------------------------------
|
||||
|
||||
$projects = "{$dir}/projects";
|
||||
$mine = new Store($projects, 'anna');
|
||||
$doc = $mine->create('Anna\'s game');
|
||||
$id = $doc['id'];
|
||||
$stranger = new Store($projects, 'bert');
|
||||
$admin = new Store($projects, 'boss', true);
|
||||
|
||||
check('a stranger does not see the project', $stranger->load($id) === null);
|
||||
check('a stranger cannot save it', $stranger->save($id, 1, ['name' => 'x']) === null);
|
||||
check('a stranger cannot delete it', $stranger->delete($id) === false);
|
||||
check('an administrator loads it', $admin->load($id)['name'] === "Anna's game");
|
||||
$saved = $admin->save($id, 1, ['name' => 'Edited by boss']);
|
||||
check('an administrator saves it', $saved !== null && $saved['revision'] === 2, $saved);
|
||||
check('the save keeps the owner', $mine->load($id)['owner'] === 'anna', $mine->load($id)['owner'] ?? null);
|
||||
check('the owner sees the edit', $mine->load($id)['name'] === 'Edited by boss');
|
||||
check('the administrator\'s own list stays their own', $admin->list() === []);
|
||||
check('listOf lists somebody else\'s projects', array_column($admin->listOf('anna'), 'id') === [$id]);
|
||||
|
||||
check('transfer moves the project to the new name', $admin->transfer('anna', 'annabel') === 1);
|
||||
$moved = (new Store($projects, 'annabel'))->load($id);
|
||||
check('the new name owns it', $moved !== null && $moved['owner'] === 'annabel');
|
||||
check('the old name does not', $mine->load($id) === null);
|
||||
check('transfer keeps the revision', $moved['revision'] === 2, $moved['revision'] ?? null);
|
||||
|
||||
$other = (new Store($projects, 'carl'))->create('Carl\'s game');
|
||||
check('an administrator deletes somebody else\'s project', $admin->delete($other['id']) && $admin->listOf('carl') === []);
|
||||
|
||||
// --- AccountRules ------------------------------------------------------------------------------
|
||||
|
||||
check('renaming yourself is refused', AccountRules::renameRefusal('boss', 'BOSS') !== null);
|
||||
check('renaming somebody else is fine', AccountRules::renameRefusal('boss', 'anna') === null);
|
||||
check('changing your own role is refused', AccountRules::roleRefusal('boss', 'boss', 'admin', 'user', 2) !== null);
|
||||
check('setting your own role to what it is is fine', AccountRules::roleRefusal('boss', 'boss', 'admin', 'admin', 1) === null);
|
||||
check('the last administrator cannot be demoted', AccountRules::roleRefusal('x', 'boss', 'admin', 'user', 1) !== null);
|
||||
check('an administrator among two can be demoted', AccountRules::roleRefusal('x', 'boss', 'admin', 'user', 2) === null);
|
||||
check('deleting yourself is refused', AccountRules::deleteRefusal('boss', 'boss', 'admin', 2) !== null);
|
||||
check('the last administrator cannot be deleted', AccountRules::deleteRefusal('x', 'boss', 'admin', 1) !== null);
|
||||
check('a user can be deleted', AccountRules::deleteRefusal('boss', 'anna', 'user', 1) === null);
|
||||
check('an account with projects is not deleted', AccountRules::ownerRefusal('anna', 2) === 'anna still owns 2 projects');
|
||||
check('one project is said as one', AccountRules::ownerRefusal('anna', 1) === 'anna still owns 1 project');
|
||||
check('an account without projects may go', AccountRules::ownerRefusal('anna', 0) === null);
|
||||
|
||||
exec('rm -rf ' . escapeshellarg($dir));
|
||||
|
||||
echo $failed === 0 ? "ALL PASSED – {$passed} checks\n" : "{$failed} FAILED, {$passed} passed\n";
|
||||
exit($failed === 0 ? 0 : 1);
|
||||
@@ -6,6 +6,7 @@ import {
|
||||
GAME_ICON_SVG,
|
||||
LIST_ICON_SVG,
|
||||
PLUGINS_ICON_SVG,
|
||||
USERS_ICON_SVG,
|
||||
} from '../../src/editor/panels/assetIcons';
|
||||
import { HEADER_ICON_SVG } from '../../src/editor/panels/headerIcons';
|
||||
import { ARROW_ICON_SVG, KIND_ICON_SVG, ROW_ACTION_ICON_SVG } from '../../src/editor/panels/markIcons';
|
||||
@@ -44,6 +45,7 @@ describe('icon files', () => {
|
||||
['list tab', LIST_ICON_SVG],
|
||||
['game tab', GAME_ICON_SVG],
|
||||
['plugins tab', PLUGINS_ICON_SVG],
|
||||
['user management tab', USERS_ICON_SVG],
|
||||
...Object.entries(TOOLBAR_ICON_SVG).map(([name, svg]) => [`toolbar ${name}`, svg]),
|
||||
...Object.entries(HEADER_ICON_SVG).map(([name, svg]) => [`header ${name}`, svg]),
|
||||
...Object.entries(UI_ICON_SVG).map(([name, svg]) => [`ui ${name}`, svg]),
|
||||
@@ -124,12 +126,18 @@ describe('icon files', () => {
|
||||
});
|
||||
|
||||
it('draws the asset icons in the 16 units a list row and a button reserve', () => {
|
||||
for (const svg of [...Object.values(ASSET_ICON_SVG), LIST_ICON_SVG, GAME_ICON_SVG, PLUGINS_ICON_SVG])
|
||||
for (const svg of [
|
||||
...Object.values(ASSET_ICON_SVG),
|
||||
LIST_ICON_SVG,
|
||||
GAME_ICON_SVG,
|
||||
PLUGINS_ICON_SVG,
|
||||
USERS_ICON_SVG,
|
||||
])
|
||||
expect(tags(svg)[0]!.attrs.viewBox).toBe('-8 -8 16 16');
|
||||
});
|
||||
|
||||
it('gives the List, Game and Plugins tabs a picture no asset type has (#176, #119)', () => {
|
||||
const tabs = [LIST_ICON_SVG, GAME_ICON_SVG, PLUGINS_ICON_SVG];
|
||||
it('gives the List, Game, Plugins and User management tabs a picture no asset type has (#176, #119, #179)', () => {
|
||||
const tabs = [LIST_ICON_SVG, GAME_ICON_SVG, PLUGINS_ICON_SVG, USERS_ICON_SVG];
|
||||
for (const svg of tabs) expect(Object.values(ASSET_ICON_SVG)).not.toContain(svg);
|
||||
expect(new Set(tabs).size).toBe(tabs.length);
|
||||
});
|
||||
|
||||
Reference in new issue
Block a user