-
-
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
fix: token LocalCache to DistributedCache #46109
fix: token LocalCache to DistributedCache #46109
Conversation
In load balanced plateform, token local Cache does not work. We think with this change that we can use Distributed Cache instead. Signed-off-by: dsisysteme <72081136+dsisysteme@users.noreply.github.com>
The token is stored in oc_authtokens and the cache there to reduce database queries. Changing local to distributed to seems like a hack to fix something that's broken somewhere else. |
Distributed cache is slow, I would prefer to avoid it. Can you outline how the local cache can lead to login loops? |
Sorry, we try to explain the problem in the issue. What we notice in the flow is that when requests come back to Nextcloud after passing through user_saml, the node that generated the token can handle the requests, whereas the second node will return a 401 and redirect to user_saml, which is already authenticated. It then returns to Nextcloud, which will generate a new token on the node handling the requests, but when a request arrives on the second node, it returns a 401 and the loop is initiated. When the token is stored in Redis, all nodes retrieve the information. Alternatively, if a node cannot find the token, it should store it in its local cache. |
I don't yet see why this would happen. The second node doesn't have the app token cached, true, but it also doesn't have a negative cache entry. So it will go to the database and should find the row, right? |
Yes, it seems that the record in the oc_authtoken table disappears immediately when the issue arises. However, logically, the second node should make a request to the database to update its local cache. In the case of distributed caching, there is ultimately only one cached record for the entire cluster. This is just for discussion because we do not know all the impacts behind it. We have conducted upgrade tests and detected that the issue appeared between version 27.1.9 and 27.1.10, if that helps to understand the reason! |
We detected that right after the SSO login (redirect from SSO to Nextcloud), we get this error and then the redirection loop starts, but the error appears only once:
|
Hello, It seems that there is a problem in the getToken() function of the PublicKeyTokenProvider.php file If we comment on this line $this->cacheInvalidHash($tokenHash); authentication works with local cache Hope this information helps you find the cause. |
Hello there, We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process. Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6 Thank you for contributing to Nextcloud and we hope to hear from you soon! (If you believe you should not receive this message, you can add yourself to the blocklist.) |
A different approach #46398 |
Close because fix by #46398 |
Fix #46165
In a load-balanced platform with multiple application servers, local token caching does not work effectively and leads to login loops. We believe that by making this change, we can use a distributed cache instead.
In a load-balanced platform without sticky sessions, local token caching does not work and causes a login loop.
Summary
Currently, the token is stored in the local cache. In the case of a load-balanced Nextcloud platform without Sticky Sessions, it is necessary to store the token in the distributed cache if it exists.
Checklist