Skip to content

Conversation

@sprytnyserek
Copy link

simplesamlphp-module-authX509/templates/X509error.twig contains references to errortitle and errordescr parameters. However they both are undefined. It should be in the method authFailed in:

public function authFailed(&$state): void

This pull request addresses this issue by putting an error code and a corresponding error message if available.

$t->data['errordescr'] = $t->data['errorcodes'][$t->data['errorcode']];
} else {
$t->data['errordescr'] = $t->data['errorcode'];
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the result of refactoring towards twig templates that was incomplete. To fully clean it up I propose the following:

  • Do not set errorcode and errorcodes as template variables but just as local variables and use those in the code in the added lines above.
  • Change the if condition in the twig template to test for presence of errordescr.

@thijskh
Copy link
Member

thijskh commented Dec 10, 2020

Thanks!

@thijskh thijskh merged commit d4b36d2 into simplesamlphp:master Dec 10, 2020
@tvdijen
Copy link
Member

tvdijen commented Dec 10, 2020

Does this require backporting to the 0.9 branch?

@sprytnyserek
Copy link
Author

The problem exists in 0.9.

Btw. I've just realized my fault in loading error info. Instead of this:

$t->data['errortitle'] = $errorcode;
        if (array_key_exists($errorcode, $errorcodes)) {
            $t->data['errordescr'] = $errorcodes[$errorcode];
        } else {
            $t->data['errordescr'] = $errorcode;
        }

I should do something like this (+ null checks etc.):

$t->data['errortitle'] = $errorcodes['title'][$errorcode];
        if (array_key_exists($errorcode, $errorcodes['descr'])) {
            $t->data['errordescr'] = $errorcodes['descr'][$errorcode];
        } else {
            $t->data['errordescr'] = $t->data['errortitle'];
        }

I'll fix it soon.

@thijskh
Copy link
Member

thijskh commented Dec 10, 2020

In 0.9 the problem exists only in the new templates (not used by default so probably therefore not detected). Fix for 0.9 will need to consider to keep old templates working aswell.

@tvdijen
Copy link
Member

tvdijen commented Dec 10, 2020

Ack, will wait for the final fix and then port it to 0.9

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants