-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
feat(settings): Add setup check for too much caching #40960
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Christoph Wurst <[email protected]>
@@ -94,6 +95,8 @@ | |||
use OCP\Security\Bruteforce\IThrottler; | |||
use OCP\Security\ISecureRandom; | |||
use Psr\Log\LoggerInterface; | |||
use function setcookie; | |||
use function time; |
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.
timefactory? :D
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.
the constructor has like 37 injected services and I didn't want to touch that beast
I can't get my local nginx to cause the issues reported in #40863. When I turn on asset caching, all resources give an HTTP404. Edit: set up nginx proxy manager for my dev env. Even with asset caching I'm not able to trigger a cached response that has set-cookie headers. |
I haven't managed an account takeover yet, but with NPM + their Cache Assets enabled there are behavior changes that are red flags:
As soon as I turn off NPM's caching things go back to normal and I can't replicate any problems. Some of these behaviors seem to be timing specific and/or depend on which session connected first (which makes sense intuitively). I think a strong generic inappropriate caching check is a good idea for us. This type of caching contradicts our recommended config and randomly appears to breaks stuff. That's enough for me to justify a check. Asides:
|
Summary
Warn admins if their server caching is caching responses that should not be cached.
The approach is simple:
Set-Cookie
response headerIf the two responses are identical, then the server didn't process the requests individually.
TODO
Checklist