add cookie_path configuration parameter #335
Conversation
| if (!$path) { | ||
| $path = $request->path; | ||
| } | ||
| if ($path == 'none') { |
There was a problem hiding this comment.
Is this a good idea to check for a special value none (which is valid just for the server-side configuration) if it might be forged by the client (see line 314)?
There was a problem hiding this comment.
I just followed the same way as cookie_domain variable is set. Indeed, if we end up with a URI path of 'none' we might default to an empty cookie path which will be a difference with the old code - it would have used 'none' as the cookie path. I don't mind removing this 'none' functionality from here. Just wanted to keep it consistent with cookie domain one.
There was a problem hiding this comment.
Sorry for the delay on this. @dumblob makes a good point, though I don't think there is any directly actionable attack vector as the result will likely be you get logged out if you forge 'none' for both cookie path or cookie domain. Regardless, I'm going to go ahead and merge this then fix both methods to eliminate that possibility.
| ; the path used for cookies. Leave this blank to let Cypht automatically | ||
| ; determine the path. You can also use the special value of "none" to force | ||
| ; Cypht to NOT set the cookie path property at all. This is not recommended | ||
| ; unless you know what you are doing! |
There was a problem hiding this comment.
The example with tiki-git would be of help here as well 😉.
|
Thanks for the PR. I will follow up with the small refactor as discussed in the comments and add some unit tests. |
Pullrequest
Add ability to specify the cookie path the same way cookie domain can be specified through configuration. It is useful in scenarios where mod_rewrite is used and current Cypht cookie path retrieval mechanism incorrectly puts the cookie inside a directory which is actually a webpage, hence the inability to access those cookies later. E.g. request URI like /tiki-git/Webmail?page=compose will end up with cookie path /tiki-git/Webmail/ which is incorrect as the path is /tiki-git/ only.
Issues
Checklist
How2Test
Specify a specific cookie path and check cookies are still set - e.g. compose message returns back the proper Message Sent response.
Todo