WIP: file-based rights: overhaul

[marschap]

This is work in progress: undocumented and untested in context - please feel free to comment., but do not merge yet

• treat the 'groups' config key similar to the 'user' config key
  • BREAKING CHANGE: make 'group' a pattern instead of a list
    • same "format" as the 'user' and 'collection' keys
    • allow for easy expression of "any": .+
  • allow giving "{group}" in the 'collection' pattern to allow for resticting to the goups a use is member of
• fix the rule matching logic
  • if both keys, 'user' and 'groups', are given, both need to match, and both can be used in the 'collection' pattern
  • if only one is given, only the given one needs to match, and only the given one can be used in the 'collection' pattern
  • if none is given, the rule will always fail

[pbiering]

@marschap - please rebase and will this be finished in the next 2 weeks or should I postpone this to 3.7.1?

[marschap]

Hi Peter,
please don#t hold your breath: don't let this issue stop or slow down releases of radicale.

I am a bit stuck here: before the new version goes live, it should have test cases added covering also groups.
Because groups seem to have been bolted on after the initial design, I am not sure how to do that without re-factoring the whole auth/* files to include groups into the return of the logins, and the Access class to also comprise the user's groups.

So:
a) I am not sure the change should go into the early 3.7.x versions, especially not without tests
b) help is welcome to allow tests with groups in test_rights.py without the full re-facturing mentioned above.

PS: I think groups should not be treated special for LDAP, because it is possible to extend other auth modules as well:

• pam.py: it already uses groups to internally - it sohuld be easy to expose them
• oauth2.py: scopes could be added and - if provided - exposed as a kind of pseudo "groups"

[pbiering]

@marschap - will you be able to fix this soon to become active in 3.7.0 planned for next weekend?

[marschap]

Hi Peter,

the situation is unchanged to 3 weeks ago: don't let this stop radicale.

I did not yet find a way to test my changes to authorization() in radicale/rights/from_file.py.
In my reading of the test code, ot looks like groups need to be passed to authorization() as an argument,
which is currently not the case: currently they are a property of the class: self._user_groups.

I appreciate any help in using this class property for creating tests with groups!!

[pbiering]

@marschap - can you provide me some examples how configs and test cases should look like?

[marschap]

@pbiering In my opinion it makes most sense to have radicale/tests/test_rights.py extended so that it can additionally deal with the groups, that a user may have, passed as an additional argument to _test_rights().
I.e.something like
def _test_rights(self, rights_type: str, user: str, path: str, mode: str, user_groups: Set[str], expected_status: int, with_auth: bool = True) -> None:
with the new argument user_groups argument then ending in self._rights._user_groups when the test is started.

With this preparation, all the existing tests need continue to pass no matter whether a user has groups or not (of course).
In addition new test cases can be added which make use of the groups: keyword and also the {group} variable in the collection: keyword from the rights file.

I hope this makes sense.

[pbiering]

you can extend your PR by following then user_groups is optional and not breaking any current test:

--- a/radicale/tests/test_rights.py
+++ b/radicale/tests/test_rights.py
@@ -19,6 +19,7 @@ Radicale tests with simple requests and rights.
 """
 
 import os
+from typing import Set
 
 from radicale.tests import BaseTest
 from radicale.tests.helpers import get_file_content
@@ -28,7 +29,9 @@ class TestBaseRightsRequests(BaseTest):
     """Tests basic requests with rights."""
 
     def _test_rights(self, rights_type: str, user: str, path: str, mode: str,
-                     expected_status: int, with_auth: bool = True) -> None:
+                     expected_status: int, with_auth: bool = True,
+                     user_groups: Set[str] = set([])
+                     ) -> None:
         assert mode in ("r", "w")
         assert user in ("", "tmp", "user@domain.test")
         htpasswd_file_path = os.path.join(self.colpath, ".htpasswd")

[pbiering]

@marschap - do you need any other support?