Feature #7585
closedEnhance logging of unexpected GET requests on Ajax sessions
0%
Description
Once a Wt Ajax session has been established, the client no longer needs to send GET requests containing wtd query parameters.
Attached, for your review, is a patch that logs unexpected GET requests on Ajax sessions and returns a 403 status. It has been lightly tested on top of github master 4.3.0-26-gd18ca1b9, but would benefit from additional review and testing.
Files
Updated by Roel Standaert over 4 years ago
Hm, it's not quite right yet, since it makes it impossible to set reload-is-new-session
to false
. GET requests with an existing wtd
should be tolerated in that case. I think if that configuration needs to be secured a bit better, then the Combined
session tracking strategy can be used.
Also, I think the check is maybe a bit late? I see that some of the code before that already changes the state of the web session.
Updated by Bruce Toll over 4 years ago
Thanks for the helpful feedback. I agree with you that the check is late. In an earlier attempt, I had placed the test in WebController::handleRequest before the web session was accessed, but it seemed like that approach required redundant work that belonged in WebSession. I will revisit. Perhaps I could add a const method in WebSession that helps determine whether a request should be rejected. I will also be sure to test with reload-is-new-session false...
Updated by Bruce Toll over 4 years ago
- File 0002-Log-unexpected-GET-requests-on-Ajax-sessions.patch 0002-Log-unexpected-GET-requests-on-Ajax-sessions.patch added
Hi Roel,
I have attached a revised patch, relocating the check so that it occurs earlier in WebSession::handleRequest. I believe that if the check fails, it should now leave the session unaffected. I also moved up the definition of requestForResource so that it could be used in the new check and a couple of other locations.
I tested with reload-is-new-session false, as well as other configuration options, with and without JS. The flexibility of the library makes for a lot of testing combinations. Please review at your convenience. I'm happy to make additional changes, but if it's easier to just change yourself, feel free to do so. Thanks. ---Bruce
Updated by Roel Standaert over 4 years ago
- Status changed from New to Resolved
Thanks, Bruce.
I pushed it to GitHub. I also backported it to Wt 3.
Updated by Roel Standaert about 4 years ago
- Status changed from Resolved to Closed