search

bwanders / dokuwiki-strata

Strata - a Semi-Structured Data plugin for Dokuwiki
https://www.dokuwiki.org/plugin:strata
17 stars 8 forks source link

First guess for an ACL extension #15

Closed pbruneck closed 8 years ago

pbruneck commented 8 years ago

This is my first attempt for ACL rules in strata ...

The basic idea is, that once in the page build process an index of blocked pages is generated

helper_plugin_strata_triples::$blocked_graphs_sql

This index is represented as a SQL filter

AND graph NOT IN ('abc:def', 'abc:xyz')

which will be injected into the SQL code in _trans_tp(). This should prevent that any private information is gathered by the plugin. For performance reasons an additional SQL index is introduced.

bwanders commented 8 years ago

Thanks for the interest and the work! This is a well-written first attempt, and I think that we can merge it without too much of a hassle after we've fixed the two above line comments, and the following general issue: dokuwiki's caching.

Dokuwiki caches the final generated HTML, and serves from the cache the next time the page is requested. This cache is not aware of user logins. With the query results depending on the ACL, cache hits will likely result in users getting HTML with too much classified information or too little information according to the ACL rules. The easiest way to fix this is to make sure that pages with queries on them are not cached of the use_acl option is enabled. (I do not have a concrete proposal to fix this, but a good starting point would be the ~~NOCACHE~~ syntax in dokuwiki.)

Originally I wanted to remark on the performance implications of enumerating all graphs and checking the ACL for each of them, but I think that the added value is high enough for those that use it, and there is virtually no performance decrease while the use_acl options is disabled. So that's fine by me. We'll fix performance once we get to the point it is annoying.

I look forward to updates to the PR!

bwanders commented 8 years ago

Without a solution to the caching problem, merging this PR will lead to unintended information leaking. I'll have to close for now... If someone wants to pick it up, reopen at your leisure.