Approvals and Evidence 3 Approver and Reviewer Access - edmozley/freeitsm GitHub Wiki
Status: built in 3.8.0. Part 3 of the #180 design, kept as the record of what was decided and why. Where the build differs from what is written here, the overview says so. Anything marked today describes 3.7.0.
What #180 asks for:
- A business approver can read and decide the requests assigned to them without being able to edit form definitions, workflow rules or unrelated submissions. In the issue's words, it was not possible to configure "an approval-only role without form-design access".
- A reviewer can read permitted records, their history and the matching reports and exports, without creating, editing, approving or deleting anything - "a useful read-only reviewer role".
- Enforced on the server, in the UI and the API alike. Hiding buttons is not enough.
- Existing installs get a documented default.
The issue asks for roles. FreeITSM's Roles today can only hold settings permissions. This part is therefore mostly about giving Roles and module access the two things they lack: permissions for everyday work, and a read-only level.
Three layers (Roles & Permissions):
| Question | Answered by | Granularity today |
|---|---|---|
| Can they use the module? | Module access (analyst_modules, team_modules, the all modules flags) |
yes or no - no read-only level |
| Can they administer this part of its settings? | a Role holding a capability | one capability per settings tab |
| Can they run System? | is_admin |
yes or no |
Approving is decided by identity, not permission: a CAB vote needs a change_cab_members row (ChangesService::voteCab()), and a catalogue decision needs to be the submission's approver_id or an administrator (catalogueApprovalDecide() in includes/catalogue_approvals.php).
Roles are deliberately "not an action-level permission system for everyday work" (Roles & Permissions). In practice that line has already been crossed a few times, quietly, by capabilities that sit on a settings tab but guard everyday work:
| Capability | Declared as the tab | Actually also guards |
|---|---|---|
projects.manage |
the Projects settings umbrella | changing or deleting any project (projectIsManager()) |
lms.manage |
the LMS umbrella | authoring and assigning courses |
files.folders |
Files settings | take ownership of any folder |
war_room.manage |
the War Room umbrella | deleting other people's messages and channels |
knowledge.manage |
the Knowledge umbrella | the article-visibility floor |
So the need is real and already met, badly: each of those is an everyday permission described as a settings tab, which nobody would guess from the Roles screen.
-
Forms design is plain module access.
forms/settings/manifest.phpsays so on purpose: "Designing FORMS and reading submissions is the everyday job and stays on plain module access."api/forms/save_form.php→FormsService::saveForm()writes the title, fields, conditional logic, publishing, portal visibility, the approver, the approval switch and the approval actions behindrequireModuleAccessJson('forms')and nothing else. The same is true ofapi/forms/create_version.php,api/forms/delete_form.php,api/forms/audience.php,api/forms/upload_image.phpandapi/forms/ai_generate.php. -
An approver needs the whole Forms module to reach
forms/approvals.php(Catalogue Request Approvals says so). Giving it to them gives them the designer, every form's submissions (api/forms/get_submissions.phptakes anyform_id), and delete (api/forms/delete_submission.php). - There is no read-only module access. Giving a reviewer Change Management lets them edit, delete and vote.
A module's manifest already declares its settings tabs, and everything about capabilities is derived from manifests (Roles - Developer Guide). It gains a second list, for permissions that are not a tab:
// forms/settings/manifest.php
'actions' => [
[
'cap' => Cap::FORMS_DESIGN,
'grant' => 'Create, edit, publish and delete forms, including who approves them and what happens next',
'setting' => 'forms_design_restricted', // the switch that makes this permission bite (below)
],
],
// change-management/settings/manifest.php
'actions' => [
[
'cap' => Cap::CHANGES_CHOOSE_APPROVERS,
'grant' => 'Choose who approves a change: the approver, the CAB and whether it is required',
'setting' => 'change_approval_setup_by',
],
],Still two places and only two - the Cap:: constant and the manifest - so nothing in the architecture's central rule changes. capRegistry() reads actions as well as tabs; capSelfCheck() counts an action as a claim; settingsVisibleTabs() ignores actions, because they are not tabs. The module umbrella covers its actions as it covers its tabs.
System → Roles shows them under each module in their own group, Everyday work, above the settings tabs, so somebody building an Approver or Form designer role can see what they are ticking. The five existing capabilities in the table above are not moved in this work - moving a capability is a rename with aliases (Roles - Developer Guide), and a separate tidy-up. They are listed as a follow-up.
Layer 2 is deny-by-default: no role, no capability. That is right for settings, which are administration. It is wrong for an everyday permission added to an existing install: on the day Design forms appeared, every analyst who builds forms today would lose the ability, and nobody would know why.
So each action names a module setting, and the permission only applies once that setting is switched on:
// includes/rbac.php
/** May this analyst do an everyday-work action? True for everyone while its module has not restricted it. */
function analystMayAct(PDO $conn, int $analystId, string $cap): bool {
$action = capAction($cap); // from the manifests; fatal on an unknown Cap::
if (!capActionRestricted($conn, $action)) { // e.g. forms_design_restricted = 0 (default)
return true; // today's behaviour: module access is enough
}
return analystHasCapability($conn, $analystId, $cap); // is_admin still bypasses, as for every capability
}
function requireActionJson(string $cap): void { /* 403 unless analystMayAct() */ }The administrator turns the restriction on after building the role and giving it to the people who should keep the ability. The settings page says that, and shows how many analysts currently hold the permission, so "0 people will be able to design forms" is visible before the switch rather than discovered after it.
🔑 Why
is_adminmay bypass Design forms but must not bypass approving your own change (Part 1 §7): designing a form is a power, and administrators hold every power. Separation of duties is not a power; it is a rule about which powers one person may combine. An administrator who could approve their own change would make the rule meaningless.
Forms → Settings → Access (new tab, Cap::FORMS_ACCESS): Who can design forms - Everyone with Forms (default) / Only people with "Design forms".
With it on, requireActionJson(Cap::FORMS_DESIGN) guards:
| Endpoint | Why it is design |
|---|---|
api/forms/save_form.php |
fields, logic, publishing, portal visibility, approver and approval switch, submission actions, collection pairing - every per-form setting goes through it, including the forms list's partial updates (the shield and portal toggles) |
api/forms/create_version.php |
a new version |
api/forms/delete_form.php |
deletes the chain and its submissions |
api/forms/audience.php (POST) |
who may request the form |
api/forms/upload_image.php |
an image placed in the designer |
api/forms/ai_generate.php |
AI building a form, and spending the AI budget |
And, because the API must not be the way round it, the same check sits in FormsService::saveForm(), createVersion() and deleteForm() for an API call, using the key's analyst. POST /forms, PATCH /forms/{id} and DELETE /forms/{id} are refused (403 forbidden) for a key whose analyst may not design - but only once the restriction is on, so no existing key changes behaviour.
The pages follow: no New form button, the designer opens read-only, the shield/portal/collection controls on the list are not rendered. That is comfort; the endpoints are the control.
Workflow rules (#180 lists them too) are a separate module, Workflow, with its own access. An approver needs neither. A form's own What happens next actions are edited through api/forms/save_form.php, so they are design and covered above.
Today an approver must be given Forms. Instead, being named as an approver becomes enough to reach the approvals inbox, and nothing else:
// includes/catalogue_approvals.php
/** Forms access, OR named as the approver of a catalogue item or of any submission. */
function catalogueApproverCanEnter(PDO $conn, int $analystId): bool {
if (analystCanAccessModule($conn, $analystId, 'forms')) return true;
$st = $conn->prepare("SELECT 1 FROM forms WHERE approver_id = ? AND requires_approval = 1
UNION SELECT 1 FROM form_submissions WHERE approver_id = ? LIMIT 1");
$st->execute([$analystId, $analystId]);
return (bool)$st->fetchColumn();
}| Analyst with Forms | Approver without Forms | |
|---|---|---|
forms/approvals.php |
as today | allowed - the only Forms page that is |
| Inbox For me / Decided by me | as today | yes |
Inbox All pending (filter=all) |
as today | no - it shows every pending request and every answer in it |
Decide (api/forms/decide_catalogue_approval.php) |
own requests (admins: any), as today | own requests only - the existing approver_id check is unchanged |
| The designer, other forms, submissions, delete | as today | no - 302 / 403, as now |
The front door - the mistake this codebase keeps making is a finished feature nobody can reach. An approver without Forms gets Approvals in the waffle menu and on the landing page, pointing at forms/approvals.php, and the catalogue_request.submitted workflow event already carries the approver so an email can link straight there. Without this the approver would have the permission and no way to find it.
ALTER TABLE analyst_modules ADD access_level VARCHAR(10) NOT NULL DEFAULT 'full'; -- 'full' | 'read'
ALTER TABLE team_modules ADD access_level VARCHAR(10) NOT NULL DEFAULT 'full';The all modules flags mean full on everything, as today. A Read only grant is a row with access_level = 'read'. Every existing row is full, so on upgrade nobody's access changes at all.
getAnalystAllowedModules() already combines the analyst's own grants with each team's, under module_permission_mode (Module Access Control). Levels combine the same way:
| Mode | Rule | Own grant | Team A | Result |
|---|---|---|---|---|
most (default) |
the highest level any source gives | Read | Full | Full |
least |
the lowest level every source gives | Full | Read | Read |
One new resolver beside the old one, cached per request like it:
// includes/functions.php
/** 'full', 'read', or null (no access). The single answer to "what may they do in this module?" */
function analystModuleLevel(PDO $conn, int $analystId, string $moduleKey): ?string { … }
function analystCanWriteModule(PDO $conn, int $analystId, string $moduleKey): bool {
return analystModuleLevel($conn, $analystId, $moduleKey) === 'full';
}
function requireModuleWriteJson(string $moduleKey): void { /* 403 'You have read-only access to this module' */ }analystCanAccessModule() stays exactly as it is - read is access - so every existing page and read endpoint lets a reviewer in without being touched.
-
Every write endpoint of a converted module calls
requireModuleWriteJson()beside its existingrequireModuleAccessJson(). For Change Management that isapi/change-management/save.php,api/change-management/update_field.php,api/change-management/delete.php,api/change-management/save_comment.php,api/change-management/delete_comment.php,api/change-management/upload_attachment.php,api/change-management/delete_attachment.php,api/change-management/save_cab_members.php,api/change-management/link_ticket.php,api/change-management/unlink_ticket.php,api/change-management/move_to_company.php,api/change-management/send_share_email.php. -
The service layer, as a backstop.
ActorContextgainsreadOnlyModules, filled byfromSession()and byfromApiKey()from the analyst's levels. Every write method inChangesServiceandFormsServicestarts with$ctx->assertCanWrite('changes'). That is what makes the API obey it: a key whose analyst was given Read only cannotPATCHa change, even if the key itself was grantedchanges.update.
🔑 Why the backstop does not change any existing key. Today the API never consults the analyst's module access (F7 on the overview), and this design does not start refusing keys whose analyst has no access - that would be a separate decision with real consequences for integrations. It only refuses writes for an analyst who has been explicitly given Read only, which nobody has until an administrator chooses it.
A reviewer must not approve (#180). An approver must. The line between them is the same identity rule as today:
-
Voting on a change needs a
change_cab_membersrow, and deciding as the approver needs to be the change'sapprover_id- plus at least Read access to see what is being approved.voteCab()anddecideApproval()do not callassertCanWrite(). - So an administrator builds a change approver by giving somebody Read only on Change Management and naming them on changes; a reviewer is somebody with Read only who is never named. Nothing else is needed.
A module only offers Read only once every write endpoint has the write guard and its service has the backstop - otherwise Read only would be a promise with a hole in it. getModuleRegistry() gains a flag:
'changes' => [..., 'read_only' => true],
'forms' => [..., 'read_only' => true],System → Modules shows the Full / Read only choice only for flagged modules. Changes and Forms are first because they are what #180 evaluated. Each further module is a conversion like the Layer 2 roll-out - and, by the same experience, an audit of that module.
Read only in Forms means: see forms, their versions and every submission; export CSV and PDF; open the approvals inbox. Not: design, delete submissions, fill in a form (a submission is a record created), or decide - unless named as the approver.
Each converted page exposes window.MODULE_LEVEL = 'read' and a module-read-only class on <body>. The module's JS leaves out New, Edit, Delete, the status menu, comment and upload boxes; the change detail shows a small Read only badge so the reviewer knows it is deliberate rather than broken. Export and the evidence pack (Part 2 §6) stay.
The effective-access tool on System → Modules shows the level with its reason: "Read only - granted via team 'Auditors'", "Full - granted directly, which beats the team's Read only (mode: most)".
#180 wants the reviewer to see "corresponding reports/exports". A reviewer gets the Changes table view and its CSV export, the evidence export, and Forms' submission exports. Report Packs read module access through rpBlockData(), so Read only already passes there - but there are no Change or Forms blocks to read. Building them is out of scope (overview §6).
The two tools that already guard module access learn the new level:
-
tests/module-access-coverage.phpfails if any write-shaped endpoint in aread_onlymodule lacksrequireModuleWriteJson(), unless it is listed with a reason (the vote and decide endpoints are the only expected entries). -
D005 reports the same as High: "A read-only analyst could write here." Today D005 treats any
moduleguard as sufficient, which is exactly the blind spot a read-only level would fall into.
"An approval-only role without form-design access" - for Priya, who approves laptop requests:
- Forms → Settings → Access: tick Only people with "Design forms" - after giving your form builders a role with Design forms.
- Name Priya as the approver on the catalogue items.
- Do not give Priya the Forms module. She sees Approvals in her menu, her requests, and nothing else of Forms.
"A useful read-only reviewer role" - for an auditor, Alex:
- System → Modules → Change Management → give Alex (or the Auditors team) Read only. Same for Forms if needed.
- Alex can open every change in their companies, the approval rounds, every revision, the audit trail, and export the evidence. Every write, in the UI or through any API key acting as Alex, is refused.
A change approver - a manager who signs changes off but does not raise them: Read only on Change Management, and named on the changes' CAB or as their approver.
| What | On upgrade |
|---|---|
analyst_modules / team_modules
|
access_level added, every row full - nobody's access changes |
| Design forms | exists; restriction off - everybody who designs forms today still can |
| Choose approvers | exists; Anyone with Change Management - as today |
| Forms approvers without Forms | gain the approvals inbox - the only visible change, and it only adds |
| API keys | unchanged |
Colour key: 🗄️ schema · ⚙️ engine · 🔌 API · 🖥️ UI · 🌍 i18n · 📄 docs · 🧪 test
| 🎨 | File | What changes |
|---|---|---|
| 🗄️ |
includes/db_verify_schema.php · database/freeitsm.sql
|
access_level on analyst_modules and team_modules
|
| ⚙️ | includes/capabilities.php |
Cap::FORMS_DESIGN, Cap::FORMS_ACCESS, Cap::CHANGES_CHOOSE_APPROVERS; capRegistry() / capSelfCheck() / capGroups() read actions; capAction()
|
| ⚙️ | includes/rbac.php |
analystMayAct(), requireActionJson(), capActionRestricted()
|
| ⚙️ | includes/functions.php |
analystModuleLevel(), analystCanWriteModule(), requireModuleWriteJson(); getModuleRegistry() read_only flags |
| ⚙️ | includes/service_context.php |
ActorContext::$readOnlyModules, assertCanWrite()
|
| ⚙️ |
includes/services/changes.php · includes/services/forms.php
|
the backstop on every write; the design check in saveForm() / createVersion() / deleteForm()
|
| ⚙️ | includes/catalogue_approvals.php |
catalogueApproverCanEnter(); filter=all needs Forms access |
| 🔌 |
api/change-management/* (the twelve write endpoints in §4) · api/forms/* (the design endpoints in §3 and the other writes) |
the guards |
| 🔌 |
api/system/save_analyst_modules.php · api/system/save_module_grants.php · api/system/get_effective_module_access.php
|
levels saved and explained |
| 🖥️ |
forms/approvals.php · forms/includes/header.php · includes/waffle-menu.php · index.php
|
the approver's way in |
| 🖥️ |
forms/settings/manifest.php · forms/settings/index.php · change-management/settings/manifest.php
|
actions; the Access tab |
| 🖥️ |
system/modules/index.php · assets/js/system-roles.js · system/roles/index.php
|
the Full / Read only choice; Everyday work in the Roles picker |
| 🖥️ |
assets/js/change-management.js · forms/index.php · forms/edit/index.php · forms/submissions.php (their page scripts) |
MODULE_LEVEL, controls left out, the badge |
| 🧪 |
tests/module-access-coverage.php · api/system/debug-tools/D005_endpoint_permissions.php
|
the write-guard check |
| 🌍 |
lang/en/*.php (+ the locales) |
new strings |
| 📄 | wiki: Roles & Permissions, Roles - Developer Guide, Module Access Control, Module Access - Developer Guide, Catalogue Request Approvals, Forms | the model gains two concepts; every page that says roles are settings-only changes |
Three actors over real HTTP with forged sessions, as in Roles - Developer Guide, reading the response body every time:
| Actor | Must be able to | Must be refused |
|---|---|---|
| Administrator | everything | nothing - except approving their own change in Enforce mode |
| Read only on Changes | open, read history and revisions, export evidence; vote when named | every write endpoint in §4, and PATCH /changes/{id} with a key acting as them that holds changes.update
|
| Approver without Forms | the approvals inbox, decide their own requests |
filter=all, the designer, other forms' submissions, delete |
| Forms user, restriction on, no Design forms | fill in forms, read submissions, decide their own approvals | every design endpoint, and PATCH /forms/{id} through a key acting as them |
| Same, restriction off | design, as today | - (the negative control: the restriction is what refuses) |
Plus the coverage test and D005 run with a write guard deliberately removed from one endpoint, to prove each reports it.