jena-fuseki-access - Propagate request/service context #1291

vtermanis · 2022-05-03T10:37:08Z

What

Process request/service context in AccessControlledDataset the same way as is done in (parent) query processor, i.e. honour the server => dataset => endpoint context value priority order.

Why

Standard SPARQL queries honour the context of the endpoint (see docs) as well as setting of reuqest-specific timeout via the timeout URL parameter. These two features allow one to:

Override context values specified at server and/or dataset level (e.g. disable access to text indexing or change the default timeout for the endpoint)
Specify per-request timeouts

(See bottom of summary of example. See also mailing list thread.)

How

Use QueryExec to create the QueryExecution instead of QueryExecutionFactory (the latter does not consider HTTPActions's context)
Use same timeout parameter logic for pre-request timeouts as in base SPARQL query processor

Note: I am not convinced that the way I've updated AccessControlledDataset (and promoted a helper method from SPARQLQueryProcessor.java to public) is the right way to go. But I can confirm that the following use-case works:

Define a dataset DS1 with jena:text indexing enabled
Define a dataset DS2, with AccessControlledDataset wrapping DS1. (The access actual rules are irrelevant here.)
Define service A exposing a query endpoint for DS1, with extended context: ja:context [ ja:cxtName "http://jena.apache.org/text#index" ; ja:cxtValue false ] ;
Define service B the same as A, but for DS2

Current behaviour (pre-patch):

jena:text is exposed only with query endpoint of B. SPARQL queries against A do not match text-indexed properties.

Expected behaviour (post patch):

Neither A nor B match text-indexed properties

vtermanis · 2022-05-03T10:38:03Z

...-fuseki2/jena-fuseki-access/src/main/java/org/apache/jena/fuseki/access/SecurityContext.java

+            .query(query)
+            .context(action.getContext())
+            ;
+        // Note: Instead of this call (and the above build), the equivalent code from


Repeating the same step here for timeout seems not right (but I wasn't sure what a better / the right approach would be).

kinow

Use-case sounds good to me. Not sure about the implementation either, so probably needs @afs 's review here.

Thanks!

(also need to confirm all tests pass in Jenkins or GH Actions [manual])

afs · 2022-05-03T11:20:47Z

From users@ thread https://lists.apache.org/thread/h0c81qjl8oc83yl2xf7xvt4l0pw4grrf

It looks like there is an issue as to whether the text dataset should push down the context setting for the index or not. Requiring endpoint configuration isn't so user-friendly.

This PR may contain a change to make anyway but this jena-fuseki-access uses context settings DataAccessCtl.symAuthorizationService itself so allowing the endpoints to modify context may be a security risk. Needs investigation - it's been a long time since I looked at the code!

An outstanding question from email:

Doesn't that [setting the context to an illegal value] cause warnings in the Fuseki log?

It'll need some tests.

vtermanis · 2022-05-03T15:02:30Z

Doesn't that [setting the context to an illegal value] cause warnings in the Fuseki log?

(copied from thread reply):

Yes it does - three (expected) warnings from TextQueryPF:

Context setting 'symbol:http://jena.apache.org/text#index'is not a TextIndex
Failed to find the text index : tried context and as a text-enabled dataset
No text index - no text search performed

It'll need some tests.

I'd be happy to assist with this (time permitting) if I'm pointed in the right direction.

vtermanis · 2022-05-03T15:08:47Z

Apologies - I updated the description since it had a mistake. The expected behaviour section has changed to:

Current behaviour (pre-patch):

jena:text is exposed only with query endpoint of B. SPARQL queries against A do not match text-indexed properties.

Expected behaviour (post patch):

Neither A nor B match text-indexed properties

vtermanis · 2022-05-04T08:05:44Z

(Changing to draft as suggested, since the best way to address this is still being debated.)

- Requires context propagation patch (see apache#1291) - Comment specified before query can be used to override security context with a given set of graphs. - Overridden security context is preserved in the execution context - Simplified regex pattern only - has to be first non-whitespace line to work.

- "#pragma acl.graphs" preamble can be used to specify one or more graphs to restriction query exectution to - The functionality is only enabled if an access:entry for a specific user contains just <urn:jena:accessGraphsDynamic> - Requires context propagation patch (see apache#1291), since query is stored in request-specific context

- "#pragma acl.graphs" preamble can be used to specify one or more graphs to restriction query exectution to - The functionality is only enabled if an access:entry for a specific user contains just <urn:jena:accessGraphsDynamic> - Only applies to SPARQL query, not GSP (since the latter can already be filtered dynamically) - Requires context propagation patch (see apache#1291), since query is stored in request-specific context

- Honour context from HttpAction (as is done in SPARQLQueryProcessor parent) - Set timeouts from request (as with SPARQLQueryProcessor) - Without this, context changes configured against an endpoint are ignored if the associated dataset is an AccessControlledDataset

- "#pragma acl.graphs" preamble can be used to specify one or more graphs to restriction query exectution to - The functionality is only enabled if an access:entry for a specific user contains just <urn:jena:accessGraphsDynamic> - Only applies to SPARQL query, not GSP (since the latter can already be filtered dynamically) - Requires context propagation patch (see apache#1291), since query is stored in request-specific context

afs · 2022-07-14T13:36:05Z

@vtermanis -- there has been a bug fixes for general context handling with queries and these are now on the main branch. I don't think it immediately relates to your report but I wanted to make sure you are working against the right state of the codebase.

vtermanis · 2022-07-28T19:57:33Z

@afs thanks for the heads-up, do you mean:

Improve handling of external transactional components #1444

Is that the main one or other there others in particular? (Though it'll probably be clear when I rebase what's changed and look at the query servlet & processor code)

afs · 2022-07-29T07:48:31Z

#1444 is about transactions.

#1375 and parts of some others around that time.

vtermanis commented May 3, 2022

View reviewed changes

kinow approved these changes May 3, 2022

View reviewed changes

vtermanis marked this pull request as draft May 4, 2022 08:05

vtermanis force-pushed the vt-add-ctx-to-fuseki-access branch from b4d73bd to 62b4adc Compare July 14, 2022 10:33

vtermanis added 2 commits July 14, 2022 11:35

fixup! jena-fuseki-access - Propagate request/service context

a1eb933

vtermanis force-pushed the vt-add-ctx-to-fuseki-access branch from 62b4adc to a1eb933 Compare July 14, 2022 10:35

Aklakan mentioned this pull request Oct 3, 2022

Make projected dataset in DynamicDataset accessible #1557

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

jena-fuseki-access - Propagate request/service context #1291

jena-fuseki-access - Propagate request/service context #1291

vtermanis commented May 3, 2022 •

edited

vtermanis May 3, 2022

kinow left a comment •

edited

afs commented May 3, 2022

vtermanis commented May 3, 2022 •

edited

vtermanis commented May 3, 2022

vtermanis commented May 4, 2022

afs commented Jul 14, 2022

vtermanis commented Jul 28, 2022

afs commented Jul 29, 2022

jena-fuseki-access - Propagate request/service context #1291

Are you sure you want to change the base?

jena-fuseki-access - Propagate request/service context #1291

Conversation

vtermanis commented May 3, 2022 • edited

What

Why

How

vtermanis May 3, 2022

Choose a reason for hiding this comment

kinow left a comment • edited

Choose a reason for hiding this comment

afs commented May 3, 2022

vtermanis commented May 3, 2022 • edited

vtermanis commented May 3, 2022

vtermanis commented May 4, 2022

afs commented Jul 14, 2022

vtermanis commented Jul 28, 2022

afs commented Jul 29, 2022

vtermanis commented May 3, 2022 •

edited

kinow left a comment •

edited

vtermanis commented May 3, 2022 •

edited