[checkmk-commits] Additional permission function cleanups

Module: check_mk Branch: master Commit: 0f1c9ba4a0eeba93b70d57e42665c085f64e0eb8 URL: http://git.mathias-kettner.de/git/?p=check_mk.git;a=commit;h=0f1c9ba4a0eeba… Author: Lars Michelsen <lm(a)mathias-kettner.de> Date: Sat Dec 1 17:56:55 2018 +0100 Additional permission function cleanups * Replaced config.permission_exists() calls with direct permission_registry checks. * Moved compatibility declare_permission() and declare_permission_section() implementations to the permission module. Aliases in config.py need to be kept for compatibilty Change-Id: Ic4356821f7e6fdb1ec806969bdcf93cc20289930 --- cmk/gui/config.py | 41 +++++------------------------------ cmk/gui/pagetypes.py | 11 +++++++--- cmk/gui/permissions.py | 31 ++++++++++++++++++++++++++ cmk/gui/visuals.py | 7 +++--- tests/unit/cmk/gui/test_gui_config.py | 4 ++-- 5 files changed, 50 insertions(+), 44 deletions(-) diff --git a/cmk/gui/config.py b/cmk/gui/config.py index 176b6cb..9f35fea 100644 --- a/cmk/gui/config.py +++ b/cmk/gui/config.py @@ -75,8 +75,7 @@ config_dir = cmk.paths.var_dir + "/web" # Stores the initial configuration values default_config = {} -# Global table of available permissions. Plugins may add their own -# permissions by calling declare_permission() +# TODO: Clean this up permission_declaration_functions = [] # Constants for BI @@ -319,37 +318,9 @@ def tag_group_title(tag): # | Declarations of permissions and roles | # '----------------------------------------------------------------------' - -# Kept for compatibility with pre 1.6 GUI plugins -def declare_permission(name, title, description, defaults): - if isinstance(name, unicode): - name = name.encode("utf-8") - - section_name, permission_name = name.split(".", 1) - - cls = type( - "LegacyPermission%s%s" % (section_name.title(), permission_name.title()), - (permissions.Permission,), { - "_section_name": section_name, - "section": property(lambda s: permissions.permission_section_registry[s._section_name]), - "permission_name": permission_name, - "name": name, - "title": title, - "description": description, - "defaults": defaults, - }) - permissions.permission_registry.register(cls) - - # Kept for compatibility with pre 1.6 GUI plugins -def declare_permission_section(name, title, prio=50, do_sort=False): - cls = type("LegacyPermissionSection%s" % name.title(), (permissions.PermissionSection,), { - "name": name, - "title": title, - "sort_index": prio, - "do_sort": do_sort, - }) - permissions.permission_section_registry.register(cls) +declare_permission = permissions.declare_permission +declare_permission_section = permissions.declare_permission_section # Some module have a non-fixed list of permissions. For example for @@ -358,21 +329,19 @@ def declare_permission_section(name, title, prio=50, do_sort=False): # that purpose module can register functions. These functions should # just call declare_permission(). They are being called in the correct # situations. +# TODO: Clean this up def declare_dynamic_permissions(func): permission_declaration_functions.append(func) # This function needs to be called by all code that needs access # to possible dynamic permissions +# TODO: Clean this up def load_dynamic_permissions(): for func in permission_declaration_functions: func() -def permission_exists(pname): - return pname in permissions.permission_registry - - def get_role_permissions(): """Returns the set of permissions for all roles""" role_permissions = {} diff --git a/cmk/gui/pagetypes.py b/cmk/gui/pagetypes.py index cd655c6..b1bef71 100644 --- a/cmk/gui/pagetypes.py +++ b/cmk/gui/pagetypes.py @@ -57,7 +57,12 @@ from cmk.gui.valuespec import ( from cmk.gui.i18n import _u, _ from cmk.gui.globals import html -from cmk.gui.exceptions import MKUserError, MKGeneralException, MKAuthException +from cmk.gui.exceptions import ( + MKUserError, + MKGeneralException, + MKAuthException, +) +from cmk.gui.permissions import permission_registry # .--Base----------------------------------------------------------------. # | ____ | @@ -499,7 +504,7 @@ class Overridable(Base): # TODO: Wie is die Semantik hier genau? Umsetzung vervollständigen! def may_see(self): perm_name = "%s.%s" % (self.type_name(), self.name()) - if config.permission_exists(perm_name) and not config.user.may(perm_name): + if perm_name in permission_registry and not config.user.may(perm_name): return False # if self.owner() == "" and not config.user.may(perm_name): @@ -788,7 +793,7 @@ class Overridable(Base): @classmethod def declare_permission(cls, page): permname = "%s.%s" % (cls.type_name(), page.name()) - if page.is_public() and not config.permission_exists(permname): + if page.is_public() and permname not in permission_registry: config.declare_permission(permname, page.title(), page.description(), ['admin', 'user', 'guest']) diff --git a/cmk/gui/permissions.py b/cmk/gui/permissions.py index f1630b6..3caca95 100644 --- a/cmk/gui/permissions.py +++ b/cmk/gui/permissions.py @@ -150,3 +150,34 @@ class PermissionRegistry(cmk.plugin_registry.ClassRegistry): permission_registry = PermissionRegistry() + + +# Kept for compatibility with pre 1.6 GUI plugins +def declare_permission_section(name, title, prio=50, do_sort=False): + cls = type("LegacyPermissionSection%s" % name.title(), (PermissionSection,), { + "name": name, + "title": title, + "sort_index": prio, + "do_sort": do_sort, + }) + permission_section_registry.register(cls) + + +# Kept for compatibility with pre 1.6 GUI plugins +def declare_permission(name, title, description, defaults): + if isinstance(name, unicode): + name = name.encode("utf-8") + + section_name, permission_name = name.split(".", 1) + + cls = type( + "LegacyPermission%s%s" % (section_name.title(), permission_name.title()), (Permission,), { + "_section_name": section_name, + "section": property(lambda s: permission_section_registry[s._section_name]), + "permission_name": permission_name, + "name": name, + "title": title, + "description": description, + "defaults": defaults, + }) + permission_registry.register(cls) diff --git a/cmk/gui/visuals.py b/cmk/gui/visuals.py index 2db1310..1246f15 100644 --- a/cmk/gui/visuals.py +++ b/cmk/gui/visuals.py @@ -66,6 +66,7 @@ from cmk.gui.plugins.visuals.utils import ( # pylint: disable=unused-import FilterTristate, FilterUnicodeFilter, FilterSite, ) from cmk.gui.plugins.visuals.utils import _infos as infos +from cmk.gui.permissions import permission_registry if not cmk.is_raw_edition(): import cmk.gui.cee.plugins.visuals @@ -265,7 +266,7 @@ def load_visuals_of_a_user(what, builtin_visuals, skip_func, lock, path, user): def declare_visual_permission(what, name, visual): permname = "%s.%s" % (what[:-1], name) - if visual["public"] and not config.permission_exists(permname): + if visual["public"] and permname not in permission_registry: config.declare_permission(permname, visual["title"], visual["description"], ['admin', 'user', 'guest']) @@ -318,7 +319,7 @@ def available(what, all_visuals): u, "general.force_" + what): # Honor original permissions for the current user permname = "%s.%s" % (permprefix, n) - if config.permission_exists(permname) \ + if permname in permission_registry \ and not config.user.may(permname): continue visuals[n] = visual @@ -337,7 +338,7 @@ def available(what, all_visuals): u, "general.publish_" + what): # Is there a builtin visual with the same name? If yes, honor permissions. permname = "%s.%s" % (permprefix, n) - if config.permission_exists(permname) \ + if permname in permission_registry \ and not config.user.may(permname): continue visuals[n] = visual diff --git a/tests/unit/cmk/gui/test_gui_config.py b/tests/unit/cmk/gui/test_gui_config.py index 318e884..1e68c71b 100644 --- a/tests/unit/cmk/gui/test_gui_config.py +++ b/tests/unit/cmk/gui/test_gui_config.py @@ -2480,9 +2480,9 @@ def test_declare_permission(monkeypatch): assert "bla" in permissions.permission_section_registry monkeypatch.setattr(permissions, "permission_registry", permissions.PermissionRegistry()) - assert not config.permission_exists("bla.blub") + assert "bla.blub" not in permissions.permission_registry config.declare_permission("bla.blub", u"bla perm", u"descrrrrr", ["admin"]) - assert config.permission_exists("bla.blub") + assert "bla.blub" in permissions.permission_registry permission = permissions.permission_registry["bla.blub"]() assert permission.section == permissions.permission_section_registry["bla"]