On 03/05/2022 15:56, Vilnis Termanis wrote:
(fixing up the quote):
My understanding is that, for updates, the stack of datasets is respected
when deciding whether access the text index whereas for querying the
dataset context is used, which is not stacked.
I'm confused. "understanding of what is supposed to happen" or
"understanding of what does happen"? What is "stacked" - do you mean the
context settings should not pushed down? That seems like a safer
approach to me and removes the burden on the user knowing to set the
endpoint context.
(The context merging is for
server + dataset + endpoint but there is only one of each.)
I think the proposed fix would cause issues only if there is an expectation
for AccessControlledDataset to behave differently when it comes to handling
endpoint context: Right now it is ignored.
Yes, if it is secure - that has be be determined. As per comment on the
PR, requiring the user to set endpoint context is making the users life
harder.
But we have two issues here - whether the access control is correctly
picking up the context (a general issue c.f. timeouts) if it is safe to
do so, and whether the text dataset should push down the context setting
(specific issues of locating the text index).
There is a subsidiary issue: cleanly unsetting a context entry.
But hopefully this is a good starting point to throw rocks at.
So should the PR be a draft?
Andy
