fix(sqlview): When current user in SQL view, do not cache

[benguaraldi]

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.

[Birkbjo]

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?

[jimgrace]

@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:

1. Force caching off when saving a the SQL view containing one of these constructs, and patch any such preexisting SQL Views, or
2. Force a result not to be cached when executing a SQL View with one of these constructs

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?

[stian-sandvold]

@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:

1. Force caching off when saving a the SQL view containing one of these constructs, and patch any such preexisting SQL Views, or
2. Force a result not to be cached when executing a SQL View with one of these constructs

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?

[Birkbjo]

Meanwhile, @Birkbjo, do you think it is reasonable to merge this change in the frontend as well?

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.

[benguaraldi]

@Birkbjo Let me know if you need anything from me before we approve and merge. Thanks!

[benguaraldi]

@Birkbjo, could you merge this PR? Or could someone else? I'm not allowed to.

Thanks!

[Birkbjo]

@benguaraldi , it's failing due to a deprecated dependency during build. Will look into fixing it, then merging this.

[benguaraldi]

Thank you so much, @Birkbjo!

[dhis2-bot]

🎉 This PR is included in version 32.31.11 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀