Timelogs GraphQL resolver authorization checks issue
When running a query to get the list of timelogs, it can happen that the result is empty but the computed fields (count
and totalTimeSpent
) are calculated. The pagination data also looks wrong: it mentions that there is more than one page, but the cursors are all null
.
This seems to be due to the authorization check that is being performed on the timelogs happens only after having retrieved the list of timelogs. The authorization logic is not even implemented inside the resolver, it looks like it's built-in into the GraphQL endpoint.
This causes the resulting set to be filtered out after the computation of the computed fields and can lead to those pagination issues.
It could be also considered a security issue since the total time spent value is still calculated taking into accout the timelogs the user doesn't have access to.
Implementation plan
This won't be a trivial change because we will need to check if the user can read the timelogs, and that is related to whether the user can read or not the issuable associated to each timelog, which is in turn related to the project access level.
One solution would be to iterate over the timelogs and check the policy to determine if each timelog can be read or not. This is not the best in terms of computation though.
Since each timelog has the ID of the project it belongs to, another solution could probably be doing something similar to what is done for the issues_finder.rb
(https://gitlab.com/gitlab-org/gitlab/-/blob/581642addf0504e7687f83c56edb59933456c906/app/finders/issues_finder.rb#L67-75), where we are filtering the list of issues based on the access level directly with the database query (much better than iterating the set after).
We might also need to preload the project associated to each timelog like it's been done for the issues_resolver.rb
: https://gitlab.com/gitlab-org/gitlab/-/blob/581642addf0504e7687f83c56edb59933456c906/app/graphql/resolvers/issues_resolver.rb#L22-27