Hello,
We had a private conversation about time management that grew into a design discussion, so I'm moving it to the appropriate list.

This is about ACI changes:
https://fedorahosted.org/freeipa/ticket/3566

The work is now tracked by additional several tickets (and corresponding design documents). In order of planned completion:
https://fedorahosted.org/freeipa/ticket/4034 - New permissions system
https://fedorahosted.org/freeipa/ticket/4032 - Anonymous and All permissions
https://fedorahosted.org/freeipa/ticket/4033 - Managed permissions
https://fedorahosted.org/freeipa/ticket/4035 - ACI audit tool

On 11/18/2013 07:49 PM, Simo Sorce wrote:
On Mon, 2013-11-18 at 19:29 +0100, Petr Viktorin wrote:
On 11/18/2013 06:19 PM, Dmitri Pal wrote:
Please factor in impact on the extensibility and API.
* Regarding extensibility:
Right now we say to add schema, create plugin and things would work.
With this change we need clear guidance on what ACI changes need to be
made and how they need to be made when IPA is customized.

We actually do ask "What are the ACIs for your entries?" in
http://www.freeipa.org/page/General_considerations

I'd like to have the default ACI definitions for plugins in the plugin
code, next to things like default_attributes, attribute_members,
password_attributes, etc.  So this is contained in the "create plugin"
step, with all plugins available as examples.
I'm attaching a patch from the set I sent earlier (rejected because more
ground work is needed); it shows the changes I envision would be needed
in a plugin.
(re-attaching for the list to see)

+1 this should make ACI more digestible too, as you can lok at them in
the context of the plugin that uses them.

However we must be careful with conflicting ACIs, we need to come up
with a well know procedure to handle cases where one plugin may try to
create permissions that conflict with those of another plugin.

We'll only have allow ACIs so changes here will not reduce functionality (but possibly security).

You'll notice that this style only supports attaching permissions to the object(s) provided by the plugin. Any other permissions would have to be supplied in update files, as today, so they'd hopefully receive the attention they do today. Or more, since they'd be exceptions.

* API/CLI
I suspect there will be case when ACI prevents exposure of an attribute
that was assumed by some logic to always be availble. I suspect API
would blow up badly. We need to prevent this and make sure we have test
that run API/CLI when different attributes are not readable.

The main use case here is configuring what is readable by anonymous vs.
by logged-in users, and you always need a ticket to use the API.

But it is correct that the API will currently fail in pretty mysterious
ways if read access is denied. Perhaps we should modify ipaldap's
get_entry & friends to double-check attribute-level rights on the
requested attributes?

Anyway we'll probably need to say that if you explicitly un-allow access
to an attribute, you're pretty much on your own.

I think we need to change this.
There may be legitimate reason to require an emergency block of an
attribute for an environment, and if at all possible that should not
completely break all functionality.

I agree here.

If you block writing to the attribute it is ok to break functions that
change stuff. If you block reading we should probably replace the
attribute with a special constructs that causes the plugin to show a
good error message or to blank out the relevant field and display a
message that tells why part of the information is not available.

Yes, for most attributes we can do that with a framework change to always check attribute-level rights.

The other part is attributes we actually process rather. From a quick scan we seem to be doing a fairly good job always checking if an attribute is present before using it, but we need to
- Ensure that is always the case, and
- Make sure the functionality makes sense if the attribute is hidden by ACIs rather than just doesn't exist. Think something like not seeing some "don't delete" flag on an entry.

That's a monumental task that we can't reasonably finish (or test). So we need to warn users they should know all the implications before hiding attributes.

--
PetrĀ³

From f4db999711d2bc48e00db08bf4976dc588d91db4 Mon Sep 17 00:00:00 2001
From: Petr Viktorin <pvikt...@redhat.com>
Date: Thu, 19 Sep 2013 17:41:04 +0200
Subject: [PATCH] Add Object metadata and update plugin for managed permissions

The default read permission is added for User as an example.

Part of the work for: https://fedorahosted.org/freeipa/ticket/3566
Design: http://www.freeipa.org/page/V3/Managed_Read_permissions
---
 ipalib/plugins/baseldap.py                         |   1 +
 ipalib/plugins/user.py                             |  25 +++++
 .../install/plugins/update_managed_permissions.py  | 103 +++++++++++++++++++++
 3 files changed, 129 insertions(+)
 create mode 100644 ipaserver/install/plugins/update_managed_permissions.py

diff --git a/ipalib/plugins/baseldap.py b/ipalib/plugins/baseldap.py
index 
4a79502708c358dd20ede017037b16f691b76766..2b1e135b020ee1af97187a331432f72cf28efe9c
 100644
--- a/ipalib/plugins/baseldap.py
+++ b/ipalib/plugins/baseldap.py
@@ -465,6 +465,7 @@ class LDAPObject(Object):
     }
     label = _('Entry')
     label_singular = _('Entry')
+    managed_permissions = {}
 
     container_not_found_msg = _('container entry (%(container)s) not found')
     parent_not_found_msg = _('%(parent)s: %(oname)s not found')
diff --git a/ipalib/plugins/user.py b/ipalib/plugins/user.py
index 
471981f48204209753eda2fb994d4c653dca0fa2..5c48290f3645079a621d4c37086462a5684f1e86
 100644
--- a/ipalib/plugins/user.py
+++ b/ipalib/plugins/user.py
@@ -221,6 +221,31 @@ class user(LDAPObject):
     bindable = True
     password_attributes = [('userpassword', 'has_password'),
                            ('krbprincipalkey', 'has_keytab')]
+    managed_permissions = {
+        'Read Users': {
+            'rights': {'read', 'search', 'compare'},
+            'attributes': {
+                'audio', 'businesscategory', 'carlicense', 'cn',
+                'departmentnumber', 'description', 'destinationindicator',
+                'displayname', 'employeenumber', 'employeetype',
+                'facsimiletelephonenumber', 'gecos', 'gidnumber', 'givenname',
+                'homedirectory', 'homephone', 'homepostaladdress',
+                'inetuserhttpurl', 'inetuserstatus', 'initials',
+                'internationalisdnnumber', 'ipasshpubkey', 'ipauniqueid',
+                'jpegphoto', 'l', 'labeleduri', 'loginshell', 'mail',
+                'manager', 'memberof', 'mobile', 'nsaccountlock', 'o',
+                'objectclass', 'ou', 'pager', 'photo',
+                'physicaldeliveryofficename', 'postofficebox',
+                'postaladdress', 'postalcode', 'preferreddeliverymethod',
+                'preferredlanguage', 'registeredaddress', 'roomnumber',
+                'secretary', 'seealso', 'sn', 'st', 'street',
+                'telephonenumber', 'teletexterminalidentifier', 'telexnumber',
+                'title', 'uid', 'uidnumber', 'usercertificate',
+                'usersmimecertificate',
+            },
+            'default privileges': {'User Administrators', 'User Readers'},
+        },
+    }
 
     label = _('Users')
     label_singular = _('User')
diff --git a/ipaserver/install/plugins/update_managed_permissions.py 
b/ipaserver/install/plugins/update_managed_permissions.py
new file mode 100644
index 
0000000000000000000000000000000000000000..0063560a943795883e20e59d9f801785049ce78e
--- /dev/null
+++ b/ipaserver/install/plugins/update_managed_permissions.py
@@ -0,0 +1,103 @@
+# Authors:
+#   Petr Viktorin <pvikt...@redhat.com>
+#
+# Copyright (C) 2013  Red Hat
+# see file 'COPYING' for use and warranty information
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+from ipalib import api, errors
+from ipalib.plugable import Registry
+from ipalib.plugins.permission import ACI_PREFIX
+from ipapython.dn import DN
+from ipaserver.install.plugins import LAST
+from ipaserver.install.plugins.baseupdate import PostUpdate
+
+
+register = Registry()
+
+
+@register()
+class update_managed_permissions(PostUpdate):
+    """Update managed permissions after an update.
+    """
+    order = LAST
+
+    def update_permission(self, obj, name, template):
+        rights = [unicode(r) for r in template['rights']]
+        new_attributes = set(template['attributes'])
+
+        if obj.name in api.Command.permission_add.params['type'].values:
+            aci_kw = {'type': unicode(obj.name), 'permissions': rights}
+        else:
+            aci_kw = {
+                'subtree': 'ldap://' + unicode(DN(
+                    (obj.uuid_attribute, '*'), obj.container_dn)),
+                'permissions': rights}
+
+        try:
+            result = api.Command.permission_show(name)
+        except errors.NotFound:
+            self.log.info('Creating managed permission: %s', name)
+            result = api.Command.permission_add(
+                name, all=True, **aci_kw)
+            updating = False
+        else:
+            self.log.info('Updating managed permission: %s', name)
+            updating = True
+
+        dn = DN(result['result']['dn'])
+        entry = api.Backend.ldap2.get_entry(dn)
+        if updating and not api.Object.permission.is_managed(entry):
+            self.log.warning('Permission %s is not MANAGED, skipping', name)
+            return
+        entry['objectclass'].append('ipaManagedPermission')
+        entry['ipapermissiontype'] = [u'SYSTEM', u'MANAGED']
+        existing_attrs = set(entry.get('ipapermissiondefaultattribute', ()))
+        entry['ipapermissiondefaultattribute'] = list(
+            existing_attrs | new_attributes)
+
+        if not updating:
+            # Set default privileges only when first creating the privilege
+            members = []
+            for privilege_name in template['default privileges']:
+                members.append(DN(('cn', privilege_name),
+                                  api.env.container_privilege,
+                                  api.env.basedn))
+            entry['member'] = members
+
+        try:
+            api.Backend.ldap2.update_entry(entry)
+        except errors.EmptyModlist:
+            self.log.debug('Entry unchanged')
+
+        try:
+            api.Command.aci_mod(name, aciprefix=ACI_PREFIX, **aci_kw)
+        except errors.EmptyModlist:
+            self.log.debug('Permissions unchanged')
+
+        try:
+            api.Command.permission_mod(name)
+        except errors.EmptyModlist:
+            self.log.debug('Attributes unchanged')
+
+    def execute(self, **options):
+        for obj in api.Object():
+            managed_permissions = getattr(obj, 'managed_permissions', {})
+            if managed_permissions:
+                self.log.debug('Updating managed permissions for %s', obj.name)
+            for name, template in managed_permissions.items():
+                self.update_permission(obj, unicode(name), template)
+
+        return False, False, ()
-- 
1.8.3.1

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to