On Sun, Oct 27, 2024 at 05:13:17PM +0000, John Rouillard wrote:
> 
> In message <20241027064407.uzxiynqywsbcypby@runtux.com>,
> Ralf Schlatterbeck writes:
> >Proposal for examples:
> >We already have a check function on query (at least in the classic
> >template) that permits users to see their own and public (private_for
> >empty) queries.
> >
> >def view_query(db, userid, itemid):
> >    private_for = db.query.get(itemid, 'private_for')
> >    if not private_for:
> >        return True
> >    return userid == private_for
> >
> >Augmenting this with a filter function:
> >
> >    def filter_query(db, userid, klass):
> >        return [dict(filterspec = dict(private_for=['-1', userid]))]
> 
> Would:
> 
>    return [{"filterspec": {"private_for": ['-1', userid]}}]
> 
> be the same? If so we should probably have an example without the use
> of dict() if possible. IIRC dict is slower than the definition using
> {} (I can't remember the name for the {}'ed definition.  Also I think
> the static definition is more familiar to users/admins as well.

Fine with me.

> In the 'private_for' list, I assume -1 means unset and that items in
> the list are or'ed together.

This is a normal Class.filter call. So, yes, -1 means unset. We search
for 'unset or userid'.

> We also need a reference link to the filter syntax. References.txt has
> a reference for filter in the hyperdb wrapper class. Is that good
> enough? I have always found it a bit difficult to understand and there
> aren't a lot of great examples that show the input filter dictionary
> and describes how it gets applied. IIRC you had a similar issue a
> while back and added some more documentation.

Hmm, yes we should add that link and make a separate ticket for
improving the filter docs. I'm not very good at writing documentation
not being a native speaker... Maybe we can get some AI to improve on
that matter? (Honestly, this *could* be an idea :-)

> I also have an uncommited section in my user_guide.txt titled:
> 
>   Advanced Searching with Property Expressions
> 
> that talks about using RPN notation in the URL for filtering. I
> haven't committed it because I haven't verified all the examples I
> mention. IIRC I wrote this before 2.4.0 was released and didn't commit
> it then because one of my examples failed. Also I am not sure that
> this location would be proper doc for the filterspec syntax.

Fine with me, I also don't know a better place.

> >Now if we also want to check if the user is the creator of a query and
> >permit access we would modify these to::
> >
> >    def view_query(db, userid, itemid):
> >        q = db.query.getnode(itemid)
> >        if not q.private_for or userid == q.private_for:
> >            return True
> >        if userid == q.creator:
> >            return True
> >
> >    def filter_query(db, userid, klass):
> 
> >        return [f1, f2]
> >
> >Note how we need a list with more than one entry here to account for the
> >OR-condition.
> 
> Understood.
> 
> Do we need AND-condition or NOT-condition examples as well?

The AND is automagically taken care of by filter (filterspec entries for
different properties are ANDed). I'm not sure we're very good at not
conditions using hyperdb.filter.

> 
> I assume (a nonsense) AND is:
> 
>    f1 = dict(filterspec = dict(private_for=['-1', userid],
>                                dict(creator=userid))

Yes.
But I don't think we need a separate example as this is covered by
filter.

> I still haven't traced through the code enough to understand how these
> end up being applied at the database level. What do the WHERE clauses
> look like when the filter is pushed down to the SQL level?

We're filtering in a set of

results = original-filter-call

for filter_args in all the filters from filter function:
    results = klass.filter (**filter_args)

Except that I'm optimizing the number of results to pass through the
filter calls by keeping two sets.

At the end I'm again filtering with the original sort/group arguments to
get the correct order.

> 
> >Another example would be the following: Consider we have a class
> >"organization". A user has a link to organization and the issue has a
> >link to organization. Users can see only issues of their own
> >organisation.
> >
> >A check function would be::
> >
> >    def view_issue(db, userid, itemid):
> >        user  = db.user.getnode(userid)
> >        if not user.organisation:
> >            return False
> >        issue = db.issue.getnode(itemid)
> >        if user.organisation == issue.organisation:
> >            return True
> >
> >The corresponding filter function::
> >
> >    def filter_issue(db, userid, klass):
> >        user = db.user.getnode(userid)
> >        if not user.organisation:
> >            return []
> >        return [dict(filterspec = dict(organisation=user.organisation))]
> >
> >Note how the filter fails early by returning an empty list of filter
> >arguments when the user has no organisation.
> >
> 
> The filter function here exactly duplicates the check function
> correct? If so, does this make the check function redundant? Can the
> permission be defined with only the filter function and no check
> function?

Yes, the filter function *always* duplicates the check function.
I don't know if this makes the check function redundant and if we could
automagically derive a check function from a filter function. I guess it
could be done by passing the current itemid through the filter,
something along the lines of

def check (db, userid, itemid, klass, filter_function):
    args = filter_function (db, userid, klass)
    if klass.filter ([itemid], **args):
        return True

Note that the check function currently has no knowledge of the klass and
the filter function. But it would certainly be possible to write a
factory that can return a check function from a given filter function,
and klass according to the recipe above. Like so:

def check_factory (klass, filter_function):
    def check (db, userid, itemid):
        args = filter_function (db, userid, klass)
        if klass.filter ([itemid], **args):
            return True
    return check

Untested. But I think this would work. It passes a single ID through the
filter mechanism filtering the single-item again with the standard
hyperdb.filter mechanism. If it returns the item we may see it. If it
doesn't it will return an empty list of results.

> Also how would that be modified to include people with an admin role?
> Are filter's ignored if the user is an Admin?

No the admin is not special. There is code that adds a permission entry
for the role admin. These do not have a check function and therefore do
not need a filter function.

Same for normal users that have access to a klass via a permission
*without* a check function: These are taken care of by the first if
condition in hyperdb.filter_with_permissions:

        if check(permission, userid, cn, skip_permissions_with_check = True):
            allowed = item_ids

If this check passes (it does when we have a permission for a klass
without a check function) no further checks are performed at all.

> I view adding 'filter' to permissions as the equivalent to the
> hyperdb's filter_sql() method. Do you agree?  (Side note filter_sql is
> missing examples.)

I wasn't aware we have that. (and it's not in hyperdb but in
backends/rdbms_common.py)

And, no I don't think this is correct, filter uses the standard
mechanisms of hyperdb. Nothing specific to SQL.

> If we are using 'filter' on a non-sql database (dbm, or future mongo
> etc.), how is the 'filter' applied? You said that the filter_function
> could be debugged on a dbm style database.

It is calling the standard hyperdb.filter. Line 1835 in
roundup/hyperdb.py in method filter_with_permissions.

The method filter_with_permissions falls back to previous behavior
(calling the check function for every item found) if not for every
permission with a check function we also have a filter function. The
debug setting simply enforces that fallback:

if not debug and sec.is_filterable(permission, userid, cn):

The else-part of that has the comment "Last resort: filter in python"

> 
> >I think this documents most of the use-cases.
> >Note that so far I've never needed the 'klass' argument to the filter
> >function. It might be needed when a filter applies to more than one
> >class, though.
> 
> I assume this would happen only if the value you are checking is
> equivalent in the two classes but has different names.

Yes. And I think it is a good idea to have klass in the signature.

> Consider a filter checking to see if the owner of the item is the
> current owner.  However you have help desk people creating issues and
> users. So the creator property is a random help desk person, not the
> actual creator. So you have a user_id property added to the user class
> and a requestr_id property added to the issue class to record the id
> for the actual person.
> 
> So you would generate a different filter against a different field
> based on the klass.
> 
> Is this an example when applying a filter to multiple classes?

No that would be a filter for different properties of the same class.
My idea would be that you might have two different issue-like classes.
In that case you could apply the same filter function to both and using
the klass argument to differentiate e.g. between different property
names or similar.

Thanks for taking the time looking into this!
kind regards
Ralf
-- 
Dr. Ralf Schlatterbeck                  Tel:   +43/2243/26465-16
Open Source Consulting                  www:   www.runtux.com
Reichergasse 131, A-3411 Weidling       email: office@runtux.com