BulkMemberAccessLoad concern does not differentiate memoization_index across models
Summary
BulkMemberAccessLoad is used to memoize max member access in projects and groups so we don't have to query the database each time we ask (Using SafeRequestStore
).
This line returns the key to store the max access in this format "max_member_access_for_#{klass.name.underscore.pluralize}:#{memoization_index}"
. But, memoization_index makes no diferentiation if the stored ID is from a project or a group. So, if you get the max access for a group with ID=6 and it is MAINTAINER
, it will be stored in the SafeRequestStore as max_member_access_for_users:6
. If you ask for the max member access for a project with ID=6 on the same request, you will get MAINTAINER
without it getting to the DB.
Steps to reproduce
Didn't get to test on the API or any other place than specs yet, so might not be possible to accomplish from any of our external APIs.
Probably easier to test in the GQL API by multiplexing. So, make a request that authorizes read_group where you have access. In the same request try to read a project with the same ID as the group, and you will get access even if you don't.
This also happens in the opposite direction, if you get max access for the project with no access, the trying to read the group will fail because no access for that group ID is already memoized.
Sample queries described in #336802 (comment 636855209)
What is the expected correct behavior?
Memoization on the request store should also store the model the ID belongs to.
Possible fixes
Possibly changing this line to use the model as part of the key is enough. Might affect several places where BulkMemberAccessLoad
's methods are called.