Skip to content

Commit

Permalink
Helpdesk form: store direct access token in session once validated
Browse files Browse the repository at this point in the history
  • Loading branch information
AdrienClairembault committed Mar 6, 2025
1 parent 1bdc49a commit 1d8aa90
Show file tree
Hide file tree
Showing 10 changed files with 27 additions and 31 deletions.
7 changes: 0 additions & 7 deletions js/modules/Forms/Condition/Engine.js
Original file line number Diff line number Diff line change
Expand Up @@ -107,13 +107,6 @@ export class GlpiFormConditionEngine
form_data.append(`answers[${entry[0]}]`, entry[1]);
}

// Included direct access token if needed.
// Not great to have to do this normally, TOOD: find a better way.
const url_params = new URLSearchParams(window.location.search);
if (url_params.has('token')) {
form_data.append('token', url_params.get('token'));
}

// Send request
const url = `${CFG_GLPI.root_doc}/Form/Condition/Engine`;
const response = await fetch(url, {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
use Glpi\Form\AccessControl\AccessVote;
use Glpi\Form\AccessControl\ControlType\AllowList;
use Glpi\Form\AccessControl\FormAccessParameters;
use Glpi\Form\Form;
use Glpi\Tests\FormBuilder;
use Glpi\Tests\FormTesterTrait;
use Glpi\Form\AccessControl\ControlType\AllowListConfig;
Expand Down Expand Up @@ -199,7 +200,7 @@ public function testCanAnswer(
$allow_list = new AllowList();
$this->assertEquals(
$expected,
$allow_list->canAnswer($config, $parameters)
$allow_list->canAnswer(new Form(), $config, $parameters)
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
use Glpi\Form\AccessControl\AccessVote;
use Glpi\Form\AccessControl\ControlType\DirectAccess;
use Glpi\Form\AccessControl\FormAccessParameters;
use Glpi\Form\Form;
use Glpi\Tests\FormBuilder;
use Glpi\Tests\FormTesterTrait;
use Glpi\Form\AccessControl\ControlType\DirectAccessConfig;
Expand Down Expand Up @@ -251,7 +252,7 @@ public function testCanAnswer(
$direct_access = new DirectAccess();
$this->assertEquals(
$expected,
$direct_access->canAnswer($config, $parameters)
$direct_access->canAnswer(new Form(), $config, $parameters)
);
}

Expand Down
4 changes: 0 additions & 4 deletions src/Glpi/Controller/Form/RendererController.php
Original file line number Diff line number Diff line change
Expand Up @@ -103,10 +103,6 @@ public function __invoke(Request $request): Response
'my_tickets_url_param' => http_build_query($my_tickets_criteria),
'visibility_engine_output' => $visibility_engine_output,
'params' => $request->query->all(),

// Direct access token must be included in the form data as it will
// be checked in the submit answers controller.
'token' => $request->query->getString('token'),
]);
}

Expand Down
7 changes: 1 addition & 6 deletions src/Glpi/Controller/Form/Utils/CanCheckAccessPolicies.php
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,7 @@ protected function checkFormAccessPolicies(Form $form, Request $request): void
// Form administrators can bypass restrictions while previewing forms.
$parameters = new FormAccessParameters(bypass_restriction: true);
} else {
// URL parameters might be sent in GET or POST requests due to some
// technical limitations.
$url_parameters = $request->isMethod('POST')
? $request->request->all()
: $request->query->all()
;
$url_parameters = $request->query->all();

// Load current user session info and URL parameters.
$parameters = new FormAccessParameters(
Expand Down
1 change: 1 addition & 0 deletions src/Glpi/Form/AccessControl/ControlType/AllowList.php
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ public function createConfigFromUserInput(array $input): AllowListConfig

#[Override]
public function canAnswer(
Form $form,
JsonFieldInterface $config,
FormAccessParameters $parameters
): AccessVote {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,13 +101,9 @@ public function createConfigFromUserInput(array $input): JsonFieldInterface;

/**
* Check if the current user can answer the given form.
*
* @param JsonFieldInterface $config
* @param FormAccessParameters $parameters
*
* @return AccessVote
*/
public function canAnswer(
Form $form,
JsonFieldInterface $config,
FormAccessParameters $parameters
): AccessVote;
Expand Down
19 changes: 17 additions & 2 deletions src/Glpi/Form/AccessControl/ControlType/DirectAccess.php
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ public function createConfigFromUserInput(array $input): DirectAccessConfig

#[Override]
public function canAnswer(
Form $form,
JsonFieldInterface $config,
FormAccessParameters $parameters
): AccessVote {
Expand All @@ -141,7 +142,7 @@ public function canAnswer(
return AccessVote::Abstain;
}

if (!$this->validateToken($config, $parameters)) {
if (!$this->validateToken($form, $config, $parameters)) {
return AccessVote::Abstain;
};

Expand All @@ -160,12 +161,26 @@ private function validateSession(
}

private function validateToken(
Form $form,
DirectAccessConfig $config,
FormAccessParameters $parameters,
): bool {
// Note: it is easy to validate the token when an user is accesing the
// form for the first time through the /Form/Render/{id} page as the
// link will contain the token as a GET parameter.
// However, for any subsequent AJAX requests, the token is not present
// in the URL. Therefore, we must rely on the session to store the token
// and validate it on each request.
$token = $parameters->getUrlParameters()['token'] ?? null;
if ($token === null) {
return false;
$session_token = $_SESSION['helpdesk_form_access_control'][$form->getId()] ?? null;
if ($session_token === null) {
return false;
} else {
$token = $session_token;
}
} else {
$_SESSION['helpdesk_form_access_control'][$form->getId()] = $token;
}

return hash_equals($config->getToken(), $token);
Expand Down
3 changes: 3 additions & 0 deletions src/Glpi/Form/AccessControl/FormAccessControlManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ public function canAnswerForm(
}

return $this->validateAccessControlsPolicies(
$form,
$access_controls_policies,
$parameters
);
Expand Down Expand Up @@ -203,6 +204,7 @@ private function addWarningIfFormHasNoActivePolicies(
}

private function validateAccessControlsPolicies(
Form $form,
array $policies,
FormAccessParameters $parameters
): bool {
Expand All @@ -212,6 +214,7 @@ private function validateAccessControlsPolicies(
/** @var FormAccessControl[] $policies */
foreach ($policies as $policiy) {
$votes[] = $policiy->getStrategy()->canAnswer(
$form,
$policiy->getConfig(),
$parameters
);
Expand Down
5 changes: 0 additions & 5 deletions templates/pages/form_renderer.html.twig
Original file line number Diff line number Diff line change
Expand Up @@ -265,11 +265,6 @@
</div>
</div>
</div>

{# Include direct access token if supplied #}
{% if token %}
<input type="hidden" name="token" value="{{ token }}" />
{% endif %}
</form>

<script>(async () => {
Expand Down

0 comments on commit 1d8aa90

Please sign in to comment.