-
Notifications
You must be signed in to change notification settings - Fork 38
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(sqlview): When current user in SQL view, do not cache #2911
base: master
Are you sure you want to change the base?
Conversation
This is probably something that should ideally also be validated by the backend? This would help, but if it can actually be a security issue, then we should follow up with backend as well? |
@Birkbjo I agree about backend support also. If the frontend is bypassed, and for preexisting SQL Views, the backend shouldn't cache the results when a SQL View specifies the current user. This could be done in either of two ways, whichever seems more reasonable to code:
Note: @DavidCKen (or @karolinelien & @stian-sandvold while David is on leave). Also: @netroms Meanwhile, @Birkbjo, do you think it is reasonable to merge this change in the frontend as well? |
I never worked on the SQL views code myself, but @jimgrace do you know if there is any use-cases we support today that prevents caching the views? Also, is this a task you would be able to create a solution for @jimgrace? |
Yeah I think it makes sense. Sorry I should've been more clear that we can merge this - but I just wanted to mention that we shouldn't only do it on the client if it's indeed a security problem. |
This code implements the request from DHIS2-9300, namely that if
${_current_user_id}
or${_current_username}
is in a SQL view, then the SQL view should not be allowed to cache, as caching would keep the same results in the SQL view if one user were to log in and another user were to log out.