Conversation
| public function test_checkIssueInstant_does_not_warn_when_request_is_within_max_age(): void | ||
| { | ||
| $maxAge = 86400; | ||
| $issueInstant = time() - $maxAge + 1; |
There was a problem hiding this comment.
1 second seems flaky to me. What if computing for some reason takes longer the test will fail without i clear message. Maybe you can increase this to 60. What do you think?
There was a problem hiding this comment.
I agree that 1 second can trigger false failures. 60 seconds is a bit much, the whole corto test suite takes 500ms seconds to complete. Adjusted to 5 seconds.
| * The `0000-00-00 00:00:00` is added for clarity/consistency, as this is probably the default behaviour of your database already. | ||
| * Removed unused index `consent.deleted_at`. Delete this from your production database if it's there. | ||
| * Added a specific error page for unsolicited SAML responses (IdP-initiated SSO without a prior AuthnRequest). | ||
| * Added `max_issue_instant_age` to parameters.yaml to configure the logging mechanism. EB will write log entries if it receives requests that are older than this value. |
There was a problem hiding this comment.
Very nitpick: the parameter reference isn't correct. It should be yml instead of yaml. As i said very nitpicky you dont have to change this.
| * debug?: DebugConfig, | ||
| * web_profiler?: WebProfilerConfig, | ||
| * }, | ||
| * "when@prod"?: array{ |
There was a problem hiding this comment.
just wondering why is this needed?
There was a problem hiding this comment.
its not needed. Just reference.php being annoying: symfony/symfony#62588 (comment)
Prior to this change, EB would not make note of the fact that old SAML requests are used. Engine does make a log notice about possible clock drift, but does so if if the time is off by 30 seconds. This change adds a warning if a request is received that is X seconds old. With the default being 1 day. Functionally, EB does proces these requests as usual, but SPs might reject the requests. Resolves #1792
johanib
force-pushed
the
feature/1792-add-expired-logging
branch
from
b917f95 to
8d30b05
Compare
2 participants
Prior to this change, EB would not make note of the fact that old SAML requests are used. Engine does make a log notice about possible clock drift, but does so if if the time is off by 30 seconds.
This change adds a warning if a request is received that is X seconds old. With the default being 1 day.
Functionally, EB does proces these requests as usual, but SPs might reject the requests.
Resolves #1792