From 5f47daa11c1b93b43440e3110811bdad8b747c08 Mon Sep 17 00:00:00 2001 From: Julien Durand <300495670+jdurand-teclib@users.noreply.github.com> Date: Thu, 5 Nov 2026 15:26:32 +0100 Subject: [PATCH 1/2] feat(46030): Add a language access policy to forms --- CHANGELOG.md | 1 + SPEC.md | 192 +++++++++ .../AccessControl/LanguageAccessControl.php | 196 +++++++++ .../LanguageAccessControlConfig.php | 68 ++++ src/Service/InitManager.php | 20 + src/Service/InstallManager.php | 13 + tasks/plan.md | 86 ++++ tasks/todo.md | 152 +++++++ templates/access_control/language.html.twig | 47 +++ tests/AdvancedFormsTestCase.php | 2 + .../LanguageAccessControlTest.php | 374 ++++++++++++++++++ tests/Service/InstallManagerTest.php | 39 ++ .../e2e/specs/language_access_control.spec.ts | 201 ++++++++++ 13 files changed, 1391 insertions(+) create mode 100644 SPEC.md create mode 100644 src/Model/AccessControl/LanguageAccessControl.php create mode 100644 src/Model/AccessControl/LanguageAccessControlConfig.php create mode 100644 tasks/plan.md create mode 100644 tasks/todo.md create mode 100644 templates/access_control/language.html.twig create mode 100644 tests/Model/AccessControl/LanguageAccessControlTest.php create mode 100644 tests/e2e/specs/language_access_control.spec.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 9456862..c1aefdf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ### Added - Add translations management through Transifex +- Add "Allow specific languages" form access policy ### Fixed diff --git a/SPEC.md b/SPEC.md new file mode 100644 index 0000000..d620b83 --- /dev/null +++ b/SPEC.md @@ -0,0 +1,192 @@ +# Spec: Language access control policy (#46030) + +## Objective + +As a GLPI form administrator, I want an **"Allow specific languages"** option in a +form's **Access controls** tab, so that I can choose the GLPI languages in which the form +is available. It grants access to users whose GLPI language is in the list and +**denies** everyone else, so the language is never ignored when combined with other +policies: "All users" + "English" means "all users whose GLPI is in English". + +Example: a user's GLPI is in French (`fr_FR`). The form's only active policy is +"Allow specific languages" with `en_GB` and `de_DE`. The form does not show for that +user in the service catalog or on helpdesk tiles, and opening its direct URL is refused. +The same user is still refused when another active policy (e.g. "All users") grants +them, because core blocks access as soon as one policy denies. + +Delivered entirely by the **advancedforms** plugin through GLPI core's existing extension +point `FormAccessControlManager::registerPluginAccessControlPolicy()`. GLPI core is not +modified. + +### Acceptance criteria + +1. The Access controls tab of a form shows a new policy, **"Allow specific languages"** + (translatable label, icon, active/inactive toggle like core policies). +2. Its config form is a multi-select of the languages in `Glpi\Locale\LanguageRegistry`. + The selection is stored as language codes (e.g. `fr_FR`). +3. Vote (core: any `Deny` blocks; otherwise at least one `Grant` is needed): + - current language **not in** list → `Deny`, authenticated or anonymous + - current language **in** list, other active policies on the form → `Abstain`: the + others decide who gets in, the language only narrows it ("group X" + French = French + users of group X; Direct access + French = token still required, logged in or not) + - current language **in** list, policy alone, authenticated → `Grant` + - current language **in** list, policy alone, anonymous → `Abstain` (see 5) +4. The "current language" is the session language `$_SESSION['glpilanguage']`, i.e. the + language GLPI is displayed in. +5. The policy never opens a form to anonymous users. `canAnswer()` never returns + `Grant` when `!$parameters->isAuthenticated()`, and `allowUnauthenticated()` + returns `false`. It still denies anonymous users whose language is not listed. The login redirect stays core's job. + The guard only stops a token bypass: when Direct access allows anonymous users, + an anonymous visitor without the token would otherwise be granted access through + their session language. +6. An active policy with an empty language list denies everyone, and `getWarnings()` + shows a warning in the Access controls tab. +7. Every place that calls core's `canAnswerForm()` enforces the policy automatically: + the service catalog, helpdesk tiles, form render/submit, and form documents. +8. Form export/import keeps the selected languages. Language codes are not DB ids, so + no mapping is needed. +9. The policy is registered whenever the plugin is active. There is **no** toggle on + the plugin config page: those toggles are for question types, and each form's + policy already has its own active switch. +10. Disabled by default: core creates any newly registered policy on each form with + `is_active => false` (`createMissingStrategyForForm`), so no extra code is needed. +11. When the plugin is deactivated, core ignores the saved rows + (`Form::getAccessControls()` skips strategies that are no longer valid), so forms + keep working. On reactivation the saved configs are used again. +12. Plugin uninstall deletes the `FormAccessControl` rows whose `strategy` is + `LanguageAccessControl`. This follows `InstallManager::uninstall()`, which already + removes the plugin's own data. + +### Non-goals + +- Restricting individual questions or sections by language +- Detecting the language from the browser's `Accept-Language` header beyond what GLPI + already puts in the session +- Translating form content (core already handles that) +- Any GLPI core change + +## Tech Stack + +PHP ≥ 8.2, GLPI 11/12 Forms (`Glpi\Form\AccessControl\*`), Twig, PHPUnit, Playwright. +No new dependency. + +## Commands + +All commands run from the plugin directory and go through `../../PluginsMakefile.mk` +(Docker): + +``` +Tests (single file): make phpunit c='tests/Model/AccessControl/LanguageAccessControlTest.php' +All lints/tests: make test +Static analysis: make phpstan +E2E: make playwright c='tests/e2e/specs/' +Locales: handled by CI (locales-update-source.yml on push to main) +License headers: make license-headers-check +``` + +## Project Structure + +``` +src/Model/AccessControl/LanguageAccessControl.php → NEW: ControlTypeInterface +src/Model/AccessControl/LanguageAccessControlConfig.php → NEW: JsonFieldInterface, holds string[] languages +templates/access_control/language.html.twig → NEW: config form (multi-select) +src/Service/InitManager.php → add registerAccessControlPolicies(), unconditional, guarded against double registration +src/Service/InstallManager.php → uninstall(): delete this policy's FormAccessControl rows +tests/Model/AccessControl/LanguageAccessControlTest.php → NEW +tests/Service/InitManagerTest.php / InstallManagerTest.php → extend +tests/e2e/specs/… → NEW: one happy path + one deny path +``` + +Core references to mirror: `src/Glpi/Form/AccessControl/ControlType/AllowList*.php` and +`templates/pages/admin/form/access_control/allow_list.html.twig`. + +## Code Style + +Follow the existing plugin: MIT license header, `final` classes, strict types, named +args, first-class callables, `__()` with domain `'advancedforms'`. + +```php +final class LanguageAccessControl implements ControlTypeInterface +{ + #[Override] + public function canAnswer( + Form $form, + JsonFieldInterface $config, + FormAccessParameters $parameters, + ): AccessVote { + if (!$config instanceof LanguageAccessControlConfig) { + throw new InvalidArgumentException('Invalid config class'); + } + + $language = $_SESSION['glpilanguage'] ?? null; + if (!in_array($language, $config->getLanguages(), true)) { + return AccessVote::Deny; + } + + if ($this->hasOtherActivePolicies($form)) { + return AccessVote::Abstain; + } + + return $parameters->isAuthenticated() ? AccessVote::Grant : AccessVote::Abstain; + } +} +``` + +`createConfigFromUserInput()` keeps only the codes that exist in +`Glpi\Locale\LanguageRegistry`. This is validation at the trust boundary. + +## Testing Strategy + +- **PHPUnit** (`tests/Model/AccessControl/`, extends `AdvancedFormsTestCase`): + - with other active policies, the language only restricts them: allow list + + language and direct access + language, logged in or not + - alone, authenticated: listed language → `Grant`; unlisted language or empty list → + `Deny`; anonymous: listed language → `Abstain`, unlisted → `Deny`; every list holds + several languages + - user input is cleaned: unknown codes are dropped, and a missing key gives an + empty list + - JSON serialize/deserialize round trip, and export/import round trip + - the warning shows when the list is empty + - integration, checked through `FormAccessControlManager::canAnswerForm()`, with + Language(`en_GB`, `de_DE`, `es_ES`): with or without "All users", the user can + answer in `en_GB` and is refused in `fr_FR` +- **InitManager test**: the policy is registered, and only once if `init()` runs again. + A new form gets the policy with `is_active = false`. +- **InstallManager test**: uninstall removes this policy's rows and leaves core + policies untouched. +- **Playwright**: configure the policy in the Access controls tab, then check that the + form shows or is hidden in the service catalog after switching the user's language. + +## Boundaries + +- **Always:** use core's plugin policy API, `make test` green before commit, license + headers, translatable strings, validate the selected languages server-side. +- **Ask first:** any DB schema or migration (none expected: config lives in core's + `glpi_forms_accesscontrols_formaccesscontrols.config` JSON), any GLPI core change, new + dependencies. +- **Never:** patch GLPI core, widen anonymous access, delete users' stored policy + configs, remove or skip existing tests. + +## Success Criteria + +- Acceptance criteria 1–12 pass, through PHPUnit and the Playwright spec. +- `make test` (phpunit, phpstan, psalm, rector, cs) passes. +- Manual check: French UI plus a form whose only active policy allows English → the + form is absent from the catalog, and its direct URL returns access denied. + +## Open Questions + +None. + +## Resolved (2026-11-05) + +- Vote: `Grant` when listed, `Deny` when not listed (decided 2026-11-05, replacing + `Grant`/`Abstain`). The language is never ignored when combined with other policies. +- Next to other active policies, a listed language gives `Abstain`, not `Grant` + (decided 2026-11-05): the policy works alone but never lets in users that another + policy refuses (e.g. outside group X, or without the direct access token). +- Label: "Allow specific languages". +- Anonymous users: never `Grant` (option B), so the policy can't be used to get + around Direct access's token; an unlisted language still gives `Deny`. +- No plugin config toggle: always registered while the plugin is active, and inactive + by default on each form through core's own default. diff --git a/src/Model/AccessControl/LanguageAccessControl.php b/src/Model/AccessControl/LanguageAccessControl.php new file mode 100644 index 0000000..80a66f3 --- /dev/null +++ b/src/Model/AccessControl/LanguageAccessControl.php @@ -0,0 +1,196 @@ +getAccessControls() as $control) { + if ( + $control->fields['is_active'] + && $control->getStrategy() instanceof self + && $this->getValidatedConfig($control->getConfig())->getLanguages() === [] + ) { + return [__('No language is selected in the "Allow specific languages" policy: it denies access to everyone.', 'advancedforms')]; + } + } + + return []; + } + + #[Override] + public function renderConfigForm(FormAccessControl $access_control): string + { + $twig = TemplateRenderer::getInstance(); + return $twig->render('@advancedforms/access_control/language.html.twig', [ + 'access_control' => $access_control, + 'config' => $this->getValidatedConfig($access_control->getConfig()), + 'languages' => Dropdown::getLanguages(), + 'input_name' => self::INPUT_NAME, + 'label' => $this->getLabel(), + ]); + } + + #[Override] + public function getWeight(): int + { + // After core policies (AllowList: 10, DirectAccess: 20). + return 30; + } + + /** @param array $input */ + #[Override] + public function createConfigFromUserInput(array $input): LanguageAccessControlConfig + { + $values = $input[self::INPUT_NAME] ?? []; + // An empty multi-select is posted as an empty string. + if (!is_array($values)) { + $values = []; + } + + // Trust boundary: keep only known GLPI language codes. + $languages = array_values(array_unique(array_filter( + $values, + fn($value): bool => is_string($value) && LanguageRegistry::has($value), + ))); + + return new LanguageAccessControlConfig($languages); + } + + #[Override] + public function canAnswer( + Form $form, + JsonFieldInterface $config, + FormAccessParameters $parameters, + ): AccessVote { + $config = $this->getValidatedConfig($config); + + // Deny (not Abstain) so the language restricts every other policy: + // "All users" + this policy means "all users using a listed language". + $language = $_SESSION['glpilanguage'] ?? null; + if (!in_array($language, $config->getLanguages(), true)) { + return AccessVote::Deny; + } + + // Next to other policies, only restrict: granting would let in users + // they refuse (e.g. outside the allowed group, or without the token). + if ($this->hasOtherActivePolicies($form)) { + return AccessVote::Abstain; + } + + // Never grant anonymous users: login stays the job of other policies. + return $parameters->isAuthenticated() ? AccessVote::Grant : AccessVote::Abstain; + } + + #[Override] + public function allowUnauthenticated(JsonFieldInterface $config): bool + { + return false; + } + + #[Override] + public function exportDynamicConfig(JsonFieldInterface $config): DynamicExportDataField + { + // Language codes are not database ids: no requirement to map on import. + return new DynamicExportDataField($config->jsonSerialize(), []); + } + + /** + * @param array $config + * @return array + */ + #[Override] + public static function prepareDynamicConfigDataForImport( + array $config, + DatabaseMapper $mapper, + ): array { + return $config; + } + + private function hasOtherActivePolicies(Form $form): bool + { + foreach ($form->getAccessControls() as $control) { + if ($control->fields['is_active'] && $control->fields['strategy'] !== self::class) { + return true; + } + } + + return false; + } + + private function getValidatedConfig(JsonFieldInterface $config): LanguageAccessControlConfig + { + if (!$config instanceof LanguageAccessControlConfig) { + throw new InvalidArgumentException('Invalid config class'); + } + + return $config; + } +} diff --git a/src/Model/AccessControl/LanguageAccessControlConfig.php b/src/Model/AccessControl/LanguageAccessControlConfig.php new file mode 100644 index 0000000..8200f89 --- /dev/null +++ b/src/Model/AccessControl/LanguageAccessControlConfig.php @@ -0,0 +1,68 @@ + $data */ + #[Override] + public static function jsonDeserialize(array $data): self + { + $languages = $data[self::KEY_LANGUAGES] ?? []; + return new self(is_array($languages) ? array_values(array_filter($languages, is_string(...))) : []); + } + + /** @return array{languages: string[]} */ + #[Override] + public function jsonSerialize(): array + { + return [self::KEY_LANGUAGES => $this->languages]; + } + + /** @return string[] */ + public function getLanguages(): array + { + return $this->languages; + } +} diff --git a/src/Service/InitManager.php b/src/Service/InitManager.php index d657c10..3fe733c 100644 --- a/src/Service/InitManager.php +++ b/src/Service/InitManager.php @@ -34,12 +34,14 @@ namespace GlpiPlugin\Advancedforms\Service; use Config; +use Glpi\Form\AccessControl\FormAccessControlManager; use Glpi\Form\Destination\FormDestinationManager; use Glpi\Form\Destination\FormDestinationTicket; use Glpi\Form\Migration\TypesConversionMapper; use Glpi\Form\QuestionType\QuestionTypesManager; use Glpi\Plugin\Hooks; use Glpi\Toolbox\SingletonTrait; +use GlpiPlugin\Advancedforms\Model\AccessControl\LanguageAccessControl; use GlpiPlugin\Advancedforms\Model\Config\ConfigTab; use GlpiPlugin\Advancedforms\Model\Destination\PreReservationField; use GlpiPlugin\Advancedforms\Model\QuestionType\AdvancedCategory; @@ -52,11 +54,15 @@ final class InitManager { use SingletonTrait; + /** Manager instance our access policies were registered on (core has no getter to check). */ + private ?FormAccessControlManager $policies_registered_on = null; + public function init(): void { $this->registerConfiguration(); $this->registerPluginTypes(); $this->registerDestinationFields(); + $this->registerAccessControlPolicies(); $this->registerTimelineHooks(); $this->registerNotifications(); $this->registerPurgeHooks(); @@ -140,6 +146,20 @@ private function registerDestinationFields(): void ); } + private function registerAccessControlPolicies(): void + { + $manager = FormAccessControlManager::getInstance(); + + // init() may run multiple times per process; a duplicate would create + // duplicate policy rows on new forms. + if ($this->policies_registered_on === $manager) { + return; + } + + $manager->registerPluginAccessControlPolicy(new LanguageAccessControl()); + $this->policies_registered_on = $manager; + } + private function registerTimelineHooks(): void { global $PLUGIN_HOOKS; diff --git a/src/Service/InstallManager.php b/src/Service/InstallManager.php index 5e56baa..ab05619 100644 --- a/src/Service/InstallManager.php +++ b/src/Service/InstallManager.php @@ -37,7 +37,9 @@ use Config; use DBConnection; use Glpi\DBAL\QueryExpression; +use Glpi\Form\AccessControl\FormAccessControl; use Glpi\Toolbox\SingletonTrait; +use GlpiPlugin\Advancedforms\Model\AccessControl\LanguageAccessControl; use GlpiPlugin\Advancedforms\Model\TicketReservationRequest; use Migration; use Notification; @@ -242,6 +244,7 @@ public function uninstall(): bool $config->deleteByCriteria(['context' => 'advancedforms']); $this->uninstallNotifications(); + $this->uninstallAccessControlPolicies(); $table = TicketReservationRequest::getTable(); if ($DB->tableExists($table, false)) { @@ -251,6 +254,16 @@ public function uninstall(): bool return true; } + /** Removes the plugin's access policies saved on forms (core ignores them once the plugin is gone). */ + public function uninstallAccessControlPolicies(): void + { + global $DB; + + $DB->delete(FormAccessControl::getTable(), [ + 'strategy' => LanguageAccessControl::class, + ]); + } + /** Removes every notification artifact seeded by installNotifications(). */ private function uninstallNotifications(): void { diff --git a/tasks/plan.md b/tasks/plan.md new file mode 100644 index 0000000..27b99ab --- /dev/null +++ b/tasks/plan.md @@ -0,0 +1,86 @@ +# Implementation Plan: Language access control policy (#46030) + +Spec: [`SPEC.md`](../SPEC.md). Task list: [`tasks/todo.md`](todo.md). + +## Overview + +Add an "Allow specific languages" form access policy to the advancedforms plugin through +GLPI core's `FormAccessControlManager::registerPluginAccessControlPolicy()`. It votes +`Grant` when the authenticated user's session language is in the configured list, and +`Deny` otherwise, so the language always restricts the other policies. No GLPI core +change and no DB schema change. + +## Architecture Decisions + +- **Mirror `AllowList` / `DirectAccess`**: two classes, the strategy + (`ControlTypeInterface`) and its config (`JsonFieldInterface`), plus one Twig template. + New namespace `GlpiPlugin\Advancedforms\Model\AccessControl`. +- **Language source**: `$_SESSION['glpilanguage']`. `SessionInfo` doesn't expose the + language, and the session language is what "GLPI is set in French" means. +- **Vote**: unlisted language → `Deny` (restricts every other policy); listed → + `Grant` if authenticated, `Abstain` if anonymous, so the policy can't be used to get + around Direct access's token. +- **Validation**: `createConfigFromUserInput()` keeps only codes that are keys of + `Glpi\Locale\LanguageRegistry` and removes duplicates. +- **Export**: `new DynamicExportDataField($config->jsonSerialize(), [])`, the same as + DirectAccess. There are no DB ids, so import passes the data through unchanged. +- **Registration**: unconditional in `InitManager`, guarded against double + registration if `init()` runs again, the same way `registerDestinationFields()` is. + Core already creates the policy inactive on each form. +- **Uninstall**: delete the `FormAccessControl` rows whose `strategy` is this class. + +## Dependency Graph + +``` +LanguageAccessControlConfig + │ +LanguageAccessControl (vote, validation, export, warnings) + │ + ├── template + InitManager registration ──→ visible in UI, enforced by canAnswerForm() + │ │ + │ └── Playwright spec + └── InstallManager uninstall cleanup +``` + +## Task List + +### Phase 1: Policy logic +- [x] Task 1: Config + strategy classes with vote, validation and export (unit tested) + +### Phase 2: Wire into GLPI +- [x] Task 2: Config form template + registration; integration through `canAnswerForm()` + +### Checkpoint A +- [x] `make phpunit` (new + existing) green, `make phpstan` clean +- [x] Manual: the policy appears inactive in a form's Access controls tab, saves the + languages, hides the form from the catalog for a non-listed language + +### Phase 3: Lifecycle and end-to-end +- [x] Task 3: Uninstall cleanup +- [x] Task 4: Playwright spec + +### Phase 4: Finish +- [x] Task 5: CHANGELOG, full `make test` (locales handled by CI workflows) + +### Checkpoint: Complete +- [x] SPEC acceptance criteria 1–12 met +- [x] `make test` green (except 8 LDAP tests needing an LDAP server) +- [x] Human review before commit/PR + +## Risks and Mitigations + +| Risk | Impact | Mitigation | +|------|--------|------------| +| Twig input naming must match what core's access-control form posts to `createConfigFromUserInput()` | High | Copy the field naming from core's `allow_list.html.twig`; the Task 2 integration test saves through the real form input path | +| Multi-select posts nothing when it is emptied, so the old value could survive | Med | A missing key → empty list; covered by a unit test | +| Language list for the select | Low | `$CFG_GLPI['languages']` is deprecated (11.0): use `Glpi\Locale\LanguageRegistry` (`has()` for validation, `all()` / `toLegacyArray()` for the select) | +| Tests depend on global `$_SESSION['glpilanguage']` | Low | Set and restore it in each test; `AdvancedFormsTestCase` resets session | +| Twig template path under the plugin namespace (`@advancedforms/...`) | Low | Same pattern as `ConfigManager::renderConfigForm()` | + +## Parallelization + +Task 3 only depends on Task 1 and can run alongside Task 2. Task 4 needs Task 2. + +## Open Questions + +None. diff --git a/tasks/todo.md b/tasks/todo.md new file mode 100644 index 0000000..691fd92 --- /dev/null +++ b/tasks/todo.md @@ -0,0 +1,152 @@ +# Todo: Language access control policy (#46030) + +See [`plan.md`](plan.md) and [`../SPEC.md`](../SPEC.md). + +## Task 1: Policy config and strategy classes + +**Description:** Create `LanguageAccessControlConfig` (holds `string[] $languages`, JSON +round trip) and `LanguageAccessControl` implementing `ControlTypeInterface`: label +"Allow specific languages", icon, weight, the vote, input validation, warnings for an +empty list, export/import, and `allowUnauthenticated() = false`. +`renderConfigForm()` may return an empty string until Task 2. + +**Acceptance criteria:** +- [x] Authenticated user: listed session language → `Grant`; not listed or empty + list → `Deny`. Anonymous: listed → `Abstain`, not listed → `Deny` +- [x] `createConfigFromUserInput()` drops unknown codes and duplicates; a missing key + gives an empty list +- [x] Config JSON round trip and export → import round trip keep the languages; + `getWarnings()` is non-empty only when the list is empty + +**Verification:** +- [x] `make phpunit c='tests/Model/AccessControl/LanguageAccessControlTest.php'` +- [x] `make phpstan` + +**Dependencies:** None + +**Files:** +- `src/Model/AccessControl/LanguageAccessControlConfig.php` (new) +- `src/Model/AccessControl/LanguageAccessControl.php` (new) +- `tests/Model/AccessControl/LanguageAccessControlTest.php` (new) + +**Scope:** S + +--- + +## Task 2: Config form and registration + +**Description:** Add the multi-select Twig template (input naming copied from core's +`allow_list.html.twig`), implement `renderConfigForm()`, and register the policy in +`InitManager::init()` through a new `registerAccessControlPolicies()` that is guarded +against double registration. + +**Acceptance criteria:** +- [x] The policy is registered once, even if `init()` runs twice; a new form gets it + with `is_active = false` +- [x] `canAnswerForm()` with Language(`en_GB`, `de_DE`, `es_ES`), with or without + "All users": true in `en_GB`, false in `fr_FR` +- [x] Token bypass guard: Direct access (anonymous allowed) + Language(`fr_FR`, `en_GB`, + `de_DE`), anonymous: `fr_FR` with no token → false; `fr_FR` with the token → + true; `es_ES` with the token → false +- [x] The config form renders and the saved languages are preselected + +**Verification:** +- [x] PHPUnit: registration/default-inactive tests live in `LanguageAccessControlTest` (not `InitManagerTest`) +- [ ] Manual (Checkpoint A): enable the policy on a form, select English, switch the + user to French → the form is gone from the service catalog and its direct URL + is refused + +**Dependencies:** Task 1 + +**Files:** +- `templates/access_control/language.html.twig` (new) +- `src/Model/AccessControl/LanguageAccessControl.php` +- `src/Service/InitManager.php` +- `tests/AdvancedFormsTestCase.php` (reset `FormAccessControlManager` singleton per test) +- `tests/Model/AccessControl/LanguageAccessControlTest.php` + +**Scope:** M + +--- + +## Checkpoint A +- [x] All PHPUnit tests green (except 8 LDAP tests needing an LDAP server), `make phpstan` clean +- [ ] Manual UI check above done +- [ ] Review with human before continuing + +--- + +## Task 3: Uninstall cleanup + +**Description:** `InstallManager::uninstall()` calls the new public +`uninstallAccessControlPolicies()`, tested directly (calling `uninstall()` in a test +would drop a table, which commits the test transaction). It deletes the `FormAccessControl` rows whose +`strategy` is `LanguageAccessControl::class`. + +**Acceptance criteria:** +- [x] After uninstall, no rows for this strategy remain +- [x] Core policy rows (AllowList, DirectAccess) are left alone + +**Verification:** +- [x] `make phpunit c='tests/Service/InstallManagerTest.php'` + +**Dependencies:** Task 1 + +**Files:** +- `src/Service/InstallManager.php` +- `tests/Service/InstallManagerTest.php` + +**Scope:** XS + +--- + +## Task 4: Playwright spec + +**Description:** E2E test that configures "Allow specific languages" in the Access +controls tab, then checks catalog visibility for a listed and a non-listed user +language. + +**Acceptance criteria:** +- [x] Policy toggled on and a language saved through the UI +- [x] Listed language → form visible in the service catalog; non-listed → hidden + +**Verification:** +- [x] `make playwright c='language_access_control'` (2 passed) + +**Dependencies:** Task 2 + +**Files:** +- `tests/e2e/specs/language_access_control.spec.ts` (new) +- `tests/e2e/pages/…` (only if a page object is needed) + +**Scope:** S + +--- + +## Task 5: Changelog and full check + +**Description:** Add a CHANGELOG entry and run the full quality gate. Locale extraction +is not needed: the `locales-update-source.yml` workflow regenerates +`locales/advancedforms.pot` on every push to `main`, and `locales-sync.yml` pulls +translations back. + +**Acceptance criteria:** +- [x] CHANGELOG "Unreleased" entry for #46030 +- [x] `make test` and `make license-headers-check` green (except 8 LDAP tests needing an LDAP server) + +**Verification:** +- [x] `make test && make license-headers-check` + +**Dependencies:** Tasks 1–4 + +**Files:** +- `CHANGELOG.md` + +**Scope:** XS + +--- + +## Checkpoint: Complete +- [x] SPEC acceptance criteria 1–12 met +- [x] `make test` green (except 8 LDAP tests needing an LDAP server) +- [ ] Human review before commit/PR diff --git a/templates/access_control/language.html.twig b/templates/access_control/language.html.twig new file mode 100644 index 0000000..92b5d56 --- /dev/null +++ b/templates/access_control/language.html.twig @@ -0,0 +1,47 @@ +{# + # ------------------------------------------------------------------------- + # advancedforms plugin for GLPI + # ------------------------------------------------------------------------- + # + # MIT License + # + # Permission is hereby granted, free of charge, to any person obtaining a copy + # of this software and associated documentation files (the "Software"), to deal + # in the Software without restriction, including without limitation the rights + # to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + # copies of the Software, and to permit persons to whom the Software is + # furnished to do so, subject to the following conditions: + # + # The above copyright notice and this permission notice shall be included in all + # copies or substantial portions of the Software. + # + # THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + # IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + # FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + # AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + # LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + # SOFTWARE. + # ------------------------------------------------------------------------- + # @copyright Copyright (C) 2025 by the advancedforms plugin team. + # @license MIT https://opensource.org/licenses/mit-license.php + # @link https://github.com/pluginsGLPI/advancedforms + # ------------------------------------------------------------------------- + #} + +{# @var \Glpi\Form\AccessControl\FormAccessControl access_control #} +{# @var \GlpiPlugin\Advancedforms\Model\AccessControl\LanguageAccessControlConfig config #} +{# @var array languages #} +{# @var string label #} + +{{ call('Dropdown::showFromArray', [ + access_control.getNormalizedInputName(input_name), + languages, + { + 'multiple' : true, + 'values' : config.getLanguages(), + 'aria_label': label, + 'width' : '100%', + 'display' : false, + } +])|raw }} diff --git a/tests/AdvancedFormsTestCase.php b/tests/AdvancedFormsTestCase.php index e259224..d2098ca 100644 --- a/tests/AdvancedFormsTestCase.php +++ b/tests/AdvancedFormsTestCase.php @@ -34,6 +34,7 @@ namespace GlpiPlugin\Advancedforms\Tests; use AuthLDAP; +use Glpi\Form\AccessControl\FormAccessControlManager; use Glpi\Form\Form; use Glpi\Form\Migration\TypesConversionMapper; use Glpi\Form\QuestionType\QuestionTypeInterface; @@ -66,6 +67,7 @@ public function setUp(): void $this->deleteSingletonInstance([ QuestionTypesManager::class, TypesConversionMapper::class, + FormAccessControlManager::class, ]); } diff --git a/tests/Model/AccessControl/LanguageAccessControlTest.php b/tests/Model/AccessControl/LanguageAccessControlTest.php new file mode 100644 index 0000000..feb6e55 --- /dev/null +++ b/tests/Model/AccessControl/LanguageAccessControlTest.php @@ -0,0 +1,374 @@ + */ + public static function provideVotes(): iterable + { + yield 'listed language' => [['en_GB', 'fr_FR', 'de_DE'], 'fr_FR', true, AccessVote::Grant]; + yield 'unlisted language' => [['en_GB', 'de_DE', 'es_ES'], 'fr_FR', true, AccessVote::Deny]; + yield 'empty list' => [[], 'fr_FR', true, AccessVote::Deny]; + yield 'anonymous, listed language' => [['en_GB', 'fr_FR', 'de_DE'], 'fr_FR', false, AccessVote::Abstain]; + yield 'anonymous, unlisted language' => [['en_GB', 'de_DE', 'es_ES'], 'fr_FR', false, AccessVote::Deny]; + } + + /** @param string[] $languages */ + #[DataProvider('provideVotes')] + public function testCanAnswer( + array $languages, + string $session_language, + bool $authenticated, + AccessVote $expected, + ): void { + $this->login(); + $_SESSION['glpilanguage'] = $session_language; + $parameters = $authenticated + ? new FormAccessParameters(session_info: Session::getCurrentSessionInfo()) + : new FormAccessParameters(); + + $vote = (new LanguageAccessControl())->canAnswer( + new Form(), + new LanguageAccessControlConfig($languages), + $parameters, + ); + + $this->assertSame($expected, $vote); + } + + public function testNeverAllowsUnauthenticated(): void + { + $this->assertFalse((new LanguageAccessControl())->allowUnauthenticated( + new LanguageAccessControlConfig(['fr_FR', 'en_GB', 'de_DE']), + )); + } + + /** @return iterable, string[]}> */ + public static function provideUserInput(): iterable + { + $key = LanguageAccessControl::INPUT_NAME; + yield 'valid codes' => [[$key => ['fr_FR', 'en_GB', 'de_DE']], ['fr_FR', 'en_GB', 'de_DE']]; + yield 'unknown code dropped' => [[$key => ['fr_FR', 'xx_XX', 'en_GB', '']], ['fr_FR', 'en_GB']]; + yield 'duplicates removed' => [[$key => ['fr_FR', 'en_GB', 'fr_FR']], ['fr_FR', 'en_GB']]; + yield 'non string dropped' => [[$key => ['fr_FR', ['en_GB'], 12, 'de_DE']], ['fr_FR', 'de_DE']]; + yield 'empty select' => [[$key => ''], []]; + yield 'missing key' => [[], []]; + } + + /** + * @param array $input + * @param string[] $expected + */ + #[DataProvider('provideUserInput')] + public function testCreateConfigFromUserInput(array $input, array $expected): void + { + $config = (new LanguageAccessControl())->createConfigFromUserInput($input); + $this->assertSame($expected, $config->getLanguages()); + } + + public function testConfigJsonRoundTrip(): void + { + $config = new LanguageAccessControlConfig(['fr_FR', 'en_GB']); + $copy = LanguageAccessControlConfig::jsonDeserialize($config->jsonSerialize()); + $this->assertSame(['fr_FR', 'en_GB'], $copy->getLanguages()); + $this->assertSame([], LanguageAccessControlConfig::jsonDeserialize([])->getLanguages()); + } + + public function testExportImportKeepsLanguages(): void + { + $this->login(); + $form = $this->createForm( + (new FormBuilder())->addAccessControl( + LanguageAccessControl::class, + new LanguageAccessControlConfig(['fr_FR', 'en_GB']), + ), + ); + + $copy = $this->exportAndImportForm($form); + + $this->assertSame(['fr_FR', 'en_GB'], $this->getAccessControl($copy, LanguageAccessControl::class)->getConfig()->getLanguages()); + } + + /** @return iterable */ + public static function provideWarnings(): iterable + { + yield 'active, empty list' => [[], true, true]; + yield 'active, with language' => [['fr_FR', 'en_GB'], true, false]; + yield 'inactive, empty list' => [[], false, false]; + yield 'inactive, with language' => [['fr_FR', 'en_GB'], false, false]; + } + + /** @param string[] $languages */ + #[DataProvider('provideWarnings')] + public function testWarnings(array $languages, bool $is_active, bool $has_warning): void + { + $this->login(); + $form = $this->createForm( + (new FormBuilder())->addAccessControl( + LanguageAccessControl::class, + new LanguageAccessControlConfig($languages), + $is_active, + ), + ); + + // Go through core: this is the list displayed in the Access controls tab, + // so it also fails if the policy is no longer registered. + InitManager::getInstance()->init(); + $warnings = FormAccessControlManager::getInstance()->getWarnings($form); + + $expected = __('No language is selected in the "Allow specific languages" policy: it denies access to everyone.', 'advancedforms'); + $this->assertSame($has_warning, in_array($expected, $warnings, true)); + } + + /** @return iterable */ + public static function provideCanAnswerForm(): iterable + { + yield 'listed language, language policy only' => ['en_GB', false, true]; + yield 'unlisted language, language policy only' => ['fr_FR', false, false]; + yield 'listed language, "All users" + language' => ['en_GB', true, true]; + yield 'unlisted language, "All users" + language' => ['fr_FR', true, false]; + } + + #[DataProvider('provideCanAnswerForm')] + public function testCanAnswerFormThroughCore( + string $session_language, + bool $with_all_users, + bool $expected, + ): void { + $this->login(); + $_SESSION['glpilanguage'] = $session_language; + $builder = (new FormBuilder()) + ->setUseDefaultAccessPolicies(false) + ->addAccessControl( + LanguageAccessControl::class, + new LanguageAccessControlConfig(['en_GB', 'de_DE', 'es_ES']), + ); + if ($with_all_users) { + $builder->addAccessControl(AllowList::class, new AllowListConfig( + user_ids: [AbstractRightsDropdown::ALL_USERS], + )); + } + + $form = $this->createForm($builder); + + $can_answer = FormAccessControlManager::getInstance()->canAnswerForm( + $form, + new FormAccessParameters(session_info: Session::getCurrentSessionInfo()), + ); + + $this->assertSame($expected, $can_answer); + } + + /** @return iterable */ + public static function provideAllowListAndLanguage(): iterable + { + yield 'allowed user, listed language' => [true, 'fr_FR', true]; + yield 'allowed user, unlisted language' => [true, 'es_ES', false]; + yield 'other user, listed language' => [false, 'fr_FR', false]; + yield 'other user, unlisted language' => [false, 'es_ES', false]; + } + + /** Next to another policy, the language only restricts it: it never lets in users the other policy refuses. */ + #[DataProvider('provideAllowListAndLanguage')] + public function testAllowListAndLanguage( + bool $in_allow_list, + string $session_language, + bool $expected, + ): void { + $this->login(); + $current_user_id = Session::getLoginUserID(); + $other_user_id = getItemByTypeName(User::class, 'tech', true); + $form = $this->createForm( + (new FormBuilder()) + ->setUseDefaultAccessPolicies(false) + ->addAccessControl(AllowList::class, new AllowListConfig( + user_ids: [$in_allow_list ? $current_user_id : $other_user_id], + )) + ->addAccessControl( + LanguageAccessControl::class, + new LanguageAccessControlConfig(['fr_FR', 'en_GB', 'de_DE']), + ), + ); + $_SESSION['glpilanguage'] = $session_language; + + $can_answer = FormAccessControlManager::getInstance()->canAnswerForm( + $form, + new FormAccessParameters(session_info: Session::getCurrentSessionInfo()), + ); + + $this->assertSame($expected, $can_answer); + } + + /** @return iterable, string, bool}> */ + public static function provideDirectAccess(): iterable + { + // Same rules through the direct link, logged in or not. + foreach (['authenticated' => true, 'anonymous' => false] as $who => $authenticated) { + yield "$who, listed language, no token" => [$authenticated, [], 'fr_FR', false]; + yield "$who, listed language, with token" => [$authenticated, ['token' => 'my_token'], 'fr_FR', true]; + yield "$who, unlisted language, with token" => [$authenticated, ['token' => 'my_token'], 'es_ES', false]; + } + } + + /** @param array $url_parameters */ + #[DataProvider('provideDirectAccess')] + public function testDirectAccess( + bool $authenticated, + array $url_parameters, + string $session_language, + bool $expected, + ): void { + $this->login(); + $form = $this->createForm( + (new FormBuilder()) + ->setUseDefaultAccessPolicies(false) + ->addAccessControl(DirectAccess::class, new DirectAccessConfig( + token: 'my_token', + allow_unauthenticated: true, + )) + ->addAccessControl( + LanguageAccessControl::class, + new LanguageAccessControlConfig(['fr_FR', 'en_GB', 'de_DE']), + ), + ); + if (!$authenticated) { + $this->logOut(); + } + + $_SESSION['glpilanguage'] = $session_language; + + $can_answer = FormAccessControlManager::getInstance()->canAnswerForm( + $form, + new FormAccessParameters( + session_info: $authenticated ? Session::getCurrentSessionInfo() : null, + url_parameters: $url_parameters, + ), + ); + + $this->assertSame($expected, $can_answer); + } + + public function testRenderConfigFormPreselectsSavedLanguages(): void + { + $this->login(); + $form = $this->createForm( + (new FormBuilder())->addAccessControl( + LanguageAccessControl::class, + new LanguageAccessControlConfig(['fr_FR', 'de_DE']), + ), + ); + $control = $this->getAccessControl($form, LanguageAccessControl::class); + + $html = new Crawler((new LanguageAccessControl())->renderConfigForm($control)); + + $name = $control->getNormalizedInputName(LanguageAccessControl::INPUT_NAME) . '[]'; + $select = $html->filter(sprintf('select[name="%s"][multiple]', $name)); + $this->assertCount(1, $select); + $this->assertEqualsCanonicalizing(['fr_FR', 'de_DE'], $select->filter('option[selected]')->each( + fn(Crawler $option): string => (string) $option->attr('value'), + )); + } + + public function testSaveThroughCoreInputPath(): void + { + $this->login(); + $form = $this->createForm( + (new FormBuilder())->addAccessControl( + LanguageAccessControl::class, + new LanguageAccessControlConfig([]), + ), + ); + $control = $this->getAccessControl($form, LanguageAccessControl::class); + + // Same steps as front/form/access_control.form.php + $input = ['id' => $control->getID(), LanguageAccessControl::INPUT_NAME => ['en_GB', 'xx_XX', 'de_DE']]; + $input['_config'] = $control->createConfigFromUserInput($input); + $this->assertTrue($control->update($input)); + + $control->getFromDB($control->getID()); + $this->assertSame(['en_GB', 'de_DE'], $control->getConfig()->getLanguages()); + } + + public function testPolicyIsRegisteredOnceAndInactiveByDefault(): void + { + $this->login(); + InitManager::getInstance()->init(); + InitManager::getInstance()->init(); + $form = $this->createForm(new FormBuilder()); + + FormAccessControlManager::getInstance()->createMissingAccessControlsForForm($form); + + $rows = (new FormAccessControl())->find([ + Form::getForeignKeyField() => $form->getID(), + 'strategy' => LanguageAccessControl::class, + ]); + $this->assertCount(1, $rows); + $this->assertSame(0, (int) current($rows)['is_active']); + } +} diff --git a/tests/Service/InstallManagerTest.php b/tests/Service/InstallManagerTest.php index a58b060..441cb17 100644 --- a/tests/Service/InstallManagerTest.php +++ b/tests/Service/InstallManagerTest.php @@ -34,6 +34,14 @@ namespace GlpiPlugin\Advancedforms\Tests\Service; use Config; +use Glpi\Form\AccessControl\ControlType\AllowList; +use Glpi\Form\AccessControl\ControlType\AllowListConfig; +use Glpi\Form\AccessControl\ControlType\DirectAccess; +use Glpi\Form\AccessControl\ControlType\DirectAccessConfig; +use Glpi\Form\AccessControl\FormAccessControl; +use Glpi\Tests\FormBuilder; +use GlpiPlugin\Advancedforms\Model\AccessControl\LanguageAccessControl; +use GlpiPlugin\Advancedforms\Model\AccessControl\LanguageAccessControlConfig; use GlpiPlugin\Advancedforms\Model\TicketReservationRequest; use GlpiPlugin\Advancedforms\Service\InstallManager; use GlpiPlugin\Advancedforms\Tests\AdvancedFormsTestCase; @@ -109,4 +117,35 @@ public function testInstallNotificationsAreIdempotent(): void ), ); } + + public function testUninstallRemovesOnlyLanguagePolicies(): void + { + // Arrange: two forms using the language policy next to core policies + $this->login(); + $forms = []; + foreach ([['fr_FR', 'en_GB'], []] as $languages) { + $forms[] = $this->createForm( + (new FormBuilder()) + ->setUseDefaultAccessPolicies(false) + ->addAccessControl(AllowList::class, new AllowListConfig(user_ids: [1])) + ->addAccessControl(DirectAccess::class, new DirectAccessConfig()) + ->addAccessControl(LanguageAccessControl::class, new LanguageAccessControlConfig($languages)), + ); + } + + $form_ids = array_map(fn($form) => $form->getID(), $forms); + $count = fn(string $strategy): int => countElementsInTable(FormAccessControl::getTable(), [ + 'forms_forms_id' => $form_ids, + 'strategy' => $strategy, + ]); + $this->assertSame(2, $count(LanguageAccessControl::class)); + + // Act + InstallManager::getInstance()->uninstallAccessControlPolicies(); + + // Assert: language rows are gone, core policies are untouched + $this->assertSame(0, $count(LanguageAccessControl::class)); + $this->assertSame(2, $count(AllowList::class)); + $this->assertSame(2, $count(DirectAccess::class)); + } } diff --git a/tests/e2e/specs/language_access_control.spec.ts b/tests/e2e/specs/language_access_control.spec.ts new file mode 100644 index 0000000..3d0d222 --- /dev/null +++ b/tests/e2e/specs/language_access_control.spec.ts @@ -0,0 +1,201 @@ +/** + * ------------------------------------------------------------------------- + * advancedforms plugin for GLPI + * ------------------------------------------------------------------------- + * + * MIT License + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + * ------------------------------------------------------------------------- + * @copyright Copyright (C) 2025 by the advancedforms plugin team. + * @license MIT https://opensource.org/licenses/mit-license.php + * @link https://github.com/pluginsGLPI/advancedforms + * ------------------------------------------------------------------------- + */ + +import { randomUUID } from 'crypto'; +import type { Browser, Page } from '@playwright/test'; +import { test, expect } from '../fixtures/advancedforms_fixture'; +import { FormPage } from '../../../../../tests/e2e/pages/FormPage'; +import { LoginPage } from '../../../../../tests/e2e/pages/LoginPage'; +import type { Api } from '../../../../../tests/e2e/utils/Api'; +import { Profiles } from '../../../../../tests/e2e/utils/Profiles'; +import { + getWorkerEntityId, + getWorkerLogin, + getWorkerUserId, +} from '../../../../../tests/e2e/utils/WorkerEntities'; + +const POLICY = 'Allow specific languages'; +const ALLOW_LIST = 'Allow specifics users, groups or profiles'; +const EMPTY_WARNING = 'No language is selected in the "Allow specific languages" policy: it denies access to everyone.'; + +/** Create an active form without any access policy and open its access controls tab. */ +async function createFormAndOpenAccessControls( + form: FormPage, + api: Api, + name: string, +): Promise { + const form_id = await api.createItem('Glpi\\Form\\Form', { + 'name': name, + 'is_active': true, + '_init_access_policies': false, + 'entities_id': getWorkerEntityId(), + }); + + const tab = 'Glpi\\Form\\AccessControl\\FormAccessControl$1'; + await form.page.goto(`/front/form/form.form.php?id=${form_id}&forcetab=${tab}`); + + return form_id; +} + +async function activatePolicy(form: FormPage, name: string): Promise { + const active = form.getRegion(name).getByRole('checkbox', { name: 'Active', exact: true }); + await expect(active).not.toBeChecked(); + await active.click(); +} + +async function selectLanguages(form: FormPage, languages: string[]): Promise { + for (const language of languages) { + await form.doSetDropdownValue(form.getDropdownByLabel(POLICY), language, true, false); + } +} + +async function saveAccessControls(page: Page): Promise { + await page.getByRole('button', { name: 'Save changes' }).click(); +} + +/** + * Log in again as the worker user, with the given language, in a session of + * its own: the language of a session is read when it is created, and a + * separate browser context keeps the session of the worker untouched. + * Self-Service is used as form administrators bypass access policies. + */ +async function openSessionWithLanguage(browser: Browser, api: Api, language: string): Promise { + await api.updateItem('User', getWorkerUserId(), { 'language': language }); + + const context = await browser.newContext({ storageState: { cookies: [], origins: [] } }); + const page = await context.newPage(); + + const login_page = new LoginPage(page); + await login_page.goto(); + await login_page.doLogin(getWorkerLogin(), getWorkerLogin()); + await page.goto('/'); + + // Labels are translated, so the profile is changed without the menu. + const csrf_token = await page.evaluate( + () => document + .querySelector('meta[property="glpi:csrf_token"]') + ?.getAttribute('content') ?? '' + ); + const response = await page.request.post('/Session/ChangeProfile', { + form: { id: Profiles.SelfService }, + headers: { + 'X-Requested-With': 'XMLHttpRequest', + 'X-Glpi-Csrf-Token': csrf_token, + }, + }); + expect(response.ok()).toBe(true); + + return page; +} + +/** Whether the form is listed in the service catalog, and the HTTP status of its direct URL. */ +async function getFormAccess( + page: Page, + form_id: number, + form_name: string, +): Promise<{ in_catalog: boolean, render_status: number | undefined }> { + // Query the catalog items directly: the search box label is translated. + const items = await page.request.get('/ServiceCatalog/Items', { + params: { filter: form_name }, + }); + expect(items.ok()).toBe(true); + const in_catalog = (await items.text()).includes(form_name); + + const response = await page.goto(`/Form/Render/${form_id}`); + await page.context().close(); + + return { in_catalog, render_status: response?.status() }; +} + +test.describe('Advanced forms - Language access control', () => { + // The language of the worker user is shared by every test of this worker. + test.afterEach(async ({ profile, api }) => { + await profile.set(Profiles.SuperAdmin); + await api.updateItem('User', getWorkerUserId(), { 'language': '' }); + }); + + test('can configure the policy and warns when no language is selected', async ({ + page, + profile, + api, + }) => { + await profile.set(Profiles.SuperAdmin); + const form = new FormPage(page); + await createFormAndOpenAccessControls(form, api, `[Tests] Language policy ${randomUUID()}`); + + // Active without any language: the form is closed to everyone, say so. + await activatePolicy(form, POLICY); + await saveAccessControls(page); + await expect(page.getByRole('alert').filter({ hasText: EMPTY_WARNING })).toBeVisible(); + + // Select several languages and save. + await selectLanguages(form, ['English', 'Deutsch']); + await saveAccessControls(page); + + await expect( + form.getRegion(POLICY).getByRole('checkbox', { name: 'Active', exact: true }) + ).toBeChecked(); + const dropdown = form.getDropdownByLabel(POLICY); + await expect(dropdown).toContainText('English'); + await expect(dropdown).toContainText('Deutsch'); + await expect(page.getByRole('alert').filter({ hasText: EMPTY_WARNING })).toHaveCount(0); + }); + + test('"All users" + languages only lets in users with a listed language', async ({ + page, + browser, + profile, + api, + }) => { + await profile.set(Profiles.SuperAdmin); + const form = new FormPage(page); + const form_name = `[Tests] Language policy ${randomUUID()}`; + const form_id = await createFormAndOpenAccessControls(form, api, form_name); + + await activatePolicy(form, ALLOW_LIST); + await form.doSetDropdownValue(form.getDropdownByLabel(ALLOW_LIST), 'All users'); + await activatePolicy(form, POLICY); + await selectLanguages(form, ['English', 'Deutsch']); + await saveAccessControls(page); + await expect(form.getDropdownByLabel(POLICY)).toContainText('English'); + + // Listed language: visible in the catalog and can be opened. + const english = await openSessionWithLanguage(browser, api, 'en_GB'); + expect(await getFormAccess(english, form_id, form_name)) + .toEqual({ in_catalog: true, render_status: 200 }); + + // Unlisted language: hidden from the catalog and the direct URL is refused, + // although "All users" grants access. + const french = await openSessionWithLanguage(browser, api, 'fr_FR'); + expect(await getFormAccess(french, form_id, form_name)) + .toEqual({ in_catalog: false, render_status: 403 }); + }); +}); From 5dd4a423d87e7ee6a9476b53227bdca8f423a3e1 Mon Sep 17 00:00:00 2001 From: Julien Durand <300495670+jdurand-teclib@users.noreply.github.com> Date: Thu, 5 Nov 2026 15:28:56 +0100 Subject: [PATCH 2/2] chore(46030): Removed unwanted files --- SPEC.md | 192 -------------------------------------------------- tasks/plan.md | 86 ---------------------- tasks/todo.md | 152 --------------------------------------- 3 files changed, 430 deletions(-) delete mode 100644 SPEC.md delete mode 100644 tasks/plan.md delete mode 100644 tasks/todo.md diff --git a/SPEC.md b/SPEC.md deleted file mode 100644 index d620b83..0000000 --- a/SPEC.md +++ /dev/null @@ -1,192 +0,0 @@ -# Spec: Language access control policy (#46030) - -## Objective - -As a GLPI form administrator, I want an **"Allow specific languages"** option in a -form's **Access controls** tab, so that I can choose the GLPI languages in which the form -is available. It grants access to users whose GLPI language is in the list and -**denies** everyone else, so the language is never ignored when combined with other -policies: "All users" + "English" means "all users whose GLPI is in English". - -Example: a user's GLPI is in French (`fr_FR`). The form's only active policy is -"Allow specific languages" with `en_GB` and `de_DE`. The form does not show for that -user in the service catalog or on helpdesk tiles, and opening its direct URL is refused. -The same user is still refused when another active policy (e.g. "All users") grants -them, because core blocks access as soon as one policy denies. - -Delivered entirely by the **advancedforms** plugin through GLPI core's existing extension -point `FormAccessControlManager::registerPluginAccessControlPolicy()`. GLPI core is not -modified. - -### Acceptance criteria - -1. The Access controls tab of a form shows a new policy, **"Allow specific languages"** - (translatable label, icon, active/inactive toggle like core policies). -2. Its config form is a multi-select of the languages in `Glpi\Locale\LanguageRegistry`. - The selection is stored as language codes (e.g. `fr_FR`). -3. Vote (core: any `Deny` blocks; otherwise at least one `Grant` is needed): - - current language **not in** list → `Deny`, authenticated or anonymous - - current language **in** list, other active policies on the form → `Abstain`: the - others decide who gets in, the language only narrows it ("group X" + French = French - users of group X; Direct access + French = token still required, logged in or not) - - current language **in** list, policy alone, authenticated → `Grant` - - current language **in** list, policy alone, anonymous → `Abstain` (see 5) -4. The "current language" is the session language `$_SESSION['glpilanguage']`, i.e. the - language GLPI is displayed in. -5. The policy never opens a form to anonymous users. `canAnswer()` never returns - `Grant` when `!$parameters->isAuthenticated()`, and `allowUnauthenticated()` - returns `false`. It still denies anonymous users whose language is not listed. The login redirect stays core's job. - The guard only stops a token bypass: when Direct access allows anonymous users, - an anonymous visitor without the token would otherwise be granted access through - their session language. -6. An active policy with an empty language list denies everyone, and `getWarnings()` - shows a warning in the Access controls tab. -7. Every place that calls core's `canAnswerForm()` enforces the policy automatically: - the service catalog, helpdesk tiles, form render/submit, and form documents. -8. Form export/import keeps the selected languages. Language codes are not DB ids, so - no mapping is needed. -9. The policy is registered whenever the plugin is active. There is **no** toggle on - the plugin config page: those toggles are for question types, and each form's - policy already has its own active switch. -10. Disabled by default: core creates any newly registered policy on each form with - `is_active => false` (`createMissingStrategyForForm`), so no extra code is needed. -11. When the plugin is deactivated, core ignores the saved rows - (`Form::getAccessControls()` skips strategies that are no longer valid), so forms - keep working. On reactivation the saved configs are used again. -12. Plugin uninstall deletes the `FormAccessControl` rows whose `strategy` is - `LanguageAccessControl`. This follows `InstallManager::uninstall()`, which already - removes the plugin's own data. - -### Non-goals - -- Restricting individual questions or sections by language -- Detecting the language from the browser's `Accept-Language` header beyond what GLPI - already puts in the session -- Translating form content (core already handles that) -- Any GLPI core change - -## Tech Stack - -PHP ≥ 8.2, GLPI 11/12 Forms (`Glpi\Form\AccessControl\*`), Twig, PHPUnit, Playwright. -No new dependency. - -## Commands - -All commands run from the plugin directory and go through `../../PluginsMakefile.mk` -(Docker): - -``` -Tests (single file): make phpunit c='tests/Model/AccessControl/LanguageAccessControlTest.php' -All lints/tests: make test -Static analysis: make phpstan -E2E: make playwright c='tests/e2e/specs/' -Locales: handled by CI (locales-update-source.yml on push to main) -License headers: make license-headers-check -``` - -## Project Structure - -``` -src/Model/AccessControl/LanguageAccessControl.php → NEW: ControlTypeInterface -src/Model/AccessControl/LanguageAccessControlConfig.php → NEW: JsonFieldInterface, holds string[] languages -templates/access_control/language.html.twig → NEW: config form (multi-select) -src/Service/InitManager.php → add registerAccessControlPolicies(), unconditional, guarded against double registration -src/Service/InstallManager.php → uninstall(): delete this policy's FormAccessControl rows -tests/Model/AccessControl/LanguageAccessControlTest.php → NEW -tests/Service/InitManagerTest.php / InstallManagerTest.php → extend -tests/e2e/specs/… → NEW: one happy path + one deny path -``` - -Core references to mirror: `src/Glpi/Form/AccessControl/ControlType/AllowList*.php` and -`templates/pages/admin/form/access_control/allow_list.html.twig`. - -## Code Style - -Follow the existing plugin: MIT license header, `final` classes, strict types, named -args, first-class callables, `__()` with domain `'advancedforms'`. - -```php -final class LanguageAccessControl implements ControlTypeInterface -{ - #[Override] - public function canAnswer( - Form $form, - JsonFieldInterface $config, - FormAccessParameters $parameters, - ): AccessVote { - if (!$config instanceof LanguageAccessControlConfig) { - throw new InvalidArgumentException('Invalid config class'); - } - - $language = $_SESSION['glpilanguage'] ?? null; - if (!in_array($language, $config->getLanguages(), true)) { - return AccessVote::Deny; - } - - if ($this->hasOtherActivePolicies($form)) { - return AccessVote::Abstain; - } - - return $parameters->isAuthenticated() ? AccessVote::Grant : AccessVote::Abstain; - } -} -``` - -`createConfigFromUserInput()` keeps only the codes that exist in -`Glpi\Locale\LanguageRegistry`. This is validation at the trust boundary. - -## Testing Strategy - -- **PHPUnit** (`tests/Model/AccessControl/`, extends `AdvancedFormsTestCase`): - - with other active policies, the language only restricts them: allow list + - language and direct access + language, logged in or not - - alone, authenticated: listed language → `Grant`; unlisted language or empty list → - `Deny`; anonymous: listed language → `Abstain`, unlisted → `Deny`; every list holds - several languages - - user input is cleaned: unknown codes are dropped, and a missing key gives an - empty list - - JSON serialize/deserialize round trip, and export/import round trip - - the warning shows when the list is empty - - integration, checked through `FormAccessControlManager::canAnswerForm()`, with - Language(`en_GB`, `de_DE`, `es_ES`): with or without "All users", the user can - answer in `en_GB` and is refused in `fr_FR` -- **InitManager test**: the policy is registered, and only once if `init()` runs again. - A new form gets the policy with `is_active = false`. -- **InstallManager test**: uninstall removes this policy's rows and leaves core - policies untouched. -- **Playwright**: configure the policy in the Access controls tab, then check that the - form shows or is hidden in the service catalog after switching the user's language. - -## Boundaries - -- **Always:** use core's plugin policy API, `make test` green before commit, license - headers, translatable strings, validate the selected languages server-side. -- **Ask first:** any DB schema or migration (none expected: config lives in core's - `glpi_forms_accesscontrols_formaccesscontrols.config` JSON), any GLPI core change, new - dependencies. -- **Never:** patch GLPI core, widen anonymous access, delete users' stored policy - configs, remove or skip existing tests. - -## Success Criteria - -- Acceptance criteria 1–12 pass, through PHPUnit and the Playwright spec. -- `make test` (phpunit, phpstan, psalm, rector, cs) passes. -- Manual check: French UI plus a form whose only active policy allows English → the - form is absent from the catalog, and its direct URL returns access denied. - -## Open Questions - -None. - -## Resolved (2026-11-05) - -- Vote: `Grant` when listed, `Deny` when not listed (decided 2026-11-05, replacing - `Grant`/`Abstain`). The language is never ignored when combined with other policies. -- Next to other active policies, a listed language gives `Abstain`, not `Grant` - (decided 2026-11-05): the policy works alone but never lets in users that another - policy refuses (e.g. outside group X, or without the direct access token). -- Label: "Allow specific languages". -- Anonymous users: never `Grant` (option B), so the policy can't be used to get - around Direct access's token; an unlisted language still gives `Deny`. -- No plugin config toggle: always registered while the plugin is active, and inactive - by default on each form through core's own default. diff --git a/tasks/plan.md b/tasks/plan.md deleted file mode 100644 index 27b99ab..0000000 --- a/tasks/plan.md +++ /dev/null @@ -1,86 +0,0 @@ -# Implementation Plan: Language access control policy (#46030) - -Spec: [`SPEC.md`](../SPEC.md). Task list: [`tasks/todo.md`](todo.md). - -## Overview - -Add an "Allow specific languages" form access policy to the advancedforms plugin through -GLPI core's `FormAccessControlManager::registerPluginAccessControlPolicy()`. It votes -`Grant` when the authenticated user's session language is in the configured list, and -`Deny` otherwise, so the language always restricts the other policies. No GLPI core -change and no DB schema change. - -## Architecture Decisions - -- **Mirror `AllowList` / `DirectAccess`**: two classes, the strategy - (`ControlTypeInterface`) and its config (`JsonFieldInterface`), plus one Twig template. - New namespace `GlpiPlugin\Advancedforms\Model\AccessControl`. -- **Language source**: `$_SESSION['glpilanguage']`. `SessionInfo` doesn't expose the - language, and the session language is what "GLPI is set in French" means. -- **Vote**: unlisted language → `Deny` (restricts every other policy); listed → - `Grant` if authenticated, `Abstain` if anonymous, so the policy can't be used to get - around Direct access's token. -- **Validation**: `createConfigFromUserInput()` keeps only codes that are keys of - `Glpi\Locale\LanguageRegistry` and removes duplicates. -- **Export**: `new DynamicExportDataField($config->jsonSerialize(), [])`, the same as - DirectAccess. There are no DB ids, so import passes the data through unchanged. -- **Registration**: unconditional in `InitManager`, guarded against double - registration if `init()` runs again, the same way `registerDestinationFields()` is. - Core already creates the policy inactive on each form. -- **Uninstall**: delete the `FormAccessControl` rows whose `strategy` is this class. - -## Dependency Graph - -``` -LanguageAccessControlConfig - │ -LanguageAccessControl (vote, validation, export, warnings) - │ - ├── template + InitManager registration ──→ visible in UI, enforced by canAnswerForm() - │ │ - │ └── Playwright spec - └── InstallManager uninstall cleanup -``` - -## Task List - -### Phase 1: Policy logic -- [x] Task 1: Config + strategy classes with vote, validation and export (unit tested) - -### Phase 2: Wire into GLPI -- [x] Task 2: Config form template + registration; integration through `canAnswerForm()` - -### Checkpoint A -- [x] `make phpunit` (new + existing) green, `make phpstan` clean -- [x] Manual: the policy appears inactive in a form's Access controls tab, saves the - languages, hides the form from the catalog for a non-listed language - -### Phase 3: Lifecycle and end-to-end -- [x] Task 3: Uninstall cleanup -- [x] Task 4: Playwright spec - -### Phase 4: Finish -- [x] Task 5: CHANGELOG, full `make test` (locales handled by CI workflows) - -### Checkpoint: Complete -- [x] SPEC acceptance criteria 1–12 met -- [x] `make test` green (except 8 LDAP tests needing an LDAP server) -- [x] Human review before commit/PR - -## Risks and Mitigations - -| Risk | Impact | Mitigation | -|------|--------|------------| -| Twig input naming must match what core's access-control form posts to `createConfigFromUserInput()` | High | Copy the field naming from core's `allow_list.html.twig`; the Task 2 integration test saves through the real form input path | -| Multi-select posts nothing when it is emptied, so the old value could survive | Med | A missing key → empty list; covered by a unit test | -| Language list for the select | Low | `$CFG_GLPI['languages']` is deprecated (11.0): use `Glpi\Locale\LanguageRegistry` (`has()` for validation, `all()` / `toLegacyArray()` for the select) | -| Tests depend on global `$_SESSION['glpilanguage']` | Low | Set and restore it in each test; `AdvancedFormsTestCase` resets session | -| Twig template path under the plugin namespace (`@advancedforms/...`) | Low | Same pattern as `ConfigManager::renderConfigForm()` | - -## Parallelization - -Task 3 only depends on Task 1 and can run alongside Task 2. Task 4 needs Task 2. - -## Open Questions - -None. diff --git a/tasks/todo.md b/tasks/todo.md deleted file mode 100644 index 691fd92..0000000 --- a/tasks/todo.md +++ /dev/null @@ -1,152 +0,0 @@ -# Todo: Language access control policy (#46030) - -See [`plan.md`](plan.md) and [`../SPEC.md`](../SPEC.md). - -## Task 1: Policy config and strategy classes - -**Description:** Create `LanguageAccessControlConfig` (holds `string[] $languages`, JSON -round trip) and `LanguageAccessControl` implementing `ControlTypeInterface`: label -"Allow specific languages", icon, weight, the vote, input validation, warnings for an -empty list, export/import, and `allowUnauthenticated() = false`. -`renderConfigForm()` may return an empty string until Task 2. - -**Acceptance criteria:** -- [x] Authenticated user: listed session language → `Grant`; not listed or empty - list → `Deny`. Anonymous: listed → `Abstain`, not listed → `Deny` -- [x] `createConfigFromUserInput()` drops unknown codes and duplicates; a missing key - gives an empty list -- [x] Config JSON round trip and export → import round trip keep the languages; - `getWarnings()` is non-empty only when the list is empty - -**Verification:** -- [x] `make phpunit c='tests/Model/AccessControl/LanguageAccessControlTest.php'` -- [x] `make phpstan` - -**Dependencies:** None - -**Files:** -- `src/Model/AccessControl/LanguageAccessControlConfig.php` (new) -- `src/Model/AccessControl/LanguageAccessControl.php` (new) -- `tests/Model/AccessControl/LanguageAccessControlTest.php` (new) - -**Scope:** S - ---- - -## Task 2: Config form and registration - -**Description:** Add the multi-select Twig template (input naming copied from core's -`allow_list.html.twig`), implement `renderConfigForm()`, and register the policy in -`InitManager::init()` through a new `registerAccessControlPolicies()` that is guarded -against double registration. - -**Acceptance criteria:** -- [x] The policy is registered once, even if `init()` runs twice; a new form gets it - with `is_active = false` -- [x] `canAnswerForm()` with Language(`en_GB`, `de_DE`, `es_ES`), with or without - "All users": true in `en_GB`, false in `fr_FR` -- [x] Token bypass guard: Direct access (anonymous allowed) + Language(`fr_FR`, `en_GB`, - `de_DE`), anonymous: `fr_FR` with no token → false; `fr_FR` with the token → - true; `es_ES` with the token → false -- [x] The config form renders and the saved languages are preselected - -**Verification:** -- [x] PHPUnit: registration/default-inactive tests live in `LanguageAccessControlTest` (not `InitManagerTest`) -- [ ] Manual (Checkpoint A): enable the policy on a form, select English, switch the - user to French → the form is gone from the service catalog and its direct URL - is refused - -**Dependencies:** Task 1 - -**Files:** -- `templates/access_control/language.html.twig` (new) -- `src/Model/AccessControl/LanguageAccessControl.php` -- `src/Service/InitManager.php` -- `tests/AdvancedFormsTestCase.php` (reset `FormAccessControlManager` singleton per test) -- `tests/Model/AccessControl/LanguageAccessControlTest.php` - -**Scope:** M - ---- - -## Checkpoint A -- [x] All PHPUnit tests green (except 8 LDAP tests needing an LDAP server), `make phpstan` clean -- [ ] Manual UI check above done -- [ ] Review with human before continuing - ---- - -## Task 3: Uninstall cleanup - -**Description:** `InstallManager::uninstall()` calls the new public -`uninstallAccessControlPolicies()`, tested directly (calling `uninstall()` in a test -would drop a table, which commits the test transaction). It deletes the `FormAccessControl` rows whose -`strategy` is `LanguageAccessControl::class`. - -**Acceptance criteria:** -- [x] After uninstall, no rows for this strategy remain -- [x] Core policy rows (AllowList, DirectAccess) are left alone - -**Verification:** -- [x] `make phpunit c='tests/Service/InstallManagerTest.php'` - -**Dependencies:** Task 1 - -**Files:** -- `src/Service/InstallManager.php` -- `tests/Service/InstallManagerTest.php` - -**Scope:** XS - ---- - -## Task 4: Playwright spec - -**Description:** E2E test that configures "Allow specific languages" in the Access -controls tab, then checks catalog visibility for a listed and a non-listed user -language. - -**Acceptance criteria:** -- [x] Policy toggled on and a language saved through the UI -- [x] Listed language → form visible in the service catalog; non-listed → hidden - -**Verification:** -- [x] `make playwright c='language_access_control'` (2 passed) - -**Dependencies:** Task 2 - -**Files:** -- `tests/e2e/specs/language_access_control.spec.ts` (new) -- `tests/e2e/pages/…` (only if a page object is needed) - -**Scope:** S - ---- - -## Task 5: Changelog and full check - -**Description:** Add a CHANGELOG entry and run the full quality gate. Locale extraction -is not needed: the `locales-update-source.yml` workflow regenerates -`locales/advancedforms.pot` on every push to `main`, and `locales-sync.yml` pulls -translations back. - -**Acceptance criteria:** -- [x] CHANGELOG "Unreleased" entry for #46030 -- [x] `make test` and `make license-headers-check` green (except 8 LDAP tests needing an LDAP server) - -**Verification:** -- [x] `make test && make license-headers-check` - -**Dependencies:** Tasks 1–4 - -**Files:** -- `CHANGELOG.md` - -**Scope:** XS - ---- - -## Checkpoint: Complete -- [x] SPEC acceptance criteria 1–12 met -- [x] `make test` green (except 8 LDAP tests needing an LDAP server) -- [ ] Human review before commit/PR