Roundcube-SQL-Global-Address-Books icon indicating copy to clipboard operation
Roundcube-SQL-Global-Address-Books copied to clipboard

Restrict global address book to specific users

Open IzzySoft opened this issue 10 years ago • 38 comments

First: congrats to this plugin – exactly what I was looking for!

Now: Domain address books can be restricted to the resp. domain, which is fine with me. I'd like to use the Global address book (all domains), but not expose it to everyone. Use case: staff members/admins should be able to see users of all domains, "normal users" should not. While this would be easy if staff members were all in one domain where no "normal users" are in, that doesn't always match: either other users are part of the same domain as their admins, or both are split across multiple domains.

Possible approaches:

  • with PostfixAdmin used (as in my case), the domain_admins table (i.e. a view on it) could be used, restricting the book to members of this table with active=1 AND domain='ALL' (full view of all domains) resp. active=1 AND domain=<domain> (selective view for subset of domains)
  • if the first cannot be used, the same might be done with a table that's manually populated
  • another possibility would be introducing a $config['_sql_gb_data_allowed_users'] setting, where all allowed users ("admin staff members") are listed explicitly. If set (and not NULL), this could either override the other hidden/allowed settings ("staff only") or be added to those (to e.g. allow one full domain plus users from this list regardless of their domain). For simplicity I'd vouch for the latter, as the former can be achieved easily using $config['_sql_gb_read_hidden'] = array('*'); if I got that right.

I'd be fine with either approach (the last one is probably easiest to achieve, and should cover most cases unless staff numbers are high – though the first approach might be easier to manage, as one cannot forget to add new/remove "old" staff members).

Feasible? Would you consider this? Or is it even possible already, and I just didn't see it? Thanks in advance!

IzzySoft avatar Aug 12 '15 09:08 IzzySoft

I understand what you mean, essentially similar to my own use except for domains which mix both admins and users. I will have a think about the best course of action and if I can find the time will implement it for you

t3chguy avatar Aug 12 '15 09:08 t3chguy

I'm thinking of adding a way of adding SQL commands to the config under the sql_xx_xxxx* so that instead of an array you'll be able to define a string which will be executed under the same MySQL Connection and then you can define your own view to achieve what you want.

Ideally I'd rewrite this plugin from scratch to be completely dynamic where you could have multiple very modular and customizable global/domain/etc books but currently it is rather restrictive, but for this I definitely do not have time.

t3chguy avatar Aug 12 '15 09:08 t3chguy

To be fair, it is possible to use SQL to populate the config already, since it is just a PHP file you can reference rcube::get_instance()->db to use Roundcube's SQL Connection. But since this is not very user-friendly I have began slow work on refactoring for a v3.

t3chguy avatar Aug 12 '15 10:08 t3chguy

Thanks for the fast response! Sounds great. I won't go the "not very user-friendly" way now (it's not that urgent for me, and might instead complicate future updates, plus there are more urgent issues I have to attend to currently) but rather wait for your solution then. So I'm looking forward to that :innocent:

IzzySoft avatar Aug 12 '15 11:08 IzzySoft

Please note that this will be pretty much an entire re-write so your config will need be scrapped and re-written from scratch anyway.

t3chguy avatar Aug 12 '15 11:08 t3chguy

Sure. But this way I haven't to do it twice ;)

IzzySoft avatar Aug 12 '15 13:08 IzzySoft

You can monitor my progress on the Development branch

t3chguy avatar Aug 12 '15 13:08 t3chguy

Thanks again! Before "bumping" the issue I'll check there #D But as I wrote, I'm still quite busy with the other components of my new server, so no need to hurry – you already impressed me with your response times :smiley_cat:

So, and this time I hope to hit the right button in the first place :see_no_evil:

IzzySoft avatar Aug 12 '15 15:08 IzzySoft

in theory the Global Addressbook works in the Development branch, not got around to testing anything other than the basics, a lot of the programming I do is theoretical until pre-release when I get to testing haha

t3chguy avatar Aug 12 '15 18:08 t3chguy

Wow, that was fast! If no changes to the database are required, just let me know when I shall start testing. Hints are welcome then, too, of course (what to test, what changed with configuration/setup, things to consider on "replace"). My current Roundcube installation is not yet productive, so there're no much things in danger here.

IzzySoft avatar Aug 12 '15 20:08 IzzySoft

Give me another 24-48 hours to get a little more progress, got a busy day tomorrow, send over your existing config (in a comment) and I'll try and convert it to the newer format for you to be able to test once I am happy for you to; Thanks

t3chguy avatar Aug 12 '15 20:08 t3chguy

Database requires no changes, just the plugin itself and config file have been rewritten from scratch but still works in a fundamentally similar way, just far more dynamic, extendable and configurable.

t3chguy avatar Aug 12 '15 20:08 t3chguy

Sounds fantastic. So basically, I could rename the current plugin dir, drop-in and configure the "beta" – and if it causes problems, just switch back to the directory with the "stable" version, right? And as it acts read-only, there's no danger anyway.

In this case, let me know when I should start (in 24-48 hours, yes ;)

As for the config: Consider it equivalent to the -dist one. Only thing I've changed for now is explicitly disabling the Global address book (i.e. sticking to the domain-wide only).

IzzySoft avatar Aug 12 '15 20:08 IzzySoft

I'm thinking of renaming it so you wouldn't even have to do that, just changing config.inc.php (of Roundcube) - need to figure out what to call it though since its not only a Global Address book, this might mean having to rename the SQL View also, do you have any naming ideas?

t3chguy avatar Aug 12 '15 20:08 t3chguy

Both fine with me.

As for names, depends whether you want it simple or sophisticated:

  • MultiBook / ManyBooks / AutoBooks (auto for the auto-populating)
  • AFABooks / APABooks (Auto-Fill/Populate-Address books)
  • AutoPop
  • MaggiBooks (I love acronyms/backronyms: Multi Address Global Group, pick something for the "i")

And speaking about "simple or sophisticated", I love double meanings too: SoSBooks. The SOS has the second meaning of asking for help/assistance, obviously.

So far my brain-storming – hope it helped a bit.

IzzySoft avatar Aug 12 '15 23:08 IzzySoft

Definitely helped a bit, I'm currently thinking MultiBook, but I'll decide when I get the chance to carry on in 24-ish hours.

Thanks

t3chguy avatar Aug 13 '15 07:08 t3chguy

That was my favorite as well (hence named first), as it reflects best what the plugin is about: Multiple address books. Plus it's easy to remember, and unique enough to stand out (e.g. when googling for something connected to it, a Google search on "multibook roundcube" currently returns 3 hits only) – which is good when deciding on a "branding".

IzzySoft avatar Aug 13 '15 11:08 IzzySoft

The functionality that you wanted should be functional. There is only currently one addressbook (Global) and only planning one more for now (Domain), but filter (hiding some users) but show and hide (choosing which users can see the book) should be functional. (including from custom SQL Queries)

t3chguy avatar Aug 14 '15 19:08 t3chguy

I already saw you've been quite active (just checked the repo an hour ago). As I'm not using the Global addressbook (yet; with the filters, that's of course a different thing) but really need the Domain one, I guess I rather wait until that's implemented as well before I start testing.

From looking at the config (which appears easy and intuitive even for newbies, if no filters come into play) I don't see where the hiding part (filters) can be defined. It probably requires a call to addFilter() – but I miss corresponding hints in the config (maybe some examples) to not have to revert to "try-and-err" methods – especially for how the parameter (array $entry) should look like. I might even be wrong here and it requires a call to addHideSQL() – so I had to guess for the SQL then (just the WHERE clause? Or the WHERE conditions only? Of course related to the view, but …).

So even if I would start testing now, I wouldn't get much further than download-unpack/clone. Need some more input :innocent:

IzzySoft avatar Aug 14 '15 20:08 IzzySoft

Hmm, I could actually implement Domain book as a subset of Global book which just sets a filter. And yeah you're right with how to add filters, the config is still for testing, last stage before release is making it user-friendly. and the $entry argument can either be array (mostly used internally) or even just a single domain (domain.co.uk) or fulluser ([email protected]), or you could feed an array of them at the same time instead of multiple calls. And the SQL actually just pulls a list of names/users/domains from SQL and adds them to who can/can't see the book, this isn't possible with MySQL as its not relevant.

If I understand correctly you want to use your domain_admins table to list who can see the Global Book, you would do something like $GlobalBook->addShowSQL('SELECT username FROM domain_admins') and with that only your domain_admins can see the Global Book (variable name is how you define it, and the username may not be the right column, I'm not sure of the columns in your db) Hide and Show are WHO can see the book Filters are which users/domains are hidden from the book, which will in the end be either the WHERE clause or a post-filter.

--Note-- Filters have just had to be renamed to Cloaks Cloaked users will be hidden from that book, for example might be useful to cloak [email protected] so that its not shown in the Global Book or whichever you mask it from. (*due to filters being a keyword I had to use to keep full Roundcube functionality)

Does that make some sense? This will hopefully be much more clear once I update the Docs/Hints in the Config.

t3chguy avatar Aug 14 '15 20:08 t3chguy

This will hopefully be much more clear once I update the Docs/Hints in the Config.

I bet on that! Yes, much clearer already now. First thing I've learned: Filters are for "what is shown", and __SQL* is "who can see". That's one step further.

Limiting the global book to domain admins (regardless of which domain) is easy, yeah. For your example, consider something like:

$DomainBook->addShowSQL("SELECT username FROM domain_admins WHERE domain IN ('ALL','%d')");
$GlobalBook->addShowSQL("SELECT username FROM domain_admins WHERE domain='ALL');

i.e. limit the DomainBook to admins of the current domain (%d placeholder plus ALL super-admin), and the GlobalBook (all domains) to super-admins.

Next: What happens when I define multiple "shows" – will it further limit (i.e. AND connect the queries) or expand (OR connect) the number of allowed viewers? Say e.g. I want to permit access to the GlobalBook for super-admins plus [email protected] – or to all super-admins but keep Joe out. Not everybody speaks SQL fluent enough to combine that into one query – and multiple simple calls also make the config easier to read. Just mentioning.

As a DBA, I of course should know how to use a simple UNION here – though as Oracle DBA, I had to check for the right syntax in MySQL:

-- all admins plus joe
$GlobalBook->addShowSQL("SELECT username FROM domain_admins WHERE domain='ALL' UNION SELECT '[email protected]');
-- all admins without joe
$GlobalBook->addShowSQL("SELECT username FROM domain_admins WHERE domain='ALL' AND username NOT IN ('[email protected]'));

A mix is trickier, as MySQL doesn't support MINUS or EXCEPT – but for that you already have the other round (FacePalm!) using addHideSQL(): Show with UNION, then Hide the rest. And if I'm not completely mistaken, I've just answered my above question: two addShowSQL() are the same as one with UNION, and addHideSQL() would run last and "take away" from that? Then I hope my rant at least provided you with some useful copy-pasta material for examples/documentation :innocent:

One last example: I have an account in multiple of the domains hosted, always with the same username. So to have the GlobalBook shown to me:

$GlobalBook->addShowSQL("SELECT username FROM mailbox WHERE username LIKE 'izzy@'");

or, to stay within the RoundcubeDB and not needing a special view:

$GlobalBook->addShowSQL("SELECT username FROM users WHERE username LIKE 'izzy@'");

--Note-- As I didn't use Filter examples, nothing needs to be cloaked on copy :smiley_cat:

IzzySoft avatar Aug 14 '15 22:08 IzzySoft

Instead of your example at the end, you could just do $GlobalBook->addShow('izzy@'); and this will allow all users which have the localpart of izzy, this would be a small bit better as no SQL Call would need to be made.

t3chguy avatar Aug 15 '15 08:08 t3chguy

And on your Union example, you can either run Two SQL Queries, or Union, or since your "extra" user was actually static, you should use ::addShow() as it won't invoke SQL. ::addShowSQL() is actually a wrapper to ::addShow() and both are in the public scope.

t3chguy avatar Aug 15 '15 10:08 t3chguy

Actually, $GlobalBook->addShow('izzy@'); would be all I need in my case, as I'm (currently) the only admin – so that's great and simple (make sure to include this example!). And as I see I finally got the idea (LOL), I'd say also mention that two "add" calls would work as "sum", where "hide" takes away from. Easier to read, even though that would mean additional SQL calls; not sure whether a mention of "UNION" as a "cheaper call" should be made (cannot hurt, though).

In any case: using it that way would need either an additional view (if we wish to check for admins), or an additional field in the current view. The latter might be easier: a column admin which is either "ALL",NULL,or the domain. Trouble with that: what if our Joe is not a super-admin ("ALL"), but admin of "example.org" and "example.net" (i.e. one user is admin of multiple but not ALL domains)? Then this approach would fail. So it looks like a second view might be required: SELECT username,domain FROM postfixdb.domain_admins WHERE active=1. Of course one only needs to create that if "access definition by admin" is intended. Will be useful for some, while not needed by most I guess – but adding that view definition SQL isn't really that much overhead (keep in mind mail/web admins are not necessarily firm enough to write up that themselves, so they'll be thankful to find it ready-to-use).

IzzySoft avatar Aug 15 '15 13:08 IzzySoft

You can obviously extend the existing View with an extra column and use addShowSQL to check it. I will add a %d replacement for the add methods soon. Glad you are understanding it a little more now

t3chguy avatar Aug 15 '15 13:08 t3chguy

As far as I see, I just need to:

  • Implement cloaking (hiding users/domains from books)
  • Implement %d
  • Test Test Test
  • Write Docs and Examples

t3chguy avatar Aug 15 '15 14:08 t3chguy

Glad you are understanding it a little more now

Yupp: investigating the code and "thinking aloud" helps – but cannot necessarily be expected from "normal users" :innocent:

You can obviously extend the existing View with an extra column and use addShowSQL to check it. I will add a %d replacement for the add methods soon.

If you do that with the existing view, be aware of the culprit I've mentioned above: what happens if a user has multiple entries in postfixdb.domain_admins? Either that results in multiple "rows" in the view (should be fine), or it would only cover one domain (not so good).

As for your "todo list": ping me when item number 3 is reached, from that point on I can help again :)

IzzySoft avatar Aug 15 '15 14:08 IzzySoft

In theory steps 1 and 2 are done. A lot of testing has to be done now before I can release 3.0.0 but if it works for you then feel free to use it. Report back any findings please

t3chguy avatar Aug 15 '15 16:08 t3chguy

OK, I'll see to give it a try ASAP – and then let you know. Thanks a lot already for the speed and implementation!

IzzySoft avatar Aug 15 '15 19:08 IzzySoft

I really hope that you can get some function from it. I've just had the worst luck, my computer just died, I can't code from my phone so I will be unable to continue this for now. I currently don't have the money to fix my computer. Sorry about this

t3chguy avatar Aug 15 '15 23:08 t3chguy