Listings by Sqlite don't respect ACLs

[michitux]

When you assign a page to a blog it will be displayed in that blog listing regardless of the permissions of the page. I think this is an issue for several reasons:

• It would be nice to be able to create draft blog posts that are already part of the blog but are protected by ACLs and then publish them by changing the ACLs
• If you have a private part of the wiki with a private blog and somewhere a page that can be edited by users and these users can guess the name of the private blog, they will be able to see the listing of these entries (possibly with the whole content or a part of it).
• If you assign a private page to a blog (or the wrong blog) by accident it will be published (and you might not even notice the problem immediately as you assume that only users with enough permissions can see it in the list)

[Klap-in]

The commit (part of listingsacl branch) referenced above added acl checks in the queries of the <blog *> syntaxes.
The new form syntax does already check acl.

Are there more cases which doesn't respect ACL?

[michitux]

What about comments in the recent comments syntax for pages the user can't view? I wonder if blogtng should also check if the page is hidden, i.e. if isHiddenPage($id) returns false (or isVisiblePage($id) returns true). I've seen that you've added ACL checks for the tag cloud. Unfortunately this slows down the tag cloud generation (see dokufreaks/plugin-cloud#20), but I don't know how bad it is in the case of blogtng compared to the tag/cloud plugin. A possibility is adding a configuration option if only public pages should be considered for the tag cloud and if an option should be enabled to chose for which user or group the ACLs should be checked (or if no ACLs should be checked) for wikis where write access is only given to trusted users and there should be a private and a public tag cloud. At least that are my ideas for the tag/cloud-plugin, I don't know if blogtng needs to support this variety of options.

[Klap-in]

• The page of the comments collected for the recent comments are checked for their acl.
• There isHiddenPage() is meant to hide pages from general listing, i will apply it too.

The aclcheck for tag cloud generation are quite similar to that of the other queries of blogTNG, when it give performance penalties, i guess the other queries are affected too.

With respect to tag cloud, the tag cloud of blogTNG doesn't calculate counts for the tags. Maybe therefore this cloud is not that slow?

I wasn't aware that the auth_quickaclcheck() method can slow down the whole stuff. I have not a big data set here. Sofar i have seen in the <blog *.> stuff the caching is default disabled. I guess that enable caching will store stuff too long.

[michitux]

The tag cloud of blogtng does calculate counts for the tags which means that sqlite needs to execute the acl check for every page that is a blog post at least once (maybe even for each tag and page once). With the check for hidden pages this also includes one triggered event for each page. The load_by_blog-function returns tag-pid-pairs and for each of these pairs the acls need to be checked, and this function is used by xhtml_tagcloud.

I'm more worried about the tag cloud than the other queries because I hope that the function will only be executed for pages that might be returned as result which shouldn't be much more than the actually returned pages (unless the newest pages are all read-protected).

[Klap-in]

I'm looking to add a function for checking page access to the Sqlite plugin.

So far i have some questions:

• is the isHidden() check really desired (not yet tested), it seems logical it will slow down. And do we want it is considered for all queries?
• what do you prefer, a HASREADACCESS that returns a boolean or a CHECKACL that returns a permission level that is compared in the query. Personally i prefer that latest, because it allows checks for other levels as well. e.g.

                  WHERE '.$blog_query.$tag_query.'
                       AND CHECKACL(page) >= '.AUTH_READ.'
                  GROUP BY A.pid

• at the moment i consider this _checkACL, which sets all hidden pages to AUTH_NONE. Do you agree?

    /**
     * Callback checks the permissions for the current user
     *
     * This function is registered as a SQL function named CHECKACL
     *
     * @param  string $pageid page ID (needs to be resolved and cleaned)
     * @return int permission level
     */
    public function _checkACL($pageid) {
        static $aclcache = array();

        if(isset($aclcache[$pageid])) {
            return $aclcache[$pageid];
        }

        if(isHiddenPage($pageid)) {
            $acl = AUTH_NONE;
        } else {
            $acl = auth_quickaclcheck($pageid);
        }
        $aclcache[$pageid] = $acl;
        return $acl;
    }

Let i mention @splitbrain and @Chris--S as well, your feedback is welcome too.

[Klap-in]

Any feedback at this idea for adding ACL and hidden checks to Sqlite plugin?

[Klap-in]

All the queries to sqlite-db are now checking per page the ACL.