Item14348: The allow_* method of access control is a complete pain in the ass.

The allow_* method of access control is far too inflexible for the case where you have a DB that is unrestricted.

I have the following database configuration:

  wiki_db_test => {
    dsn => 'dbi:ODBC:wiki_db_test',
    password => '****',
    user => 'wiki'
  }

and a simple (contrived) query:

%DBI_QUERY{"wiki_db_test"}%
SELECT
    name
  FROM
    topic
  WHERE
    web = '%QUERYWEB%'
%DBI_QUERY%

I want to be able include this query in any topic, and have any user perform it. But there is no way to specify this level of access in the DatabaseContrib. Something like:

    allow_query => {
        default => [ '*' ]
    }

(maybe there is a way to do this; if so, it's not documented and I can't find it in the code)

Further, I'm a member of the AdminGroup (or at least my LDAP id is) but the access controls still deny me even when

    allow_query => {
        default => [ 'AdminGroup' ]
    }

is set.

The following patch works for me:

diff --git a/lib/Foswiki/Contrib/DatabaseContrib.pm b/lib/Foswiki/Contrib/DatabaseContrib.pm
index 05f97d5..ad94d20 100644
--- a/lib/Foswiki/Contrib/DatabaseContrib.pm
+++ b/lib/Foswiki/Contrib/DatabaseContrib.pm
@@ -376,6 +376,9 @@ sub access_allowed {
 #say STDERR "  " x $nesting, "Got '" . ($match // '*undef*') . "' for $parent_acl";
             last if defined $match;
         }
+    } elsif ( !defined $connection->{$access_type}
+              && !defined $acl_inheritance->{$access_type} ) {
+        $match = 1;
     }
 
 #say STDERR "  " x $nesting, "Returning $access_type result for $user on $topic for $conname: '", $match // '*undef*', "'";

However, I need to be able to restrict access to DB's based on user alone, i.e. for all topics. I can't see how to do that with the current system.

-- Main.CrawfordCurrie - 22 Mar 2017 - 13:50

I see your point... Hm, I always considered this plugin to be rather a security pain in the ass and the most relaxed access control level was to have a group permissions.

Regarding the LDAP issue – perhaps I'm using a wrong API call to check if a user belongs to a group. But I currently don't know how to test the issue.

BTW, it is time to move the repos from my profile to project's collection. I shall do it in first place.

-- VadimBelman - 22 Mar 2017

You are using the right API to determine group membership. The problem was mine, ultimately, but very hard to track down without debugging support in the DatabaseContrib

Restricting access by user isn't relaxed, it's a fundamental requirement.

• Access has to be open by default, and it has to be possible to restrict access according to:
  • source of the request (might be topic, or all topics in a web)
  • user making the request (including restriction by group membership)
• Would also be nice to restrict access to certain queries
  • for example, I pre-declare a query called "fred" in a template, and then restrict access to that query to certain topics, users

Perhaps some of these are already possible, but the lack of examples in the documentation make it really hard to work out.

It would have been nice if the DBIQueryPlugin followed the standard syntax for sections, too. Using commonTagsHandler is a real drain. e.g.

%STARTSECTION{type="dbi_query"}% ........... %ENDSECTION{type="dbi_query"}%
%STARTSECTION{type="dbi_do"}% ........... %ENDSECTION{type="dbi_do"}%

I admit it's not particularly clear how to extend this syntax from a Contrib, and on review I see it'not as efficient as I first thought. Maybe we need some work on declaring section handlers from plugins/contribs.

-- Main.CrawfordCurrie - 23 Mar 2017 - 08:15

I'm afraid the amount of changes you request is too much for me now. I simply don't have enough time, unfortunately. It turned out that the version of contrib published on foswiki.org was modified comparing to what I left on twiki years ago. Then I was migrating to foswiki on my corporate site and simply converted the old version skipping over foswiki's modifications.

As a result I now have two barely compatible branches which has to be merged before any other fixes could be done. I'll try to find some spare hours for this but absolutely not sure when it will happen.

To the section syntax – don't know if it was there when the plugin was initially developed. Anyway, I didn't know it exists. But one of the ideas behind using commonTagsHandler was to let the plugin generate wiki syntax and let it be processed normally.

-- VadimBelman - 23 Mar 2017

OK. You didn't set a Modification Policy on the contrib form, so I'm going to assume it's "feel free to change" and apply my patch, as the contrib in it's current state is unusable.

Fixes have let me downgrade this to "Enhancement"

There's nothing wrong with the DatabaseContrib on the access control mechanism per se. It's the way it's used in the DBIQueryPlugin that is a PITA. The access control mechanism uses the topic to define the context of the query, but doesn't allow for any wildcards on the definition of that context.

-- Main.CrawfordCurrie - 24 Mar 2017 - 08:18

Sure, feel free to patch it.

I planned to merge foswiki's code with my personal patches but as long as it gonna take more time than I'm ready to spend now the task would be postponed for undefined period. Perhaps the only patch I would eventually apply will be the one implementing so called 'permission inclusion' which was used to make it possible to make allow_do permissions a part of allow_query. I.e. if a user is mentioned in allow_do then no need to define allow_query separately for him. Otherwise there is no big features worth merging.

-- VadimBelman - 24 Mar 2017

Don't want to edit the previous comment, just a note: it turns out that I totally forgot about what's been done for the contrib previously. So, the second paragraph must be ignored.

-- VadimBelman - 24 Mar 2017

Could you, please, check out Item14348 branch? I have implemented glob-matching on both sides – for contexts and users/groups. There are examples in DatabaseContribTestCommon.pm.

The logic behind contexts is simple: get a context we're matching permission against and run it through all contexts defined in the config. Though it slows down the procedure comparing to simple hash matching but provides more flexibility. Like, for example, making it possible to match against all Tasks.Item* topics or even the whole web, or a group of webs – whatever.

For user/group it is more complicated. It first checks if we're dealing with a glob by looking for an asterisk (for now it ignores the fact that the asterisk could be prepended with \). If we are not then the old logic applies. For a glob it tries to match against user name normalized to WikiName form. If fails then checks if the glob ends with 'Group'. For a group glob it tries to match against all user groups. If succeeds then either user or group matching the glob is returned.

Regarding your commend on undefined group order – it'd be a rare case but I'll redo the code to use two greps instead of sort. This will preserver the order as defined for a context. But for a glob everything would depend on what the iterator returns.

-- VadimBelman - 25 Mar 2017

Did you push the branch? I don't see it. Also, please check how I rewrote the doc, make sure it reflects your intention. And not my comment about mapping FW users to DB users - it's random which group mapping is used, if a user is in multiple matching groups - probably not what you intended.

-- Main.CrawfordCurrie - 27 Mar 2017 - 07:41

The branch is there – check out https://github.com/foswiki/DatabaseContrib/tree/Item14348

Let's get back to the doc if we give the globs 'good to go' as there will be some more changes. By quick overlooking I didn't see anything wrong about your changes.

The multiple groups issue – I'll redo in today. So far, I only considered one side of the dime – permissions, where a user either granted access or not. Then the order is not relevant. But the other side – which mapping is used – was escaping my eye until you pointed at it.

-- VadimBelman - 27 Mar 2017

As I dug deeper into the group order issue it appears to me that there is no clear solution for it. The best idea I've came up with is adding a 'prio' key to entries in the usermap key. It would globally define what groups are gonna be matched first against a user. Entries with no 'prio' are considered unordered and are getting last into the action with no order defined.

If a user is an exception of this rule he must be defined individually in the map.

For a reason I don't understand yet this approach doesn't make me happy. On the other hand I don't understand what is wrong about it what makes me unsatisfied.

-- VadimBelman - 27 Mar 2017

The simplest, most obvious, answer to the usermap problem is to change it from a map to an array and enumerate it in order. Or am I missing something? Of course that means individual users may not resolve exactly as you want them to, but then all you need is to add them individually, as you say. Which is fine.

-- Main.CrawfordCurrie - 28 Mar 2017 - 14:47

Being overloaded with work I didn't have time and it seems to be for better. It gave me some time to think before acting.

Initially I was worried about compatibility issues of switching the usermap from hash to array format. But with the latest commit I simply made it supporting both. Arrays are preferred, though. But that's not documented yet.

Can you test and report back to me if the branch is working for you? I would merge it back to the master then and publish the new version.

-- VadimBelman - 02 Apr 2017

After some thinking of what you said on the IRC I decided to follow your proposal. So, for an array usermap there is no additional sorting applied.

Adapted it for testing and added a new test for the usermap.

-- VadimBelman - 06 Apr 2017

Commenting is disabled while not logged in