From d2d9c59d3c32fa18f815fc3641fe5b79e42b0a58 Mon Sep 17 00:00:00 2001 From: Sebastien Monterisi Date: Wed, 19 Aug 2026 10:56:32 +0200 Subject: [PATCH 1/3] Document the re-authentication replay refactoring New: ReAuthReplayListener, ReAuthManager::RESTORE_REFERER_PARAM and ReAuthManager::getReplayData() (replaces getRequestedPostData()), which restore the origin page as the referer of the replayed request, GET replays included. --- source/devapi/reauthentication.rst | 37 ++++++++++++++++++++++++++--- source/plugins/reauthentication.rst | 6 ++++- 2 files changed, 39 insertions(+), 4 deletions(-) diff --git a/source/devapi/reauthentication.rst b/source/devapi/reauthentication.rst index 7cc8295..5f57a8e 100644 --- a/source/devapi/reauthentication.rst +++ b/source/devapi/reauthentication.rst @@ -81,6 +81,10 @@ entry points on ``CommonGLPI`` / ``CommonDBTM``. * - ``Glpi\Controller\Security\ReAuthController`` - Routes ``/ReAuth/Prompt`` (display the form) and ``/ReAuth/Verify`` (verify, then replay the initial request). + * - ``Glpi\Kernel\Listener\RequestListener\ReAuthReplayListener`` + - Request listener restoring the origin page as the referer of the replayed request, so + that what reads it — ``Html::back()`` most notably — sends the user back where they + came from instead of into the re-authentication flow. ``ReAuthManager`` uses ``SingletonTrait``, and is registered as an autowirable service in ``dependency_injection/services.php`` (a factory on ``getInstance()``). @@ -139,11 +143,37 @@ Request flow └─ on success: ├─ ReAuthManager::authenticate() → opens the 15 min window └─ renders pages/redirect_post.html.twig, which replays the initial request - (same URL, same method, same POST data) + (same URL, same method, same data — ReAuthManager::getReplayData()) + + 4. the replayed request + └─ ReAuthReplayListener restores the origin page as the referer The replay is what makes the detour transparent: a submitted form is not lost, the user lands on the page they asked for. +Coming back to the origin page +++++++++++++++++++++++++++++++ + +The replay is an auto-submitted form served by the verification page, so the browser reports +that page as the ``Referer`` of the replayed request. Anything reading it — ``Html::back()`` +most notably — would then send the user back into the re-authentication flow instead of the +page the action was triggered from. + +``ReAuthManager::getReplayData()`` therefore adds one parameter to the replayed request, +``_glpi_reauth_restore_referer`` (``ReAuthManager::RESTORE_REFERER_PARAM``). It lands in the +query string of a GET replay and in the body of a POST one, and ``ReAuthReplayListener`` acts +upon it before any controller runs — legacy scripts included: it reads the origin URL from the +session and writes it both on the Symfony request headers and on ``$_SERVER['HTTP_REFERER']``, +so both worlds see the same referer. + +The parameter carries no value of its own: the URL is read from the session, never from the +request. A forged parameter can therefore only send the user back to their own origin page. + +.. note:: + + A replayed request carries that extra parameter. Code reading the whole query string or the + whole POST payload must tolerate it. + Session keys used +++++++++++++++++ @@ -160,9 +190,10 @@ Session keys used * - ``glpi_reauth_requested_httpmethod`` - The HTTP method of the replayed request. * - ``glpi_reauth_requested_post_data`` - - The POST data of the replayed request. + - The data of the replayed request: the POST payload, or the query parameters of a GET. * - ``glpi_reauth_origin_url`` - - The referer, used for the "Cancel" button. + - The page the action was triggered from. Used by the "Cancel" button of the prompt, and + restored as the referer of the replayed request. .. warning:: diff --git a/source/plugins/reauthentication.rst b/source/plugins/reauthentication.rst index 1c85fb3..0f99b72 100644 --- a/source/plugins/reauthentication.rst +++ b/source/plugins/reauthentication.rst @@ -232,7 +232,7 @@ Your endpoint is then responsible for the last three steps of the flow. It must return $this->render('pages/redirect_post.html.twig', [ 'http_method' => $this->reAuthManager->getRequestedMethod(), 'url' => $this->reAuthManager->getRequestedURL(), - 'post_data' => $this->reAuthManager->getRequestedPostData(), + 'replay_data' => $this->reAuthManager->getReplayData(), ]); } } @@ -246,6 +246,10 @@ Points of attention for such an endpoint: * When the provider answers asynchronously (redirect back from the provider, callback), make sure the state you check cannot be forged or replayed, and only then call ``authenticate()``. * Do not skip step 3, otherwise the user loses the action they had triggered. +* Replay the request with ``getReplayData()``, not with the raw stored data: it adds the + parameter that restores the origin page as the referer of the replayed request (see + :doc:`the core documentation `). Building the payload yourself + would send the user back into the re-authentication flow on the next ``Html::back()``. Security considerations for strategy authors ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ From 3aefaa6531cd6a4b1aba5edfea8c2c8b418dcab2 Mon Sep 17 00:00:00 2001 From: Sebastien Monterisi Date: Wed, 19 Aug 2026 10:59:43 +0200 Subject: [PATCH 2/3] Re-authentication doc: template guards and proofreading Add the has_itemtype_right() / has_profile_right() pitfall on links and buttons, complete the list of core sensitive itemtypes. --- source/devapi/reauthentication.rst | 34 ++++++++++++++++++++++++------ 1 file changed, 28 insertions(+), 6 deletions(-) diff --git a/source/devapi/reauthentication.rst b/source/devapi/reauthentication.rst index 5f57a8e..3b178cf 100644 --- a/source/devapi/reauthentication.rst +++ b/source/devapi/reauthentication.rst @@ -56,8 +56,8 @@ Key properties to keep in mind while developing: Architecture ^^^^^^^^^^^^ -Everything lives in the ``Glpi\Security\ReAuth`` namespace, plus a controller and a few -entry points on ``CommonGLPI`` / ``CommonDBTM``. +Most of it lives in the ``Glpi\Security\ReAuth`` namespace, plus a controller, a request +listener, and a few entry points on ``CommonGLPI`` / ``CommonDBTM``. .. list-table:: :header-rows: 1 @@ -231,8 +231,9 @@ by default): } } -Core examples: ``User``, ``Profile``, ``Profile_User``, ``Group``, ``Group_User``, ``Config``, -``AuthLDAP``, ``AuthMail``, ``OAuthClient``, ``Glpi\Event``, ``Glpi\Inventory\Conf``. +Core examples: ``User``, ``Profile``, ``Profile_User``, ``Group``, ``Group_User``, +``Preference``, ``Config``, ``AuthLDAP``, ``AuthMail``, ``OAuthClient``, ``Glpi\Event``, +``Glpi\System\Log\LogViewer``, ``Glpi\Inventory\Conf``. The derived state is read through the ``final`` method ``CommonGLPI::isUserReauthenticationNeeded()``, which returns ``true`` only when the itemtype @@ -278,8 +279,8 @@ check by an item check. This is what was done for the plugin and marketplace pag .. warning:: - ``Session::checkRight()`` and similar functions silently bypass re-authentication. If a sensitive page - keeps using them, it stays unprotected even though its itemtype declares + ``Session::checkRight()`` and similar functions silently bypass re-authentication. If a + sensitive page keeps using them, it stays unprotected even though its itemtype declares ``itemTypeRequiresReauthentication()``. The ``$reauth_needed`` by-reference parameter of ``can()`` / ``canGlobal()`` exists for the @@ -339,6 +340,27 @@ Generic controllers ``$class::checkReAuthenticationOrRedirect()``. An itemtype served by them is protected as soon as it declares ``itemTypeRequiresReauthentication()``. +Links and buttons in templates +++++++++++++++++++++++++++++++ + +The Twig helper ``has_itemtype_right()`` goes through ``CommonDBTM::canGlobal()``, so it +returns ``false`` as long as the re-authentication is missing. Guarding a link with it +therefore **hides** it instead of letting the user click it and get the prompt — and a user who +never sees the entry has no way to reach the action at all. + +Guard such a link with ``has_profile_right()``, a plain right check: the page it points at +runs the re-authentication check on its own. + +.. code-block:: diff + + - {% if has_itemtype_right('Config', constant('UPDATE')) %} + + {% if has_profile_right('config', constant('UPDATE')) %} + +.. note:: + + The two helpers do not take the same argument: ``has_itemtype_right()`` expects an itemtype, + ``has_profile_right()`` a right name. + AJAX and non-HTML endpoints +++++++++++++++++++++++++++ From 646ec3901062696f83e05ff1fc2f28720d48b4b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Monterisi?= Date: Thu, 20 Aug 2026 12:21:04 +0200 Subject: [PATCH 3/3] Breath ! Co-authored-by: Romain B. <8530352+Rom1-B@users.noreply.github.com> --- source/devapi/reauthentication.rst | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/source/devapi/reauthentication.rst b/source/devapi/reauthentication.rst index 3b178cf..9609423 100644 --- a/source/devapi/reauthentication.rst +++ b/source/devapi/reauthentication.rst @@ -161,10 +161,11 @@ page the action was triggered from. ``ReAuthManager::getReplayData()`` therefore adds one parameter to the replayed request, ``_glpi_reauth_restore_referer`` (``ReAuthManager::RESTORE_REFERER_PARAM``). It lands in the -query string of a GET replay and in the body of a POST one, and ``ReAuthReplayListener`` acts -upon it before any controller runs — legacy scripts included: it reads the origin URL from the -session and writes it both on the Symfony request headers and on ``$_SERVER['HTTP_REFERER']``, -so both worlds see the same referer. +query string of a GET replay and in the body of a POST one. + +``ReAuthReplayListener`` acts upon it before any controller runs, legacy scripts included: it +reads the origin URL from the session and writes it both on the Symfony request headers and on +``$_SERVER['HTTP_REFERER']``, so both worlds see the same referer. The parameter carries no value of its own: the URL is read from the session, never from the request. A forged parameter can therefore only send the user back to their own origin page.