-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
clean up logged error messages #258
Conversation
fixes error shown in test log output: SimpleSAML\Error\Exception: Warning - Undefined array key "add" at /data/vendor/simplesamlphp/simplesamlphp/modules/profilereview/src/Auth/Process/ProfileReview.php:234
fixes error shown in test log output: Caused by: Twig\Error\RuntimeError: An exception has been thrown during the rendering of a template ("SimpleSAML\XHTML\Template::getEntityDisplayName(): Argument #1 ($data) must be of type array, null given, called in /data/vendor/twig/twig/src/Environment.php(392) : eval()'d code on line 82").
and remove incorrect log message
fixes error shown in test log output: Module exampleauth:UserPass configured in legacy mode. Please put your username:password entries under the "users" key in your authsource.
fixes error: impleSAML\Error\Exception: Warning - Undefined array key "mfaLearnMoreUrl" at /data/vendor/simplesamlphp/simplesamlphp/modules/profilereview/public/nag.php:37
Quality Gate passedIssues Measures |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me
$spmd = $state['SPMetadata']; | ||
$this->log('Updated SP metadata from ' . $this->spEntityId . ' to ' . $spmd['entityid']); | ||
} | ||
assert($state && array_key_exists('SPMetadata', $state)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we want to depend on PHP's built in assert()
function, since it is typically only run in debug or test mode, and is disabled in production mode (per the docs).
If we want to ensure something is asserted every time, it seems to be better to use other/normal PHP code (either an if
and throw new Exception
, or something like WebMozart's "Assert" library).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I didn't realize that. Thanks.
Fixed
$state['mfaLearnMoreUrl']
for the review pageexampleauth:UserPass
users inusers
keyif
toassert
to catch error condition earlier